[C-841] Put getFeatureEnabled in storeContext#1727
Conversation
dylanjeffers
left a comment
There was a problem hiding this comment.
looks great, isn't there also the getFeatureEnabled in the audius-backend-instance? we using the same one there and here? Also just wanted to double check regarding the NATIVE_MOBILE removals!
| export const storeContext: CommonStoreContext = { | ||
| getLocalStorageItem: async (key) => AsyncStorage.getItem(key), | ||
| setLocalStorageItem: async (key, value) => AsyncStorage.setItem(key, value), | ||
| getFeatureEnabled, |
There was a problem hiding this comment.
so can we use remoteConfigInstance to call getFeatureEnabled, or are they separate things?
There was a problem hiding this comment.
They are separate things right now. The getFeatureFlag helper checks local storage for the feature flag overrides, but I think I can consolidate it
There was a problem hiding this comment.
Actually we do need both, the one in remoteConfig is used by the feature flag modal to show the default state. The helper is used by actual features to check if they're enabled accounting for overrides. I could return the one from audiusBackendInstance out and update the usages to use that one, but it seems like it makes more sense at the top level of the storeContext. wdyt?
There was a problem hiding this comment.
yeah im good with top level, but was just wondering why it seemed like we had duplicate things. maybe when everything is sorted, we take a loot at context and see if we can consolidate? things like getLocalStorageItem vs localStorage, is another example
| const audiusBackendInstance = yield* getContext('audiusBackendInstance') | ||
| yield* call(waitForBackendSetup) | ||
| yield* call(audiusBackendInstance.markAllNotificationAsViewed) | ||
| if (NATIVE_MOBILE) { |
There was a problem hiding this comment.
cool this is going away now! is it safe to do in this intermediate phase?
There was a problem hiding this comment.
No this shouldn't have been part of this commit, my b
|
Preview this change https://demo.audius.co/sk-c-841-getfeatureenabled-storecontext |
Description
Moves
getFeatureEnabledto storeContext in preparation for notification sagasDragons
Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.
How will this change be monitored?
For features that are critical or could fail silently please describe the monitoring/alerting being added.
Feature Flags
Are all new features properly feature flagged? Describe added feature flags.