Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read Submissions from Firestore using new proto-based format #1918

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d758cba
partial implementation of submission data converter
rfontanarosa Jul 17, 2024
5578cf7
removed unused code
rfontanarosa Jul 18, 2024
190bbe9
fixed import error
rfontanarosa Jul 18, 2024
7485b88
moved test into separate file
rfontanarosa Jul 18, 2024
2e0629f
added all different taskData conditions
rfontanarosa Jul 18, 2024
028ca6c
added all different taskData conditions
rfontanarosa Jul 18, 2024
2a891c7
added else if chain to make it clear there's no fall-through logic
rfontanarosa Jul 18, 2024
b909545
fixed missing other option and missing capture location
rfontanarosa Jul 19, 2024
3652181
fixed missing Timestamp case
rfontanarosa Jul 19, 2024
4d8e077
fixed auditinfo user conversion
rfontanarosa Jul 19, 2024
1a34b55
fixed auditinfo dates conversion
rfontanarosa Jul 19, 2024
c6de267
fixed wrong index (submission id instead of task id)
rfontanarosa Jul 22, 2024
83d1ad4
full working multipleChoiceResponses task type
rfontanarosa Jul 22, 2024
26f1ae8
fixed date conversion
rfontanarosa Jul 22, 2024
a5e47fa
it's not an error when task is not available
rfontanarosa Jul 22, 2024
d95e2ce
Merge branch 'master' into rfontanarosa/1857/use-protos-when-reading-…
rfontanarosa Jul 24, 2024
2718a1f
Use field IDs to query
sufyanAbbasi Jul 25, 2024
626c585
Make sure all Firestore queries use field numbers
sufyanAbbasi Jul 25, 2024
6763772
Merge branch 'master' into rfontanarosa/1857/use-protos-when-reading-…
rfontanarosa Jul 25, 2024
2256319
Ensure that Timestamps render correctly
sufyanAbbasi Jul 25, 2024
6f35125
Merge branch 'master' into rfontanarosa/1857/use-protos-when-reading-…
rfontanarosa Jul 30, 2024
99065ce
fixed some comments
rfontanarosa Jul 30, 2024
67cb342
removed useless "any" returning type
rfontanarosa Jul 30, 2024
8d975eb
assigned field number to constants
rfontanarosa Jul 30, 2024
e7e84ee
added FieldNumbers constant
rfontanarosa Jul 30, 2024
5889caf
missing file
rfontanarosa Jul 30, 2024
6b79423
removed useless comments
rfontanarosa Jul 30, 2024
7441b43
used constant instead of hardcoded value
rfontanarosa Jul 30, 2024
0d247d4
used role constants
rfontanarosa Jul 30, 2024
9fd6445
reverted use of FieldNumbers
rfontanarosa Jul 30, 2024
7777758
fixed recursive dependency
rfontanarosa Jul 31, 2024
4f5125b
missing field
rfontanarosa Jul 31, 2024
31e9fde
missing field
rfontanarosa Jul 31, 2024
4db5fcb
fixed owner id instead of owner email
rfontanarosa Jul 31, 2024
11a48e2
fixed variable names for constistency
rfontanarosa Jul 31, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions functions/src/common/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
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';

/**
*
*/
type pseudoGeoJsonGeometry = {
type: string;
coordinates: any;

Check warning on line 27 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type
};

