Merge branch 'google:master' into master

firestore/firestore.rules

@@ -26,10 +26,12 @@
     }
 
    /**
-     * Returns the role of the specified email address in the given survey.
+     * Returns the current user's role in the given survey.
      */
     function getRole(survey) {
-      return survey.acl[request.auth.token.email];
+      // Get role from `acl` nested object (field 4).
+      // TODO(#1858): Delete fallback to 'acl' collection once migration is complete.
+      return ("4" in survey ? survey["4"] : survey.acl)[request.auth.token.email];
     }
 
     /**
@@ -63,28 +65,41 @@
     }
 
     /**
-     * Returns true iff the user with the given email has one of the specified
-     * roles in the given survey.
+     * Returns true if the current user has one of the specified roles in the
+     * given survey.
      */
     function isOneOf(survey, roles) {
-      return survey.acl[request.auth.token.email] in roles;
+      // TODO(#1858): Delete fallback to 'acl' collection once migration is complete.
+     return ("4" in survey ? survey["4"] : survey.acl)[request.auth.token.email] in roles;
     }
 
     /**
-     * Returns true iff the user with the given email can manage the specified
-     * survey (modify the survey document, edit user data, etc.).
+     * Returns true if the current user is passlisted and has the `SURVEY_ORGANIZER` role
+     * in the specified survey. Note that this include survey owners, since they are
+     * assigned this role by default.
      */
     function canManageSurvey(survey) {
-      return canAccess() && isOneOf(survey, ['OWNER', 'SURVEY_ORGANIZER']);
+      // TODO(#1858): Delete string role keys once migration is complete.
+      return canAccess() && isOneOf(survey, [
+        'OWNER',
+        'SURVEY_ORGANIZER',
+        3 /* SURVEY_ORGANIZER */
+      ]);
     }
 
     /**
-     * Returns true iff the user with the given email can contribute LOIs 
-     * and submissions to the specified survey (i.e., add/edit LOIs and
-     * submissions).
+     * Returns true iff the current user with the given email can contribute LOIs 
+     * and submissions to the specified survey.
      */
     function canCollectData(survey) {
-      return canAccess() && isOneOf(survey, ['OWNER', 'SURVEY_ORGANIZER', 'DATA_COLLECTOR']);
+      // TODO(#1858): Delete string role keys once migration is complete.
+      return canAccess() && isOneOf(survey, [
+        'OWNER',
+        'SURVEY_ORGANIZER',
+        'DATA_COLLECTOR',
+        2 /* DATA_COLLECTOR */,
+        3 /* SURVEY_ORGANIZER */
+      ]);
     }
 
     /**

functions/src/common/auth.ts

@@ -23,7 +23,7 @@ import {EmulatorIdToken} from '../handlers';
 // https://firebase.google.com/docs/hosting/manage-cache#using_cookies
 export const SESSION_COOKIE_NAME = '__session';
 export const OWNER_ROLE = 'OWNER';
-const DATA_COLLECTOR_ROLE = 'DATA_COLLECTOR';
+const SURVEY_ORGANIZER_ROLE = 'SURVEY_ORGANIZER';
 
 /**
  * Returns the encoded auth token from the "Authorization: Bearer" HTTP header
@@ -101,5 +101,5 @@ export function canImport(
   survey: DocumentSnapshot
 ): boolean {
   const role = getRole(user, survey);
-  return role === OWNER_ROLE || role === DATA_COLLECTOR_ROLE;
+  return role === OWNER_ROLE || role === SURVEY_ORGANIZER_ROLE;
 }

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<QuerySnapshot<DocumentData, DocumentData>> {
     return this.db_
       .collection(lois(surveyId))
       .where('jobId', '==', jobId)

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 = {
@@ -36,7 +37,19 @@ type Task = {
   readonly hasOtherOption?: boolean;
 };
 
-// TODO: Refactor into meaningful pieces.
+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<FirebaseFirestore.DocumentData>;
+
+/**
+ * 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<any>,
@@ -61,16 +74,9 @@ 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 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 loiDocs = await db.fetchLocationsOfInterestByJobId(survey.id, jobId);
+  const loiProperties = getPropertyNames(loiDocs);
+  const headers = getHeaders(tasks, loiProperties);
 
   res.type('text/csv');
   res.setHeader(
@@ -87,49 +93,83 @@ export async function exportCsvHandler(
   });
   csvStream.pipe(res);
 
-  const submissions = await db.fetchSubmissionsByJobId(survey.id, jobId);
+  const submissionsByLoi = await getSubmissionsByLoi(survey.id, jobId);
+
+  loiDocs.forEach(loiDoc => {
+    submissionsByLoi[loiDoc.id]?.forEach(submission =>
+      writeRow(csvStream, loiProperties, tasks, loiDoc, submission)
+    );
+  });
+  csvStream.end();
+}
+
+function getHeaders(
+  tasks: Map<string, Task>,
+  loiProperties: Set<string>
+): 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;
+}
 
-  // 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 submissionsByLocationOfInterest: {[name: string]: any[]} = {};
+/**
+ * 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<SubmissionDict> {
+  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[] = submissionsByLocationOfInterest[loiId] || [];
+    const arr: any[] = submissionsByLoi[loiId] || [];
     arr.push(submission.data());
-    submissionsByLocationOfInterest[loiId] = arr;
+    submissionsByLoi[loiId] = arr;
   });
+  return submissionsByLoi;
+}
 
-  lois.forEach(loi => {
-    const loiId = loi.id;
-    const submissions = submissionsByLocationOfInterest[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, allLoiProperties));
-      // 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);
-    });
-  });
-  csvStream.end();
+function writeRow(
+  csvStream: csv.CsvFormatterStream<csv.Row, csv.Row>,
+  loiProperties: Set<string>,
+  tasks: Map<string, Task>,
+  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);
 }
 
 /**
@@ -211,9 +251,7 @@ function getFileName(jobName: string) {
   return `${fileBase}.csv`;
 }
 
-function getPropertyNames(
-  lois: FirebaseFirestore.QuerySnapshot<FirebaseFirestore.DocumentData>
-): Set<string> {
+function getPropertyNames(lois: QuerySnapshot<DocumentData>): Set<string> {
   return new Set(
     lois.docs
       .map(loi =>
@@ -226,11 +264,11 @@ function getPropertyNames(
 }
 
 function getPropertiesByName(
-  loi: FirebaseFirestore.QueryDocumentSnapshot<FirebaseFirestore.DocumentData>,
-  allLoiProperties: Set<string>
+  loiDoc: LoiDocument,
+  loiProperties: Set<string>
 ): List<string> {
   // Fill the list with the value associated with a prop, if the LOI has it, otherwise leave empty.
-  return List.of(...allLoiProperties).map(
-    prop => (loi.get('properties') || {})[prop] || ''
+  return List.of(...loiProperties).map(
+    prop => (loiDoc.get('properties') || {})[prop] || ''
   );
 }

functions/src/on-create-loi.ts

@@ -51,6 +51,10 @@ export async function onCreateLoiHandler(
       properties,
       wkt
     );
+
+    Object.keys(properties)
+      .filter(key => typeof properties[key] === 'object')
+      .forEach(key => (properties[key] = JSON.stringify(properties[key])));
   }
 
   await db.updateLoiProperties(surveyId, loiId, properties);

proto/src/google/ground/v1beta1/job.proto

@@ -243,5 +243,9 @@ message Task {
     // Required. List of multiple choice option IDs which trigger the associated
     // task. The task is shown if one or more of the related options are selected.
     repeated string option_ids = 1;
+
+    // Required. The system-defined unique identifier of the task to which this
+    // MultipleChoiceSelection refers.
+    string task_id = 2;
   }
 }

proto/src/google/ground/v1beta1/loi.proto

@@ -30,7 +30,8 @@ message LocationOfInterest {
   // Required. The system-defined unique identifier of this entity.
   string id = 1;
 
-  // Required. The system-defined of the job to which this LOI belongs.
+  // Required. The system-defined unique identifier of the job to which this
+  // LOI belongs.
   string job_id = 2;
 
   // Required. The geometry associated with this LOI.

web/src/app/converters/firebase-data-converter.ts

@@ -422,12 +422,12 @@ export class FirebaseDataConverter {
     };
   }
 
-  static toUser(data: DocumentData): User | undefined {
+  static toUser(data: DocumentData, uid: string): User | undefined {
     if (!data) {
       return;
     }
     return new User(
-      data.id,
+      uid,
       data.email,
       data.isAuthenticated || true,
       data.displayName,