Api cleanup #92

engelke · 2021-07-02T20:55:34Z

replaced resource-specific method handlers with common one
separated database access for method to different module
added list sub-resource method (e.g., all donations to a campaign)
added tests

Inadvertent rename, later restored

content-api/data/cloud_firestore.py

content-api/main.py

content-api/main_test.py

content-api/resources/methods.py

docs/resource.json

ace-n · 2021-07-07T20:41:47Z

content-api/data/cloud_firestore.py

+        return None
+
+    resource = snapshot_to_resource(resource_snapshot)
+    resource = canonical_resource(resource, resource_kind, resource_fields)


Nit: reusing the same variable is a tad confusing IMO. Should we combine these into one statement, as on lines 70-72?

ace-n · 2021-07-07T20:43:04Z

content-api/data/cloud_firestore.py

+    resource_reference = client.document("{}/{}".format(resource_kind, id))
+
+    @firestore.transactional
+    def update_in_transaction(transaction, resource_reference, representation):


Yeah - I was wondering if we could somehow call that out in the name of this method.

(I'm not sure if there's a Python convention for that though. If not, then we can probably leave it as-is.)

ace-n · 2021-07-07T20:45:01Z

content-api/data/cloud_firestore.py

+            snapshot_to_resource(resource_snapshot), resource_kind, resource_fields
+        )
+
+        if match_etag is not None:  # Only apply if resource has not changed


This code is used in multiple places; I was thinking we could move it into a single place.

* Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review

* Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>

* OpenAPI spec and generated Python library * More operations in spec * Api cleanup (#92) (#101) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * OpenAPI complete, not finished * Client library generated * Update branch to match main (#113) * Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>

* Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review

* OpenAPI spec and generated Python library * More operations in spec * Api cleanup (#92) (#101) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * OpenAPI complete, not finished * Client library generated * Update branch to match main (#113) * Api cleanup (#92) * Markdown API docs * Refactored all resource methods to one package * Restored accidentally deleted resources/base * Partial separation of db access from methods * DB separated from method handlers * Added first test * More tests * API functionally complete per current spec * Reformatted by black * Delete , Inadvertent rename, later restored * Address PR #92 comments * Used existing helper method more often * Respond to PR review * Unit tests (#94) * Unit tests * Adding directory * Using alpine image and installing pytest * Using slim image instead * Adding descriptive ID's for the steps * productivity(terraform): correct terraform violations via PR suggestions (#98) * productivity(terraform): correct terraform violations via PR suggestions (fixes #50) * Switch to secrets.GITHUB_TOKEN Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> * Update decisions.md (#108) New decision record * Terraform and setup.sh (#93) Initial terraform setup. Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com> Co-authored-by: Dina Graves Portman <dinagraves@google.com> Co-authored-by: Adam Ross <adamross@google.com>

engelke added 9 commits June 24, 2021 16:17

Markdown API docs

6b16995

Refactored all resource methods to one package

9906ddf

Restored accidentally deleted resources/base

0261a23

Partial separation of db access from methods

d04fc2d

DB separated from method handlers

87768c7

Added first test

0675be2

More tests

51b4f40

API functionally complete per current spec

b228d6c

Reformatted by black

0fe9645

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jul 2, 2021

Merge branch 'main' into api_cleanup

3b5e382

engelke requested a review from ace-n July 2, 2021 20:59

Delete ,

895f5e7

Inadvertent rename, later restored

ace-n suggested changes Jul 2, 2021

View reviewed changes

engelke added 2 commits July 7, 2021 12:48

Address PR #92 comments

2409661

Used existing helper method more often

edf1d94

engelke requested a review from ace-n July 9, 2021 22:25

ace-n reviewed Jul 9, 2021

View reviewed changes

Respond to PR review

3d63216

ace-n approved these changes Jul 9, 2021

View reviewed changes

engelke merged commit 61561c7 into main Jul 9, 2021

engelke deleted the api_cleanup branch July 9, 2021 23:24

engelke mentioned this pull request Jul 12, 2021

Merge updated main to new branch #101

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Api cleanup #92

Api cleanup #92

engelke commented Jul 2, 2021

ace-n Jul 7, 2021

ace-n Jul 7, 2021

ace-n Jul 7, 2021

Api cleanup #92

Api cleanup #92

Conversation

engelke commented Jul 2, 2021

ace-n Jul 7, 2021

Choose a reason for hiding this comment

ace-n Jul 7, 2021

Choose a reason for hiding this comment

ace-n Jul 7, 2021

Choose a reason for hiding this comment