Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MBL-658] Replace Real Optimizely Client With Mock #1810

Merged
merged 7 commits into from
Apr 10, 2023

Conversation

msadoon
Copy link
Contributor

@msadoon msadoon commented Apr 9, 2023

πŸ“² What

To address our deadline of shipping the removal of Optimizely by April 17, here is a tiddy solution that should hold us over until we implement our next feature flagging tool. Probably Firebase Remote Config.

This document goes into more detail about the hows and whys.

πŸ€” Why

Removing Optimizely saves us an expensive subscription that can be replaced for free by more useful tools like Firebase Remote Config.

πŸ›  How

MockOptimizelyClient is actually a 1-1 replacement of the core functionality of the real Optimizely client.

In fact we pass around a OptimizelyClientType which is a protocol that is mapped to the functions used in Optimizely. we pass it around via our AppEnvironment.current.optimizelyClient singleton variable. MockOptimizelyClient conforms to this protocol.

So in the sake of saving the time to remove each individual flag throughout our app, why not hardcode those values as they are now on Optimizely dashboard?

We don't need to worry about older versions either, because as that document goes into more detail, even if the configuration of the real Optimizely SDK fails, we handle all experiments as a control case and all features as a false case. So at worst features that were enabled for versions 5.4.0 (project story tab), 5.5.0 (stripe payment settings) and 5.6.0 (stripe payment sheet) stop working until users update to this version 5.7.0. No crashes should occur because we handle the errors in our use cases.

Updating those users is probably going to be a campaign with in-app notifications down the line.

The thing to note is this MockOptimizelyClient is good enough that it covers of the entire scope of work for this ticket. So none of the subtickets need to be done until we actually have Remote Config setup to create that feature/experiment.

Aside: To get a sense of our active daily users for each version, check our app analytics in the document above.

βœ… Acceptance criteria

  • Ensure existing features and experiments behave exactly as they do in the production app version.

⏰ Developer QA

Step 1. Record the optimizely variable values after launching this pr in a build. Hint: use breakpoints

Experiments
DiscoveryViewModel activateNativeProjectCardsExperiment β€”> .control
OptimizelyExperiment nativeProjectCardsExperimentVariant β€”> .control
AppDelegateViewModel shouldSeeCategoryPersonalization β€”> .control
AppDelegateViewModel shouldGoToLandingPage β€”> .control
DiscoveryPageViewModel shouldShowPersonalization β€”> .control

Features

AppDelegateViewModel featureConsentManagementDialogEnabled β€”> false
ProjectPamphletMainCellViewModel featureProjectPageStoryTabEnabled β€”> true
ProjectNavigationSelectorViewModel featureProjectPageStoryTabEnabled β€”> true
CommentCellViewModel featureCommentFlaggingIsEnabled β€”> false
PledgePaymentMethodsViewModel featurePaymentSheetEnabled β€”> true
PaymentMethodsFooterView featureSettingsPaymentSheetEnabled β€”> true
AppDelegateViewModel featureConsentManagementDialog β€”> false

OptimizelyFeatureFlagToolsViewModel
comment flagging isEnabled fromServer == false, isEnabledFromUserDefaults nil
consent management isEnabledfromServer == false, isEnabledFromUserDefaults nil
conversions_api isEnabledFromServer == false, isEnabledFromUserDefaults nil
facebook deprecation isEnabledFromServer == false, isEnabledfromuserDefaults nil
payment sheet isEnabledFromServer == true, isEnabledFromUserDefaults nil
project story tab isEnabledFromServer == true, isEnabledFromUserDetauls nil
settings payment sheet isEnabledFromServer == true, isEnabledFromUserDefaults nil

Untested experiments: native_onboarding_series_new_backers
Untested features: ios_facebook_deprecation, ios_facebook_conversions_api

Step 2. Ensure UserDefaults is being activated through Beta tools
[Experiments are not stored in UserDefaults]

Instructions: a) turn on/off feature through Optimizely Feature Flags. b) kill app, don’t delete, relaunch through XCode c) ensure feature is being displayed as expected.

To record values, you might need to set breakpoints. Should be opposite values of what's recorded above.

CommentCellViewModel featureCommentFlaggingIsEnabled β€”> true
AppDelegateViewModel featureConsentManagementDialogEnabled β€”> true
PledgePaymentMethodsViewModel featurePaymentSheetEnabled β€”> false
ProjectPamphletMainCellViewModel featureProjectPageStoryTabEnabled β€”> false
ProjectNavigationSelectorViewModel featureProjectPageStoryTabEnabled β€”> false
PaymentMethodsFooterView featureSettingsPaymentSheetEnabled β€”> true

Untested experiments: native_onboarding_series_new_backers β€”> requires new users.
Untested features: ios_facebook_deprecation, ios_facebook_conversions_api β€”> requires facebook login, unused

Step 3. Repeat for Step 1 for version of app with Optimizely and check that variables are being set the same way as this pr build

Caveat:
Things are still named around Optimizely so we should update that once we have our new feature flagging tool set up.

#❓Questions/Future Work

  • Conversions api ff not used, can we remove?
  • Update native-secrets to remove optimizely secret keys
  • Fix weird bug found on old payment sheet in settings: cannot open payment sheet after cancelling unless re-enter page.

@msadoon msadoon self-assigned this Apr 9, 2023
@msadoon msadoon added this to the release-5.7.0 milestone Apr 9, 2023
@msadoon msadoon changed the base branch from release-5.7.0 to main April 9, 2023 21:24
@msadoon msadoon requested a review from scottkicks April 9, 2023 21:53
[
OptimizelyFeature.commentFlaggingEnabled.rawValue: false,
OptimizelyFeature.consentManagementDialogEnabled.rawValue: false,
OptimizelyFeature.facebookConversionsAPI.rawValue: false,
Copy link
Contributor

@scottkicks scottkicks Apr 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we can remove this one since the CAPI code is gated behind the user's consent preferences instead of this flag

Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we just need to update some optimizely tests to reflect the removal of the conversions flag. Good to go after that.

Ran through the Developer QA steps and everything was working as expected πŸŽ‰

@msadoon
Copy link
Contributor Author

msadoon commented Apr 10, 2023

Looks like we just need to update some optimizely tests to reflect the removal of the conversions flag. Good to go after that.

Ran through the Developer QA steps and everything was working as expected πŸŽ‰

Thanks for quick and thorough review on this! I just have to fix the tests and we'll merge

@msadoon msadoon merged commit 0c642e4 into main Apr 10, 2023
@msadoon msadoon deleted the mbl-658/replace-real-optimizely-with-mock branch April 10, 2023 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants