diff --git a/firestore/firestore.rules b/firestore/firestore.rules index 47b10bb83..eea7c1238 100644 --- a/firestore/firestore.rules +++ b/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 */ + ]); } /** diff --git a/functions/src/common/auth.ts b/functions/src/common/auth.ts index 3b9f70746..6d8c99471 100644 --- a/functions/src/common/auth.ts +++ b/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; } 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) diff --git a/functions/src/export-csv.ts b/functions/src/export-csv.ts index 9001df320..d9c95d932 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 = { @@ -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; + +/** + * 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, @@ -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, + 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; +} - // 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 { + 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, + 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); } /** @@ -211,9 +251,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 => @@ -226,11 +264,11 @@ function getPropertyNames( } function getPropertiesByName( - loi: FirebaseFirestore.QueryDocumentSnapshot, - allLoiProperties: Set + 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(...allLoiProperties).map( - prop => (loi.get('properties') || {})[prop] || '' + return List.of(...loiProperties).map( + prop => (loiDoc.get('properties') || {})[prop] || '' ); } diff --git a/functions/src/on-create-loi.ts b/functions/src/on-create-loi.ts index 9bf6be373..4553a9f1b 100644 --- a/functions/src/on-create-loi.ts +++ b/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); diff --git a/proto/src/google/ground/v1beta1/job.proto b/proto/src/google/ground/v1beta1/job.proto index 48e65015b..10f77cefa 100644 --- a/proto/src/google/ground/v1beta1/job.proto +++ b/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; } } diff --git a/proto/src/google/ground/v1beta1/loi.proto b/proto/src/google/ground/v1beta1/loi.proto index 33265792a..1d1019d06 100644 --- a/proto/src/google/ground/v1beta1/loi.proto +++ b/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. diff --git a/web/src/app/converters/firebase-data-converter.ts b/web/src/app/converters/firebase-data-converter.ts index ebfdced5e..b99ab416e 100644 --- a/web/src/app/converters/firebase-data-converter.ts +++ b/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, diff --git a/web/src/app/converters/proto-model-converter.ts b/web/src/app/converters/proto-model-converter.ts index 4209cf80a..9e5be9b30 100644 --- a/web/src/app/converters/proto-model-converter.ts +++ b/web/src/app/converters/proto-model-converter.ts @@ -17,11 +17,15 @@ import {DocumentData} from '@angular/fire/firestore'; import {toDocumentData} from '@ground/lib'; import {GroundProtos} from '@ground/proto'; -import {Map} from 'immutable'; +import {List, 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 { + Cardinality, + MultipleChoice, +} from 'app/models/task/multiple-choice.model'; +import {TaskCondition} from 'app/models/task/task-condition.model'; import {Task, TaskType} from 'app/models/task/task.model'; import Pb = GroundProtos.google.ground.v1beta1; @@ -45,7 +49,7 @@ export function roleToProtoRole(role: Role) { } /** - * Creates a proto rapresentation of a Survey. + * Returns the proto representation of a Survey model object. */ export function newSurveyToDocument( name: string, @@ -64,7 +68,7 @@ export function newSurveyToDocument( } /** - * Creates a proto rapresentation of a Survey. + * Returns the proto representation of a partial Survey model object. */ export function partialSurveyToDocument( name: string, @@ -79,7 +83,7 @@ export function partialSurveyToDocument( } /** - * Creates a proto rapresentation of a survey access control list. + * Returns the proto representation of a Survey ACL model object. */ export function aclToDocument(acl: Map): DocumentData | Error { return toDocumentData( @@ -90,7 +94,7 @@ export function aclToDocument(acl: Map): DocumentData | Error { } /** - * Creates a proto rapresentation of a Job. + * Returns the proto representation of a Job model object. */ export function jobToDocument(job: Job): DocumentData { return toDocumentData( @@ -99,32 +103,22 @@ export function jobToDocument(job: Job): DocumentData { 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(), + tasks: job.tasks?.toList().map(toTaskMessage).toArray(), }) ); } +export function tasksToDocument(tasks: List): DocumentData { + return toDocumentData(tasks.toList().map(toTaskMessage).toArray()); +} + /** - * Creates a partial rapresentation of a Task. + * Returns a Protobuf message representing a partial Task model. */ -function taskTypeToPartialMessage(task: Task): Pb.ITask { - const {type: taskType, multipleChoice: taskMultipleChoice} = task; - +function toTaskTypeMessage( + taskType: TaskType, + taskMultipleChoice?: MultipleChoice +): Pb.ITask { switch (taskType) { case TaskType.TEXT: return { @@ -139,7 +133,7 @@ function taskTypeToPartialMessage(task: Task): Pb.ITask { taskMultipleChoice!.cardinality === Cardinality.SELECT_ONE ? Pb.Task.MultipleChoiceQuestion.Type.SELECT_ONE : Pb.Task.MultipleChoiceQuestion.Type.SELECT_MULTIPLE, - hasOtherOption: task.multipleChoice!.hasOtherOption, + hasOtherOption: taskMultipleChoice!.hasOtherOption, options: taskMultipleChoice!.options .map( option => @@ -207,11 +201,11 @@ function taskTypeToPartialMessage(task: Task): Pb.ITask { } /** - * Creates a partial rapresentation of a Task. + * Returns a Protobuf message representing a TaskCondition model. */ -function taskConditionToPartialMessage(task: Task): Pb.Task.ICondition[] { - const {condition: taskCondition} = task; - +function toTaskConditionMessage( + taskCondition?: TaskCondition +): Pb.Task.ICondition[] { return ( taskCondition?.expressions .map( @@ -219,9 +213,27 @@ function taskConditionToPartialMessage(task: Task): Pb.Task.ICondition[] { new Pb.Task.Condition({ multipleChoice: new Pb.Task.MultipleChoiceSelection({ optionIds: expression.optionIds.toArray(), + taskId: expression.taskId, }), }) ) .toArray() || [] ); } + +/** + * Returns a Protobuf messager epresenting a Task model. + */ +function toTaskMessage(task: Task): Pb.ITask { + return new Pb.Task({ + ...toTaskTypeMessage(task.type, task.multipleChoice), + 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: toTaskConditionMessage(task.condition), + }); +} 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 930b75386..e239395a5 100644 --- a/web/src/app/services/data-store/data-store.service.ts +++ b/web/src/app/services/data-store/data-store.service.ts @@ -15,7 +15,10 @@ */ import {Injectable} from '@angular/core'; -import {AngularFirestore} from '@angular/fire/compat/firestore'; +import { + AngularFirestore, + CollectionReference, +} from '@angular/fire/compat/firestore'; import { DocumentData, FieldPath, @@ -34,6 +37,7 @@ import { jobToDocument, newSurveyToDocument, partialSurveyToDocument, + tasksToDocument, } from 'app/converters/proto-model-converter'; import {Job} from 'app/models/job.model'; import {LocationOfInterest} from 'app/models/loi.model'; @@ -336,7 +340,9 @@ export class DataStoreService { return this.db .doc(`users/${uid}`) .valueChanges() - .pipe(map(data => FirebaseDataConverter.toUser(data as DocumentData))); + .pipe( + map(data => FirebaseDataConverter.toUser(data as DocumentData, uid)) + ); } private toLocationsOfInterest( @@ -351,10 +357,10 @@ export class DataStoreService { } /** - * Returns a stream containing all the Location of Interests based - * on provided parameters. + * Returns an Observable that loads and emits all the locations of interest + * based on provided parameters. * - * @param id the id of the survey instance. + * @param survey the survey instance. * @param userEmail the email of the user to filter the results. * @param canManageSurvey a flag indicating whether the user has survey organizer or owner level permissions of the survey. */ @@ -394,19 +400,23 @@ export class DataStoreService { } /** - * Returns an Observable that loads and emits the submissions with the specified - * uuid. + * Returns an Observable that loads and emits all the Submissions + * based on provided parameters. * - * @param id the id of the requested survey (it should have tasks inside). - * @param loiId the id of the requested loi. + * @param survey the survey instance. + * @param loi the loi instance. + * @param userEmail the email of the user to filter the results. + * @param canManageSurvey a flag indicating whether the user has survey organizer or owner level permissions of the survey. */ - submissions$( + getAccessibleSubmissions$( survey: Survey, - loi: LocationOfInterest + loi: LocationOfInterest, + userEmail: string, + canManageSurvey: boolean ): Observable> { return this.db .collection(`${SURVEYS_COLLECTION_NAME}/${survey.id}/submissions`, ref => - ref.where('loiId', '==', loi.id) + this.canViewSubmissions(ref, loi.id, userEmail, canManageSurvey) ) .valueChanges({idField: 'id'}) .pipe( @@ -527,9 +537,10 @@ export class DataStoreService { .collection(SURVEYS_COLLECTION_NAME) .doc(surveyId) .update({ - [`jobs.${jobId}.tasks`]: FirebaseDataConverter.tasksToJS( - this.convertTasksListToMap(tasks) - ), + [`jobs.${jobId}.tasks`]: { + ...FirebaseDataConverter.tasksToJS(this.convertTasksListToMap(tasks)), + ...tasksToDocument(tasks), + }, }); } @@ -553,4 +564,21 @@ export class DataStoreService { } return true; } + + /** + * Creates a new Query object to filter submissions based on two + * criteria: loi and user + */ + private canViewSubmissions( + ref: CollectionReference, + loiId: string, + userEmail: string, + canManageSurvey: boolean + ) { + return canManageSurvey + ? ref.where('loiId', '==', loiId) + : ref + .where('loiId', '==', loiId) + .where('lastModified.user.email', '==', userEmail); + } } diff --git a/web/src/app/services/submission/submission.service.ts b/web/src/app/services/submission/submission.service.ts index 026424446..8b25a9efb 100644 --- a/web/src/app/services/submission/submission.service.ts +++ b/web/src/app/services/submission/submission.service.ts @@ -52,9 +52,9 @@ export class SubmissionService { constructor( private dataStore: DataStoreService, - surveyService: SurveyService, - loiService: LocationOfInterestService, - authService: AuthService + private surveyService: SurveyService, + private loiService: LocationOfInterestService, + private authService: AuthService ) { this.subscription.add( this.selectedSubmissionId$ @@ -125,7 +125,18 @@ export class SubmissionService { survey: Survey, loi: LocationOfInterest ): Observable> { - return this.dataStore.submissions$(survey, loi); + return this.authService + .getUser$() + .pipe( + switchMap(user => + this.dataStore.getAccessibleSubmissions$( + survey, + loi, + user.email, + this.surveyService.canManageSurvey() + ) + ) + ); } selectSubmission(submissionId: string) {