refactor: Introduce the (outlines of) the Firestore Native-based v2 backend by rjhuijsman · Pull Request #1965 · WorldHealthOrganization/app

rjhuijsman · 2021-03-22T07:18:19Z

This PR lays the groundwork for the v2 backend, which will be based on Firestore in Native mode.

Reviewer suggestion:

Begin with the README.md files to get some idea of what the moving pieces are.
Look at the current Firestore Security Rules in app/server/firestore.rules and its unit tests in app/server/functions/firestore_rules.spec.ts.
Look at the app/server/functions/src/index.ts file containing the new Cloud Functions.
Look at the development setup for the Cloud Functions in app/server/functions/package.json.
Look at the changes to the Terraform config in app/server/terraform.
Look at how Firebase specifies what it will deploy in app/server/firebase.json.
Optionally, try deploying a v2 backend for yourself, and let me know how it goes!

Screenshots

None

How did you test the change?

iOS Simulator
iOS Device
Android Simulator
Android Device
curl to a dev App Engine server
other, please describe: Unit tests, and manual verification using Google Cloud Console.

Checklist:

Followed the Contributor Guidelines and verified that all contributions are properly licensed pursuant to the LICENSE.

…ackend. This PR lays the groundwork for the v2 backend, which will be based on Firestore in Native mode.

matthewblain

Looks like the right direction. My comments are mostly procedural/naive, as I've never used firestore or typescript.

server/.firebaserc

server/.gitignore

server/WHO.postman_collection.json

server/fetch-static-serving.sh

server/public/index.html

server/terraform/README.md

server/terraform/modules/myhealth/main.tf

server/terraform/v2-dev1/.terraform.lock.hcl

server/terraform/modules/myhealth/main.tf

client/README.md

server/.gitignore

server/README.md

server/WHO.postman_collection.json

server/terraform/README.md

brunobowden · 2021-03-25T07:29:57Z

server/terraform/README.md

 terraform init
 terraform apply
-# Should create all resources without any errors
+# THIS MAY THROW AN ERROR:


@matthewblain - I'm sure you'll have a comment on this. It is tricky to do this entirely without errors.

server/terraform/README.md

server/terraform/modules/myhealth/main.tf

brunobowden · 2021-03-25T07:43:48Z

tools/package-lock.json

@@ -1,7 +1,7408 @@
 {
  "name": "app-dev-tools",
+  "lockfileVersion": 2,


@theswerd - did we discuss upgrading to lockfileVersion 2 before? Any other implications to worry about?

@theswerd - please comment if there's any issues with lockfileversion 2

brunobowden

@rjhuijsman - I'm happy at this point but would want @matthewblain to look it over a again. All my feedback is relatively straight forward.

server/fetch-static-serving.sh

server/README.md

server/functions/src/index.ts

server/README.md

server/functions/src/index.ts

server/terraform/README.md

matthewblain

lg

server/functions/src/index.ts

server/README.md

server/WHO.postman_collection.json

server/terraform/modules/myhealth-v2/main.tf

server/fetch-static-serving.sh

server/functions/casestats.json

theswerd

Mostly LGTM - Please take a look at comments before merging

server/firebase.json

server/firestore.rules

server/functions/.eslintrc.js

server/functions/.prettierrc

server/functions/tsconfig.json

Co-authored-by: Bruno Bowden <github@brunobowden.com>

… location matches the project's Firestore location

rjhuijsman · 2021-03-30T08:55:32Z

I've addressed all of your comments. I believe we're all happy to merge this version? Would somebody with The Power be willing to press that button @brunobowden @matthewblain @theswerd?

server/.firebaserc

server/README.md

Hi Ben, dismissing your review as I think RJ has addressed it and we want to get this merged in

refactor: Introduce the (outlines of) the Firestore Native-based v2 b…

c37537c

…ackend. This PR lays the groundwork for the v2 backend, which will be based on Firestore in Native mode.

rjhuijsman requested review from britannio, brunobowden, matthewblain, theswerd and yukuairoy as code owners March 22, 2021 07:18

matthewblain reviewed Mar 24, 2021

View reviewed changes

brunobowden reviewed Mar 25, 2021

View reviewed changes

Review comments

0d57ea0

rjhuijsman force-pushed the rjh.backend branch from ea6a755 to 0d57ea0 Compare March 26, 2021 07:55

rjhuijsman requested review from brunobowden and matthewblain March 26, 2021 07:56

brunobowden reviewed Mar 26, 2021

View reviewed changes

matthewblain reviewed Mar 28, 2021

View reviewed changes

theswerd previously requested changes Mar 29, 2021

View reviewed changes

server/firebase.json Show resolved Hide resolved

server/firestore.rules Show resolved Hide resolved

server/functions/.eslintrc.js Show resolved Hide resolved

server/functions/.prettierrc Show resolved Hide resolved

server/functions/tsconfig.json Show resolved Hide resolved

rjhuijsman and others added 4 commits March 30, 2021 00:43

Apply suggestions from code review

499f95c

Co-authored-by: Bruno Bowden <github@brunobowden.com>

Address review comments

060af07

Add a pre-deploy test that checks that the configured Cloud Functions…

b69e230

… location matches the project's Firestore location

Teach ESLint to import *.ts files

12f4ebd

rjhuijsman requested review from brunobowden, matthewblain and theswerd March 30, 2021 08:54

brunobowden reviewed Mar 31, 2021

View reviewed changes

server/.firebaserc Show resolved Hide resolved

brunobowden approved these changes Mar 31, 2021

View reviewed changes

server/README.md Outdated Show resolved Hide resolved

server/README.md Outdated Show resolved Hide resolved

server/README.md Outdated Show resolved Hide resolved

rjhuijsman added 2 commits March 31, 2021 11:59

Fix linter error

3b6c6b1

Address review comments

9d075fb

rjhuijsman requested a review from brunobowden March 31, 2021 19:33

brunobowden merged commit daef675 into WorldHealthOrganization:master Mar 31, 2021

Comments

Conversation

rjhuijsman commented Mar 22, 2021

Screenshots

How did you test the change?

Checklist:

Uh oh!

matthewblain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunobowden Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brunobowden Mar 25, 2021

Choose a reason for hiding this comment

Uh oh!

brunobowden Mar 31, 2021

Choose a reason for hiding this comment

Uh oh!

brunobowden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewblain left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theswerd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rjhuijsman commented Mar 30, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!