/**
Expand Down Expand Up @@ -119,7 +120,7 @@
fetchSubmissionsByJobId(surveyId: string, jobId: string) {
return this.db_
.collection(submissions(surveyId))
.where('jobId', '==', jobId)
.where(FieldNumbers.Submission.job_id, '==', jobId)
.get();
}

Expand All @@ -133,7 +134,7 @@
): Promise<QuerySnapshot<DocumentData, DocumentData>> {
return this.db_
.collection(lois(surveyId))
.where('jobId', '==', jobId)
.where(FieldNumbers.LocationOfInterest.job_id, '==', jobId)
.get();
}

Expand All @@ -150,7 +151,11 @@
loiId: string
): Promise<number> {
const submissionsRef = this.db_.collection(submissions(surveyId));
const submissionsForLoiQuery = submissionsRef.where('loiId', '==', loiId);
const submissionsForLoiQuery = submissionsRef.where(
FieldNumbers.Submission.loi_id,
'==',
loiId
);
const snapshot = await submissionsForLoiQuery.count().get();
return snapshot.data().count;
}
Expand All @@ -169,7 +174,7 @@
await loiRef.update({properties});
}

static toFirestoreMap(geometry: any) {

Check warning on line 177 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type
return Object.fromEntries(
Object.entries(geometry).map(([key, value]) => [
key,
Expand All @@ -178,7 +183,7 @@
);
}

static toFirestoreValue(value: any): any {

Check warning on line 186 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type

Check warning on line 186 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type
if (value === null) {
return null;
}
Expand Down Expand Up @@ -206,7 +211,7 @@
*
* @returns GeoJSON geometry object (with geometry as list of lists)
*/
static fromFirestoreMap(geoJsonGeometry: any): any {

Check warning on line 214 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type

Check warning on line 214 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type
const geometryObject = geoJsonGeometry as pseudoGeoJsonGeometry;
if (!geometryObject) {
throw new Error(
Expand All @@ -221,7 +226,7 @@
return geometryObject;
}

static fromFirestoreValue(coordinates: any) {

Check warning on line 229 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type
if (coordinates instanceof GeoPoint) {
// Note: GeoJSON coordinates are in lng-lat order.
return [coordinates.longitude, coordinates.latitude];
Expand All @@ -230,7 +235,7 @@
if (typeof coordinates !== 'object') {
return coordinates;
}
const result = new Array<any>(coordinates.length);

Check warning on line 238 in functions/src/common/datastore.ts

View workflow job for this annotation

GitHub Actions / Check

Unexpected any. Specify a different type

Object.entries(coordinates).map(([i, nestedValue]) => {
const index = Number.parseInt(i);
Expand Down
3 changes: 2 additions & 1 deletion functions/src/on-write-submission.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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';

const test = require('firebase-functions-test')();

Expand Down Expand Up @@ -62,7 +63,7 @@ describe('onWriteSubmission()', () => {
.and.returnValue({
where: jasmine
.createSpy('where')
.withArgs('loiId', '==', loiId)
.withArgs(FieldNumbers.Submission.loi_id, '==', loiId)
.and.returnValue(newCountQuery(count)),
} as any);
}
Expand Down
2 changes: 2 additions & 0 deletions lib/src/firestore-to-proto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ function toFieldValue(
case 'bool':
case 'double':
return firestoreValue;
case '.google.protobuf.Timestamp':
return toMessageOrEnumValue(['google', 'protobuf'], 'Timestamp', firestoreValue);
default:
return toMessageOrEnumValue(messageTypePath, fieldType, firestoreValue);
}
Expand Down
1 change: 1 addition & 0 deletions lib/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +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';
32 changes: 32 additions & 0 deletions lib/src/proto-field-numbers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* 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'
}
};
4 changes: 3 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"build-all-and-test": "npm run build-all && npm run test",
"watch": "npm run build -- --watch",
"start": "npm run build && ng serve -c $npm_config_config",
"build-and-start": "npm run build && npm run start",
"build-and-start": "npm run build && npm run start",
"build-all-and-start": "npm run build-all && npm run start",
"pretest": "./pretest.sh",
"test": "ng test",
Expand Down Expand Up @@ -50,6 +50,7 @@
"functions": "^1.0.9",
"hammerjs": "^2.0.8",
"immutable": "^4.3.6",
"long": "^5.2.3",
"ngx-autosize-input": "^17",
"ngx-color": "^9.0.0",
"rxjs": "^7.5.7",
Expand Down
130 changes: 0 additions & 130 deletions web/src/app/converters/firebase-data-converter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,141 +14,11 @@
* limitations under the License.
*/

import {Timestamp} from '@angular/fire/firestore';
import {List, Map} from 'immutable';

import {Job} from 'app/models/job.model';
import {Role} from 'app/models/role.model';
import {Submission} from 'app/models/submission/submission.model';
import {
Cardinality,
MultipleChoice,
} from 'app/models/task/multiple-choice.model';
import {Option} from 'app/models/task/option.model';
import {Task, TaskType} from 'app/models/task/task.model';

import {FirebaseDataConverter} from './firebase-data-converter';

class MockFirebaseData {
static submission001 = {
created: {
clientTimestamp: undefined,
serverTimestamp: undefined,
user: {
displayName: 'Creator',
email: '[email protected]',
id: 'creator001',
},
},
lastModified: {
clientTimestamp: undefined,
serverTimestamp: undefined,
user: {
displayName: 'Modifier',
email: '[email protected]',
id: 'modifier001',
},
},
loiId: 'loi001',
jobId: 'job001',
data: {
task001: 'text result',
task002: ['option001', 'option002'],
task003: 123,
task004: new Timestamp(1641533340, 0),
task005: new Timestamp(1641534444, 0),
},
};
}

class MockModel {
static task001: Task = new Task(
'task001',
TaskType.TEXT,
'Text Field',
/*required=*/ true,
0
);

static task002: Task = new Task(
'task002',
TaskType.MULTIPLE_CHOICE,
'Multiple Select',
/*required=*/ true,
1,
new MultipleChoice(
Cardinality.SELECT_MULTIPLE,
List([
new Option(
'option001',
'code001',
'option 1',
/* index= */
0
),
new Option(
'option002',
'code002',
'option 2',
/* index= */
0
),
])
)
);

static task003: Task = new Task(
'task003',
TaskType.NUMBER,
'How many sloths are there?',
/*required=*/ true,
2
);

static task004: Task = new Task(
'task004',
TaskType.DATE,
'What is the current date?',
/*required=*/ true,
2
);

static task005: Task = new Task(
'task005',
TaskType.TIME,
'What time is it?',
/*required=*/ true,
2
);

static job001: Job = new Job(
/* id= */ 'job001',
/* index= */ 0,
'#ffffff',
'Test job',
Map({
task001: MockModel.task001,
task002: MockModel.task002,
task003: MockModel.task003,
task004: MockModel.task004,
task005: MockModel.task005,
})
);
}

describe('FirebaseDataConverter', () => {
it('Submission converts back and forth without loosing data.', () => {
expect(
FirebaseDataConverter.submissionToJS(
FirebaseDataConverter.toSubmission(
MockModel.job001,
'submission001',
MockFirebaseData.submission001
) as Submission
)
).toEqual(MockFirebaseData.submission001);
});

describe('toRole()', () => {
it('converts enums to strings', () => {
expect(FirebaseDataConverter.toRoleId(Role.OWNER)).toEqual('OWNER');
Expand Down
Loading
Loading