1492174 - Update CFR Message Definitions to reflect planned cohorts for upcoming study

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P2)

Description

[Tim Spurway [:tspurway]]

Let's make sure the actual messages for the CFR study target the cohorts outlined here:

https://gist.github.com/k88hudson/8e1fb4a7a84cd521832aaf36cb6f62e3

We might as well also constrain the locale to en-US while we are in there (as referenced in https://bugzilla.mozilla.org/show_bug.cgi?id=1490697)

Comment 1

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4434

Comment 2

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/96790ab6b06cf2dbcd71ea77fe5ce7f48de34177
Bug 1492174 - Update CFR Message to reflect planned cohorts

Comment 3

[Kate Hudson :k88hudson]

Attachment: Bug 1492174 - Update CFR Messages to reflect planned cohorts

Comment 4

[Ursula Sarracini (:ursula)]

Comment on attachment 9010410 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Ursula Sarracini (:ursula) has approved the revision.

Comment 5

[Kate Hudson :k88hudson]

Comment on attachment 9010410 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Approval Request Comment
[Feature/Bug causing the regression]: CFR messages need cohort matching
[User impact if declined]: CFR experiment cannot run if cohorts are not defined
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no, but being exported today and tested locally
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: 
[Is the change risky?]: no
[Why is the change risky/not risky?]: small change to targeting, no major code changes
[String changes made/needed]: none

Comment 6

[Kate Hudson :k88hudson]

[Tracking Requested - why for this release]:

Comment 7

[Kate Hudson :k88hudson]

Attachment: Bug 1492174 - Update CFR Messages to reflect planned cohorts

Comment 8

[Ursula Sarracini (:ursula)]

Comment on attachment 9010699 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Ursula Sarracini (:ursula) has approved the revision.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fba337077a68
Update CFR Messages to reflect planned cohorts r=ursula

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/fba337077a68

Comment 11

[Pascal Chevrel:pascalc]

Kate, can you confirm that the patch you need to have uplifted is the one in comment #5 or will you make a new uplift request for the patch that landed in comment #10? (the https://phabricator.services.mozilla.com/D6414 and https://phabricator.services.mozilla.com/D6310 patches are identical?) Thanks

Comment 12

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/activity-stream/pull/4444

Comment 13

[[github robot]]

Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/1bb4a0464a0dcc425d4406fadd09068aa334bcdc
Bug 1492174 - Part 2. Add variants for lifetime caps

Comment 14

[Kate Hudson :k88hudson]

Attachment: Bug 1492174 - Add variants for lifetime caps in CFR messages

Comment 15

[Kate Hudson :k88hudson]

It's fine to merge the patch in comment #5.

Additionally, we will need attachment 9011015 [details] in order to fix an issue we saw on nightly this morning.

Comment 16

[Kate Hudson :k88hudson]

Comment on attachment 9011015 [details]
Bug 1492174 - Add variants for lifetime caps in CFR messages

Approval Request Comment
[Feature/Bug causing the regression]: The contextual feature recommendation experiment needs a variant implemented with frequency cap of 3 per recommendation
[User impact if declined]: We will be unable to study the impact of showing recommendations more frequently on user engagement and retention
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Not as of 2:30pm EDT, we will be merging to mozilla central as soon as possible.
[Needs manual test from QE? If yes, steps to reproduce]: Yes, see QA test plan in https://docs.google.com/spreadsheets/d/1H71t0luls86meTlDaCAR5lszej52mHQ5TrSFc-IRASo/edit?ts=5ba3b037#gid=0 for Branch 2 around verifying frequency caps.
[List of other uplifts needed for the feature/fix]: Yes, the additional patch in this bug (see comment #5) is required to land first for this to merge cleanly.
[Is the change risky?]: No
[Why is the change risky/not risky?]: This feature is behind a pref and does not affect users in release (outside a shield study)
[String changes made/needed]: No

Comment 17

[Ursula Sarracini (:ursula)]

Comment on attachment 9011015 [details]
Bug 1492174 - Add variants for lifetime caps in CFR messages

Ursula Sarracini (:ursula) has approved the revision.

Comment 18

[Pascal Chevrel:pascalc]

(In reply to Kate Hudson :k88hudson from comment #15)
> It's fine to merge the patch in comment #5.
> 
> Additionally, we will need attachment 9011015 [details] in order to fix an
> issue we saw on nightly this morning.

Kate, is that https://bugzilla.mozilla.org/show_bug.cgi?id=1492454? If that is the case, it needs to land first on mozilla-central and the uplift request should be done in the bug where the code landed.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ae51750c377
Add variants for lifetime caps in CFR messages r=ursula

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1ae51750c377

Comment 21

[Kate Hudson :k88hudson]

https://bugzilla.mozilla.org/show_bug.cgi?id=1492454 Is a separate unrelated issue for daily frequency caps. The second patch here is for implementing a missing lifetime frequency cap variant (i.e. a variant of each message in which it is suggested a total of three times, rather than just once) for the purposes of the study.

Comment 22

[Pascal Chevrel:pascalc]

Comment on attachment 9011015 [details]
Bug 1492174 - Add variants for lifetime caps in CFR messages

Thanks, uplift approved for 63 beta 9.

Comment 23

[Pascal Chevrel:pascalc]

Comment on attachment 9010410 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Uplift approved for 63 beta 9. (note to sheriffs: there are two patches to uplift in this bug)

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/fc233c29a740
https://hg.mozilla.org/releases/mozilla-beta/rev/57154ebfdfb5

Comment 25

[Marius Coman [:mcoman], Ecosystem QA]

I have verified this on Windows 10 x64, Arch Linux and Mac 10.13.3 with the latest version of "Firefox Beta" (63.0b9 - Build ID 20180924202103) and "Firefox Nightly" (64.0a1 - Build ID 20180924220042) installed and I can confirm that the "CFR Message Definitions" are reflecting the planned cohorts for the "CFR" shield study.