WordPress-FluxC-Android
WordPress-FluxC-Android copied to clipboard
wordpress-mobile
Adds: DB layer for remote feature flags
Parent Issue https://github.com/wordpress-mobile/WordPress-Android/issues/17050#event-7216422667
Parent PR - https://github.com/wordpress-mobile/WordPress-Android/pull/17101
@AjeshRPai - So far, so good! My only hesitation is what we should call this. FeatureFlags are just one part of Remote Config. Eventually we'll add String values which then won't fit in with Map<String, Boolean>. If we rename to include "FeatureFlags" somewhere in the DAO, it may serve us better in the future. Would love to hear your thoughts.
@zwarm Thanks for taking a look at it.
(1)
RemoteConfigValueSource- do we also need to track that the value is from manual debug setting? I should probably look at the WP companion PR first, but it's in my brain so I am asking now.
I am not sure whether we need to do that or not 🤔 . My intention for tracking the RemoteConfigValueSource was to check in case of whether the value is synced from the remote or not. We are checking at the time of TTL, a combination of the source and the last modified time. Can you think of any scenario where we would need to track the source from a manual debug setting? I think it would be needed in case we have a remote value and manual debug setting, in that case, we don't want to override the manual value with the debug value. Do you think that's a valid scenario?
(2) I see we are no longer passing buildNumber and marketingVersion in the endpoint call; did the API change?
I will add the build number logic. Removed it accidentally. Am not sure about the marketingVersion. From where do we get that value to pass it on?
I am not sure whether we need to do that or not 🤔
Your explanation makes perfect sense - the DB is used for caching the data, not determining value to actual use in the code. If this changes, then yeah, we need to adjust.
Am not sure about the marketingVersion. From where do we get that value to pass it on?
See pbArwn-57S-p2#comment-6394 for details regarding marketing vs. build versions. Both are available in packagemanager ... it's just picking the right one. We can brainstorm together.
@AjeshRPai - Thanks for renaming from ff to remote config ❤️ The clean up looks nice. I posted a question p1665497442802759-slack-C03T04HETR8 with regards to "what" query parameters we will need to pass. I don't think we'll be using identifier as we had previously defined it (build package name). I suspect future changes to this PR. 👍
I removed the Not ready to merge label on this PR based on the testing that we did yesterday. :shipit:
hi @AjeshRPai 👋
This PR bumped the database version to 9 while not adding generated schema configuration (file 9.json). Could you please add it? Thanks!
Hey @wzieba 👋🏼 Sorry about that. Forgot to push it from my local. 😞 . Will create a separate PR for that.