Change FxNimbus/Nimbus startup sequence #25266
Conversation
| components.analytics.experiments.register(object : NimbusInterface.Observer { | ||
| override fun onUpdatesApplied(updated: List<EnrolledExperiment>) { | ||
| restore() | ||
| } | ||
| }) |
There was a problem hiding this comment.
Is it Ok if we added as part of the existing onUpdatesApplied?
There was a problem hiding this comment.
I combined the two observers into one.
| } | ||
|
|
||
| private fun setupMessaging() { | ||
| if (!FeatureFlags.messagingFeature) { |
There was a problem hiding this comment.
@jhugman is it OK to remove the Settings.isExperimentationEnabled check?
There was a problem hiding this comment.
I see you're already checking isExperimentationEnabled in the HomeFragment, in which case I don't know what the desired behaviour is (if the user flips the studies switch, should they start seeing messages immediately, or on the next startup?).
This is a bit strange, but I added back in.
There was a problem hiding this comment.
It's a bit confusing. To make sure users are in a consistent state when they turn on/off experiments, we let them know if they proceed the app will restart.
There was a problem hiding this comment.
Tested manually and it works expected.
I would prefer if we could reuse the existing event listener, instead of introducing a new one, I also tested this alternative and it worked as expected too.
jhugman
force-pushed
the
jhugman/exp-2395-change-fxnimbus-initialization
branch
from
c36d989 to
a627077
Compare
jhugman
added
the
pr:needs-landing-squashed
| FxNimbus.api = NimbusDisabled(testContext) | ||
|
|
There was a problem hiding this comment.
The api property is no longer public; this is no longer the way we want to initialize the FxNimbus object.
This is a reup of the #25089 and #25234.
The problem seemed to be two fold:
MessagingAction.Restorewas being called from theonExperimentsFetchedobserver method.This caused the message not to be ready for the first display of the home screen, which was caught by the performance test.
The fix is to move the restore action to be dispatched explicitly after the nimbus SDK is ready.
Pull Request checklist
To download an APK when reviewing a PR: