Update CFR Message Definitions to reflect planned cohorts for upcoming study

VERIFIED FIXED in Firefox 63

Status

()

defect
P2
normal
VERIFIED FIXED
10 months ago
28 days ago

People

(Reporter: tspurway, Assigned: k88hudson)

Tracking

(Blocks 1 bug)

63 Branch
Firefox 63
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63+ verified, firefox64 verified)

Details

Attachments

(5 attachments)

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: 10 months 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.