fix(app, android): require default firebase.json boolean key #4791
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 fixes the case where firebase.json existed but the key being query was missing. Previously the config system would silently fail on the missing key and return false even though the default for some keys should be true In particular this caused analytics auto collection to be disabled if you had a firebase.json but no analytics_auto_collection_enabled = true entry, even though the default should have been true Credit to @WadhahEssam for discovery, diagnosis, and general solution
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/invertase/react-native-firebase/8cxwoofco |
|
As soon as this passes CI I will merge and release 10.4.2 @WadhahEssam |
Codecov Report
@@ Coverage Diff @@
## master #4791 +/- ##
=======================================
Coverage 89.09% 89.09%
=======================================
Files 109 109
Lines 3712 3712
Branches 347 347
=======================================
Hits 3307 3307
Misses 363 363
Partials 42 42 |
|
Actually @WadhahEssam - since I know you are watching this :-), it will be 10.5.0 because I'm going to merge the feature / minor bump to the underlying SDKs when they finish too #4792 |
8 tasks
hmhm2292
pushed a commit
to hmhm2292/react-native-firebase
that referenced
this pull request
Jul 13, 2021
…se#4791) This fixes the case where firebase.json existed but the key being query was missing. Previously the config system would silently fail on the missing key and return false even though the default for some keys should be true In particular this caused analytics auto collection to be disabled if you had a firebase.json but no analytics_auto_collection_enabled = true entry, even though the default should have been true Credit to @WadhahEssam for discovery, diagnosis, and general solution
androidIsForVivek
pushed a commit
to androidIsForVivek/react-native-firebase
that referenced
this pull request
Aug 9, 2021
…se#4791) This fixes the case where firebase.json existed but the key being query was missing. Previously the config system would silently fail on the missing key and return false even though the default for some keys should be true In particular this caused analytics auto collection to be disabled if you had a firebase.json but no analytics_auto_collection_enabled = true entry, even though the default should have been true Credit to @WadhahEssam for discovery, diagnosis, and general solution
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.
Description
This fixes the case where firebase.json existed but the key being
query was missing. Previously the config system would silently fail on
the missing key and return false even though the default for some keys
should be true
In particular this caused analytics auto collection to be disabled
if you had a firebase.json but no analytics_auto_collection_enabled = true entry,
even though the default should have been true
Credit to @WadhahEssam for discovery, diagnosis, and general solution
Related issues
Fixes #4784
Obsoletes #4786 (different API shape, same solution though)
Checklist
AndroidiOSe2etests added or updated inpackages/\*\*/e2ejesttests added or updated inpackages/\*\*/__tests__Test Plan
I tested:
Seems to work fine although I did discover some problems and was able to break my tests - the tests I was doing were testing it and I proved it locally by toggling back and forth
the logic is not engaged if firebase.json is not present
Think
react-native-firebaseis great? Please consider supporting the project with any of the below:React Native FirebaseandInvertaseon Twitter