Skip to content

Improve retrieval of properties from LOI documents #2235

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rfontanarosa wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Improve retrieval of properties from LOI documents #2235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
01ca149
improved block to retrieve properties
rfontanarosa Jul 18, 2025
316c856
fixed wrong condition
rfontanarosa Sep 2, 2025
0110fab
removed comments
rfontanarosa Sep 3, 2025
d8177e4
Merge branch 'master' into rfontanarosa/fix-export-csv-properties
rfontanarosa Nov 21, 2025
12ce678
improved readibility
rfontanarosa Nov 21, 2025
514f517
Merge branch 'master' into rfontanarosa/fix-export-csv-properties
rfontanarosa Dec 9, 2025
b98cafd
Merge branch 'master' into rfontanarosa/fix-export-csv-properties
rfontanarosa Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions functions/src/common/datastore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
*/
type pseudoGeoJsonGeometry = {
type: string;
coordinates: any;

Check warning on line 34 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type
};

/**
Expand Down Expand Up @@ -165,6 +165,19 @@
.get();
}

fetchPartialLocationsOfInterest(
surveyId: string,
jobId: string,
limit: number
) {
return this.db_
.collection(lois(surveyId))
.where(l.jobId, '==', jobId)
.orderBy(FieldPath.documentId())
.select('5', '9', '10')
.limit(limit);
}

fetchMailTemplate(templateId: string) {
return this.fetchDoc_(mailTemplate(templateId));
}
Expand Down Expand Up @@ -238,7 +251,7 @@
await loiRef.update({[l.properties]: loiDoc[l.properties]});
}

static toFirestoreMap(geometry: any) {

Check warning on line 254 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type
return Object.fromEntries(
Object.entries(geometry).map(([key, value]) => [
key,
Expand All @@ -247,7 +260,7 @@
);
}

static toFirestoreValue(value: any): any {

Check warning on line 263 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type

Check warning on line 263 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type
if (value === null) {
return null;
}
Expand Down Expand Up @@ -275,7 +288,7 @@
*
* @returns GeoJSON geometry object (with geometry as list of lists)
*/
static fromFirestoreMap(geoJsonGeometry: any): any {

Check warning on line 291 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type

Check warning on line 291 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type
const geometryObject = geoJsonGeometry as pseudoGeoJsonGeometry;
if (!geometryObject) {
throw new Error(
Expand All @@ -290,7 +303,7 @@
return geometryObject;
}

static fromFirestoreValue(coordinates: any) {

Check warning on line 306 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type
if (coordinates instanceof GeoPoint) {
// Note: GeoJSON coordinates are in lng-lat order.
return [coordinates.longitude, coordinates.latitude];
Expand All @@ -299,7 +312,7 @@
if (typeof coordinates !== 'object') {
return coordinates;
}
const result = new Array<any>(coordinates.length);

Check warning on line 315 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check 

Unexpected any. Specify a different type

Object.entries(coordinates).map(([i, nestedValue]) => {
const index = Number.parseInt(i);
Expand Down
45 changes: 25 additions & 20 deletions functions/src/export-csv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {getDatastore} from './common/context';
import * as HttpStatus from 'http-status-codes';
import {DecodedIdToken} from 'firebase-admin/auth';
import {List} from 'immutable';
import {QuerySnapshot} from 'firebase-admin/firestore';
import {timestampToInt, toMessage} from '@ground/lib';
import {GroundProtos} from '@ground/proto';
import {toGeoJsonGeometry} from '@ground/lib';
Expand Down Expand Up @@ -84,8 +83,28 @@ export async function exportCsvHandler(
const ownerIdFilter = canViewAll ? null : userId;

const tasks = job.tasks.sort((a, b) => a.index! - b.index!);
const snapshot = await db.fetchLocationsOfInterest(surveyId, jobId);
const loiProperties = createProperySetFromSnapshot(snapshot, ownerIdFilter);

const loiProperties = new Set<string>();
let query = db.fetchPartialLocationsOfInterest(surveyId, jobId, 1000);
let lastVisible = null;
do {
const snapshot = await query.get();
if (snapshot.empty) break;
await Promise.all(
snapshot.docs.map(async doc => {
const loi = doc.data();
if (ownerIdFilter) {
Copy link
Collaborator

@gino-m gino-m Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think this is still a bit confusing.. maybe we want something like this:

if (canViewAll || loiOwnerId === userId) {
  return extractLoiProperties(loi);
} else {
  return undefined;
}

Then we don't need ownerIdFilter at all

if (!(loi[9] === 2 && loi[5] === ownerIdFilter)) {
Copy link
Collaborator

@gino-m gino-m Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have named constants we can refer to field numbers instead of using literals?

return;
}
}
Object.keys(loi[10] || {}).forEach(key => loiProperties.add(key));
})
);
lastVisible = snapshot.docs[snapshot.docs.length - 1];
query = query.startAfter(lastVisible);
} while (lastVisible);

const headers = getHeaders(tasks, loiProperties);

res.type('text/csv');
Expand Down Expand Up @@ -291,23 +310,9 @@ function getFileName(jobName: string | null) {
return `${fileBase}.csv`;
}

function createProperySetFromSnapshot(
snapshot: QuerySnapshot,
ownerId: string | null
): Set<string> {
const allKeys = new Set<string>();
snapshot.forEach(doc => {
const loi = toMessage(doc.data(), Pb.LocationOfInterest);
if (loi instanceof Error) return;
if (!isAccessibleLoi(loi, ownerId)) return;
const properties = loi.properties;
for (const key of Object.keys(properties || {})) {
allKeys.add(key);
}
});
return allKeys;
}

/**
* Retrieves the values of specified properties from a LocationOfInterest object.
*/
function getPropertiesByName(
loi: Pb.LocationOfInterest,
properties: Set<string | number>
Expand Down
Loading