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 9 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
9 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
4dc2949
Merge branch 'master' into rfontanarosa/fix-export-csv-properties
rfontanarosa Mar 10, 2026
25161c5
Merge branch 'master' into rfontanarosa/fix-export-csv-properties
rfontanarosa Mar 10, 2026
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 @@ -165,6 +165,19 @@ export class Datastore {
.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
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 @@ -24,7 +24,6 @@ import { getDatastore } from './common/context';
import { DecodedIdToken } from 'firebase-admin/auth';
import { StatusCodes } from 'http-status-codes';
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 @@ -85,8 +84,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 @@ -293,23 +312,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