local storage dev root id #2407
Draft
+33
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This puts a random id in local storage and uses it for the Firestore and Realtime database root ids. This approach is currently broken though, because the Firebase functions have not been updated to use this same root id. The functions continue to use the Firebase user id instead. See this PR for more info: #2403
collaborative-learning
|
||||||||||||||||||||||||||||
| Project |
collaborative-learning
|
| Branch Review |
local-storage-dev-root
|
| Run status |
|
| Run duration | 01m 41s |
| Commit |
|
| Committer | Scott Cytacki |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
1
|
| View all changes introduced in this branch ↗︎ | |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2407 +/- ##
===========================================
- Coverage 85.90% 54.70% -31.20%
===========================================
Files 737 728 -9
Lines 37824 37685 -139
Branches 9624 9604 -20
===========================================
- Hits 32492 20617 -11875
- Misses 5027 16045 +11018
- Partials 305 1023 +718
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
scytacki
added
the
punted
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Do not merge
This PR uses a random id in local storage for the root id when the appMode is dev. This restores a similar behavior to how appMode dev worked before the switch to use session storage for the firebase credentials.
To enable this:
However:
This approach breaks the Firebase functions which create and read data in the Realtime database and Firestore. These functions compute the root based on a context and the firebase auth that is passed to them. Previously the dev root was firebase auth uid, so that is what these functions are using. To support the new dev root approach, we'll need to send the random id from local storage to the functions. This complicates things because it is used by almost all the functions. We haven't migrated the old functions to the new functions-v2 folder, so it is harder to work on them.
The functions that would be broken in appMode dev are:
All of these functions should not crash with this PR, they just are looking in or updating the wrong root in Firebase. So they save documents in the wrong place, or return no documents because the runtime is writing to a different place than the functions are looking.