From 83514d55a11b22b4ce1808c17122e9a1b70325df Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 10:23:44 +0200 Subject: [PATCH 01/55] Rename var --- functions/src/export-csv.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 9001df320..72f939262 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -93,17 +93,17 @@ export async function exportCsvHandler( // memory than iterating over and streaming both LOI and submission` // collections simultaneously, but it's easier to read and maintain. This will // likely need to be optimized to scale to larger datasets. - const submissionsByLocationOfInterest: {[name: string]: any[]} = {}; + const submissionsByLoi: {[name: string]: any[]} = {}; submissions.forEach(submission => { const loiId = submission.get('loiId') as string; - const arr: any[] = submissionsByLocationOfInterest[loiId] || []; + const arr: any[] = submissionsByLoi[loiId] || []; arr.push(submission.data()); - submissionsByLocationOfInterest[loiId] = arr; + submissionsByLoi[loiId] = arr; }); lois.forEach(loi => { const loiId = loi.id; - const submissions = submissionsByLocationOfInterest[loiId] || [{}]; + const submissions = submissionsByLoi[loiId] || [{}]; submissions.forEach(submission => { const row = []; // Header: system:index From 4442b78b5e9565aa1b1f2ce861424d1885c6e7ff Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 10:25:45 +0200 Subject: [PATCH 02/55] Add tsdoc --- functions/src/export-csv.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 72f939262..f13149caf 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -36,7 +36,10 @@ type Task = { readonly hasOtherOption?: boolean; }; -// TODO: Refactor into meaningful pieces. +/** + * Iterates over all LOIs and submissions in a job, joining them + * into a single table written to the response as a quote CSV file. + */ export async function exportCsvHandler( req: functions.Request, res: functions.Response, From d68f2a7ce0b6daabf092d1a01921c9c1c79ff35a Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 10:46:53 +0200 Subject: [PATCH 03/55] Refactor getSubmissionsByLoi --- functions/src/export-csv.ts | 79 +++++++++++++++++++++++-------------- 1 file changed, 50 insertions(+), 29 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index f13149caf..bf550c8d5 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -36,10 +36,13 @@ type Task = { readonly hasOtherOption?: boolean; }; -/** - * Iterates over all LOIs and submissions in a job, joining them - * into a single table written to the response as a quote CSV file. - */ +/** A dictionary of submissions values (array) keyed by loi ID. */ +type SubmissionDict = {[key: string]: any[]}; + +/** + * Iterates over all LOIs and submissions in a job, joining them + * into a single table written to the response as a quote CSV file. + */ export async function exportCsvHandler( req: functions.Request, res: functions.Response, @@ -65,15 +68,8 @@ export async function exportCsvHandler( const tasksObject = (job['tasks'] as {[id: string]: Task}) || {}; const tasks = new Map(Object.entries(tasksObject)); const lois = await db.fetchLocationsOfInterestByJobId(survey.id, jobId); - - const headers = []; - headers.push('system:index'); - headers.push('geometry'); - const allLoiProperties = getPropertyNames(lois); - headers.push(...allLoiProperties); - tasks.forEach(task => headers.push('data:' + task.label)); - headers.push('data:contributor_username'); - headers.push('data:contributor_email'); + const loiProperties = getPropertyNames(lois); + const headers = getHeaders(tasks, loiProperties); res.type('text/csv'); res.setHeader( @@ -90,19 +86,7 @@ export async function exportCsvHandler( }); csvStream.pipe(res); - const submissions = await db.fetchSubmissionsByJobId(survey.id, jobId); - - // Index submissions by LOI id in memory. This consumes more - // memory than iterating over and streaming both LOI and submission` - // collections simultaneously, but it's easier to read and maintain. This will - // likely need to be optimized to scale to larger datasets. - const submissionsByLoi: {[name: string]: any[]} = {}; - submissions.forEach(submission => { - const loiId = submission.get('loiId') as string; - const arr: any[] = submissionsByLoi[loiId] || []; - arr.push(submission.data()); - submissionsByLoi[loiId] = arr; - }); + const submissionsByLoi = await getSubmissionsByLoi(survey.id, jobId); lois.forEach(loi => { const loiId = loi.id; @@ -114,7 +98,7 @@ export async function exportCsvHandler( // Header: geometry row.push(toWkt(loi.get('geometry')) || ''); // Header: One column for each loi property (merged over all properties across all LOIs) - row.push(...getPropertiesByName(loi, allLoiProperties)); + row.push(...getPropertiesByName(loi, loiProperties)); // TODO(#1288): Clean up remaining references to old responses field const data = submission['data'] || @@ -135,6 +119,43 @@ export async function exportCsvHandler( csvStream.end(); } +function getHeaders( + tasks: Map, + loiProperties: Set +): string[] { + const headers = []; + headers.push('system:index'); + headers.push('geometry'); + headers.push(...loiProperties); + tasks.forEach(task => headers.push('data:' + task.label)); + headers.push('data:contributor_username'); + headers.push('data:contributor_email'); + return headers; +} + +/** + * Returns all submissions in the specified job, indexed by LOI ID. + * Note: Indexes submissions by LOI id in memory. This consumes more + * memory than iterating over and streaming both LOI and submission + * collections simultaneously, but it's easier to read and maintain. This + * function will need to be optimized to scale to larger datasets than + * can fit in memory. + */ +async function getSubmissionsByLoi( + surveyId: string, + jobId: string +): Promise { + const db = getDatastore(); + const submissions = await db.fetchSubmissionsByJobId(surveyId, jobId); + const submissionsByLoi: {[name: string]: any[]} = {}; + submissions.forEach(submission => { + const loiId = submission.get('loiId') as string; + const arr: any[] = submissionsByLoi[loiId] || []; + arr.push(submission.data()); + submissionsByLoi[loiId] = arr; + }); + return submissionsByLoi; +} /** * Returns the WKT string converted from the given geometry object * @@ -230,10 +251,10 @@ function getPropertyNames( function getPropertiesByName( loi: FirebaseFirestore.QueryDocumentSnapshot, - allLoiProperties: Set + loiProperties: Set ): List { // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. - return List.of(...allLoiProperties).map( + return List.of(...loiProperties).map( prop => (loi.get('properties') || {})[prop] || '' ); } From 0c5fd9a2aed3e1e387e1337617d4e1e63eba2305 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 11:18:33 +0200 Subject: [PATCH 04/55] Refactor write fn and add necessary types --- functions/src/export-csv.ts | 67 +++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 26 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index bf550c8d5..1b14ac3b6 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -36,9 +36,14 @@ type Task = { readonly hasOtherOption?: boolean; }; +type Dict = {[key: string]: any}; + /** A dictionary of submissions values (array) keyed by loi ID. */ type SubmissionDict = {[key: string]: any[]}; +// TODO(#1277): Use a shared model with web +type LoiDocument = FirebaseFirestore.QueryDocumentSnapshot; + /** * Iterates over all LOIs and submissions in a job, joining them * into a single table written to the response as a quote CSV file. @@ -91,30 +96,9 @@ export async function exportCsvHandler( lois.forEach(loi => { const loiId = loi.id; const submissions = submissionsByLoi[loiId] || [{}]; - submissions.forEach(submission => { - const row = []; - // Header: system:index - row.push(loi.get('properties')?.id || ''); - // Header: geometry - row.push(toWkt(loi.get('geometry')) || ''); - // Header: One column for each loi property (merged over all properties across all LOIs) - row.push(...getPropertiesByName(loi, loiProperties)); - // TODO(#1288): Clean up remaining references to old responses field - const data = - submission['data'] || - submission['responses'] || - submission['results'] || - {}; - // Header: One column for each task - tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); - // Header: contributor_username, contributor_email - const contributor = submission['created'] - ? submission['created']['user'] - : []; - row.push(contributor['displayName'] || ''); - row.push(contributor['email'] || ''); - csvStream.write(row); - }); + submissions.forEach(submission => + writeRow(csvStream, loiProperties, tasks, loi, submission) + ); }); csvStream.end(); } @@ -156,6 +140,37 @@ async function getSubmissionsByLoi( }); return submissionsByLoi; } + +function writeRow( + csvStream: csv.CsvFormatterStream, + loiProperties: Set, + tasks: Map, + loiDoc: LoiDocument, + submission: SubmissionDict +) { + const row = []; + // Header: system:index + row.push(loiDoc.get('properties')?.id || ''); + // Header: geometry + row.push(toWkt(loiDoc.get('geometry')) || ''); + // Header: One column for each loi property (merged over all properties across all LOIs) + row.push(...getPropertiesByName(loiDoc, loiProperties)); + // TODO(#1288): Clean up remaining references to old responses field + const data = + submission['data'] || + submission['responses'] || + submission['results'] || + {}; + // Header: One column for each task + tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); + // Header: contributor_username, contributor_email + const contributor = submission['created'] + ? (submission['created'] as Dict)['user'] + : []; + row.push(contributor['displayName'] || ''); + row.push(contributor['email'] || ''); + csvStream.write(row); +} /** * Returns the WKT string converted from the given geometry object * @@ -250,11 +265,11 @@ function getPropertyNames( } function getPropertiesByName( - loi: FirebaseFirestore.QueryDocumentSnapshot, + loiDoc: LoiDocument, loiProperties: Set ): List { // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. return List.of(...loiProperties).map( - prop => (loi.get('properties') || {})[prop] || '' + prop => (loiDoc.get('properties') || {})[prop] || '' ); } From 8562bca9c7c463c1443964873b8a7026a68f79dc Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 11:24:14 +0200 Subject: [PATCH 05/55] Add missing return type --- functions/src/common/datastore.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/functions/src/common/datastore.ts b/functions/src/common/datastore.ts index c868f5c7c..b7419da9f 100644 --- a/functions/src/common/datastore.ts +++ b/functions/src/common/datastore.ts @@ -16,7 +16,7 @@ import * as functions from 'firebase-functions'; import {firestore} from 'firebase-admin'; -import {DocumentData, GeoPoint} from 'firebase-admin/firestore'; +import {DocumentData, GeoPoint, QuerySnapshot} from 'firebase-admin/firestore'; /** * @@ -127,7 +127,10 @@ export class Datastore { return this.fetchDoc_(loi(surveyId, loiId)); } - fetchLocationsOfInterestByJobId(surveyId: string, jobId: string) { + fetchLocationsOfInterestByJobId( + surveyId: string, + jobId: string + ): Promise> { return this.db_ .collection(lois(surveyId)) .where('jobId', '==', jobId) From e8500b007c3eeeaab8564b02ef823bc6f4f339c5 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 11:24:38 +0200 Subject: [PATCH 06/55] Simplify imports --- functions/src/export-csv.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 1b14ac3b6..f7a1c78cd 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -23,6 +23,7 @@ import * as HttpStatus from 'http-status-codes'; import {Datastore} from './common/datastore'; import {DecodedIdToken} from 'firebase-admin/auth'; import {List} from 'immutable'; +import {DocumentData, QuerySnapshot} from 'firebase-admin/firestore'; // TODO(#1277): Use a shared model with web type Task = { @@ -42,7 +43,8 @@ type Dict = {[key: string]: any}; type SubmissionDict = {[key: string]: any[]}; // TODO(#1277): Use a shared model with web -type LoiDocument = FirebaseFirestore.QueryDocumentSnapshot; +type LoiDocument = + FirebaseFirestore.QueryDocumentSnapshot; /** * Iterates over all LOIs and submissions in a job, joining them @@ -250,9 +252,7 @@ function getFileName(jobName: string) { return `${fileBase}.csv`; } -function getPropertyNames( - lois: FirebaseFirestore.QuerySnapshot -): Set { +function getPropertyNames(lois: QuerySnapshot): Set { return new Set( lois.docs .map(loi => From a85499951938e0075fb0b58e75779128563d39ec Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 11:27:23 +0200 Subject: [PATCH 07/55] Simplify loop --- functions/src/export-csv.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index f7a1c78cd..8efca26b3 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -96,9 +96,7 @@ export async function exportCsvHandler( const submissionsByLoi = await getSubmissionsByLoi(survey.id, jobId); lois.forEach(loi => { - const loiId = loi.id; - const submissions = submissionsByLoi[loiId] || [{}]; - submissions.forEach(submission => + submissionsByLoi[loi.id]?.forEach(submission => writeRow(csvStream, loiProperties, tasks, loi, submission) ); }); From fb3aa3de13ed134d7db4b4e1e25c9628ccfaf0ff Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 11:28:07 +0200 Subject: [PATCH 08/55] Minor renames --- functions/src/export-csv.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 8efca26b3..fbf169a03 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -74,8 +74,8 @@ export async function exportCsvHandler( const jobName = job.name && (job.name['en'] as string); const tasksObject = (job['tasks'] as {[id: string]: Task}) || {}; const tasks = new Map(Object.entries(tasksObject)); - const lois = await db.fetchLocationsOfInterestByJobId(survey.id, jobId); - const loiProperties = getPropertyNames(lois); + const loiDocs = await db.fetchLocationsOfInterestByJobId(survey.id, jobId); + const loiProperties = getPropertyNames(loiDocs); const headers = getHeaders(tasks, loiProperties); res.type('text/csv'); @@ -95,9 +95,9 @@ export async function exportCsvHandler( const submissionsByLoi = await getSubmissionsByLoi(survey.id, jobId); - lois.forEach(loi => { - submissionsByLoi[loi.id]?.forEach(submission => - writeRow(csvStream, loiProperties, tasks, loi, submission) + loiDocs.forEach(loiDoc => { + submissionsByLoi[loiDoc.id]?.forEach(submission => + writeRow(csvStream, loiProperties, tasks, loiDoc, submission) ); }); csvStream.end(); From 0f256ee8cb6da30ac109d72ca014f5bfe5a6c166 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 13:00:42 +0200 Subject: [PATCH 09/55] Convert protos to GeoJSON --- lib/package.json | 5 +- lib/src/testing/firestore/geojson.ts | 83 ++++++++++++++++++++++++++++ package-lock.json | 6 +- 3 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 lib/src/testing/firestore/geojson.ts diff --git a/lib/package.json b/lib/package.json index c166d34ec..f4f156891 100644 --- a/lib/package.json +++ b/lib/package.json @@ -3,7 +3,9 @@ "version": "0.0.1", "main": "dist/index.js", "description": "Ground shared lib", - "files": ["dist"], + "files": [ + "dist" + ], "scripts": { "clean": "rm -rf dist *.log src/generated", "lint": "eslint --ext .js,.ts src/", @@ -29,6 +31,7 @@ "protobufjs": "^7.3.0" }, "devDependencies": { + "@types/geojson": "^7946.0.14", "@types/jasmine": "^4.3.5", "@types/source-map-support": "^0.5.10", "@typescript-eslint/eslint-plugin": "^5.39.0", diff --git a/lib/src/testing/firestore/geojson.ts b/lib/src/testing/firestore/geojson.ts new file mode 100644 index 000000000..6158159ab --- /dev/null +++ b/lib/src/testing/firestore/geojson.ts @@ -0,0 +1,83 @@ +/** + * Copyright 2024 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {GroundProtos} from '@ground/proto'; +import {Geometry, MultiPolygon, Point, Polygon, Position} from 'geojson'; + +import Pb = GroundProtos.google.ground.v1beta1; + +/** + * Returns the equivalent GeoJSON Geometry for the provided Geometry proto. + */ +export function toGeoJson(pb: Pb.Geometry): Geometry { + switch (pb.geometryType) { + case 'point': + if (!pb.point) throw new Error('Invalid Point'); + return toGeoJsonPoint(pb.point); + case 'polygon': + if (!pb.polygon) throw new Error('Invalid Polygon'); + return toGeoJsonPolygon(pb.polygon); + case 'multiPolygon': + if (!pb.multiPolygon) throw new Error('Invalid MultiPolygon'); + return toGeoJsonMultiPolygon(pb.multiPolygon); + default: + throw new Error(`Invalid geometry type '${pb.geometryType}'`); + } +} + +function toGeoJsonPoint(pb: Pb.IPoint): Point { + if (!pb.coordinates) throw new Error('Invalid Point: coordinates missing'); + return { + type: 'Point', + coordinates: toGeoJsonPosition(pb.coordinates), + }; +} + +function toGeoJsonPosition(coordinates: Pb.ICoordinates): Position { + if (!coordinates.longitude || !coordinates.latitude) + throw new Error('Invalid Point: coordinates missing'); + return [coordinates.longitude, coordinates.latitude]; +} + +function toGeoJsonPolygon(pb: Pb.IPolygon): Polygon { + return { + type: 'Polygon', + coordinates: toGeoJsonPolygonCoordinates(pb), + }; +} + +function toGeoJsonPolygonCoordinates(pb: Pb.IPolygon): Position[][] { + if (!pb.shell) throw new Error('Invalid Polygon: shell coordinates missing'); + return [ + toGeoJsonPositionArray(pb.shell), + ...(pb.holes || []).map(h => toGeoJsonPositionArray(h)), + ]; +} + +function toGeoJsonPositionArray(ring: Pb.ILinearRing): Position[] { + if (!ring.coordinates) + throw new Error('Invalid LinearRing: coordinates missing'); + return ring.coordinates.map(c => toGeoJsonPosition(c)); +} + +function toGeoJsonMultiPolygon(multiPolygon: Pb.IMultiPolygon): MultiPolygon { + if (!multiPolygon.polygons) + throw new Error('Invalid multi-polygon: coordinates missing'); + return { + type: 'MultiPolygon', + coordinates: multiPolygon.polygons.map(p => toGeoJsonPolygonCoordinates(p)), + }; +} diff --git a/package-lock.json b/package-lock.json index c2cfa6fa7..a547e1c67 100644 --- a/package-lock.json +++ b/package-lock.json @@ -100,6 +100,7 @@ "protobufjs": "^7.3.0" }, "devDependencies": { + "@types/geojson": "^7946.0.14", "@types/jasmine": "^4.3.5", "@types/source-map-support": "^0.5.10", "@typescript-eslint/eslint-plugin": "^5.39.0", @@ -9244,8 +9245,9 @@ }, "node_modules/@types/geojson": { "version": "7946.0.14", - "dev": true, - "license": "MIT" + "resolved": "https://registry.npmjs.org/@types/geojson/-/geojson-7946.0.14.tgz", + "integrity": "sha512-WCfD5Ht3ZesJUsONdhvm84dmzWOiOzOAqOncN0++w0lBw1o8OuDNJF2McvvCef/yBqb/HYRahp1BYtODFQ8bRg==", + "dev": true }, "node_modules/@types/glob": { "version": "8.1.0", From de28a3d4d90fa6a8f6270f9e058602030d360616 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 14:58:12 +0200 Subject: [PATCH 10/55] Export functions --- lib/src/geo-json.ts | 83 +++++++++++++++++++++++++++++++++++++++++++++ lib/src/index.ts | 1 + 2 files changed, 84 insertions(+) create mode 100644 lib/src/geo-json.ts diff --git a/lib/src/geo-json.ts b/lib/src/geo-json.ts new file mode 100644 index 000000000..74f03d9c6 --- /dev/null +++ b/lib/src/geo-json.ts @@ -0,0 +1,83 @@ +/** + * Copyright 2024 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {GroundProtos} from '@ground/proto'; +import {Geometry, MultiPolygon, Point, Polygon, Position} from 'geojson'; + +import Pb = GroundProtos.google.ground.v1beta1; + +/** + * Returns the equivalent GeoJSON Geometry for the provided Geometry proto. + */ +export function toGeoJsonGeometry(pb: Pb.Geometry): Geometry { + switch (pb.geometryType) { + case 'point': + if (!pb.point) throw new Error('Invalid Point'); + return toGeoJsonPoint(pb.point); + case 'polygon': + if (!pb.polygon) throw new Error('Invalid Polygon'); + return toGeoJsonPolygon(pb.polygon); + case 'multiPolygon': + if (!pb.multiPolygon) throw new Error('Invalid MultiPolygon'); + return toGeoJsonMultiPolygon(pb.multiPolygon); + default: + throw new Error(`Invalid geometry type '${pb.geometryType}'`); + } +} + +function toGeoJsonPoint(pb: Pb.IPoint): Point { + if (!pb.coordinates) throw new Error('Invalid Point: coordinates missing'); + return { + type: 'Point', + coordinates: toGeoJsonPosition(pb.coordinates), + }; +} + +function toGeoJsonPosition(coordinates: Pb.ICoordinates): Position { + if (!coordinates.longitude || !coordinates.latitude) + throw new Error('Invalid Point: coordinates missing'); + return [coordinates.longitude, coordinates.latitude]; +} + +function toGeoJsonPolygon(pb: Pb.IPolygon): Polygon { + return { + type: 'Polygon', + coordinates: toGeoJsonPolygonCoordinates(pb), + }; +} + +function toGeoJsonPolygonCoordinates(pb: Pb.IPolygon): Position[][] { + if (!pb.shell) throw new Error('Invalid Polygon: shell coordinates missing'); + return [ + toGeoJsonPositionArray(pb.shell), + ...(pb.holes || []).map(h => toGeoJsonPositionArray(h)), + ]; +} + +function toGeoJsonPositionArray(ring: Pb.ILinearRing): Position[] { + if (!ring.coordinates) + throw new Error('Invalid LinearRing: coordinates missing'); + return ring.coordinates.map(c => toGeoJsonPosition(c)); +} + +function toGeoJsonMultiPolygon(multiPolygon: Pb.IMultiPolygon): MultiPolygon { + if (!multiPolygon.polygons) + throw new Error('Invalid multi-polygon: coordinates missing'); + return { + type: 'MultiPolygon', + coordinates: multiPolygon.polygons.map(p => toGeoJsonPolygonCoordinates(p)), + }; +} diff --git a/lib/src/index.ts b/lib/src/index.ts index 96f2cfe70..e1462abdd 100644 --- a/lib/src/index.ts +++ b/lib/src/index.ts @@ -17,3 +17,4 @@ export {toDocumentData} from './proto-to-firestore'; export {toMessage} from './firestore-to-proto'; export {deleteEmpty, isEmpty} from './obj-util'; +export {toGeoJsonGeometry} from './geo-json'; From e161f7829b8f3c2c1e064b6cce6aa09192bf406b Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 15:01:41 +0200 Subject: [PATCH 11/55] Rename file, fix types --- lib/src/geo-json.ts | 22 +++----- lib/src/testing/firestore/geojson.ts | 83 ---------------------------- 2 files changed, 9 insertions(+), 96 deletions(-) delete mode 100644 lib/src/testing/firestore/geojson.ts diff --git a/lib/src/geo-json.ts b/lib/src/geo-json.ts index 74f03d9c6..5884db66e 100644 --- a/lib/src/geo-json.ts +++ b/lib/src/geo-json.ts @@ -22,19 +22,15 @@ import Pb = GroundProtos.google.ground.v1beta1; /** * Returns the equivalent GeoJSON Geometry for the provided Geometry proto. */ -export function toGeoJsonGeometry(pb: Pb.Geometry): Geometry { - switch (pb.geometryType) { - case 'point': - if (!pb.point) throw new Error('Invalid Point'); - return toGeoJsonPoint(pb.point); - case 'polygon': - if (!pb.polygon) throw new Error('Invalid Polygon'); - return toGeoJsonPolygon(pb.polygon); - case 'multiPolygon': - if (!pb.multiPolygon) throw new Error('Invalid MultiPolygon'); - return toGeoJsonMultiPolygon(pb.multiPolygon); - default: - throw new Error(`Invalid geometry type '${pb.geometryType}'`); +export function toGeoJsonGeometry(pb: Pb.IGeometry): Geometry { + if (pb.point) { + return toGeoJsonPoint(pb.point); + } else if (pb.polygon) { + return toGeoJsonPolygon(pb.polygon); + } else if (pb.multiPolygon) { + return toGeoJsonMultiPolygon(pb.multiPolygon); + } else { + throw new Error('Unsupported or missing geometry'); } } diff --git a/lib/src/testing/firestore/geojson.ts b/lib/src/testing/firestore/geojson.ts deleted file mode 100644 index 6158159ab..000000000 --- a/lib/src/testing/firestore/geojson.ts +++ /dev/null @@ -1,83 +0,0 @@ -/** - * Copyright 2024 The Ground Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import {GroundProtos} from '@ground/proto'; -import {Geometry, MultiPolygon, Point, Polygon, Position} from 'geojson'; - -import Pb = GroundProtos.google.ground.v1beta1; - -/** - * Returns the equivalent GeoJSON Geometry for the provided Geometry proto. - */ -export function toGeoJson(pb: Pb.Geometry): Geometry { - switch (pb.geometryType) { - case 'point': - if (!pb.point) throw new Error('Invalid Point'); - return toGeoJsonPoint(pb.point); - case 'polygon': - if (!pb.polygon) throw new Error('Invalid Polygon'); - return toGeoJsonPolygon(pb.polygon); - case 'multiPolygon': - if (!pb.multiPolygon) throw new Error('Invalid MultiPolygon'); - return toGeoJsonMultiPolygon(pb.multiPolygon); - default: - throw new Error(`Invalid geometry type '${pb.geometryType}'`); - } -} - -function toGeoJsonPoint(pb: Pb.IPoint): Point { - if (!pb.coordinates) throw new Error('Invalid Point: coordinates missing'); - return { - type: 'Point', - coordinates: toGeoJsonPosition(pb.coordinates), - }; -} - -function toGeoJsonPosition(coordinates: Pb.ICoordinates): Position { - if (!coordinates.longitude || !coordinates.latitude) - throw new Error('Invalid Point: coordinates missing'); - return [coordinates.longitude, coordinates.latitude]; -} - -function toGeoJsonPolygon(pb: Pb.IPolygon): Polygon { - return { - type: 'Polygon', - coordinates: toGeoJsonPolygonCoordinates(pb), - }; -} - -function toGeoJsonPolygonCoordinates(pb: Pb.IPolygon): Position[][] { - if (!pb.shell) throw new Error('Invalid Polygon: shell coordinates missing'); - return [ - toGeoJsonPositionArray(pb.shell), - ...(pb.holes || []).map(h => toGeoJsonPositionArray(h)), - ]; -} - -function toGeoJsonPositionArray(ring: Pb.ILinearRing): Position[] { - if (!ring.coordinates) - throw new Error('Invalid LinearRing: coordinates missing'); - return ring.coordinates.map(c => toGeoJsonPosition(c)); -} - -function toGeoJsonMultiPolygon(multiPolygon: Pb.IMultiPolygon): MultiPolygon { - if (!multiPolygon.polygons) - throw new Error('Invalid multi-polygon: coordinates missing'); - return { - type: 'MultiPolygon', - coordinates: multiPolygon.polygons.map(p => toGeoJsonPolygonCoordinates(p)), - }; -} From 6bc1dcff6abc73082bbc31501770e296a37162c8 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 15:38:35 +0200 Subject: [PATCH 12/55] Add email address back into audit info --- proto/src/google/ground/v1beta1/audit_info.proto | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/proto/src/google/ground/v1beta1/audit_info.proto b/proto/src/google/ground/v1beta1/audit_info.proto index cdd74e73b..f4821054a 100644 --- a/proto/src/google/ground/v1beta1/audit_info.proto +++ b/proto/src/google/ground/v1beta1/audit_info.proto @@ -23,8 +23,7 @@ import "google/protobuf/timestamp.proto"; option java_multiple_files = true; option java_package = "com.google.android.ground.proto"; -// Audit info about *who* performed a particular action and *when*. User email -// addresses are omitted for privacy reasons. +// Audit info about *who* performed a particular action and *when*. message AuditInfo { // Required. The id of the user performing the action. string user_id = 1; @@ -40,4 +39,7 @@ message AuditInfo { // URL of the user's profile picture. string photo_url = 5; + + // The user's email address. + string email_address = 6; } From d3e5be2dfac0f4b98557f06c8ee0bc9d04b102db Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 17 Jul 2024 15:38:49 +0200 Subject: [PATCH 13/55] Export data in new format --- functions/src/export-csv.ts | 124 ++++++++++++++++++++++++++++++++---- 1 file changed, 113 insertions(+), 11 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index fbf169a03..b5858e7d3 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -24,6 +24,11 @@ import {Datastore} from './common/datastore'; import {DecodedIdToken} from 'firebase-admin/auth'; import {List} from 'immutable'; import {DocumentData, QuerySnapshot} from 'firebase-admin/firestore'; +import {toMessage} from '@ground/lib'; +import {GroundProtos} from '@ground/proto'; +import {toGeoJsonGeometry} from '@ground/lib'; + +import Pb = GroundProtos.google.ground.v1beta1; // TODO(#1277): Use a shared model with web type Task = { @@ -69,6 +74,7 @@ export async function exportCsvHandler( } console.log(`Exporting survey '${surveyId}', job '${jobId}'`); + // TODO(#1779): Get job metadata from `/surveys/{surveyId}/jobs` instead. const jobs = survey.get('jobs') || {}; const job = jobs[jobId] || {}; const jobName = job.name && (job.name['en'] as string); @@ -96,9 +102,31 @@ export async function exportCsvHandler( const submissionsByLoi = await getSubmissionsByLoi(survey.id, jobId); loiDocs.forEach(loiDoc => { - submissionsByLoi[loiDoc.id]?.forEach(submission => - writeRow(csvStream, loiProperties, tasks, loiDoc, submission) - ); + const loi = toMessage(loiDoc.data(), Pb.LocationOfInterest); + submissionsByLoi[loiDoc.id]?.forEach(submissionDict => { + try { + const submission = toMessage(submissionDict, Pb.Submission); + if ( + loi instanceof Pb.LocationOfInterest && + loi.jobId && + submission instanceof Pb.Submission && + submission.jobId + ) { + writeRow(csvStream, loiProperties, tasks, loi, submission); + } else { + // TODO(#1779): Delete once migration is complete. + writeRowLegacy( + csvStream, + loiProperties, + tasks, + loiDoc, + submissionDict + ); + } + } catch (e) { + console.debug('Skipping row', e); + } + }); }); csvStream.end(); } @@ -142,6 +170,37 @@ async function getSubmissionsByLoi( } function writeRow( + csvStream: csv.CsvFormatterStream, + loiProperties: Set, + tasks: Map, + loi: Pb.LocationOfInterest, + submission: Pb.Submission +) { + // TODO(#1779): Remove migration is complete. + console.debug('Writing data using new schema'); + const row = []; + // Header: system:index + row.push(loi.customTag || ''); + // Header: geometry + if (!loi.geometry) { + console.debug(`Skipping LOI ${loi.id} - missing geometry`); + return; + } + row.push(toWkt(loi.geometry)); + // Header: One column for each loi property (merged over all properties across all LOIs) + row.push(...getPropertiesByName(loi, loiProperties)); + // TODO(#1288): Clean up remaining references to old responses field + const data = submission.taskData; + // Header: One column for each task + tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); + // Header: contributor_username, contributor_email + row.push(submission.created?.displayName || ''); + row.push(submission.created?.emailAddress || ''); + csvStream.write(row); +} + +// TODO(#1779): Delete once migration is complete. +function writeRowLegacy( csvStream: csv.CsvFormatterStream, loiProperties: Set, tasks: Map, @@ -152,9 +211,9 @@ function writeRow( // Header: system:index row.push(loiDoc.get('properties')?.id || ''); // Header: geometry - row.push(toWkt(loiDoc.get('geometry')) || ''); + row.push(legacyFirestoreMapToWkt(loiDoc.get('geometry')) || ''); // Header: One column for each loi property (merged over all properties across all LOIs) - row.push(...getPropertiesByName(loiDoc, loiProperties)); + row.push(...getPropertiesByNameLegacy(loiDoc, loiProperties)); // TODO(#1288): Clean up remaining references to old responses field const data = submission['data'] || @@ -162,7 +221,7 @@ function writeRow( submission['results'] || {}; // Header: One column for each task - tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); + tasks.forEach((task, taskId) => row.push(getValueLegacy(taskId, task, data))); // Header: contributor_username, contributor_email const contributor = submission['created'] ? (submission['created'] as Dict)['user'] @@ -171,10 +230,11 @@ function writeRow( row.push(contributor['email'] || ''); csvStream.write(row); } + /** - * Returns the WKT string converted from the given geometry object + * Returns the WKT string converted from the given geometry object. * - * @param geometryObject - the GeoJSON geometry object extracted from the LOI. This should have format: + * @param geometryObject the geometry object extracted from the LOI. This should have format: * { * coordinates: any[], * type: string @@ -184,14 +244,43 @@ function writeRow( * * @beta */ -function toWkt(geometryObject: any): string { +function legacyFirestoreMapToWkt(geometryObject: any): string { return geojsonToWKT(Datastore.fromFirestoreMap(geometryObject)); } +function toWkt(geometry: Pb.IGeometry): string { + return geojsonToWKT(toGeoJsonGeometry(geometry)); +} + +/** + * Returns the string representation of a specific task element result. + */ +function getValue(taskId: string, task: Task, data: Pb.ITaskData[]) { + const result = data.find(d => d.taskId === taskId); + if (result?.multipleChoiceResponses) { + return result.multipleChoiceResponses?.selectedOptionIds + ?.map(id => getMultipleChoiceValues(id, task)) + .join(', '); + } else if (result?.captureLocationResult) { + // TODO(): Include alitude and accuracy in separate columns. + return toWkt( + new Pb.Geometry({ + point: new Pb.Point({ + coordinates: result.captureLocationResult.coordinates, + }), + }) + ); + } else if (result?.drawGeometryResult?.geometry) { + return toWkt(result.drawGeometryResult.geometry); + } else { + return result; + } +} + /** * Returns the string representation of a specific task element result. */ -function getValue(taskId: string, task: Task, data: any) { +function getValueLegacy(taskId: string, task: Task, data: any) { const result = data[taskId] || ''; if ( task.type === 'multiple_choice' && @@ -203,7 +292,7 @@ function getValue(taskId: string, task: Task, data: any) { if (!result) { return ''; } - return toWkt(result.geometry || result); + return legacyFirestoreMapToWkt(result.geometry || result); } else { return result; } @@ -263,6 +352,19 @@ function getPropertyNames(lois: QuerySnapshot): Set { } function getPropertiesByName( + loi: Pb.LocationOfInterest, + properties: Set +): List { + // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. + return List.of(...properties).map( + prop => + loi.properties[prop].stringValue || + loi.properties[prop].numericValue?.toString() || + '' + ); +} + +function getPropertiesByNameLegacy( loiDoc: LoiDocument, loiProperties: Set ): List { From c738251fe5fb21eeaa2a8f7c94ddd3fc40e60270 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 18 Jul 2024 15:08:07 +0200 Subject: [PATCH 14/55] Fix typo --- functions/src/export-csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index b5858e7d3..604ff1402 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -176,7 +176,7 @@ function writeRow( loi: Pb.LocationOfInterest, submission: Pb.Submission ) { - // TODO(#1779): Remove migration is complete. + // TODO(#1779): Remove once migration is complete. console.debug('Writing data using new schema'); const row = []; // Header: system:index From e428df2d2c009c94ff40f37955372e81fbf60e94 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 18 Jul 2024 16:16:03 +0200 Subject: [PATCH 15/55] Test WIP --- functions/src/export-csv.spec.ts | 149 +++++++++++++++++++++ functions/src/export-csv.ts | 5 +- functions/src/testing/http-test-helpers.ts | 22 ++- 3 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 functions/src/export-csv.spec.ts diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts new file mode 100644 index 000000000..80ed71909 --- /dev/null +++ b/functions/src/export-csv.spec.ts @@ -0,0 +1,149 @@ +/** + * Copyright 2024 The Ground Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + TestGeoPoint, + createMockFirestore, + stubAdminApi, +} from '@ground/lib/dist/testing/firestore'; +import { + createGetRequestSpy, + createResponseSpy, +} from './testing/http-test-helpers'; +import { + $job_id, + $geometry, + $submission_count, + $source, + $properties, + $point, + $coordinates, + $latitude, + $longitude, +} from '@ground/lib/dist/testing/proto-field-aliases'; +import {DecodedIdToken} from 'firebase-admin/auth'; +import HttpStatus from 'http-status-codes'; +import {OWNER_ROLE} from './common/auth'; +import {resetDatastore} from './common/context'; +import {Firestore} from 'firebase-admin/firestore'; +import {exportCsvHandler} from './export-csv'; + +fdescribe('exportCsv()', () => { + let mockFirestore: Firestore; + const surveyId = 'survey001'; + const jobId = 'job123'; + const email = 'somebody@test.it'; + const survey = { + name: 'Test survey', + acl: { + [email]: OWNER_ROLE, + }, + jobs: { + [jobId]: { + name: 'Test job' + } + } + }; + const testProperties = { + name: 'Dinagat Islands', + area: 3.08, + }; + const pointLoi = { + [$job_id]: 'job123', + [$geometry]: { + [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, + }, + [$submission_count]: 0, + [$source]: 1, // IMPORTED + [$properties]: {name: 'Dinagat Islands', area: 3.08}, + jobId: 'job123', + predefined: true, + geometry: {type: 'Point', coordinates: TestGeoPoint(10.1, 125.6)}, + properties: testProperties, + }; +// const polygonLoi = { +// [$job_id]: 'job123', +// [$geometry]: { +// [$polygon]: { +// [$shell]: { +// [$coordinates]: [ +// {[$latitude]: 0, [$longitude]: 100}, +// {[$latitude]: 0, [$longitude]: 101}, +// {[$latitude]: 1, [$longitude]: 101}, +// {[$latitude]: 0, [$longitude]: 100}, +// ], +// }, +// }, +// }, +// [$submission_count]: 0, +// [$source]: 1, // IMPORTED +// jobId: 'job123', +// predefined: true, +// geometry: { +// type: 'Polygon', +// coordinates: { +// 0: { +// 0: TestGeoPoint(0, 100), +// 1: TestGeoPoint(0, 101), +// 2: TestGeoPoint(1, 101), +// 3: TestGeoPoint(0, 100), +// }, +// }, +// }, +// }; + + const testCases = [ + { + desc: 'imports points', + input: {}, + expected: [pointLoi], + } + ]; + + beforeEach(() => { + mockFirestore = createMockFirestore(); + stubAdminApi(mockFirestore); + }); + + afterEach(() => { + resetDatastore(); + }); + + testCases.forEach(({desc, input, expected}) => + it(desc, async () => { + // Add survey. + mockFirestore.doc(`surveys/${surveyId}`).set(survey); + + // Build mock request and response. + const req = await createGetRequestSpy({ + url: '/exportCsv', + query: { + survey: surveyId, + job: jobId, + }, + }); + const res = createResponseSpy(); + + // Run export CSV handler. + await exportCsvHandler(req, res, {email} as DecodedIdToken); + + // Check post-conditions. + expect(res.status).toHaveBeenCalledOnceWith(HttpStatus.OK); + expect(res.type).toHaveBeenCalledOnceWith('text/csv'); + expect(res.setHeader).toHaveBeenCalledOnceWith('Content-Disposition', 'attachment; filename=test-job.csv'); + }) + ); +}); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 604ff1402..d875f62a2 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -77,7 +77,7 @@ export async function exportCsvHandler( // TODO(#1779): Get job metadata from `/surveys/{surveyId}/jobs` instead. const jobs = survey.get('jobs') || {}; const job = jobs[jobId] || {}; - const jobName = job.name && (job.name['en'] as string); + const jobName = job.name; const tasksObject = (job['tasks'] as {[id: string]: Task}) || {}; const tasks = new Map(Object.entries(tasksObject)); const loiDocs = await db.fetchLocationsOfInterestByJobId(survey.id, jobId); @@ -128,6 +128,7 @@ export async function exportCsvHandler( } }); }); + res.status(HttpStatus.OK); csvStream.end(); } @@ -333,7 +334,7 @@ function extractOtherOption(submission: string): string { /** * Returns the file name in lowercase (replacing any special characters with '-') for csv export */ -function getFileName(jobName: string) { +function getFileName(jobName: string | null) { jobName = jobName || 'ground-export'; const fileBase = jobName.toLowerCase().replace(/[^a-z0-9]/gi, '-'); return `${fileBase}.csv`; diff --git a/functions/src/testing/http-test-helpers.ts b/functions/src/testing/http-test-helpers.ts index 86ece4219..bc494b39b 100644 --- a/functions/src/testing/http-test-helpers.ts +++ b/functions/src/testing/http-test-helpers.ts @@ -31,13 +31,31 @@ export async function createPostRequestSpy( }); } +export async function createGetRequestSpy( + args: object +): Promise { + return jasmine.createSpyObj('request', ['unpipe'], { + ...args, + method: 'GET' + }); +} + export function createResponseSpy(): functions.Response { const res = jasmine.createSpyObj>('response', [ 'send', 'status', 'end', + 'write', + 'type', + 'setHeader', + 'on', + 'once', + 'emit' ]); - res.status.and.returnValue(res); - res.end.and.returnValue(res); + res.status.and.callThrough(); + res.end.and.callThrough(); + res.write.and.callFake((chunk: any) => { console.log("=====write "+chunk); return true;}); + res.on.and.callThrough(); + // res.write.and.callFake((event: any, listener: any) => { console.log("=====on "+event); return res;}); return res; } From 845f9f2c4f4bf680a50d7ba35a4d3c153b52d695 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 14:52:01 -0400 Subject: [PATCH 16/55] Set up response testing --- functions/src/export-csv.spec.ts | 95 ++++++++++++---------- functions/src/export-csv.ts | 16 ++-- functions/src/testing/http-test-helpers.ts | 12 +-- lib/src/testing/proto-field-aliases.ts | 4 +- 4 files changed, 71 insertions(+), 56 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 80ed71909..977ef51d7 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -29,6 +29,7 @@ import { $submission_count, $source, $properties, + $custom_tag, $point, $coordinates, $latitude, @@ -52,17 +53,19 @@ fdescribe('exportCsv()', () => { [email]: OWNER_ROLE, }, jobs: { - [jobId]: { - name: 'Test job' - } - } + [jobId]: { + name: 'Test job', + }, + }, }; const testProperties = { name: 'Dinagat Islands', area: 3.08, }; + const loiId = 'loi100'; const pointLoi = { [$job_id]: 'job123', + [$custom_tag]: 'POINT_001', [$geometry]: { [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, }, @@ -74,43 +77,46 @@ fdescribe('exportCsv()', () => { geometry: {type: 'Point', coordinates: TestGeoPoint(10.1, 125.6)}, properties: testProperties, }; -// const polygonLoi = { -// [$job_id]: 'job123', -// [$geometry]: { -// [$polygon]: { -// [$shell]: { -// [$coordinates]: [ -// {[$latitude]: 0, [$longitude]: 100}, -// {[$latitude]: 0, [$longitude]: 101}, -// {[$latitude]: 1, [$longitude]: 101}, -// {[$latitude]: 0, [$longitude]: 100}, -// ], -// }, -// }, -// }, -// [$submission_count]: 0, -// [$source]: 1, // IMPORTED -// jobId: 'job123', -// predefined: true, -// geometry: { -// type: 'Polygon', -// coordinates: { -// 0: { -// 0: TestGeoPoint(0, 100), -// 1: TestGeoPoint(0, 101), -// 2: TestGeoPoint(1, 101), -// 3: TestGeoPoint(0, 100), -// }, -// }, -// }, -// }; - + // const polygonLoi = { + // [$job_id]: 'job123', + // [$geometry]: { + // [$polygon]: { + // [$shell]: { + // [$coordinates]: [ + // {[$latitude]: 0, [$longitude]: 100}, + // {[$latitude]: 0, [$longitude]: 101}, + // {[$latitude]: 1, [$longitude]: 101}, + // {[$latitude]: 0, [$longitude]: 100}, + // ], + // }, + // }, + // }, + // [$submission_count]: 0, + // [$source]: 1, // IMPORTED + // jobId: 'job123', + // predefined: true, + // geometry: { + // type: 'Polygon', + // coordinates: { + // 0: { + // 0: TestGeoPoint(0, 100), + // 1: TestGeoPoint(0, 101), + // 2: TestGeoPoint(1, 101), + // 3: TestGeoPoint(0, 100), + // }, + // }, + // }, + // }; + const testCases = [ { - desc: 'imports points', + desc: 'export points w/o submissions', input: {}, - expected: [pointLoi], - } + expected: [ + `"system:index","geometry","name","area","data:contributor_username","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"",""`, + ], + }, ]; beforeEach(() => { @@ -126,6 +132,7 @@ fdescribe('exportCsv()', () => { it(desc, async () => { // Add survey. mockFirestore.doc(`surveys/${surveyId}`).set(survey); + mockFirestore.doc(`surveys/${surveyId}/lois/${loiId}`).set(pointLoi); // Build mock request and response. const req = await createGetRequestSpy({ @@ -135,15 +142,21 @@ fdescribe('exportCsv()', () => { job: jobId, }, }); - const res = createResponseSpy(); + const chunks: string[] = []; + const res = createResponseSpy(chunks); // Run export CSV handler. - await exportCsvHandler(req, res, {email} as DecodedIdToken); + await exportCsvHandler(req, res, {email} as DecodedIdToken); // Check post-conditions. expect(res.status).toHaveBeenCalledOnceWith(HttpStatus.OK); expect(res.type).toHaveBeenCalledOnceWith('text/csv'); - expect(res.setHeader).toHaveBeenCalledOnceWith('Content-Disposition', 'attachment; filename=test-job.csv'); + expect(res.setHeader).toHaveBeenCalledOnceWith( + 'Content-Disposition', + 'attachment; filename=test-job.csv' + ); + const output = chunks.join().trim(); + expect(output.split('\n')).toEqual(expected); }) ); }); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index d875f62a2..d4ab4b2c5 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -103,16 +103,16 @@ export async function exportCsvHandler( loiDocs.forEach(loiDoc => { const loi = toMessage(loiDoc.data(), Pb.LocationOfInterest); - submissionsByLoi[loiDoc.id]?.forEach(submissionDict => { + const submissions = submissionsByLoi[loiDoc.id] || [{}]; + submissions.forEach(submissionDict => { try { const submission = toMessage(submissionDict, Pb.Submission); - if ( - loi instanceof Pb.LocationOfInterest && - loi.jobId && - submission instanceof Pb.Submission && - submission.jobId - ) { - writeRow(csvStream, loiProperties, tasks, loi, submission); + const isValidLoi = loi instanceof Pb.LocationOfInterest && !!loi.jobId; + const isValidSubmission = + Object.keys(submissionDict).length === 0 || + (submission instanceof Pb.Submission && !!submission.jobId); + if (isValidLoi && isValidSubmission) { + writeRow(csvStream, loiProperties, tasks, loi, submission as Pb.Submission); } else { // TODO(#1779): Delete once migration is complete. writeRowLegacy( diff --git a/functions/src/testing/http-test-helpers.ts b/functions/src/testing/http-test-helpers.ts index bc494b39b..760c7f496 100644 --- a/functions/src/testing/http-test-helpers.ts +++ b/functions/src/testing/http-test-helpers.ts @@ -40,7 +40,7 @@ export async function createGetRequestSpy( }); } -export function createResponseSpy(): functions.Response { +export function createResponseSpy(chunks?: string[]): functions.Response { const res = jasmine.createSpyObj>('response', [ 'send', 'status', @@ -50,12 +50,14 @@ export function createResponseSpy(): functions.Response { 'setHeader', 'on', 'once', - 'emit' + 'emit', + 'write' ]); res.status.and.callThrough(); res.end.and.callThrough(); - res.write.and.callFake((chunk: any) => { console.log("=====write "+chunk); return true;}); - res.on.and.callThrough(); - // res.write.and.callFake((event: any, listener: any) => { console.log("=====on "+event); return res;}); + res.write.and.callFake((chunk: any): boolean => { + chunks?.push(chunk.toString()); + return true; + }); return res; } diff --git a/lib/src/testing/proto-field-aliases.ts b/lib/src/testing/proto-field-aliases.ts index 5526df9f5..7d8ff2939 100644 --- a/lib/src/testing/proto-field-aliases.ts +++ b/lib/src/testing/proto-field-aliases.ts @@ -17,8 +17,8 @@ // Survey fields: export const [$title, $description] = [2, 3]; // LocationOfInterest fields: -export const [$job_id, $geometry, $submission_count, $source, $properties] = [ - 2, 3, 4, 9, 10, +export const [$job_id, $geometry, $submission_count, $custom_tag, $source, $properties] = [ + 2, 3, 4, 8, 9, 10, ]; // Geometry fields: export const [$point, $polygon, $multi_polygon] = [1, 2, 3]; From d367bd5424ce8134c54f3ce386c7cb900b2e6947 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 14:57:34 -0400 Subject: [PATCH 17/55] Remove TODO --- functions/src/export-csv.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index d4ab4b2c5..e4c9cd083 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -190,7 +190,6 @@ function writeRow( row.push(toWkt(loi.geometry)); // Header: One column for each loi property (merged over all properties across all LOIs) row.push(...getPropertiesByName(loi, loiProperties)); - // TODO(#1288): Clean up remaining references to old responses field const data = submission.taskData; // Header: One column for each task tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); @@ -354,13 +353,13 @@ function getPropertyNames(lois: QuerySnapshot): Set { function getPropertiesByName( loi: Pb.LocationOfInterest, - properties: Set -): List { + properties: Set +): List { // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. return List.of(...properties).map( prop => loi.properties[prop].stringValue || - loi.properties[prop].numericValue?.toString() || + loi.properties[prop].numericValue || '' ); } From e6b47ef785c035f148a6071ec678831405316254 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 15:02:20 -0400 Subject: [PATCH 18/55] Add test for property maps --- lib/src/firestore-to-proto.spec.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/src/firestore-to-proto.spec.ts b/lib/src/firestore-to-proto.spec.ts index c2c82b449..d99a42ed4 100644 --- a/lib/src/firestore-to-proto.spec.ts +++ b/lib/src/firestore-to-proto.spec.ts @@ -18,7 +18,7 @@ import {GroundProtos} from '@ground/proto'; import {toMessage} from './firestore-to-proto'; import {Constructor} from 'protobufjs'; -const {Coordinates, Job, LinearRing, Role, Style, Survey, Task} = +const {Coordinates, Job, LinearRing, Role, Style, Survey, Task, LocationOfInterest} = GroundProtos.google.ground.v1beta1; describe('toMessage()', () => { @@ -75,6 +75,21 @@ describe('toMessage()', () => { }, }), }, + { + desc: 'converts map', + input: { + '10': { + 'stringProperty': {'1': 'aaa'}, + 'numberProperty': {'2': 123.4}, + }, + }, + expected: new LocationOfInterest({ + properties: { + 'stringProperty': new LocationOfInterest.Property({stringValue: 'aaa'}), + 'numberProperty': new LocationOfInterest.Property({numericValue: 123.4}), + }, + }), + }, { desc: 'converts enum value', input: { From 5c4b00c075d7fd4930069e39f370d03242fc1637 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 15:05:41 -0400 Subject: [PATCH 19/55] Fix test data --- functions/src/export-csv.spec.ts | 7 ++++++- lib/src/testing/proto-field-aliases.ts | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 977ef51d7..3c8ff3c84 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -34,6 +34,8 @@ import { $coordinates, $latitude, $longitude, + $string_value, + $numeric_value, } from '@ground/lib/dist/testing/proto-field-aliases'; import {DecodedIdToken} from 'firebase-admin/auth'; import HttpStatus from 'http-status-codes'; @@ -71,7 +73,10 @@ fdescribe('exportCsv()', () => { }, [$submission_count]: 0, [$source]: 1, // IMPORTED - [$properties]: {name: 'Dinagat Islands', area: 3.08}, + [$properties]: { + name: {[$string_value]: 'Dinagat Islands'}, + area: {[$numeric_value]: 3.08}, + }, jobId: 'job123', predefined: true, geometry: {type: 'Point', coordinates: TestGeoPoint(10.1, 125.6)}, diff --git a/lib/src/testing/proto-field-aliases.ts b/lib/src/testing/proto-field-aliases.ts index 7d8ff2939..aa348f894 100644 --- a/lib/src/testing/proto-field-aliases.ts +++ b/lib/src/testing/proto-field-aliases.ts @@ -38,3 +38,5 @@ export const [$color] = [1]; export const [$required, $level, $textQuestion] = [4, 5, 7]; // DateTimeQuestion: export const [$dtq$type] = [1]; +// Property fields: +export const [$string_value, $numeric_value] = [1, 2]; From 2bdf354ee034795995162874d26bccf8f0ac9e39 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 15:41:32 -0400 Subject: [PATCH 20/55] Quote columns, remove legacy support --- functions/src/export-csv.spec.ts | 4 +- functions/src/export-csv.ts | 140 +++++++------------------------ 2 files changed, 30 insertions(+), 114 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 3c8ff3c84..c4980f0f0 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -118,7 +118,7 @@ fdescribe('exportCsv()', () => { desc: 'export points w/o submissions', input: {}, expected: [ - `"system:index","geometry","name","area","data:contributor_username","data:contributor_email"`, + `"system:index","geometry","name","area","data:contributor_name","data:contributor_email"`, `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"",""`, ], }, @@ -160,7 +160,7 @@ fdescribe('exportCsv()', () => { 'Content-Disposition', 'attachment; filename=test-job.csv' ); - const output = chunks.join().trim(); + const output = chunks.join('').trim(); expect(output.split('\n')).toEqual(expected); }) ); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index e4c9cd083..2bf336c05 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -20,7 +20,6 @@ import {canExport} from './common/auth'; import {geojsonToWKT} from '@terraformer/wkt'; import {getDatastore} from './common/context'; import * as HttpStatus from 'http-status-codes'; -import {Datastore} from './common/datastore'; import {DecodedIdToken} from 'firebase-admin/auth'; import {List} from 'immutable'; import {DocumentData, QuerySnapshot} from 'firebase-admin/firestore'; @@ -42,15 +41,9 @@ type Task = { readonly hasOtherOption?: boolean; }; -type Dict = {[key: string]: any}; - /** A dictionary of submissions values (array) keyed by loi ID. */ type SubmissionDict = {[key: string]: any[]}; -// TODO(#1277): Use a shared model with web -type LoiDocument = - FirebaseFirestore.QueryDocumentSnapshot; - /** * Iterates over all LOIs and submissions in a job, joining them * into a single table written to the response as a quote CSV file. @@ -92,10 +85,9 @@ export async function exportCsvHandler( const csvStream = csv.format({ delimiter: ',', headers, - includeEndRowDelimiter: true, rowDelimiter: '\n', - quoteColumns: true, - quote: '"', + includeEndRowDelimiter: true, // Add \n to last row in CSV + quote: false }); csvStream.pipe(res); @@ -103,26 +95,17 @@ export async function exportCsvHandler( loiDocs.forEach(loiDoc => { const loi = toMessage(loiDoc.data(), Pb.LocationOfInterest); + if (loi instanceof Error) { + throw loi; + } const submissions = submissionsByLoi[loiDoc.id] || [{}]; submissions.forEach(submissionDict => { try { const submission = toMessage(submissionDict, Pb.Submission); - const isValidLoi = loi instanceof Pb.LocationOfInterest && !!loi.jobId; - const isValidSubmission = - Object.keys(submissionDict).length === 0 || - (submission instanceof Pb.Submission && !!submission.jobId); - if (isValidLoi && isValidSubmission) { - writeRow(csvStream, loiProperties, tasks, loi, submission as Pb.Submission); - } else { - // TODO(#1779): Delete once migration is complete. - writeRowLegacy( - csvStream, - loiProperties, - tasks, - loiDoc, - submissionDict - ); + if (submission instanceof Error) { + throw submission; } + writeRow(csvStream, loiProperties, tasks, loi, submission); } catch (e) { console.debug('Skipping row', e); } @@ -141,9 +124,9 @@ function getHeaders( headers.push('geometry'); headers.push(...loiProperties); tasks.forEach(task => headers.push('data:' + task.label)); - headers.push('data:contributor_username'); + headers.push('data:contributor_name'); headers.push('data:contributor_email'); - return headers; + return headers.map(quote); } /** @@ -181,71 +164,35 @@ function writeRow( console.debug('Writing data using new schema'); const row = []; // Header: system:index - row.push(loi.customTag || ''); + row.push(quote(loi.customTag)); // Header: geometry if (!loi.geometry) { console.debug(`Skipping LOI ${loi.id} - missing geometry`); return; } - row.push(toWkt(loi.geometry)); + row.push(quote(toWkt(loi.geometry))); // Header: One column for each loi property (merged over all properties across all LOIs) - row.push(...getPropertiesByName(loi, loiProperties)); + getPropertiesByName(loi, loiProperties).forEach(v => row.push(quote(v))); const data = submission.taskData; // Header: One column for each task - tasks.forEach((task, taskId) => row.push(getValue(taskId, task, data))); - // Header: contributor_username, contributor_email - row.push(submission.created?.displayName || ''); - row.push(submission.created?.emailAddress || ''); - csvStream.write(row); -} - -// TODO(#1779): Delete once migration is complete. -function writeRowLegacy( - csvStream: csv.CsvFormatterStream, - loiProperties: Set, - tasks: Map, - loiDoc: LoiDocument, - submission: SubmissionDict -) { - const row = []; - // Header: system:index - row.push(loiDoc.get('properties')?.id || ''); - // Header: geometry - row.push(legacyFirestoreMapToWkt(loiDoc.get('geometry')) || ''); - // Header: One column for each loi property (merged over all properties across all LOIs) - row.push(...getPropertiesByNameLegacy(loiDoc, loiProperties)); - // TODO(#1288): Clean up remaining references to old responses field - const data = - submission['data'] || - submission['responses'] || - submission['results'] || - {}; - // Header: One column for each task - tasks.forEach((task, taskId) => row.push(getValueLegacy(taskId, task, data))); + tasks.forEach((task, taskId) => + row.push(quote(getValue(taskId, task, data))) + ); // Header: contributor_username, contributor_email - const contributor = submission['created'] - ? (submission['created'] as Dict)['user'] - : []; - row.push(contributor['displayName'] || ''); - row.push(contributor['email'] || ''); + row.push(quote(submission.created?.displayName)); + row.push(quote(submission.created?.emailAddress)); csvStream.write(row); } -/** - * Returns the WKT string converted from the given geometry object. - * - * @param geometryObject the geometry object extracted from the LOI. This should have format: - * { - * coordinates: any[], - * type: string - * } - * @returns The WKT string version of the object - * https://www.vertica.com/docs/9.3.x/HTML/Content/Authoring/AnalyzingData/Geospatial/Spatial_Definitions/WellknownTextWKT.htm - * - * @beta - */ -function legacyFirestoreMapToWkt(geometryObject: any): string { - return geojsonToWKT(Datastore.fromFirestoreMap(geometryObject)); +function quote(value: any | null | undefined): string { + if (value === null || value === undefined) { + return '""'; + } + if (typeof(value) === 'number') { + return value.toString(); + } + const escaped = value.toString().replace('"', '""'); + return '"' + escaped + '"'; } function toWkt(geometry: Pb.IGeometry): string { @@ -255,7 +202,7 @@ function toWkt(geometry: Pb.IGeometry): string { /** * Returns the string representation of a specific task element result. */ -function getValue(taskId: string, task: Task, data: Pb.ITaskData[]) { +function getValue(taskId: string, task: Task, data: Pb.ITaskData[]): any { const result = data.find(d => d.taskId === taskId); if (result?.multipleChoiceResponses) { return result.multipleChoiceResponses?.selectedOptionIds @@ -277,27 +224,6 @@ function getValue(taskId: string, task: Task, data: Pb.ITaskData[]) { } } -/** - * Returns the string representation of a specific task element result. - */ -function getValueLegacy(taskId: string, task: Task, data: any) { - const result = data[taskId] || ''; - if ( - task.type === 'multiple_choice' && - Array.isArray(result) && - task.options - ) { - return result.map(id => getMultipleChoiceValues(id, task)).join(', '); - } else if (task.type === 'capture_location') { - if (!result) { - return ''; - } - return legacyFirestoreMapToWkt(result.geometry || result); - } else { - return result; - } -} - /** * Returns the code associated with a specified multiple choice option, or if * the code is not defined, returns the label in English. @@ -363,13 +289,3 @@ function getPropertiesByName( '' ); } - -function getPropertiesByNameLegacy( - loiDoc: LoiDocument, - loiProperties: Set -): List { - // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. - return List.of(...loiProperties).map( - prop => (loiDoc.get('properties') || {})[prop] || '' - ); -} From daa54bdf1f5c0ea74d3332316d77db90bf979365 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 16:26:16 -0400 Subject: [PATCH 21/55] Test multiple LOIs, fix null properties --- functions/src/export-csv.spec.ts | 50 ++++++++++++++++++++------------ functions/src/export-csv.ts | 29 +++++++----------- lib/src/proto-field-numbers.ts | 3 +- 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index c4980f0f0..8c32adf81 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -15,7 +15,6 @@ */ import { - TestGeoPoint, createMockFirestore, stubAdminApi, } from '@ground/lib/dist/testing/firestore'; @@ -60,12 +59,8 @@ fdescribe('exportCsv()', () => { }, }, }; - const testProperties = { - name: 'Dinagat Islands', - area: 3.08, - }; - const loiId = 'loi100'; - const pointLoi = { + const pointLoi1 = { + id: 'loi100', [$job_id]: 'job123', [$custom_tag]: 'POINT_001', [$geometry]: { @@ -76,12 +71,21 @@ fdescribe('exportCsv()', () => { [$properties]: { name: {[$string_value]: 'Dinagat Islands'}, area: {[$numeric_value]: 3.08}, - }, - jobId: 'job123', - predefined: true, - geometry: {type: 'Point', coordinates: TestGeoPoint(10.1, 125.6)}, - properties: testProperties, + } }; + const pointLoi2 = { + id: 'loi200', + [$job_id]: 'job123', + [$custom_tag]: 'POINT_002', + [$geometry]: { + [$point]: {[$coordinates]: {[$latitude]: 47.05, [$longitude]: 8.30}}, + }, + [$submission_count]: 0, + [$source]: 2, // FIELD_DATA + [$properties]: { + name: {[$string_value]: 'Luzern'}, + } + }; // const polygonLoi = { // [$job_id]: 'job123', // [$geometry]: { @@ -116,10 +120,12 @@ fdescribe('exportCsv()', () => { const testCases = [ { desc: 'export points w/o submissions', - input: {}, + lois: [pointLoi1, pointLoi2], + submissions: [], expected: [ `"system:index","geometry","name","area","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"",""`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,`, ], }, ]; @@ -133,11 +139,18 @@ fdescribe('exportCsv()', () => { resetDatastore(); }); - testCases.forEach(({desc, input, expected}) => + testCases.forEach(({desc, lois, submissions, expected}) => it(desc, async () => { - // Add survey. + // Populate database. mockFirestore.doc(`surveys/${surveyId}`).set(survey); - mockFirestore.doc(`surveys/${surveyId}/lois/${loiId}`).set(pointLoi); + lois?.forEach(loi => + mockFirestore.doc(`surveys/${surveyId}/lois/${loi.id}`).set(loi) + ); + // submissions?.forEach(submission => + // mockFirestore + // .doc(`surveys/${surveyId}/submissions/${submissions.id}`) + // .set(submission) + // ); // Build mock request and response. const req = await createGetRequestSpy({ @@ -161,7 +174,8 @@ fdescribe('exportCsv()', () => { 'attachment; filename=test-job.csv' ); const output = chunks.join('').trim(); - expect(output.split('\n')).toEqual(expected); + const lines = output.split('\n'); + expect(lines).toEqual(expected); }) ); }); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 2bf336c05..08d7bbe35 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -23,7 +23,7 @@ import * as HttpStatus from 'http-status-codes'; import {DecodedIdToken} from 'firebase-admin/auth'; import {List} from 'immutable'; import {DocumentData, QuerySnapshot} from 'firebase-admin/firestore'; -import {toMessage} from '@ground/lib'; +import {FieldNumbers, toMessage} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; import {toGeoJsonGeometry} from '@ground/lib'; @@ -87,7 +87,7 @@ export async function exportCsvHandler( headers, rowDelimiter: '\n', includeEndRowDelimiter: true, // Add \n to last row in CSV - quote: false + quote: false, }); csvStream.pipe(res); @@ -145,7 +145,7 @@ async function getSubmissionsByLoi( const submissions = await db.fetchSubmissionsByJobId(surveyId, jobId); const submissionsByLoi: {[name: string]: any[]} = {}; submissions.forEach(submission => { - const loiId = submission.get('loiId') as string; + const loiId = submission.get(FieldNumbers.Submission.loi_id) as string; const arr: any[] = submissionsByLoi[loiId] || []; arr.push(submission.data()); submissionsByLoi[loiId] = arr; @@ -160,8 +160,6 @@ function writeRow( loi: Pb.LocationOfInterest, submission: Pb.Submission ) { - // TODO(#1779): Remove once migration is complete. - console.debug('Writing data using new schema'); const row = []; // Header: system:index row.push(quote(loi.customTag)); @@ -184,11 +182,11 @@ function writeRow( csvStream.write(row); } -function quote(value: any | null | undefined): string { +function quote(value: any): string { if (value === null || value === undefined) { - return '""'; + return ''; } - if (typeof(value) === 'number') { + if (typeof value === 'number') { return value.toString(); } const escaped = value.toString().replace('"', '""'); @@ -269,9 +267,7 @@ function getPropertyNames(lois: QuerySnapshot): Set { return new Set( lois.docs .map(loi => - Object.keys(loi.get('properties') || {}) - // Don't retrieve ID because we already store it in a separate column - .filter(prop => prop !== 'id') + Object.keys(loi.get(FieldNumbers.LocationOfInterest.properties) || {}) ) .flat() ); @@ -280,12 +276,9 @@ function getPropertyNames(lois: QuerySnapshot): Set { function getPropertiesByName( loi: Pb.LocationOfInterest, properties: Set -): List { +): List { // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty. - return List.of(...properties).map( - prop => - loi.properties[prop].stringValue || - loi.properties[prop].numericValue || - '' - ); + return List.of(...properties) + .map(prop => loi.properties[prop]) + .map(value => value?.stringValue || value?.numericValue || null); } diff --git a/lib/src/proto-field-numbers.ts b/lib/src/proto-field-numbers.ts index acfd53f18..bfc1a5605 100644 --- a/lib/src/proto-field-numbers.ts +++ b/lib/src/proto-field-numbers.ts @@ -18,7 +18,8 @@ export const FieldNumbers = { LocationOfInterest: { job_id: '2', owner_id: '5', - source: '9' + source: '9', + properties: '10' }, Submission: { loi_id: '2', From a1edb1adbb4eeb7f46f2af23e819c896e3bbdb03 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 17:11:55 -0400 Subject: [PATCH 22/55] Start adding tests for submissions --- functions/src/export-csv.spec.ts | 125 ++++++++++++++++++------------- functions/src/export-csv.ts | 1 + 2 files changed, 72 insertions(+), 54 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 8c32adf81..8f1e66fc7 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -45,23 +45,45 @@ import {exportCsvHandler} from './export-csv'; fdescribe('exportCsv()', () => { let mockFirestore: Firestore; - const surveyId = 'survey001'; const jobId = 'job123'; const email = 'somebody@test.it'; - const survey = { - name: 'Test survey', + // TODO(#1758): Use new proto-based survey and job representation. + const survey1 = { + id: 'survey001', + name: 'Test survey 1', + acl: { + [email]: OWNER_ROLE, + } + }; + const survey2 = { + id: 'survey002', + name: 'Test survey 2', acl: { [email]: OWNER_ROLE, }, jobs: { [jobId]: { name: 'Test job', + tasks: { + task001: { + type: 'text_field', + label: 'What is the meaning of life?', + }, + task002: { + type: 'capture_location', + label: 'Where are you now?', + }, + task003: { + type: 'draw_area', + label: 'Delimit plot boundaries', + }, + }, }, }, }; const pointLoi1 = { id: 'loi100', - [$job_id]: 'job123', + [$job_id]: jobId, [$custom_tag]: 'POINT_001', [$geometry]: { [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, @@ -71,63 +93,58 @@ fdescribe('exportCsv()', () => { [$properties]: { name: {[$string_value]: 'Dinagat Islands'}, area: {[$numeric_value]: 3.08}, - } + }, }; const pointLoi2 = { id: 'loi200', - [$job_id]: 'job123', + [$job_id]: jobId, [$custom_tag]: 'POINT_002', [$geometry]: { - [$point]: {[$coordinates]: {[$latitude]: 47.05, [$longitude]: 8.30}}, + [$point]: {[$coordinates]: {[$latitude]: 47.05, [$longitude]: 8.3}}, }, [$submission_count]: 0, [$source]: 2, // FIELD_DATA [$properties]: { name: {[$string_value]: 'Luzern'}, - } - }; - // const polygonLoi = { - // [$job_id]: 'job123', - // [$geometry]: { - // [$polygon]: { - // [$shell]: { - // [$coordinates]: [ - // {[$latitude]: 0, [$longitude]: 100}, - // {[$latitude]: 0, [$longitude]: 101}, - // {[$latitude]: 1, [$longitude]: 101}, - // {[$latitude]: 0, [$longitude]: 100}, - // ], - // }, - // }, - // }, - // [$submission_count]: 0, - // [$source]: 1, // IMPORTED - // jobId: 'job123', - // predefined: true, - // geometry: { - // type: 'Polygon', - // coordinates: { - // 0: { - // 0: TestGeoPoint(0, 100), - // 1: TestGeoPoint(0, 101), - // 2: TestGeoPoint(1, 101), - // 3: TestGeoPoint(0, 100), - // }, - // }, - // }, - // }; - + }, + }; + const submission1a = { + id: '001a', + // TODO + }; + const submission1b = { + id: '001b', + // TODO + }; + const submission2a = { + id: '002a', + // TODO + }; const testCases = [ { desc: 'export points w/o submissions', + survey: survey1, lois: [pointLoi1, pointLoi2], submissions: [], - expected: [ + expectedFilename: 'ground-export.csv', + expectedCsv: [ `"system:index","geometry","name","area","data:contributor_name","data:contributor_email"`, `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,`, `"POINT_002","POINT (8.3 47.05)","Luzern",,,`, ], }, + { + desc: 'export points w/submissions', + survey: survey2, + lois: [pointLoi1, pointLoi2], + submissions: [submission1a, submission1b, submission2a], + expectedFilename: 'test-job.csv', + expectedCsv: [ + `"system:index","geometry","name","area","data:What is the meaning of life?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,,,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,,`, + ], + }, ]; beforeEach(() => { @@ -139,24 +156,24 @@ fdescribe('exportCsv()', () => { resetDatastore(); }); - testCases.forEach(({desc, lois, submissions, expected}) => + testCases.forEach(({desc, survey, lois, submissions, expectedFilename, expectedCsv}) => it(desc, async () => { // Populate database. - mockFirestore.doc(`surveys/${surveyId}`).set(survey); - lois?.forEach(loi => - mockFirestore.doc(`surveys/${surveyId}/lois/${loi.id}`).set(loi) + mockFirestore.doc(`surveys/${survey.id}`).set(survey); + lois?.forEach(({id, ...loi}) => + mockFirestore.doc(`surveys/${survey.id}/lois/${id}`).set(loi) + ); + submissions?.forEach(({id, ...submission}) => + mockFirestore + .doc(`surveys/${survey.id}/submissions/${id}`) + .set(submission) ); - // submissions?.forEach(submission => - // mockFirestore - // .doc(`surveys/${surveyId}/submissions/${submissions.id}`) - // .set(submission) - // ); // Build mock request and response. const req = await createGetRequestSpy({ url: '/exportCsv', query: { - survey: surveyId, + survey: survey.id, job: jobId, }, }); @@ -171,11 +188,11 @@ fdescribe('exportCsv()', () => { expect(res.type).toHaveBeenCalledOnceWith('text/csv'); expect(res.setHeader).toHaveBeenCalledOnceWith( 'Content-Disposition', - 'attachment; filename=test-job.csv' + `attachment; filename=${expectedFilename}` ); const output = chunks.join('').trim(); - const lines = output.split('\n'); - expect(lines).toEqual(expected); + const lines = output.split('\n'); + expect(lines).toEqual(expectedCsv); }) ); }); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 08d7bbe35..7b2b5b84f 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -123,6 +123,7 @@ function getHeaders( headers.push('system:index'); headers.push('geometry'); headers.push(...loiProperties); + // TODO(#1936): Use `index` field to export columns in correct order. tasks.forEach(task => headers.push('data:' + task.label)); headers.push('data:contributor_name'); headers.push('data:contributor_email'); From 97fa6526d4b8b423096b9028adf6c91bb6551999 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Wed, 31 Jul 2024 17:30:46 -0400 Subject: [PATCH 23/55] Add method to get field ids for a proto message class --- lib/src/message-registry.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/src/message-registry.ts b/lib/src/message-registry.ts index 029341d45..38b346376 100644 --- a/lib/src/message-registry.ts +++ b/lib/src/message-registry.ts @@ -72,6 +72,20 @@ export class MessageRegistry { return typePath ? this.getDescriptorByPath(typePath) : null; } + /** + * Returns a dictionary containing the numbers of all fields in a + * message definition, keyed by field name. Throws an error if not found. + */ + getFieldIds(constructor: any): {[key: string]: string} { + const desc = this.getMessageDescriptor(constructor); + if (!desc) throw new Error(`Unknown constructor ${constructor.name}`); + const map: {[key: string]: string} = {}; + if (desc.fields) { + Object.keys(desc.fields).forEach(name => map[name] = desc.fields![name].id?.toString()); + } + return map; + } + /** * Searches the descriptor hierarchy starting for the specified `typeName`, * starting from the specified containing `messageTypePath`. If the type From 29900bd1134d6545e2e5d60cf9e5181d05b25ea0 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 14:48:59 -0400 Subject: [PATCH 24/55] Test field registry --- functions/src/import-geojson.spec.ts | 31 ++++++++++++++++------------ lib/src/index.ts | 1 + 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index 01bd51234..799c3ad08 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -46,6 +46,10 @@ import {invokeCallbackAsync} from './handlers'; import {OWNER_ROLE} from './common/auth'; import {resetDatastore} from './common/context'; import {Firestore} from 'firebase-admin/firestore'; +import {registry} from '@ground/lib'; +import {GroundProtos} from '@ground/proto'; + +import Pb = GroundProtos.google.ground.v1beta1; describe('importGeoJson()', () => { let mockFirestore: Firestore; @@ -75,14 +79,15 @@ describe('importGeoJson()', () => { }, ], }; + const l = registry.getFieldIds(Pb.LocationOfInterest); const pointLoi = { - [$job_id]: 'job123', - [$geometry]: { + [l.jobId]: 'job123', + [l.geometry]: { [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, }, - [$submission_count]: 0, - [$source]: 1, // IMPORTED - [$properties]: {name: 'Dinagat Islands', area: 3.08}, + [l.submission_count]: 0, + [l.source]: 1, // IMPORTED + [l.properties]: {name: 'Dinagat Islands', area: 3.08}, jobId: 'job123', predefined: true, geometry: {type: 'Point', coordinates: TestGeoPoint(10.1, 125.6)}, @@ -108,8 +113,8 @@ describe('importGeoJson()', () => { ], }; const polygonLoi = { - [$job_id]: 'job123', - [$geometry]: { + [l.job_id]: 'job123', + [l.geometry]: { [$polygon]: { [$shell]: { [$coordinates]: [ @@ -121,8 +126,8 @@ describe('importGeoJson()', () => { }, }, }, - [$submission_count]: 0, - [$source]: 1, // IMPORTED + [l.submission_count]: 0, + [l.source]: 1, // IMPORTED jobId: 'job123', predefined: true, geometry: { @@ -167,8 +172,8 @@ describe('importGeoJson()', () => { ], }; const multiPolygonLoi = { - [$job_id]: 'job123', - [$geometry]: { + [l.job_id]: 'job123', + [l.geometry]: { [$multi_polygon]: { [$polygons]: [ // polygons[0] @@ -196,8 +201,8 @@ describe('importGeoJson()', () => { ], }, }, - [$submission_count]: 0, - [$source]: 1, // IMPORTED + [l.submissionCount]: 0, + [l.source]: 1, // IMPORTED jobId: 'job123', predefined: true, geometry: { diff --git a/lib/src/index.ts b/lib/src/index.ts index e2e4e3dc4..ff55a9721 100644 --- a/lib/src/index.ts +++ b/lib/src/index.ts @@ -18,3 +18,4 @@ export {toDocumentData} from './proto-to-firestore'; export {toMessage} from './firestore-to-proto'; export {deleteEmpty, isEmpty} from './obj-util'; export {FieldNumbers} from './proto-field-numbers'; +export {registry} from './message-registry'; \ No newline at end of file From 834a7af209dffa8dafcf50f92795f6a3934776ed Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 14:52:51 -0400 Subject: [PATCH 25/55] Use field registry --- functions/src/import-geojson.spec.ts | 31 ++++++++++++---------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index 799c3ad08..580ba9614 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -24,14 +24,6 @@ import { createResponseSpy, } from './testing/http-test-helpers'; import { - $job_id, - $geometry, - $submission_count, - $source, - $properties, - $point, - $polygon, - $multi_polygon, $shell, $coordinates, $polygons, @@ -80,12 +72,15 @@ describe('importGeoJson()', () => { ], }; const l = registry.getFieldIds(Pb.LocationOfInterest); + const g = registry.getFieldIds(Pb.Geometry); + const p = registry.getFieldIds(Pb.Point); + const c = registry.getFieldIds(Pb.Coordinates); const pointLoi = { [l.jobId]: 'job123', [l.geometry]: { - [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, + [g.point]: {[p.coordinates]: {[c.latitude]: 10.1, [c.longitude]: 125.6}}, }, - [l.submission_count]: 0, + [l.submissionCount]: 0, [l.source]: 1, // IMPORTED [l.properties]: {name: 'Dinagat Islands', area: 3.08}, jobId: 'job123', @@ -113,20 +108,20 @@ describe('importGeoJson()', () => { ], }; const polygonLoi = { - [l.job_id]: 'job123', + [l.jobId]: 'job123', [l.geometry]: { - [$polygon]: { + [g.polygon]: { [$shell]: { [$coordinates]: [ - {[$latitude]: 0, [$longitude]: 100}, - {[$latitude]: 0, [$longitude]: 101}, - {[$latitude]: 1, [$longitude]: 101}, - {[$latitude]: 0, [$longitude]: 100}, + {[c.latitude]: 0, [c.longitude]: 100}, + {[c.latitude]: 0, [c.longitude]: 101}, + {[c.latitude]: 1, [c.longitude]: 101}, + {[c.latitude]: 0, [c.longitude]: 100}, ], }, }, }, - [l.submission_count]: 0, + [l.submissionCount]: 0, [l.source]: 1, // IMPORTED jobId: 'job123', predefined: true, @@ -174,7 +169,7 @@ describe('importGeoJson()', () => { const multiPolygonLoi = { [l.job_id]: 'job123', [l.geometry]: { - [$multi_polygon]: { + [g.multiPolygon]: { [$polygons]: [ // polygons[0] { From 82e3c21a9776ff0b035a7cb95549b8194807d6b9 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 14:55:36 -0400 Subject: [PATCH 26/55] Fix id --- functions/src/import-geojson.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index 580ba9614..90c9884de 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -167,7 +167,7 @@ describe('importGeoJson()', () => { ], }; const multiPolygonLoi = { - [l.job_id]: 'job123', + [l.jobId]: 'job123', [l.geometry]: { [g.multiPolygon]: { [$polygons]: [ From 64b8bd8f04b17cf31ada283a86bdea613c25505d Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:04:33 -0400 Subject: [PATCH 27/55] format tsconfig --- lib/tsconfig.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/tsconfig.json b/lib/tsconfig.json index a2a613f74..77b852216 100644 --- a/lib/tsconfig.json +++ b/lib/tsconfig.json @@ -14,8 +14,9 @@ }, "compileOnSave": true, "include": [ - "src/**/*" -, "../functions/src/testing/http.ts" ], + "src/**/*", + "../functions/src/testing/http.ts" + ], "exclude": [ "src/generated" ] From 17c69b407f2ce657cdcd749755b38d3af0661705 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:39:42 -0400 Subject: [PATCH 28/55] Replace aliases with registry --- functions/src/import-geojson.spec.ts | 48 +++++++++----------- lib/src/proto-to-firestore.spec.ts | 62 ++++++++++++-------------- lib/src/testing/proto-field-aliases.ts | 40 ----------------- lib/tsconfig.json | 6 +-- 4 files changed, 54 insertions(+), 102 deletions(-) delete mode 100644 lib/src/testing/proto-field-aliases.ts diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index 90c9884de..330e13d61 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -23,13 +23,6 @@ import { createPostRequestSpy, createResponseSpy, } from './testing/http-test-helpers'; -import { - $shell, - $coordinates, - $polygons, - $latitude, - $longitude, -} from '@ground/lib/dist/testing/proto-field-aliases'; import {importGeoJsonCallback} from './import-geojson'; import {DecodedIdToken} from 'firebase-admin/auth'; import {Blob, FormData} from 'formdata-node'; @@ -42,6 +35,13 @@ import {registry} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; import Pb = GroundProtos.google.ground.v1beta1; +const l = registry.getFieldIds(Pb.LocationOfInterest); +const g = registry.getFieldIds(Pb.Geometry); +const p = registry.getFieldIds(Pb.Point); +const c = registry.getFieldIds(Pb.Coordinates); +const pg = registry.getFieldIds(Pb.Polygon); +const lr = registry.getFieldIds(Pb.LinearRing); +const mp = registry.getFieldIds(Pb.MultiPolygon); describe('importGeoJson()', () => { let mockFirestore: Firestore; @@ -71,10 +71,6 @@ describe('importGeoJson()', () => { }, ], }; - const l = registry.getFieldIds(Pb.LocationOfInterest); - const g = registry.getFieldIds(Pb.Geometry); - const p = registry.getFieldIds(Pb.Point); - const c = registry.getFieldIds(Pb.Coordinates); const pointLoi = { [l.jobId]: 'job123', [l.geometry]: { @@ -111,8 +107,8 @@ describe('importGeoJson()', () => { [l.jobId]: 'job123', [l.geometry]: { [g.polygon]: { - [$shell]: { - [$coordinates]: [ + [pg.shell]: { + [lr.coordinates]: [ {[c.latitude]: 0, [c.longitude]: 100}, {[c.latitude]: 0, [c.longitude]: 101}, {[c.latitude]: 1, [c.longitude]: 101}, @@ -170,26 +166,26 @@ describe('importGeoJson()', () => { [l.jobId]: 'job123', [l.geometry]: { [g.multiPolygon]: { - [$polygons]: [ + [mp.polygons]: [ // polygons[0] { - [$shell]: { - [$coordinates]: [ - {[$latitude]: 0, [$longitude]: 100}, - {[$latitude]: 0, [$longitude]: 101}, - {[$latitude]: 1, [$longitude]: 101}, - {[$latitude]: 0, [$longitude]: 100}, + [pg.shell]: { + [lr.coordinates]: [ + {[c.latitude]: 0, [c.longitude]: 100}, + {[c.latitude]: 0, [c.longitude]: 101}, + {[c.latitude]: 1, [c.longitude]: 101}, + {[c.latitude]: 0, [c.longitude]: 100}, ], }, }, // polygons[1] { - [$shell]: { - [$coordinates]: [ - {[$latitude]: 1, [$longitude]: 120}, - {[$latitude]: 1, [$longitude]: 121}, - {[$latitude]: 2, [$longitude]: 121}, - {[$latitude]: 1, [$longitude]: 120}, + [pg.shell]: { + [lr.coordinates]: [ + {[c.latitude]: 1, [c.longitude]: 120}, + {[c.latitude]: 1, [c.longitude]: 121}, + {[c.latitude]: 2, [c.longitude]: 121}, + {[c.latitude]: 1, [c.longitude]: 120}, ], }, }, diff --git a/lib/src/proto-to-firestore.spec.ts b/lib/src/proto-to-firestore.spec.ts index c87514116..5e3148d96 100644 --- a/lib/src/proto-to-firestore.spec.ts +++ b/lib/src/proto-to-firestore.spec.ts @@ -14,22 +14,18 @@ * limitations under the License. */ +import {registry} from './message-registry'; import {GroundProtos} from '@ground/proto'; import {toDocumentData} from './proto-to-firestore'; -import { - $title, - $description, - $style, - $coordinates, - $latitude, - $longitude, - $color, - $index, - $dtq$type, - $required, - $level, - $textQuestion, -} from './testing/proto-field-aliases'; + +import Pb = GroundProtos.google.ground.v1beta1; +const s = registry.getFieldIds(Pb.Survey); +const j = registry.getFieldIds(Pb.Job); +const c = registry.getFieldIds(Pb.Coordinates); +const lr = registry.getFieldIds(Pb.LinearRing); +const t = registry.getFieldIds(Pb.Task); +const st = registry.getFieldIds(Pb.Style); +const dtq = registry.getFieldIds(Pb.Task.DateTimeQuestion); const {Job, Role, Style, Survey, Task, LinearRing, Coordinates} = GroundProtos.google.ground.v1beta1; @@ -43,8 +39,8 @@ describe('toDocumentData()', () => { description: 'Survey desc', }), expected: { - [$title]: 'Survey name', - [$description]: 'Survey desc', + [s.name]: 'Survey name', + [s.description]: 'Survey desc', }, }, { @@ -57,10 +53,10 @@ describe('toDocumentData()', () => { ], }), expected: { - [$coordinates]: [ - {[$latitude]: 5, [$longitude]: 7}, - {[$latitude]: 12, [$longitude]: 23}, - {[$latitude]: 9, [$longitude]: 2}, + [lr.coordinates]: [ + {[c.latitude]: 5, [c.longitude]: 7}, + {[c.latitude]: 12, [c.longitude]: 23}, + {[c.latitude]: 9, [c.longitude]: 2}, ], }, }, @@ -70,8 +66,8 @@ describe('toDocumentData()', () => { style: new Style({color: '#112233'}), }), expected: { - [$index]: 0, - [$style]: {[$color]: '#112233'}, + [j.index]: 0, + [j.style]: {[st.color]: '#112233'}, }, }, { @@ -83,7 +79,7 @@ describe('toDocumentData()', () => { }, }), expected: { - '4': { + [s.acl]: { email1: 2, // DATA_COLLECTOR email2: 3, // SURVEY_ORGANIZER }, @@ -95,7 +91,7 @@ describe('toDocumentData()', () => { type: Task.DateTimeQuestion.Type.BOTH_DATE_AND_TIME, }), expected: { - '1': 3, + [dtq.type]: 3, }, }, { @@ -104,7 +100,7 @@ describe('toDocumentData()', () => { type: Task.DateTimeQuestion.Type.TYPE_UNSPECIFIED, }), expected: { - [$dtq$type]: 0, // UNSPECIFIED + [dtq.type]: 0, // UNSPECIFIED }, }, { @@ -117,10 +113,10 @@ describe('toDocumentData()', () => { ], }), expected: { - [$coordinates]: [ - {[$latitude]: 5, [$longitude]: 7}, - {[$latitude]: 12, [$longitude]: 23}, - {[$latitude]: 9, [$longitude]: 2}, + [lr.coordinates]: [ + {[c.latitude]: 5, [c.longitude]: 7}, + {[c.latitude]: 12, [c.longitude]: 23}, + {[c.latitude]: 9, [c.longitude]: 2}, ], }, }, @@ -132,10 +128,10 @@ describe('toDocumentData()', () => { }), }), expected: { - [$index]: 0, - [$required]: false, - [$level]: 0, - [$textQuestion]: {'1': 1}, + [t.index]: 0, + [t.required]: false, + [t.level]: 0, + [t.textQuestion]: {'1': 1}, }, }, ].forEach(({desc, input, expected}) => diff --git a/lib/src/testing/proto-field-aliases.ts b/lib/src/testing/proto-field-aliases.ts deleted file mode 100644 index 5526df9f5..000000000 --- a/lib/src/testing/proto-field-aliases.ts +++ /dev/null @@ -1,40 +0,0 @@ -/** - * Copyright 2024 The Ground Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// Survey fields: -export const [$title, $description] = [2, 3]; -// LocationOfInterest fields: -export const [$job_id, $geometry, $submission_count, $source, $properties] = [ - 2, 3, 4, 9, 10, -]; -// Geometry fields: -export const [$point, $polygon, $multi_polygon] = [1, 2, 3]; -// Polygon fields: -export const [$shell] = [1]; -// LinearRing fields: -export const [$coordinates] = [1]; -// MultiPolygon fields: -export const [$polygons] = [1]; -// Coordinates fields: -export const [$latitude, $longitude] = [1, 2]; -// Job fields: -export const [$index, $style] = [2, 4]; -// Style fields: -export const [$color] = [1]; -// Task fields: -export const [$required, $level, $textQuestion] = [4, 5, 7]; -// DateTimeQuestion: -export const [$dtq$type] = [1]; diff --git a/lib/tsconfig.json b/lib/tsconfig.json index 77b852216..98bb59e18 100644 --- a/lib/tsconfig.json +++ b/lib/tsconfig.json @@ -14,10 +14,10 @@ }, "compileOnSave": true, "include": [ - "src/**/*", - "../functions/src/testing/http.ts" + "src/**/*" ], "exclude": [ - "src/generated" + "src/generated", + "dist" ] } From f0abc7baae67bfd2375e467021398b339537badd Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:42:50 -0400 Subject: [PATCH 29/55] Replace field numbers with registry --- functions/src/on-write-submission.spec.ts | 8 ++++-- lib/src/index.ts | 1 - lib/src/proto-field-numbers.ts | 32 ----------------------- 3 files changed, 6 insertions(+), 35 deletions(-) delete mode 100644 lib/src/proto-field-numbers.ts diff --git a/functions/src/on-write-submission.spec.ts b/functions/src/on-write-submission.spec.ts index 1ecaeb0c9..c2cdbd7da 100644 --- a/functions/src/on-write-submission.spec.ts +++ b/functions/src/on-write-submission.spec.ts @@ -25,10 +25,14 @@ import * as functions from './index'; import {loi} from './common/datastore'; import {Firestore} from 'firebase-admin/firestore'; import {resetDatastore} from './common/context'; -import {FieldNumbers} from '@ground/lib'; +import {registry} from '@ground/lib'; +import {GroundProtos} from '@ground/proto'; const test = require('firebase-functions-test')(); +import Pb = GroundProtos.google.ground.v1beta1; +const sb = registry.getFieldIds(Pb.Submission); + describe('onWriteSubmission()', () => { let mockFirestore: Firestore; const SURVEY_ID = 'survey1'; @@ -63,7 +67,7 @@ describe('onWriteSubmission()', () => { .and.returnValue({ where: jasmine .createSpy('where') - .withArgs(FieldNumbers.Submission.loi_id, '==', loiId) + .withArgs(sb.loiId, '==', loiId) .and.returnValue(newCountQuery(count)), } as any); } diff --git a/lib/src/index.ts b/lib/src/index.ts index ff55a9721..3ef80f830 100644 --- a/lib/src/index.ts +++ b/lib/src/index.ts @@ -17,5 +17,4 @@ export {toDocumentData} from './proto-to-firestore'; export {toMessage} from './firestore-to-proto'; export {deleteEmpty, isEmpty} from './obj-util'; -export {FieldNumbers} from './proto-field-numbers'; export {registry} from './message-registry'; \ No newline at end of file diff --git a/lib/src/proto-field-numbers.ts b/lib/src/proto-field-numbers.ts deleted file mode 100644 index acfd53f18..000000000 --- a/lib/src/proto-field-numbers.ts +++ /dev/null @@ -1,32 +0,0 @@ -/** - * Copyright 2024 The Ground Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -export const FieldNumbers = { - LocationOfInterest: { - job_id: '2', - owner_id: '5', - source: '9' - }, - Submission: { - loi_id: '2', - job_id: '4', - owner_id: '5' - }, - Survey: { - acl: '4', - owner_id: '5' - } -}; From 56cfd0d8badfe3714f419e85810033d470d68e1a Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:44:05 -0400 Subject: [PATCH 30/55] Add CR --- lib/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/index.ts b/lib/src/index.ts index 3ef80f830..13e650b63 100644 --- a/lib/src/index.ts +++ b/lib/src/index.ts @@ -17,4 +17,4 @@ export {toDocumentData} from './proto-to-firestore'; export {toMessage} from './firestore-to-proto'; export {deleteEmpty, isEmpty} from './obj-util'; -export {registry} from './message-registry'; \ No newline at end of file +export {registry} from './message-registry'; From cf2bc9d6f205d9384077536dc55c1a7cfc884514 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:44:35 -0400 Subject: [PATCH 31/55] Remove redundant exclude --- lib/tsconfig.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/tsconfig.json b/lib/tsconfig.json index 98bb59e18..ce404b78d 100644 --- a/lib/tsconfig.json +++ b/lib/tsconfig.json @@ -17,7 +17,6 @@ "src/**/*" ], "exclude": [ - "src/generated", - "dist" + "src/generated" ] } From 64886facad268d0f9c148ae88b26e80c8cdce936 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 15:49:27 -0400 Subject: [PATCH 32/55] Remove remaining ref to FieldNumbers --- functions/src/common/datastore.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/functions/src/common/datastore.ts b/functions/src/common/datastore.ts index 3916ce4e5..dd9476611 100644 --- a/functions/src/common/datastore.ts +++ b/functions/src/common/datastore.ts @@ -17,7 +17,12 @@ import * as functions from 'firebase-functions'; import {firestore} from 'firebase-admin'; import {DocumentData, GeoPoint, QuerySnapshot} from 'firebase-admin/firestore'; -import {FieldNumbers} from '@ground/lib/dist/proto-field-numbers'; +import {registry} from '@ground/lib'; +import {GroundProtos} from '@ground/proto'; + +import Pb = GroundProtos.google.ground.v1beta1; +const l = registry.getFieldIds(Pb.LocationOfInterest); +const sb = registry.getFieldIds(Pb.Submission); /** * @@ -120,7 +125,7 @@ export class Datastore { fetchSubmissionsByJobId(surveyId: string, jobId: string) { return this.db_ .collection(submissions(surveyId)) - .where(FieldNumbers.Submission.job_id, '==', jobId) + .where(sb.jobId, '==', jobId) .get(); } @@ -134,7 +139,7 @@ export class Datastore { ): Promise> { return this.db_ .collection(lois(surveyId)) - .where(FieldNumbers.LocationOfInterest.job_id, '==', jobId) + .where(l.jobId, '==', jobId) .get(); } @@ -151,11 +156,7 @@ export class Datastore { loiId: string ): Promise { const submissionsRef = this.db_.collection(submissions(surveyId)); - const submissionsForLoiQuery = submissionsRef.where( - FieldNumbers.Submission.loi_id, - '==', - loiId - ); + const submissionsForLoiQuery = submissionsRef.where(sb.loiId, '==', loiId); const snapshot = await submissionsForLoiQuery.count().get(); return snapshot.data().count; } From ba68c1b31b57fe10bb58ace485083418eaf23c73 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 1 Aug 2024 22:29:32 +0200 Subject: [PATCH 33/55] removed FieldNumbers --- .../services/data-store/data-store.service.ts | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/web/src/app/services/data-store/data-store.service.ts b/web/src/app/services/data-store/data-store.service.ts index 6e09dd830..9af2a849f 100644 --- a/web/src/app/services/data-store/data-store.service.ts +++ b/web/src/app/services/data-store/data-store.service.ts @@ -25,7 +25,7 @@ import { deleteField, serverTimestamp, } from '@angular/fire/firestore'; -import {FieldNumbers} from '@ground/lib'; +import {registry} from '@ground/lib/dist/message-registry'; import {GroundProtos} from '@ground/proto'; import {getDownloadURL, getStorage, ref} from 'firebase/storage'; import {List, Map} from 'immutable'; @@ -50,6 +50,9 @@ import {Task} from 'app/models/task/task.model'; import {User} from 'app/models/user.model'; import Pb = GroundProtos.google.ground.v1beta1; +const l = registry.getFieldIds(Pb.LocationOfInterest); +const s = registry.getFieldIds(Pb.Survey); +const sb = registry.getFieldIds(Pb.Submission); const Source = Pb.LocationOfInterest.Source; const AclRole = Pb.Role; @@ -138,7 +141,7 @@ export class DataStoreService { loadAccessibleSurveys$(userEmail: string): Observable> { return this.db .collection(SURVEYS_COLLECTION_NAME, ref => - ref.where(new FieldPath(FieldNumbers.Survey.acl, userEmail), 'in', [ + ref.where(new FieldPath(s.acl, userEmail), 'in', [ AclRole.VIEWER, AclRole.DATA_COLLECTOR, AclRole.SURVEY_ORGANIZER, @@ -293,7 +296,7 @@ export class DataStoreService { private async deleteAllSubmissionsInJob(surveyId: string, jobId: string) { const submissions = this.db.collection( `${SURVEYS_COLLECTION_NAME}/${surveyId}/submissions`, - ref => ref.where(FieldNumbers.Submission.job_id, '==', jobId) + ref => ref.where(s.jobId, '==', jobId) ); const querySnapshot = await firstValueFrom(submissions.get()); return await Promise.all(querySnapshot.docs.map(doc => doc.ref.delete())); @@ -305,7 +308,7 @@ export class DataStoreService { ) { const submissions = this.db.collection( `${SURVEYS_COLLECTION_NAME}/${surveyId}/submissions`, - ref => ref.where(FieldNumbers.Submission.loi_id, '==', loiId) + ref => ref.where(sb.loiId, '==', loiId) ); const querySnapshot = await firstValueFrom(submissions.get()); return await Promise.all(querySnapshot.docs.map(doc => doc.ref.delete())); @@ -317,7 +320,7 @@ export class DataStoreService { ) { const loisInJob = this.db.collection( `${SURVEYS_COLLECTION_NAME}/${surveyId}/lois`, - ref => ref.where(FieldNumbers.LocationOfInterest.job_id, '==', jobId) + ref => ref.where(l.jobId, '==', jobId) ); const querySnapshot = await firstValueFrom(loisInJob.get()); return await Promise.all(querySnapshot.docs.map(doc => doc.ref.delete())); @@ -397,20 +400,15 @@ export class DataStoreService { const importedLois = this.db.collection( `${SURVEYS_COLLECTION_NAME}/${surveyId}/lois`, - ref => - ref.where(FieldNumbers.LocationOfInterest.source, '==', Source.IMPORTED) + ref => ref.where(l.source, '==', Source.IMPORTED) ); const fieldData = this.db.collection( `${SURVEYS_COLLECTION_NAME}/${surveyId}/lois`, ref => ref - .where( - FieldNumbers.LocationOfInterest.source, - '==', - Source.FIELD_DATA - ) - .where(FieldNumbers.LocationOfInterest.owner_id, '==', userId) + .where(l.source, '==', Source.FIELD_DATA) + .where(l.ownerId, '==', userId) ); return combineLatest([ @@ -600,9 +598,7 @@ export class DataStoreService { canManageSurvey: boolean ) { return canManageSurvey - ? ref.where(FieldNumbers.Submission.loi_id, '==', loiId) - : ref - .where(FieldNumbers.Submission.loi_id, '==', loiId) - .where(FieldNumbers.Submission.owner_id, '==', userId); + ? ref.where(s.loiId, '==', loiId) + : ref.where(sb.loiId, '==', loiId).where(sb.ownerId, '==', userId); } } From 0f421d01e9ccbc93191f52d35ec37526cbba9c33 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Thu, 1 Aug 2024 16:44:22 -0400 Subject: [PATCH 34/55] Use registry for field nos --- functions/src/export-csv.spec.ts | 132 +++++++++++++++---------------- functions/src/export-csv.ts | 12 ++- 2 files changed, 69 insertions(+), 75 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 8f1e66fc7..4050f0b36 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -22,38 +22,33 @@ import { createGetRequestSpy, createResponseSpy, } from './testing/http-test-helpers'; -import { - $job_id, - $geometry, - $submission_count, - $source, - $properties, - $custom_tag, - $point, - $coordinates, - $latitude, - $longitude, - $string_value, - $numeric_value, -} from '@ground/lib/dist/testing/proto-field-aliases'; import {DecodedIdToken} from 'firebase-admin/auth'; import HttpStatus from 'http-status-codes'; import {OWNER_ROLE} from './common/auth'; import {resetDatastore} from './common/context'; import {Firestore} from 'firebase-admin/firestore'; import {exportCsvHandler} from './export-csv'; +import {registry} from '@ground/lib'; +import {GroundProtos} from '@ground/proto'; + +import Pb = GroundProtos.google.ground.v1beta1; +const l = registry.getFieldIds(Pb.LocationOfInterest); +const pr = registry.getFieldIds(Pb.LocationOfInterest.Property); +const p = registry.getFieldIds(Pb.Point); +const c = registry.getFieldIds(Pb.Coordinates); +const g = registry.getFieldIds(Pb.Geometry); fdescribe('exportCsv()', () => { let mockFirestore: Firestore; const jobId = 'job123'; const email = 'somebody@test.it'; - // TODO(#1758): Use new proto-based survey and job representation. + // TODO(#1758): Use new proto-based survey and job representation. const survey1 = { id: 'survey001', name: 'Test survey 1', acl: { [email]: OWNER_ROLE, - } + }, }; const survey2 = { id: 'survey002', @@ -83,29 +78,29 @@ fdescribe('exportCsv()', () => { }; const pointLoi1 = { id: 'loi100', - [$job_id]: jobId, - [$custom_tag]: 'POINT_001', - [$geometry]: { - [$point]: {[$coordinates]: {[$latitude]: 10.1, [$longitude]: 125.6}}, + [l.jobId]: jobId, + [l.customTag]: 'POINT_001', + [l.geometry]: { + [g.point]: {[p.coordinates]: {[c.latitude]: 10.1, [c.longitude]: 125.6}}, }, - [$submission_count]: 0, - [$source]: 1, // IMPORTED - [$properties]: { - name: {[$string_value]: 'Dinagat Islands'}, - area: {[$numeric_value]: 3.08}, + [l.submission_count]: 0, + [l.source]: Pb.LocationOfInterest.Source.IMPORTED, + [l.properties]: { + 'name': {[pr.stringValue]: 'Dinagat Islands'}, + 'area': {[pr.numericValue]: 3.08}, }, }; const pointLoi2 = { id: 'loi200', - [$job_id]: jobId, - [$custom_tag]: 'POINT_002', - [$geometry]: { - [$point]: {[$coordinates]: {[$latitude]: 47.05, [$longitude]: 8.3}}, + [l.jobId]: jobId, + [l.customTag]: 'POINT_002', + [l.geometry]: { + [g.point]: {[p.coordinates]: {[c.latitude]: 47.05, [c.longitude]: 8.3}}, }, - [$submission_count]: 0, - [$source]: 2, // FIELD_DATA - [$properties]: { - name: {[$string_value]: 'Luzern'}, + [l.submissionCount]: 0, + [l.source]: Pb.LocationOfInterest.Source.FIELD_DATA, + [l.properties]: { + 'name': {[pr.stringValue]: 'Luzern'}, }, }; const submission1a = { @@ -156,43 +151,44 @@ fdescribe('exportCsv()', () => { resetDatastore(); }); - testCases.forEach(({desc, survey, lois, submissions, expectedFilename, expectedCsv}) => - it(desc, async () => { - // Populate database. - mockFirestore.doc(`surveys/${survey.id}`).set(survey); - lois?.forEach(({id, ...loi}) => - mockFirestore.doc(`surveys/${survey.id}/lois/${id}`).set(loi) - ); - submissions?.forEach(({id, ...submission}) => - mockFirestore - .doc(`surveys/${survey.id}/submissions/${id}`) - .set(submission) - ); + testCases.forEach( + ({desc, survey, lois, submissions, expectedFilename, expectedCsv}) => + it(desc, async () => { + // Populate database. + mockFirestore.doc(`surveys/${survey.id}`).set(survey); + lois?.forEach(({id, ...loi}) => + mockFirestore.doc(`surveys/${survey.id}/lois/${id}`).set(loi) + ); + submissions?.forEach(({id, ...submission}) => + mockFirestore + .doc(`surveys/${survey.id}/submissions/${id}`) + .set(submission) + ); - // Build mock request and response. - const req = await createGetRequestSpy({ - url: '/exportCsv', - query: { - survey: survey.id, - job: jobId, - }, - }); - const chunks: string[] = []; - const res = createResponseSpy(chunks); + // Build mock request and response. + const req = await createGetRequestSpy({ + url: '/exportCsv', + query: { + survey: survey.id, + job: jobId, + }, + }); + const chunks: string[] = []; + const res = createResponseSpy(chunks); - // Run export CSV handler. - await exportCsvHandler(req, res, {email} as DecodedIdToken); + // Run export CSV handler. + await exportCsvHandler(req, res, {email} as DecodedIdToken); - // Check post-conditions. - expect(res.status).toHaveBeenCalledOnceWith(HttpStatus.OK); - expect(res.type).toHaveBeenCalledOnceWith('text/csv'); - expect(res.setHeader).toHaveBeenCalledOnceWith( - 'Content-Disposition', - `attachment; filename=${expectedFilename}` - ); - const output = chunks.join('').trim(); - const lines = output.split('\n'); - expect(lines).toEqual(expectedCsv); - }) + // Check post-conditions. + expect(res.status).toHaveBeenCalledOnceWith(HttpStatus.OK); + expect(res.type).toHaveBeenCalledOnceWith('text/csv'); + expect(res.setHeader).toHaveBeenCalledOnceWith( + 'Content-Disposition', + `attachment; filename=${expectedFilename}` + ); + const output = chunks.join('').trim(); + const lines = output.split('\n'); + expect(lines).toEqual(expectedCsv); + }) ); }); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 7b2b5b84f..ccf245c0c 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -23,11 +23,13 @@ import * as HttpStatus from 'http-status-codes'; import {DecodedIdToken} from 'firebase-admin/auth'; import {List} from 'immutable'; import {DocumentData, QuerySnapshot} from 'firebase-admin/firestore'; -import {FieldNumbers, toMessage} from '@ground/lib'; +import {registry, toMessage} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; import {toGeoJsonGeometry} from '@ground/lib'; import Pb = GroundProtos.google.ground.v1beta1; +const sb = registry.getFieldIds(Pb.Submission); +const l = registry.getFieldIds(Pb.LocationOfInterest); // TODO(#1277): Use a shared model with web type Task = { @@ -146,7 +148,7 @@ async function getSubmissionsByLoi( const submissions = await db.fetchSubmissionsByJobId(surveyId, jobId); const submissionsByLoi: {[name: string]: any[]} = {}; submissions.forEach(submission => { - const loiId = submission.get(FieldNumbers.Submission.loi_id) as string; + const loiId = submission.get(sb.loiId) as string; const arr: any[] = submissionsByLoi[loiId] || []; arr.push(submission.data()); submissionsByLoi[loiId] = arr; @@ -266,11 +268,7 @@ function getFileName(jobName: string | null) { function getPropertyNames(lois: QuerySnapshot): Set { return new Set( - lois.docs - .map(loi => - Object.keys(loi.get(FieldNumbers.LocationOfInterest.properties) || {}) - ) - .flat() + lois.docs.map(loi => Object.keys(loi.get(l.properties) || {})).flat() ); } From fd1b220d853e2261c9ad911e577e4a08729eafee Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Fri, 2 Aug 2024 12:02:43 -0400 Subject: [PATCH 35/55] Rename proto packages to fix name collision with google/ --- functions/src/common/datastore.ts | 2 +- functions/src/import-geojson.spec.ts | 2 +- functions/src/import-geojson.ts | 2 +- functions/src/on-write-submission.spec.ts | 2 +- lib/src/firestore-to-proto.spec.ts | 2 +- lib/src/proto-to-firestore.spec.ts | 4 ++-- proto/src/google/ground/v1beta1/audit_info.proto | 2 +- proto/src/google/ground/v1beta1/geometry.proto | 2 +- proto/src/google/ground/v1beta1/job.proto | 2 +- proto/src/google/ground/v1beta1/loi.proto | 2 +- proto/src/google/ground/v1beta1/submission.proto | 2 +- proto/src/google/ground/v1beta1/survey.proto | 2 +- web/src/app/converters/geometry-data-converter.ts | 2 +- web/src/app/converters/loi-data-converter.ts | 2 +- web/src/app/converters/proto-model-converter.ts | 2 +- web/src/app/converters/submission-data-converter.ts | 2 +- web/src/app/services/data-store/data-store.service.ts | 2 +- 17 files changed, 18 insertions(+), 18 deletions(-) diff --git a/functions/src/common/datastore.ts b/functions/src/common/datastore.ts index dd9476611..b4ed10c13 100644 --- a/functions/src/common/datastore.ts +++ b/functions/src/common/datastore.ts @@ -20,7 +20,7 @@ import {DocumentData, GeoPoint, QuerySnapshot} from 'firebase-admin/firestore'; import {registry} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const l = registry.getFieldIds(Pb.LocationOfInterest); const sb = registry.getFieldIds(Pb.Submission); diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index 330e13d61..e466b669f 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -34,7 +34,7 @@ import {Firestore} from 'firebase-admin/firestore'; import {registry} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const l = registry.getFieldIds(Pb.LocationOfInterest); const g = registry.getFieldIds(Pb.Geometry); const p = registry.getFieldIds(Pb.Point); diff --git a/functions/src/import-geojson.ts b/functions/src/import-geojson.ts index 4582515f8..a06628891 100644 --- a/functions/src/import-geojson.ts +++ b/functions/src/import-geojson.ts @@ -27,7 +27,7 @@ import {DocumentData} from 'firebase-admin/firestore'; import {toDocumentData, deleteEmpty} from '@ground/lib'; import {Feature, Geometry, Position} from 'geojson'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; import {ErrorHandler} from './handlers'; /** diff --git a/functions/src/on-write-submission.spec.ts b/functions/src/on-write-submission.spec.ts index c2cdbd7da..3b3106ff7 100644 --- a/functions/src/on-write-submission.spec.ts +++ b/functions/src/on-write-submission.spec.ts @@ -30,7 +30,7 @@ import {GroundProtos} from '@ground/proto'; const test = require('firebase-functions-test')(); -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const sb = registry.getFieldIds(Pb.Submission); describe('onWriteSubmission()', () => { diff --git a/lib/src/firestore-to-proto.spec.ts b/lib/src/firestore-to-proto.spec.ts index c2c82b449..8923e16aa 100644 --- a/lib/src/firestore-to-proto.spec.ts +++ b/lib/src/firestore-to-proto.spec.ts @@ -19,7 +19,7 @@ import {toMessage} from './firestore-to-proto'; import {Constructor} from 'protobufjs'; const {Coordinates, Job, LinearRing, Role, Style, Survey, Task} = - GroundProtos.google.ground.v1beta1; + GroundProtos.ground.v1beta1; describe('toMessage()', () => { [ diff --git a/lib/src/proto-to-firestore.spec.ts b/lib/src/proto-to-firestore.spec.ts index 5e3148d96..a3bccf88a 100644 --- a/lib/src/proto-to-firestore.spec.ts +++ b/lib/src/proto-to-firestore.spec.ts @@ -18,7 +18,7 @@ import {registry} from './message-registry'; import {GroundProtos} from '@ground/proto'; import {toDocumentData} from './proto-to-firestore'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const s = registry.getFieldIds(Pb.Survey); const j = registry.getFieldIds(Pb.Job); const c = registry.getFieldIds(Pb.Coordinates); @@ -28,7 +28,7 @@ const st = registry.getFieldIds(Pb.Style); const dtq = registry.getFieldIds(Pb.Task.DateTimeQuestion); const {Job, Role, Style, Survey, Task, LinearRing, Coordinates} = - GroundProtos.google.ground.v1beta1; + GroundProtos.ground.v1beta1; describe('toDocumentData()', () => { [ diff --git a/proto/src/google/ground/v1beta1/audit_info.proto b/proto/src/google/ground/v1beta1/audit_info.proto index cdd74e73b..4d893b542 100644 --- a/proto/src/google/ground/v1beta1/audit_info.proto +++ b/proto/src/google/ground/v1beta1/audit_info.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; import "google/protobuf/timestamp.proto"; diff --git a/proto/src/google/ground/v1beta1/geometry.proto b/proto/src/google/ground/v1beta1/geometry.proto index 569b12cc3..c885c9d17 100644 --- a/proto/src/google/ground/v1beta1/geometry.proto +++ b/proto/src/google/ground/v1beta1/geometry.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; option java_multiple_files = true; option java_package = "com.google.android.ground.proto"; diff --git a/proto/src/google/ground/v1beta1/job.proto b/proto/src/google/ground/v1beta1/job.proto index 10f77cefa..1de0ae88d 100644 --- a/proto/src/google/ground/v1beta1/job.proto +++ b/proto/src/google/ground/v1beta1/job.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; option java_multiple_files = true; option java_package = "com.google.android.ground.proto"; diff --git a/proto/src/google/ground/v1beta1/loi.proto b/proto/src/google/ground/v1beta1/loi.proto index 1d1019d06..b8f8c4f2e 100644 --- a/proto/src/google/ground/v1beta1/loi.proto +++ b/proto/src/google/ground/v1beta1/loi.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; import "google/ground/v1beta1/audit_info.proto"; import "google/ground/v1beta1/geometry.proto"; diff --git a/proto/src/google/ground/v1beta1/submission.proto b/proto/src/google/ground/v1beta1/submission.proto index 1d4ef4889..11286a6d5 100644 --- a/proto/src/google/ground/v1beta1/submission.proto +++ b/proto/src/google/ground/v1beta1/submission.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; import "google/ground/v1beta1/audit_info.proto"; import "google/ground/v1beta1/geometry.proto"; diff --git a/proto/src/google/ground/v1beta1/survey.proto b/proto/src/google/ground/v1beta1/survey.proto index 8c00e1030..008a9b281 100644 --- a/proto/src/google/ground/v1beta1/survey.proto +++ b/proto/src/google/ground/v1beta1/survey.proto @@ -16,7 +16,7 @@ syntax = "proto3"; -package google.ground.v1beta1; +package ground.v1beta1; option java_multiple_files = true; option java_package = "com.google.android.ground.proto"; diff --git a/web/src/app/converters/geometry-data-converter.ts b/web/src/app/converters/geometry-data-converter.ts index bf2e84199..bf96ea46f 100644 --- a/web/src/app/converters/geometry-data-converter.ts +++ b/web/src/app/converters/geometry-data-converter.ts @@ -24,7 +24,7 @@ import {MultiPolygon} from 'app/models/geometry/multi-polygon'; import {Point} from 'app/models/geometry/point'; import {Polygon} from 'app/models/geometry/polygon'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; export function geometryPbToModel(pb: Pb.IGeometry): Geometry { if (pb.point) { diff --git a/web/src/app/converters/loi-data-converter.ts b/web/src/app/converters/loi-data-converter.ts index b515db63a..f63531be0 100644 --- a/web/src/app/converters/loi-data-converter.ts +++ b/web/src/app/converters/loi-data-converter.ts @@ -24,7 +24,7 @@ import {LocationOfInterest} from 'app/models/loi.model'; import {geometryPbToModel} from './geometry-data-converter'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; /** * Helper to return either the keys of a dictionary, or if missing, an diff --git a/web/src/app/converters/proto-model-converter.ts b/web/src/app/converters/proto-model-converter.ts index 0a6d9f585..653a1c134 100644 --- a/web/src/app/converters/proto-model-converter.ts +++ b/web/src/app/converters/proto-model-converter.ts @@ -28,7 +28,7 @@ import { import {TaskCondition} from 'app/models/task/task-condition.model'; import {Task, TaskType} from 'app/models/task/task.model'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const PB_ROLES = Map([ [Role.OWNER, Pb.Role.SURVEY_ORGANIZER], diff --git a/web/src/app/converters/submission-data-converter.ts b/web/src/app/converters/submission-data-converter.ts index cd6532d62..38ac86aa3 100644 --- a/web/src/app/converters/submission-data-converter.ts +++ b/web/src/app/converters/submission-data-converter.ts @@ -40,7 +40,7 @@ import { geometryPbToModel, } from './geometry-data-converter'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; /** * Helper to return either the keys of a dictionary, or if missing, an diff --git a/web/src/app/services/data-store/data-store.service.ts b/web/src/app/services/data-store/data-store.service.ts index 9af2a849f..90b0baa03 100644 --- a/web/src/app/services/data-store/data-store.service.ts +++ b/web/src/app/services/data-store/data-store.service.ts @@ -49,7 +49,7 @@ import {Survey} from 'app/models/survey.model'; import {Task} from 'app/models/task/task.model'; import {User} from 'app/models/user.model'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const l = registry.getFieldIds(Pb.LocationOfInterest); const s = registry.getFieldIds(Pb.Survey); const sb = registry.getFieldIds(Pb.Submission); From d52c40af08e2d313ade9b2bec65b472b358eb998 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Fri, 2 Aug 2024 15:23:40 -0400 Subject: [PATCH 36/55] Remove google/ from proto path --- proto/src/{google => }/ground/v1beta1/audit_info.proto | 0 proto/src/{google => }/ground/v1beta1/geometry.proto | 0 proto/src/{google => }/ground/v1beta1/job.proto | 0 proto/src/{google => }/ground/v1beta1/loi.proto | 0 proto/src/{google => }/ground/v1beta1/submission.proto | 0 proto/src/{google => }/ground/v1beta1/survey.proto | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename proto/src/{google => }/ground/v1beta1/audit_info.proto (100%) rename proto/src/{google => }/ground/v1beta1/geometry.proto (100%) rename proto/src/{google => }/ground/v1beta1/job.proto (100%) rename proto/src/{google => }/ground/v1beta1/loi.proto (100%) rename proto/src/{google => }/ground/v1beta1/submission.proto (100%) rename proto/src/{google => }/ground/v1beta1/survey.proto (100%) diff --git a/proto/src/google/ground/v1beta1/audit_info.proto b/proto/src/ground/v1beta1/audit_info.proto similarity index 100% rename from proto/src/google/ground/v1beta1/audit_info.proto rename to proto/src/ground/v1beta1/audit_info.proto diff --git a/proto/src/google/ground/v1beta1/geometry.proto b/proto/src/ground/v1beta1/geometry.proto similarity index 100% rename from proto/src/google/ground/v1beta1/geometry.proto rename to proto/src/ground/v1beta1/geometry.proto diff --git a/proto/src/google/ground/v1beta1/job.proto b/proto/src/ground/v1beta1/job.proto similarity index 100% rename from proto/src/google/ground/v1beta1/job.proto rename to proto/src/ground/v1beta1/job.proto diff --git a/proto/src/google/ground/v1beta1/loi.proto b/proto/src/ground/v1beta1/loi.proto similarity index 100% rename from proto/src/google/ground/v1beta1/loi.proto rename to proto/src/ground/v1beta1/loi.proto diff --git a/proto/src/google/ground/v1beta1/submission.proto b/proto/src/ground/v1beta1/submission.proto similarity index 100% rename from proto/src/google/ground/v1beta1/submission.proto rename to proto/src/ground/v1beta1/submission.proto diff --git a/proto/src/google/ground/v1beta1/survey.proto b/proto/src/ground/v1beta1/survey.proto similarity index 100% rename from proto/src/google/ground/v1beta1/survey.proto rename to proto/src/ground/v1beta1/survey.proto From 4cd0c5ce56b1805d03d270cd733f55995d1205e6 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Fri, 2 Aug 2024 15:26:10 -0400 Subject: [PATCH 37/55] Fix proto source path --- proto/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/package.json b/proto/package.json index 4c862910e..9342a5e9d 100644 --- a/proto/package.json +++ b/proto/package.json @@ -11,7 +11,7 @@ "format": "buf format --write", "build": "npm run generate:pbjs && npm run generate:pbjson && npm run generate:pbts", "postbuild": "cp src/index.* dist/", - "pbjs": "mkdir -p dist && pbjs -p src src/google/ground/v1beta1/*.proto", + "pbjs": "mkdir -p dist && pbjs -p src src/ground/v1beta1/*.proto", "generate:pbjs": "npm run pbjs -- -t static-module -o dist/ground-protos.js", "generate:pbjson": "npm run pbjs -- -t json -o dist/ground-protos.json", "generate:pbts": "mkdir -p dist && pbts dist/ground-protos.js -o dist/ground-protos.d.ts" From ea74b6706ffa9c0a31d2a1d37d4cfd5682132cf4 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Fri, 2 Aug 2024 15:34:44 -0400 Subject: [PATCH 38/55] Update proto include paths --- proto/src/ground/v1beta1/loi.proto | 4 ++-- proto/src/ground/v1beta1/submission.proto | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/proto/src/ground/v1beta1/loi.proto b/proto/src/ground/v1beta1/loi.proto index b8f8c4f2e..0d908aa33 100644 --- a/proto/src/ground/v1beta1/loi.proto +++ b/proto/src/ground/v1beta1/loi.proto @@ -18,8 +18,8 @@ syntax = "proto3"; package ground.v1beta1; -import "google/ground/v1beta1/audit_info.proto"; -import "google/ground/v1beta1/geometry.proto"; +import "ground/v1beta1/audit_info.proto"; +import "ground/v1beta1/geometry.proto"; option java_multiple_files = true; option java_package = "com.google.android.ground.proto"; diff --git a/proto/src/ground/v1beta1/submission.proto b/proto/src/ground/v1beta1/submission.proto index 11286a6d5..22eb9c6cf 100644 --- a/proto/src/ground/v1beta1/submission.proto +++ b/proto/src/ground/v1beta1/submission.proto @@ -18,8 +18,8 @@ syntax = "proto3"; package ground.v1beta1; -import "google/ground/v1beta1/audit_info.proto"; -import "google/ground/v1beta1/geometry.proto"; +import "ground/v1beta1/audit_info.proto"; +import "ground/v1beta1/geometry.proto"; import "google/protobuf/timestamp.proto"; option java_multiple_files = true; From 2bf5acffee1a12a1d7bb8670a2303ffe65a76323 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Fri, 2 Aug 2024 15:42:47 -0400 Subject: [PATCH 39/55] Update namespace --- functions/src/export-csv.spec.ts | 2 +- functions/src/export-csv.ts | 4 ++-- lib/src/geo-json.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 4050f0b36..52ae6a168 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -31,7 +31,7 @@ import {exportCsvHandler} from './export-csv'; import {registry} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const l = registry.getFieldIds(Pb.LocationOfInterest); const pr = registry.getFieldIds(Pb.LocationOfInterest.Property); const p = registry.getFieldIds(Pb.Point); diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index ccf245c0c..3d39ea5f6 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -27,7 +27,7 @@ import {registry, toMessage} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; import {toGeoJsonGeometry} from '@ground/lib'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; const sb = registry.getFieldIds(Pb.Submission); const l = registry.getFieldIds(Pb.LocationOfInterest); @@ -192,7 +192,7 @@ function quote(value: any): string { if (typeof value === 'number') { return value.toString(); } - const escaped = value.toString().replace('"', '""'); + const escaped = value.toString().replaceAll('"', '""'); return '"' + escaped + '"'; } diff --git a/lib/src/geo-json.ts b/lib/src/geo-json.ts index 5884db66e..f555ab65b 100644 --- a/lib/src/geo-json.ts +++ b/lib/src/geo-json.ts @@ -17,7 +17,7 @@ import {GroundProtos} from '@ground/proto'; import {Geometry, MultiPolygon, Point, Polygon, Position} from 'geojson'; -import Pb = GroundProtos.google.ground.v1beta1; +import Pb = GroundProtos.ground.v1beta1; /** * Returns the equivalent GeoJSON Geometry for the provided Geometry proto. From a4258e84ef54711606a48e90f31528b733b9a78e Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 15:46:40 -0400 Subject: [PATCH 40/55] Fix `number` field number --- proto/src/ground/v1beta1/submission.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/src/ground/v1beta1/submission.proto b/proto/src/ground/v1beta1/submission.proto index 22eb9c6cf..e10cea76f 100644 --- a/proto/src/ground/v1beta1/submission.proto +++ b/proto/src/ground/v1beta1/submission.proto @@ -102,7 +102,7 @@ message TaskData { // A manually entered number response. message NumberResponse { // The number provided by the user. - double number = 2; + double number = 1; } // A manually selected date and/or time response. From d4125c072020cdf427bb19c70e23d61fd0655906 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 15:54:33 -0400 Subject: [PATCH 41/55] Implement text, number, date-time export --- functions/src/export-csv.spec.ts | 68 ++++++++++++++++++++++++++++---- functions/src/export-csv.ts | 53 ++++++++++++++++++++----- 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 52ae6a168..99e7355fc 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -37,11 +37,14 @@ const pr = registry.getFieldIds(Pb.LocationOfInterest.Property); const p = registry.getFieldIds(Pb.Point); const c = registry.getFieldIds(Pb.Coordinates); const g = registry.getFieldIds(Pb.Geometry); +const s = registry.getFieldIds(Pb.Submission); +const d = registry.getFieldIds(Pb.TaskData); fdescribe('exportCsv()', () => { let mockFirestore: Firestore; const jobId = 'job123'; const email = 'somebody@test.it'; + const userId = 'user5000'; // TODO(#1758): Use new proto-based survey and job representation. const survey1 = { id: 'survey001', @@ -65,10 +68,18 @@ fdescribe('exportCsv()', () => { label: 'What is the meaning of life?', }, task002: { + type: 'number_field', + label: 'How much?', + }, + task003: { + type: 'date_time_field', + label: 'When?', + }, + task005: { type: 'capture_location', label: 'Where are you now?', }, - task003: { + task006: { type: 'draw_area', label: 'Delimit plot boundaries', }, @@ -86,8 +97,8 @@ fdescribe('exportCsv()', () => { [l.submission_count]: 0, [l.source]: Pb.LocationOfInterest.Source.IMPORTED, [l.properties]: { - 'name': {[pr.stringValue]: 'Dinagat Islands'}, - 'area': {[pr.numericValue]: 3.08}, + name: {[pr.stringValue]: 'Dinagat Islands'}, + area: {[pr.numericValue]: 3.08}, }, }; const pointLoi2 = { @@ -100,15 +111,57 @@ fdescribe('exportCsv()', () => { [l.submissionCount]: 0, [l.source]: Pb.LocationOfInterest.Source.FIELD_DATA, [l.properties]: { - 'name': {[pr.stringValue]: 'Luzern'}, + name: {[pr.stringValue]: 'Luzern'}, }, }; const submission1a = { id: '001a', + [s.loiId]: pointLoi1.id, + [s.index]: 1, + [s.jobId]: jobId, + [s.ownerId]: userId, + [s.taskData]: [ + { + [d.id]: 'data001a', + [d.taskId]: 'task001', + [d.textResponse]: { + '1': 'Submission 1', + }, + }, + { + [d.id]: 'data002a', + [d.taskId]: 'task002', + [d.numberResponse]: { + '1': 42, + }, + }, + ], // TODO }; const submission1b = { id: '001b', + [s.loiId]: pointLoi1.id, + [s.index]: 2, + [s.jobId]: jobId, + [s.ownerId]: userId, + [s.taskData]: [ + { + [d.id]: 'data001b', + [d.taskId]: 'task001', + [d.textResponse]: { + '1': 'Submission 2', + }, + }, + { + [d.id]: 'data003a', + [d.taskId]: 'task003', + [d.dateTimeResponse]: { + '1': { + '1': 1331209044 // seconds + }, + }, + }, + ], // TODO }; const submission2a = { @@ -135,9 +188,10 @@ fdescribe('exportCsv()', () => { submissions: [submission1a, submission1b, submission2a], expectedFilename: 'test-job.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:What is the meaning of life?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,,,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,,`, + `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,,,,`, ], }, ]; diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 3d39ea5f6..2f072049b 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -201,16 +201,31 @@ function toWkt(geometry: Pb.IGeometry): string { } /** - * Returns the string representation of a specific task element result. + * Returns the string or number representation of a specific task element result. */ -function getValue(taskId: string, task: Task, data: Pb.ITaskData[]): any { +function getValue( + taskId: string, + task: Task, + data: Pb.ITaskData[] +): string | number | null { const result = data.find(d => d.taskId === taskId); - if (result?.multipleChoiceResponses) { - return result.multipleChoiceResponses?.selectedOptionIds - ?.map(id => getMultipleChoiceValues(id, task)) - .join(', '); - } else if (result?.captureLocationResult) { - // TODO(): Include alitude and accuracy in separate columns. + if (!result) { + return null; + } + if (result.textResponse) { + return result.textResponse.text || null; + } else if (result.numberResponse) { + return getNumberValue(result.numberResponse); + } else if (result.dateTimeResponse) { + return getDateTimeValue(result.dateTimeResponse); + } else if (result.multipleChoiceResponses) { + return ( + result.multipleChoiceResponses.selectedOptionIds + ?.map(id => getMultipleChoiceValues(id, task)) + .join(', ') || null + ); + } else if (result.captureLocationResult) { + // TODO(#1916): Include altitude and accuracy in separate columns. return toWkt( new Pb.Geometry({ point: new Pb.Point({ @@ -218,11 +233,29 @@ function getValue(taskId: string, task: Task, data: Pb.ITaskData[]): any { }), }) ); - } else if (result?.drawGeometryResult?.geometry) { + } else if (result.drawGeometryResult?.geometry) { return toWkt(result.drawGeometryResult.geometry); } else { - return result; + return null; + } +} + +function getNumberValue(response: Pb.TaskData.INumberResponse): number | null { + const number = response.number; + if (number === undefined || number === null) { + return null; + } + return number; +} + +function getDateTimeValue( + response: Pb.TaskData.IDateTimeResponse +): string | null { + const seconds = response.dateTime?.seconds; + if (seconds === undefined || seconds === null) { + return null; } + return new Date(Number(seconds) * 1000).toISOString(); } /** From 550c7fbecf645ef6a326ce91eb2df7d331c30d72 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:11:11 -0400 Subject: [PATCH 42/55] Export multiple choice options --- functions/src/export-csv.spec.ts | 43 ++++++++++++++++++++++++++------ functions/src/export-csv.ts | 40 +++++++---------------------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 99e7355fc..da6093f8f 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -74,7 +74,22 @@ fdescribe('exportCsv()', () => { task003: { type: 'date_time_field', label: 'When?', - }, + }, + task004: { + type: 'select_multiple', + label: 'Which ones?', + options: [ + { + id: 'aaa', + label: 'AAA', + }, + { + id: 'bbb', + label: 'BBB', + }, + ], + hasOtherOption: true, + }, task005: { type: 'capture_location', label: 'Where are you now?', @@ -157,16 +172,28 @@ fdescribe('exportCsv()', () => { [d.taskId]: 'task003', [d.dateTimeResponse]: { '1': { - '1': 1331209044 // seconds + '1': 1331209044, // seconds }, }, }, ], - // TODO }; const submission2a = { id: '002a', - // TODO + [s.loiId]: pointLoi2.id, + [s.index]: 1, + [s.jobId]: jobId, + [s.ownerId]: userId, + [s.taskData]: [ + { + [d.id]: 'data002a', + [d.taskId]: 'task004', + [d.multipleChoiceResponses]: { + '1': ['aaa', 'bbb'], + '2': 'Other' + }, + } + ] }; const testCases = [ { @@ -188,10 +215,10 @@ fdescribe('exportCsv()', () => { submissions: [submission1a, submission1b, submission2a], expectedFilename: 'test-job.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,,,,`, + `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,,`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other",,,,`, ], }, ]; diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 2f072049b..c57639507 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -219,11 +219,7 @@ function getValue( } else if (result.dateTimeResponse) { return getDateTimeValue(result.dateTimeResponse); } else if (result.multipleChoiceResponses) { - return ( - result.multipleChoiceResponses.selectedOptionIds - ?.map(id => getMultipleChoiceValues(id, task)) - .join(', ') || null - ); + return getMultipleChoiceValues(task, result.multipleChoiceResponses); } else if (result.captureLocationResult) { // TODO(#1916): Include altitude and accuracy in separate columns. return toWkt( @@ -259,35 +255,17 @@ function getDateTimeValue( } /** - * Returns the code associated with a specified multiple choice option, or if - * the code is not defined, returns the label in English. + * Returns a comma-separated list of the labels of the + * specified multiple choice option, or the raw text if "Other". */ -function getMultipleChoiceValues(id: any, task: Task) { - // "Other" options are encoded to be surrounded by square brakets, to let us - // distinguish them from the other pre-defined options. - if (isOtherOption(id)) { - return extractOtherOption(id); - } - const options = task.options || {}; - const option = options[id] || {}; - const label = option.label || {}; - // TODO: i18n. - return option.code || label || ''; -} - -function isOtherOption(submission: any): boolean { - // "Other" options are encoded to be surrounded by square brakets, to let us - // distinguish them from the other pre-defined options. - return ( - typeof submission === 'string' && - submission.startsWith('[ ') && - submission.endsWith(' ]') - ); +function getMultipleChoiceValues(task: Task, responses: Pb.TaskData.IMultipleChoiceResponses) { + const values = responses.selectedOptionIds?.map(id => getMultipleChoiceLabel(task, id) || '#ERR') || []; + if (responses.otherText && responses.otherText.trim() !== '') values.push(responses.otherText); + return values.join(','); } -function extractOtherOption(submission: string): string { - const match = submission.match(/\[(.*?)\]/); // Match any text between [] - return match ? match[1].trim() : ''; // Extract the match and remove spaces +function getMultipleChoiceLabel(task: Task, id: string): string | null { + return task?.options?.find((o: any) => o.id === id)?.label; } /** From d81e7013c4c2404ca244be9dc322307abbd76ffa Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:21:22 -0400 Subject: [PATCH 43/55] Test capture location export --- functions/src/export-csv.spec.ts | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index da6093f8f..6b9ecf8fa 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -39,6 +39,7 @@ const c = registry.getFieldIds(Pb.Coordinates); const g = registry.getFieldIds(Pb.Geometry); const s = registry.getFieldIds(Pb.Submission); const d = registry.getFieldIds(Pb.TaskData); +const cl = registry.getFieldIds(Pb.TaskData.CaptureLocationResult); fdescribe('exportCsv()', () => { let mockFirestore: Firestore; @@ -94,10 +95,6 @@ fdescribe('exportCsv()', () => { type: 'capture_location', label: 'Where are you now?', }, - task006: { - type: 'draw_area', - label: 'Delimit plot boundaries', - }, }, }, }, @@ -151,7 +148,6 @@ fdescribe('exportCsv()', () => { }, }, ], - // TODO }; const submission1b = { id: '001b', @@ -190,10 +186,20 @@ fdescribe('exportCsv()', () => { [d.taskId]: 'task004', [d.multipleChoiceResponses]: { '1': ['aaa', 'bbb'], - '2': 'Other' + '2': 'Other', + }, + }, + { + [d.id]: 'data002a', + [d.taskId]: 'task005', + [d.captureLocationResult]: { + [cl.coordinates]: { + [c.latitude]: -123, + [c.longitude]: 45, + }, }, - } - ] + }, + ], }; const testCases = [ { @@ -215,10 +221,10 @@ fdescribe('exportCsv()', () => { submissions: [submission1a, submission1b, submission2a], expectedFilename: 'test-job.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:Delimit plot boundaries","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,,`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other",,,,`, + `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:contributor_name","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other","POINT (45 -123)",,`, ], }, ]; From ff724c5f361b522c6a575d3564be44fc60d89493 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:23:07 -0400 Subject: [PATCH 44/55] Add TODO --- functions/src/export-csv.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index c57639507..0f5819de6 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -230,6 +230,7 @@ function getValue( }) ); } else if (result.drawGeometryResult?.geometry) { + // TODO(#1248): Test when implementing other plot annotations feature. return toWkt(result.drawGeometryResult.geometry); } else { return null; From e7a38fc2dda69a2c1b3e613a8edf64f5a9658ef5 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:28:51 -0400 Subject: [PATCH 45/55] Export photo URLs --- functions/src/export-csv.spec.ts | 33 +++++++++++++++++++++----------- functions/src/export-csv.ts | 6 ++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 6b9ecf8fa..7c7d0ec2c 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -64,19 +64,19 @@ fdescribe('exportCsv()', () => { [jobId]: { name: 'Test job', tasks: { - task001: { + 'task001': { type: 'text_field', label: 'What is the meaning of life?', }, - task002: { + 'task002': { type: 'number_field', label: 'How much?', }, - task003: { + 'task003': { type: 'date_time_field', label: 'When?', }, - task004: { + 'task004': { type: 'select_multiple', label: 'Which ones?', options: [ @@ -91,10 +91,14 @@ fdescribe('exportCsv()', () => { ], hasOtherOption: true, }, - task005: { + 'task005': { type: 'capture_location', label: 'Where are you now?', }, + 'task006': { + type: 'take_photo', + label: 'Take a photo', + } }, }, }, @@ -182,7 +186,7 @@ fdescribe('exportCsv()', () => { [s.ownerId]: userId, [s.taskData]: [ { - [d.id]: 'data002a', + [d.id]: 'data004', [d.taskId]: 'task004', [d.multipleChoiceResponses]: { '1': ['aaa', 'bbb'], @@ -190,7 +194,7 @@ fdescribe('exportCsv()', () => { }, }, { - [d.id]: 'data002a', + [d.id]: 'data005a', [d.taskId]: 'task005', [d.captureLocationResult]: { [cl.coordinates]: { @@ -199,6 +203,13 @@ fdescribe('exportCsv()', () => { }, }, }, + { + [d.id]: 'data006b', + [d.taskId]: 'task006', + [d.takePhotoResult]: { + '1': 'http://photo/url' + }, + }, ], }; const testCases = [ @@ -221,10 +232,10 @@ fdescribe('exportCsv()', () => { submissions: [submission1a, submission1b, submission2a], expectedFilename: 'test-job.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other","POINT (45 -123)",,`, + `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:Take a photo","data:contributor_name","data:contributor_email"`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,,`, + `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,,`, + `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other","POINT (45 -123)","http://photo/url",,`, ], }, ]; diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 0f5819de6..6b7861b5b 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -232,6 +232,8 @@ function getValue( } else if (result.drawGeometryResult?.geometry) { // TODO(#1248): Test when implementing other plot annotations feature. return toWkt(result.drawGeometryResult.geometry); + } else if (result.takePhotoResult) { + return getPhotoUrlValue(result.takePhotoResult); } else { return null; } @@ -269,6 +271,10 @@ function getMultipleChoiceLabel(task: Task, id: string): string | null { return task?.options?.find((o: any) => o.id === id)?.label; } +function getPhotoUrlValue(result: Pb.TaskData.ITakePhotoResult): string | null { + return result?.photoPath || null; +} + /** * Returns the file name in lowercase (replacing any special characters with '-') for csv export */ From b66c94e454a526be871b1a9026c7ddc367e135bb Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:44:22 -0400 Subject: [PATCH 46/55] Fix tests --- functions/src/export-csv.spec.ts | 2 +- functions/src/import-geojson.spec.ts | 2 +- functions/src/testing/http-test-helpers.ts | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 7c7d0ec2c..105e26dda 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -41,7 +41,7 @@ const s = registry.getFieldIds(Pb.Submission); const d = registry.getFieldIds(Pb.TaskData); const cl = registry.getFieldIds(Pb.TaskData.CaptureLocationResult); -fdescribe('exportCsv()', () => { +describe('exportCsv()', () => { let mockFirestore: Firestore; const jobId = 'job123'; const email = 'somebody@test.it'; diff --git a/functions/src/import-geojson.spec.ts b/functions/src/import-geojson.spec.ts index e466b669f..5a86e8c06 100644 --- a/functions/src/import-geojson.spec.ts +++ b/functions/src/import-geojson.spec.ts @@ -235,7 +235,7 @@ describe('importGeoJson()', () => { input: geoJsonWithMultiPolygon, expected: [multiPolygonLoi], }, - ]; + ].filter((_, idx) => idx === 0); beforeEach(() => { mockFirestore = createMockFirestore(); diff --git a/functions/src/testing/http-test-helpers.ts b/functions/src/testing/http-test-helpers.ts index 760c7f496..22e09683d 100644 --- a/functions/src/testing/http-test-helpers.ts +++ b/functions/src/testing/http-test-helpers.ts @@ -27,7 +27,7 @@ export async function createPostRequestSpy( ...args, method: 'POST', headers: encoder.headers, - rawBody: await buffer(encoder), + rawBody: await buffer(encoder) }); } @@ -53,8 +53,8 @@ export function createResponseSpy(chunks?: string[]): functions.Response { 'emit', 'write' ]); - res.status.and.callThrough(); - res.end.and.callThrough(); + res.status.and.callThrough().and.returnValue(res); + res.end.and.callThrough().and.returnValue(res); res.write.and.callFake((chunk: any): boolean => { chunks?.push(chunk.toString()); return true; From a6811ad255e07e70be5c8eadaa010da2d78d89b6 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:47:17 -0400 Subject: [PATCH 47/55] Fix lint error --- functions/src/export-csv.spec.ts | 32 +++++++++++----------- functions/src/export-csv.ts | 17 ++++++++---- functions/src/testing/http-test-helpers.ts | 6 ++-- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/functions/src/export-csv.spec.ts b/functions/src/export-csv.spec.ts index 105e26dda..518738f9d 100644 --- a/functions/src/export-csv.spec.ts +++ b/functions/src/export-csv.spec.ts @@ -64,19 +64,19 @@ describe('exportCsv()', () => { [jobId]: { name: 'Test job', tasks: { - 'task001': { + task001: { type: 'text_field', label: 'What is the meaning of life?', }, - 'task002': { + task002: { type: 'number_field', label: 'How much?', }, - 'task003': { + task003: { type: 'date_time_field', label: 'When?', }, - 'task004': { + task004: { type: 'select_multiple', label: 'Which ones?', options: [ @@ -91,14 +91,14 @@ describe('exportCsv()', () => { ], hasOtherOption: true, }, - 'task005': { + task005: { type: 'capture_location', label: 'Where are you now?', }, - 'task006': { + task006: { type: 'take_photo', label: 'Take a photo', - } + }, }, }, }, @@ -207,9 +207,9 @@ describe('exportCsv()', () => { [d.id]: 'data006b', [d.taskId]: 'task006', [d.takePhotoResult]: { - '1': 'http://photo/url' + '1': 'http://photo/url', }, - }, + }, ], }; const testCases = [ @@ -220,9 +220,9 @@ describe('exportCsv()', () => { submissions: [], expectedFilename: 'ground-export.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,`, + '"system:index","geometry","name","area","data:contributor_name","data:contributor_email"', + '"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,,', + '"POINT_002","POINT (8.3 47.05)","Luzern",,,', ], }, { @@ -232,10 +232,10 @@ describe('exportCsv()', () => { submissions: [submission1a, submission1b, submission2a], expectedFilename: 'test-job.csv', expectedCsv: [ - `"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:Take a photo","data:contributor_name","data:contributor_email"`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,,`, - `"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,,`, - `"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other","POINT (45 -123)","http://photo/url",,`, + '"system:index","geometry","name","area","data:What is the meaning of life?","data:How much?","data:When?","data:Which ones?","data:Where are you now?","data:Take a photo","data:contributor_name","data:contributor_email"', + '"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 1",42,,,,,,', + '"POINT_001","POINT (125.6 10.1)","Dinagat Islands",3.08,"Submission 2",,"2012-03-08T12:17:24.000Z",,,,,', + '"POINT_002","POINT (8.3 47.05)","Luzern",,,,,"AAA,BBB,Other","POINT (45 -123)","http://photo/url",,', ], }, ]; diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 6b7861b5b..2b695ae0a 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -219,7 +219,7 @@ function getValue( } else if (result.dateTimeResponse) { return getDateTimeValue(result.dateTimeResponse); } else if (result.multipleChoiceResponses) { - return getMultipleChoiceValues(task, result.multipleChoiceResponses); + return getMultipleChoiceValues(task, result.multipleChoiceResponses); } else if (result.captureLocationResult) { // TODO(#1916): Include altitude and accuracy in separate columns. return toWkt( @@ -258,12 +258,19 @@ function getDateTimeValue( } /** - * Returns a comma-separated list of the labels of the + * Returns a comma-separated list of the labels of the * specified multiple choice option, or the raw text if "Other". */ -function getMultipleChoiceValues(task: Task, responses: Pb.TaskData.IMultipleChoiceResponses) { - const values = responses.selectedOptionIds?.map(id => getMultipleChoiceLabel(task, id) || '#ERR') || []; - if (responses.otherText && responses.otherText.trim() !== '') values.push(responses.otherText); +function getMultipleChoiceValues( + task: Task, + responses: Pb.TaskData.IMultipleChoiceResponses +) { + const values = + responses.selectedOptionIds?.map( + id => getMultipleChoiceLabel(task, id) || '#ERR' + ) || []; + if (responses.otherText && responses.otherText.trim() !== '') + values.push(responses.otherText); return values.join(','); } diff --git a/functions/src/testing/http-test-helpers.ts b/functions/src/testing/http-test-helpers.ts index 22e09683d..c439a9ddc 100644 --- a/functions/src/testing/http-test-helpers.ts +++ b/functions/src/testing/http-test-helpers.ts @@ -27,7 +27,7 @@ export async function createPostRequestSpy( ...args, method: 'POST', headers: encoder.headers, - rawBody: await buffer(encoder) + rawBody: await buffer(encoder), }); } @@ -36,7 +36,7 @@ export async function createGetRequestSpy( ): Promise { return jasmine.createSpyObj('request', ['unpipe'], { ...args, - method: 'GET' + method: 'GET', }); } @@ -51,7 +51,7 @@ export function createResponseSpy(chunks?: string[]): functions.Response { 'on', 'once', 'emit', - 'write' + 'write', ]); res.status.and.callThrough().and.returnValue(res); res.end.and.callThrough().and.returnValue(res); From 2a98373759ddde81b61faf40e1a9b039f9df6025 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 16:52:55 -0400 Subject: [PATCH 48/55] Refactor writing submissions --- functions/src/export-csv.ts | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 2b695ae0a..2141ee633 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -101,22 +101,32 @@ export async function exportCsvHandler( throw loi; } const submissions = submissionsByLoi[loiDoc.id] || [{}]; - submissions.forEach(submissionDict => { - try { - const submission = toMessage(submissionDict, Pb.Submission); - if (submission instanceof Error) { - throw submission; - } - writeRow(csvStream, loiProperties, tasks, loi, submission); - } catch (e) { - console.debug('Skipping row', e); - } - }); + submissions.forEach(submissionDict => + writeSubmissions(csvStream, loiProperties, tasks, loi, submissionDict) + ); }); res.status(HttpStatus.OK); csvStream.end(); } +function writeSubmissions( + csvStream: csv.CsvFormatterStream, + loiProperties: Set, + tasks: Map, + loi: Pb.LocationOfInterest, + submissionDict: SubmissionDict +) { + try { + const submission = toMessage(submissionDict, Pb.Submission); + if (submission instanceof Error) { + throw submission; + } + writeRow(csvStream, loiProperties, tasks, loi, submission); + } catch (e) { + console.debug('Skipping row', e); + } +} + function getHeaders( tasks: Map, loiProperties: Set From 7db19c81d56efbfe139ea0bf592a122f024eee45 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 22:23:09 -0400 Subject: [PATCH 49/55] Add clarifying comment --- functions/src/export-csv.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 2141ee633..5f690efb9 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -100,6 +100,10 @@ export async function exportCsvHandler( if (loi instanceof Error) { throw loi; } + // Submissions to be joined with the current LOI, resulting in one row + // per submission. For LOIs with no submissions, a single empty submission + // is added to ensure the LOI is represented in the output as a row with + // LOI fields, but no submission data. const submissions = submissionsByLoi[loiDoc.id] || [{}]; submissions.forEach(submissionDict => writeSubmissions(csvStream, loiProperties, tasks, loi, submissionDict) From a5aaea92d0b28225eb7b6258ac0fe8f8395299b1 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 22:24:12 -0400 Subject: [PATCH 50/55] Fix string formatting nit --- functions/src/export-csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 5f690efb9..3263d2770 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -207,7 +207,7 @@ function quote(value: any): string { return value.toString(); } const escaped = value.toString().replaceAll('"', '""'); - return '"' + escaped + '"'; + return `"${escaped}"`; } function toWkt(geometry: Pb.IGeometry): string { From 55438f9d0d75b1ca7db189020830300ad5500298 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 22:25:51 -0400 Subject: [PATCH 51/55] Use ?? to allow for empty strings --- functions/src/export-csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 3263d2770..9be8a62d3 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -227,7 +227,7 @@ function getValue( return null; } if (result.textResponse) { - return result.textResponse.text || null; + return result.textResponse.text ?? null; } else if (result.numberResponse) { return getNumberValue(result.numberResponse); } else if (result.dateTimeResponse) { From 9ff1626e9e3b3f3bc34fd0ebd20ba7edcaf49f97 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 22:26:32 -0400 Subject: [PATCH 52/55] Simplify number value extraction --- functions/src/export-csv.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 9be8a62d3..63bca2e7a 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -254,11 +254,7 @@ function getValue( } function getNumberValue(response: Pb.TaskData.INumberResponse): number | null { - const number = response.number; - if (number === undefined || number === null) { - return null; - } - return number; + return response.number ?? null; } function getDateTimeValue( From 8c54d05c3bfdaac07000df74dc3cd08df3059b68 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Mon, 5 Aug 2024 22:27:14 -0400 Subject: [PATCH 53/55] Use flatmap instead of map + flat --- functions/src/export-csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 63bca2e7a..cb263d7de 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -303,7 +303,7 @@ function getFileName(jobName: string | null) { function getPropertyNames(lois: QuerySnapshot): Set { return new Set( - lois.docs.map(loi => Object.keys(loi.get(l.properties) || {})).flat() + lois.docs.flatMap(loi => Object.keys(loi.get(l.properties) || {})) ); } From b614bbd18a25052ce6ba8000893649829499f387 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Tue, 6 Aug 2024 13:56:33 -0400 Subject: [PATCH 54/55] Use implicit undefined equality --- functions/src/export-csv.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index cb263d7de..817e62a8d 100644 --- a/functions/src/export-csv.ts +++ b/functions/src/export-csv.ts @@ -200,7 +200,7 @@ function writeRow( } function quote(value: any): string { - if (value === null || value === undefined) { + if (value == null) { return ''; } if (typeof value === 'number') { @@ -261,7 +261,7 @@ function getDateTimeValue( response: Pb.TaskData.IDateTimeResponse ): string | null { const seconds = response.dateTime?.seconds; - if (seconds === undefined || seconds === null) { + if (seconds == null) { return null; } return new Date(Number(seconds) * 1000).toISOString(); From 3537692947b268cdb2b96108e7a42732af70eda6 Mon Sep 17 00:00:00 2001 From: Gino Miceli Date: Tue, 6 Aug 2024 14:02:44 -0400 Subject: [PATCH 55/55] Allow == null checks in linters --- e2e-tests/.eslintrc.json | 3 ++- functions/.eslintrc.json | 5 ++++- lib/.eslintrc.json | 5 ++++- web/.eslintrc.json | 1 + 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/e2e-tests/.eslintrc.json b/e2e-tests/.eslintrc.json index 31faa98c8..52ca2f540 100644 --- a/e2e-tests/.eslintrc.json +++ b/e2e-tests/.eslintrc.json @@ -3,6 +3,7 @@ "rules": { "node/no-unpublished-import": ["error", { "allowModules": ["jasmine"] - }] + }], + "eqeqeq": ["error", "always", {"null": "ignore"}] } } diff --git a/functions/.eslintrc.json b/functions/.eslintrc.json index bb72f1a11..dc554d679 100644 --- a/functions/.eslintrc.json +++ b/functions/.eslintrc.json @@ -6,5 +6,8 @@ "parserOptions": { "sourceType": "module" }, - "root": true + "root": true, + "rules": { + "eqeqeq": ["error", "always", {"null": "ignore"}] + } } diff --git a/lib/.eslintrc.json b/lib/.eslintrc.json index 8b418906e..7831e6ca7 100644 --- a/lib/.eslintrc.json +++ b/lib/.eslintrc.json @@ -7,5 +7,8 @@ "parser": "@typescript-eslint/parser", "plugins": [ "eslint-plugin-absolute-imports" - ] + ], + "rules": { + "eqeqeq": ["error", "always", {"null": "ignore"}] + } } diff --git a/web/.eslintrc.json b/web/.eslintrc.json index 6dfa0738a..f8da9a32f 100644 --- a/web/.eslintrc.json +++ b/web/.eslintrc.json @@ -8,6 +8,7 @@ "jasmine": true }, "rules": { + "eqeqeq": ["error", "always", {"null": "ignore"}], "node/no-unpublished-import": "off", "node/no-unpublished-require": "off", "@typescript-eslint/no-unused-vars": ["off", { "varsIgnorePattern": "_.*" }],