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
feat(cwts): Add CWTS to the /signup page in Sync optional flow #3498
Conversation
212517f
to
715a2a0
Compare
Hmm, these two tests are failing in all of my PRs right now:
|
98b0972
to
1c988ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shane-tomlinson This LGTM 👍🏽, should we target 151?
packages/fxa-content-server/app/scripts/lib/channels/senders/web-channel.js
Show resolved
Hide resolved
...ges/fxa-content-server/app/scripts/lib/experiments/grouping-rules/cwts-on-signup-password.js
Show resolved
Hide resolved
packages/fxa-content-server/app/scripts/views/mixins/sync-choice-mixin.js
Outdated
Show resolved
Hide resolved
packages/fxa-content-server/tests/functional/fx_browser_relier.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works well in my testing.
At what point are we gonna clean up the logic for this and remove the experiment (so this is the only code path)?
...ages/fxa-content-server/app/scripts/views/mixins/cwts-on-signup-password-experiment-mixin.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At what point are we gonna clean up the logic for this and remove the experiment (so this is the only code path)?
I'd like to as soon as we are confident we aren't going to roll back. Seems like it should be somewhat straight forward.
12bfb94
to
95b9c20
Compare
@sv-sdeiac - this can be tested on https://stomlinson.dev.lcip.org To test:
|
Add CWTS on the /signup page as an experiment, 20% total, 10% in each bucket. If user deselects all of the buckets, then does not send Sync information to the browser in the login message. As part of this, did a little refactor to how WebChannel events are stored for testing, instead of always storing them, store events only if in an automated testing environment. This made me feel more comfortable storing the event metadata into sessionStorage too, where it can be queried from within the functional tests. fixes #3490
95b9c20
to
071a75f
Compare
I'm removing the needs:qa label considering that this implementation has been verified by the QA team and the signoff mail, with the testing results, was sent yesterday. |
Add CWTS on the /signup page as an experiment, 20% total,
10% in each bucket.
If user deselects all of the buckets, then does not send
Sync information to the browser in the login message.
As part of this, did a little refactor to how WebChannel
events are stored for testing, instead of always storing
them, store events only if in an automated testing
environment. This made me feel more comfortable storing
the event metadata into sessionStorage too, where it can
be queried from within the functional tests.
fixes #3490
@vladikoff and @mozilla/fxa-devs - r?