Closed Bug 1492174 Opened 2 years ago Closed 2 years ago

Update CFR Message Definitions to reflect planned cohorts for upcoming study

Categories

(Firefox :: Messaging System, defect, P2)

63 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Iteration:
64.2 - Sep 28
Tracking Status
firefox63 + verified
firefox64 --- verified

People

(Reporter: tspurway, Assigned: k88hudson)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

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)
Assignee: nobody → khudson
Comment on attachment 9010410 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Ursula Sarracini (:ursula) has approved the revision.
Attachment #9010410 - Flags: review+
Target Milestone: --- → Firefox 63
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
Attachment #9010410 - Flags: approval-mozilla-beta?
[Tracking Requested - why for this release]:
Comment on attachment 9010699 [details]
Bug 1492174 - Update CFR Messages to reflect planned cohorts

Ursula Sarracini (:ursula) has approved the revision.
Attachment #9010699 - Flags: review+
Blocks: 1491379
Pushed by khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fba337077a68
Update CFR Messages to reflect planned cohorts r=ursula
https://hg.mozilla.org/mozilla-central/rev/fba337077a68
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
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
Flags: needinfo?(khudson)
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.
Flags: needinfo?(khudson)
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
Attachment #9011015 - Flags: approval-mozilla-beta?
Comment on attachment 9011015 [details]
Bug 1492174 - Add variants for lifetime caps in CFR messages

Ursula Sarracini (:ursula) has approved the revision.
Attachment #9011015 - Flags: review+
(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.
Pushed by khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ae51750c377
Add variants for lifetime caps in CFR messages r=ursula
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 on attachment 9011015 [details]
Bug 1492174 - Add variants for lifetime caps in CFR messages

Thanks, uplift approved for 63 beta 9.
Attachment #9011015 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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)
Attachment #9010410 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Status: RESOLVED → VERIFIED
Component: Activity Streams: Newtab → Messaging System
You need to log in before you can comment on or make changes to this bug.