[Edit survey] Write using new proto-based Firestore representation (g…

…oogle#1889)

firestore/firestore.rules

@@ -138,5 +138,15 @@
       // Allow if user is owner of the existing submission or can manage survey.
       allow write: if isCreator(resource) || canManageSurvey(getSurvey(surveyId));
     }
+
+    // Apply passlist and survey-level ACLs to job documents.
+    match /surveys/{surveyId}/jobs/{jobId} {
+      // Allow if user has has read access to the survey.
+      allow read: if canViewSurvey(getSurvey(surveyId));
+      // Allow if user is owner of the new submission and can collect data.
+      allow create: if canManageSurvey(getSurvey(surveyId)) || isCreator(request.resource);
+      // Allow if user is owner of the existing submission or can manage survey.
+      allow write: if canManageSurvey(getSurvey(surveyId)) || isCreator(resource);
+    }
   }
 }

web/src/app/converters/proto-model-converter.ts

@@ -19,9 +19,12 @@ import {toDocumentData} from '@ground/lib';
 import {GroundProtos} from '@ground/proto';
 import {Map} from 'immutable';
 
+import {Job} from 'app/models/job.model';
 import {Role} from 'app/models/role.model';
+import {Cardinality} from 'app/models/task/multiple-choice.model';
+import {Task, TaskType} from 'app/models/task/task.model';
 
