Merge branch 'master' into rfontanarosa/1833/code-health-add-lint-check-for-copyright-headers

# Conflicts:
#	pnpm-lock.yaml

Author: rfontanarosa

README.md

@@ -93,3 +93,25 @@ To deploy to your own production Firebase:
 ### Next steps
 
 For instructions on how to deploy to your own production Firebase project, see the [Ground Developer's Guide](https://github.com/google/ground-platform/wiki/Ground-Developer's-Guide).
+
+## Common commands
+
+All commands use [Nx](https://nx.dev) and [pnpm](https://pnpm.io). Run them from the repo root.
+
+| Command | Description |
+| ------- | ----------- |
+| `nx start` | Build and start the local Firebase emulator and Angular dev server |
+| `nx run root:build` | Build all packages |
+| `nx run root:test` | Run all tests |
+| `nx run root:lint` | Lint all packages |
+| `nx run web:serve` | Serve the web app only (no emulator) |
+| `nx run web:serve:staging` | Serve the web app against the live staging environment |
+| `nx run web:test` | Run web tests |
+| `nx run web:extract-i18n` | Extract i18n messages from the web app |
+| `nx run functions:test` | Run Cloud Functions tests |
+| `nx run functions:emulate` | Start the Firebase emulator for functions only |
+| `nx run functions:logs` | Tail Cloud Functions logs |
+| `nx run functions:shell` | Open an interactive Cloud Functions shell |
+| `nx run root:export:local` | Save current emulator data to disk for use on next run |
+| `nx run root:deploy:staging` | Build and deploy all to staging |
+| `nx run root:deploy:production` | Build and deploy all to production |

docs/Gemfile.lock

@@ -1,31 +1,34 @@
 GEM
   remote: https://rubygems.org/
   specs:
-    activesupport (7.1.3.4)
+    activesupport (7.2.3.1)
       base64
+      benchmark (>= 0.3)
       bigdecimal
-      concurrent-ruby (~> 1.0, >= 1.0.2)
+      concurrent-ruby (~> 1.0, >= 1.3.1)
       connection_pool (>= 2.2.5)
       drb
       i18n (>= 1.6, < 2)
-      minitest (>= 5.1)
-      mutex_m
-      tzinfo (~> 2.0)
+      logger (>= 1.4.2)
+      minitest (>= 5.1, < 6)
+      securerandom (>= 0.3)
+      tzinfo (~> 2.0, >= 2.0.5)
     addressable (2.8.0)
       public_suffix (>= 2.0.2, < 5.0)
-    base64 (0.2.0)
-    bigdecimal (3.1.8)
+    base64 (0.3.0)
+    benchmark (0.5.0)
+    bigdecimal (4.0.1)
     coffee-script (2.4.1)
       coffee-script-source
       execjs
     coffee-script-source (1.11.1)
     colorator (1.1.0)
     commonmarker (0.23.10)
-    concurrent-ruby (1.3.3)
-    connection_pool (2.4.1)
+    concurrent-ruby (1.3.6)
+    connection_pool (3.0.2)
     dnsruby (1.61.7)
       simpleidn (~> 0.1)
-    drb (2.2.1)
+    drb (2.2.3)
     em-websocket (0.5.2)
       eventmachine (>= 0.12.9)
       http_parser.rb (~> 0.6.0)
@@ -97,7 +100,7 @@ GEM
       activesupport (>= 2)
       nokogiri (>= 1.4)
     http_parser.rb (0.6.0)
-    i18n (1.14.5)
+    i18n (1.14.8)
       concurrent-ruby (~> 1.0)
     jekyll (3.9.3)
       addressable (~> 2.4)
@@ -223,8 +226,7 @@ GEM
       jekyll (>= 3.5, < 5.0)
       jekyll-feed (~> 0.9)
       jekyll-seo-tag (~> 2.1)
-    minitest (5.23.1)
-    mutex_m (0.2.0)
+    minitest (5.27.0)
     net-http (0.9.1)
       uri (>= 0.11.1)
     nokogiri (1.19.1)
@@ -252,6 +254,7 @@ GEM
     sawyer (0.9.3)
       addressable (>= 2.3.5)
       faraday (>= 0.17.3, < 3)
+    securerandom (0.4.1)
     simpleidn (0.2.1)
       unf (~> 0.1.4)
     terminal-table (1.8.0)

firestore/firestore.rules

@@ -182,8 +182,8 @@ rules_version = '2';
       allow get: if canViewLoi(getSurvey(surveyId), resource.data);
       // Allow all survey users to list LOIs.
       allow list: if canViewSurvey(getSurvey(surveyId));
-      // Allow if user is owner of the existing LOI or can manage survey.
-      allow update, delete: if isLoiOwner(resource.data) || canManageSurvey(getSurvey(surveyId));
+      // Allow if user is owner of the existing LOI and can collect data, or can manage survey.
+      allow update, delete: if (isLoiOwner(resource.data) && canCollectData(getSurvey(surveyId))) || canManageSurvey(getSurvey(surveyId));
     }
 
     // Apply survey-level permissions to submission documents.