-const Pb = GroundProtos.google.ground.v1beta1;
+import Pb = GroundProtos.google.ground.v1beta1;
 
 const PB_ROLES = Map([
   [Role.OWNER, Pb.Role.SURVEY_ORGANIZER],
@@ -33,7 +36,7 @@ const PB_ROLES = Map([
 /**
  * Converts Role instance to its proto message type.
  */
-export function roleToProto(role: Role) {
+export function roleToProtoRole(role: Role) {
   const pbRole = PB_ROLES.get(role);
 
   if (!pbRole) throw new Error(`Invalid role encountered: ${role}`);
@@ -44,7 +47,7 @@ export function roleToProto(role: Role) {
 /**
  * Creates a proto rapresentation of a Survey.
  */
-export function newSurveyToProto(
+export function newSurveyToDocument(
   name: string,
   description: string,
   acl: Map<string, Role>,
@@ -54,7 +57,7 @@ export function newSurveyToProto(
     new Pb.Survey({
       name,
       description,
-      acl: acl.map(role => roleToProto(role)).toObject(),
+      acl: acl.map(role => roleToProtoRole(role)).toObject(),
       ownerId,
     })
   );
@@ -63,7 +66,7 @@ export function newSurveyToProto(
 /**
  * Creates a proto rapresentation of a Survey.
  */
-export function partialSurveyToProto(
+export function partialSurveyToDocument(
   name: string,
   description?: string
 ): DocumentData | Error {
@@ -74,3 +77,151 @@ export function partialSurveyToProto(
     })
   );
 }
+
+/**
+ * Creates a proto rapresentation of a survey access control list.
+ */
+export function aclToDocument(acl: Map<string, Role>): DocumentData | Error {
+  return toDocumentData(
+    new Pb.Survey({
+      acl: acl.map(role => roleToProtoRole(role)).toObject(),
+    })
+  );
+}
+
+/**
+ * Creates a proto rapresentation of a Job.
+ */
+export function jobToDocument(job: Job): DocumentData {
+  return toDocumentData(
+    new Pb.Job({
+      id: job.id,
+      index: job.index,
+      name: job.name,
+      style: new Pb.Style({color: job.color}),
+      tasks: job.tasks
+        ?.map(task => {
+          return new Pb.Task({
+            ...taskTypeToPartialMessage(task),
+            id: task.id,
+            index: task.index,
+            prompt: task.label,
+            required: task.required,
+            level: task.addLoiTask
+              ? Pb.Task.DataCollectionLevel.LOI_DATA
+              : Pb.Task.DataCollectionLevel.LOI_METADATA,
+            conditions: taskConditionToPartialMessage(task),
+          });
+        })
+        .toList()
+        .toArray(),
+    })
+  );
+}
+
+/**
+ * Creates a partial rapresentation of a Task.
+ */
+function taskTypeToPartialMessage(task: Task): Pb.ITask {
+  const {type: taskType, multipleChoice: taskMultipleChoice} = task;
+
+  switch (taskType) {
+    case TaskType.TEXT:
+      return {
+        textQuestion: new Pb.Task.TextQuestion({
+          type: Pb.Task.TextQuestion.Type.SHORT_TEXT,
+        }),
+      };
+    case TaskType.MULTIPLE_CHOICE:
+      return {
+        multipleChoiceQuestion: new Pb.Task.MultipleChoiceQuestion({
+          type:
+            taskMultipleChoice!.cardinality === Cardinality.SELECT_ONE
+              ? Pb.Task.MultipleChoiceQuestion.Type.SELECT_ONE
+              : Pb.Task.MultipleChoiceQuestion.Type.SELECT_MULTIPLE,
+          hasOtherOption: task.multipleChoice!.hasOtherOption,
+          options: taskMultipleChoice!.options
+            .map(
+              option =>
+                new Pb.Task.MultipleChoiceQuestion.Option({
+                  id: option.id,
+                  index: option.index,
+                  label: option.label,
+                })
+            )
+            .toArray(),
+        }),
+      };
+    case TaskType.PHOTO:
+      return {
+        takePhoto: new Pb.Task.TakePhoto({
+          minHeadingDegrees: 0,
+          maxHeadingDegrees: 360,
+        }),
+      };
+    case TaskType.NUMBER:
+      return {
+        numberQuestion: new Pb.Task.NumberQuestion({
+          type: Pb.Task.NumberQuestion.Type.FLOAT,
+        }),
+      };
+    case TaskType.DATE:
+      return {
+        dateTimeQuestion: new Pb.Task.DateTimeQuestion({
+          type: Pb.Task.DateTimeQuestion.Type.DATE_ONLY,
+        }),
+      };
+    case TaskType.TIME:
+      return {
+        dateTimeQuestion: new Pb.Task.DateTimeQuestion({
+          type: Pb.Task.DateTimeQuestion.Type.TIME_ONLY,
+        }),
+      };
+    case TaskType.DATE_TIME:
+      return {
+        dateTimeQuestion: new Pb.Task.DateTimeQuestion({
+          type: Pb.Task.DateTimeQuestion.Type.BOTH_DATE_AND_TIME,
+        }),
+      };
+    case TaskType.DRAW_AREA:
+      return {
+        drawGeometry: new Pb.Task.DrawGeometry({
+          allowedMethods: [Pb.Task.DrawGeometry.Method.DRAW_AREA],
+        }),
+      };
+    case TaskType.DROP_PIN:
+      return {
+        drawGeometry: new Pb.Task.DrawGeometry({
+          allowedMethods: [Pb.Task.DrawGeometry.Method.DROP_PIN],
+        }),
+      };
+    case TaskType.CAPTURE_LOCATION:
+      return {
+        captureLocation: new Pb.Task.CaptureLocation({
+          minAccuracyMeters: null,
+        }),
+      };
+    default:
+      throw new Error(`Invalid role encountered: ${taskType}`);
+  }
+}
+
+/**
+ * Creates a partial rapresentation of a Task.
+ */
+function taskConditionToPartialMessage(task: Task): Pb.Task.ICondition[] {
+  const {condition: taskCondition} = task;
+
+  return (
+    taskCondition?.expressions
+      .map(
+        expression =>
+          new Pb.Task.Condition({
+            multipleChoice: new Pb.Task.MultipleChoiceSelection({
+              optionIds: expression.optionIds.toArray(),
+            }),
+          })
+      )
+      .toArray() || []
+  );
+}

web/src/app/services/data-store/data-store.service.ts

@@ -28,13 +28,12 @@ import {Observable, combineLatest, firstValueFrom} from 'rxjs';
 import {map} from 'rxjs/operators';
 
 import {FirebaseDataConverter} from 'app/converters/firebase-data-converter';
+import {loiDocToModel} from 'app/converters/loi-data-converter';
 import {
-  LegacyLoiDataConverter,
-  loiDocToModel,
-} from 'app/converters/loi-data-converter';
-import {
-  newSurveyToProto,
-  partialSurveyToProto,
+  aclToDocument,
+  jobToDocument,
+  newSurveyToDocument,
+  partialSurveyToDocument,
 } from 'app/converters/proto-model-converter';
 import {Job} from 'app/models/job.model';
 import {LocationOfInterest} from 'app/models/loi.model';
@@ -155,26 +154,35 @@ export class DataStoreService {
    *  submissions that are related to the jobs to be deleted.
    */
 
-  updateSurvey(survey: Survey, jobIdsToDelete: List<string>): Promise<void> {
-    const {title, description, id} = survey;
+  async updateSurvey(
+    survey: Survey,
+    jobIdsToDelete: List<string>
+  ): Promise<void> {
+    const {title, description, id: surveyId, jobs} = survey;
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const surveyJS: {[key: string]: any} = {
       title: title,
       description: description,
     };
-    survey.jobs.forEach(
-      job => (surveyJS[`jobs.${job.id}`] = FirebaseDataConverter.jobToJS(job))
-    );
-    jobIdsToDelete.forEach(jobId => {
-      this.deleteAllLocationsOfInterestInJob(id, jobId);
-      this.deleteAllSubmissionsInJob(id, jobId);
-      surveyJS[`jobs.${jobId}`] = deleteField();
+    jobs.forEach(job => {
+      const {id: jobId} = job;
+      if (jobIdsToDelete.includes(jobId)) {
+        this.deleteAllLocationsOfInterestInJob(surveyId, jobId);
+        this.deleteAllSubmissionsInJob(surveyId, jobId);
+        surveyJS[`jobs.${jobId}`] = deleteField();
+      } else {
+        surveyJS[`jobs.${jobId}`] = FirebaseDataConverter.jobToJS(job);
+      }
     });
-
-    return this.db.firestore
+    await this.db.firestore
       .collection(SURVEYS_COLLECTION_NAME)
-      .doc(survey.id)
-      .update(surveyJS);
+      .doc(surveyId)
+      .update({
+        ...surveyJS,
+        ...partialSurveyToDocument(title, description),
+      });
+
+    await Promise.all(jobs.map(job => this.updateJob(surveyId, job)));
   }
 
   /**
@@ -187,7 +195,10 @@ export class DataStoreService {
     return this.db
       .collection(SURVEYS_COLLECTION_NAME)
       .doc(surveyId)
-      .set({title: newName, ...partialSurveyToProto(newName)}, {merge: true});
+      .set(
+        {title: newName, ...partialSurveyToDocument(newName)},
+        {merge: true}
+      );
   }
 
   /**
@@ -209,19 +220,29 @@ export class DataStoreService {
         {
           title: newName,
           description: newDescription,
-          ...partialSurveyToProto(newName, newDescription),
+          ...partialSurveyToDocument(newName, newDescription),
         },
         {merge: true}
       );
   }
 
-  addOrUpdateJob(surveyId: string, job: Job): Promise<void> {
+  addOrUpdateJob(surveyId: string, job: Job): Promise<[void, void]> {
+    return Promise.all([
+      this.db
+        .collection(SURVEYS_COLLECTION_NAME)
+        .doc(surveyId)
+        .update({
+          [`jobs.${job.id}`]: FirebaseDataConverter.jobToJS(job),
+        }),
+      this.updateJob(surveyId, job),
+    ]);
+  }
+
+  updateJob(surveyId: string, job: Job): Promise<void> {
     return this.db
-      .collection(SURVEYS_COLLECTION_NAME)
-      .doc(surveyId)
-      .update({
-        [`jobs.${job.id}`]: FirebaseDataConverter.jobToJS(job),
-      });
+      .collection(`${SURVEYS_COLLECTION_NAME}/${surveyId}/jobs`)
+      .doc(job.id)
+      .set(jobToDocument(job));
   }
 
   async deleteSurvey(survey: Survey) {
@@ -459,7 +480,7 @@ export class DataStoreService {
     return this.db
       .collection(SURVEYS_COLLECTION_NAME)
       .doc(surveyId)
-      .update({acl: FirebaseDataConverter.aclToJs(acl)});
+      .update({acl: FirebaseDataConverter.aclToJs(acl), ...aclToDocument(acl)});
   }
 
   generateId() {
@@ -488,7 +509,7 @@ export class DataStoreService {
       .doc(surveyId)
       .set({
         ...FirebaseDataConverter.newSurveyToJS(name, description, acl),
-        ...newSurveyToProto(name, description, acl, ownerId),
+        ...newSurveyToDocument(name, description, acl, ownerId),
       });
     return Promise.resolve(surveyId);
   }

web/src/app/services/job/job.service.ts

@@ -117,7 +117,7 @@ export class JobService {
   /**
    * Adds/Updates the job of a survey with a given job value.
    */
-  async addOrUpdateJob(surveyId: string, job: Job): Promise<void> {
+  async addOrUpdateJob(surveyId: string, job: Job): Promise<[void, void]> {
     if (job.index === -1) {
       const index = await this.getJobCount();
       job = job.copyWith({index});