@@ -194,8 +194,8 @@ rules_version = '2';
       allow get: if canViewSubmission(getSurvey(surveyId), resource.data);
       // Allow all survey users to list submissions.
       allow list: if canViewSurvey(getSurvey(surveyId));
-      // Allow if user is owner of the existing submission or can manage survey.
-      allow update, delete: if isSubmissionOwner(resource.data) || canManageSurvey(getSurvey(surveyId));
+      // Allow if user is owner of the existing submission and can collect data, or can manage survey.
+      allow update, delete: if (isSubmissionOwner(resource.data) && canCollectData(getSurvey(surveyId))) || canManageSurvey(getSurvey(surveyId));
     }
 
     // Apply survey-level permissions to job documents.

functions/src/common/broadcast-survey-update.ts

@@ -25,6 +25,11 @@ import * as admin from 'firebase-admin';
  * See also: https://firebase.google.com/docs/cloud-messaging/concept-options#collapsible_and_non-collapsible_messages
  */
 export async function broadcastSurveyUpdate(topic: string): Promise<string> {
+  if (process.env.FUNCTIONS_EMULATOR === 'true') {
+    console.debug(`Skipping FCM message to ${topic} (emulator mode)`);
+    return '';
+  }
+
   console.debug(`Sending message to ${topic}`);
 
   return admin.messaging().send({ topic });

functions/src/handlers.ts

@@ -16,7 +16,7 @@
 
 import cors from 'cors';
 import { DecodedIdToken } from 'firebase-admin/auth';
-import { onRequest, Request } from 'firebase-functions/v2/https';
+import { onRequest, HttpsOptions, Request } from 'firebase-functions/v2/https';
 import type { Response } from 'express';
 import { StatusCodes } from 'http-status-codes';
 import { getDecodedIdToken } from './common/auth';
@@ -74,8 +74,11 @@ export type HttpsRequestHandler = (
 /**
  * A synchronous HTTPS request handler. The HTTPS request is closed as soon as the handler resolves.
  */
-export function onHttpsRequest(handler: HttpsRequestHandler) {
-  return onRequest((req: Request, res: Response) =>
+export function onHttpsRequest(
+  handler: HttpsRequestHandler,
+  options: HttpsOptions = {}
+) {
+  return onRequest(options, (req: Request, res: Response) =>
     corsMiddleware(req, res, () =>
       cookieParser()(
         req as any,

functions/src/import-geojson.spec.ts

@@ -28,7 +28,7 @@ import { Blob, FormData } from 'formdata-node';
 import { StatusCodes } from 'http-status-codes';
 import { invokeCallbackAsync } from './handlers';
 import { SURVEY_ORGANIZER_ROLE } from './common/auth';
-import { resetDatastore } from './common/context';
+import { getDatastore, resetDatastore } from './common/context';
 import { Firestore } from 'firebase-admin/firestore';
 import { registry } from '@ground/lib';
 import { GroundProtos } from '@ground/proto';
@@ -274,13 +274,37 @@ describe('importGeoJson()', () => {
         await invokeCallbackAsync(importGeoJsonCallback, req, res, {
           email,
         } as DecodedIdToken);
-      } catch (error) {
-        console.log(error);
+      } catch (err) {
+        console.log(err);
       }
 
       // Check post-conditions.
       expect(res.status).toHaveBeenCalledOnceWith(expectedStatus);
       expect(await loiData(surveyId)).toEqual(expected);
     })
   );
+
+  it('surfaces errors thrown during async file processing', async () => {
+    // Make fetchSurvey reject to simulate an unexpected async error in the file handler.
+    const db = getDatastore();
+    spyOn(db, 'fetchSurvey').and.returnValue(
+      Promise.reject(new Error('Database connection failed'))
+    );
+
+    const req = await createPostRequestSpy(
+      { url: '/importGeoJson' },
+      createPostData(surveyId, jobId, geoJsonWithPoint)
+    );
+    const res = createResponseSpy();
+
+    try {
+      await invokeCallbackAsync(importGeoJsonCallback, req, res, {
+        email,
+      } as DecodedIdToken);
+    } catch {
+      // Expected to reject.
+    }
+
+    expect(res.status).toHaveBeenCalledWith(StatusCodes.INTERNAL_SERVER_ERROR);
+  });
 });

functions/src/import-geojson.ts

@@ -68,41 +68,45 @@ export function importGeoJsonCallback(
 
   // This code will process each file uploaded.
   busboy.on('file', async (_fieldname, fileStream) => {
-    const { survey: surveyId, job: jobId } = params;
-    if (!surveyId || !jobId) {
-      return error(StatusCodes.BAD_REQUEST, 'Missing survey and/or job ID');
-    }
-    const survey = await db.fetchSurvey(surveyId);
-    if (!survey.exists) {
-      return error(StatusCodes.NOT_FOUND, `Survey ${surveyId} not found`);
-    }
-    if (!canImport(user, survey)) {
-      return error(
-        StatusCodes.FORBIDDEN,
-        `User does not have permission to import into survey ${surveyId}`
+    try {
+      const { survey: surveyId, job: jobId } = params;
+      if (!surveyId || !jobId) {
+        return error(StatusCodes.BAD_REQUEST, 'Missing survey and/or job ID');
+      }
+      const survey = await db.fetchSurvey(surveyId);
+      if (!survey.exists) {
+        return error(StatusCodes.NOT_FOUND, `Survey ${surveyId} not found`);
+      }
+      if (!canImport(user, survey)) {
+        return error(
+          StatusCodes.FORBIDDEN,
+          `User does not have permission to import into survey ${surveyId}`
+        );
+      }
+
+      console.debug(
+        `Importing GeoJSON into survey '${surveyId}', job '${jobId}'`
       );
-    }
 
-    console.debug(
-      `Importing GeoJSON into survey '${surveyId}', job '${jobId}'`
-    );
+      const parser = JSONStream.parse(['features', true], undefined);
 
-    const parser = JSONStream.parse(['features', true], undefined);
-
-    fileStream.pipe(
-      parser
-        .on('header', (data: any) => {
-          try {
-            onGeoJsonType(data.type);
-            if (data.crs) onGeoJsonCrs(data.crs);
-          } catch (error: any) {
-            busboy.emit('error', error);
-          }
-        })
-        .on('data', (data: any) => {
-          if (!hasError) onGeoJsonFeature(data, surveyId, jobId);
-        })
-    );
+      fileStream.pipe(
+        parser
+          .on('header', (data: any) => {
+            try {
+              onGeoJsonType(data.type);
+              if (data.crs) onGeoJsonCrs(data.crs);
+            } catch (error: any) {
+              busboy.emit('error', error);
+            }
+          })
+          .on('data', (data: any) => {
+            if (!hasError) onGeoJsonFeature(data, surveyId, jobId);
+          })
+      );
+    } catch (err) {
+      busboy.emit('error', err);
+    }
   });
 
   // Handle non-file fields in the task. survey and job must appear

functions/src/index.ts

@@ -70,9 +70,17 @@ export const onCreatePasslistEntry = onDocumentCreated(
 
 export const importGeoJson = onHttpsRequestAsync(importGeoJsonCallback);
 
-export const exportCsv = onHttpsRequest(exportCsvHandler);
-
-export const exportGeojson = onHttpsRequest(exportGeojsonHandler);
+export const exportCsv = onHttpsRequest(exportCsvHandler, {
+  memory: '4GiB',
+  timeoutSeconds: 3600,
+  cpu: 2,
+});
+
+export const exportGeojson = onHttpsRequest(exportGeojsonHandler, {
+  memory: '4GiB',
+  timeoutSeconds: 3600,
+  cpu: 2,
+});
 
 export const onCreateLoi = onDocumentCreated(
   loiPathTemplate,