Closed Bug 1591584 Opened 4 months ago Closed 3 months ago

Add dynamic triplet branch that shows sync, monitor and mobile cards based off targeting

Categories

(Firefox :: Messaging System, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 72
Iteration:
72.2 - Nov 4 - 17
Tracking Status
firefox72 --- verified

People

(Reporter: pdahiya, Assigned: pdahiya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

To support dynamic extended triplets we need messaging bundle with messages greater than 3. Scope of this bug is to implement a first run dynamic triplets campaign with template similar to extended_triplets and messages more than 3

https://searchfox.org/mozilla-central/source/browser/components/newtab/lib/OnboardingMessageProvider.jsm#207

Click on the CTA in a card should replace the card with a new message from the bundle on refresh or opening a new tab

Assignee: nobody → pdahiya
Iteration: --- → 72.1 - Oct 21 - Nov 3
Priority: -- → P1
Summary: Add dynamic extended triplet message with bundle length greater than 3 → Add dynamic triplet branch that shows sync and mobile cards based off targeting

Scope of this bug is to create a dynamic triplet branch . Dynamic triplet will show sync and mobile cards based of below targeting
a) Show sync cards only if user doesn't use firefox sync - https://github.com/mozilla/activity-stream/blob/master/content-src/asrouter/docs/targeting-attributes.md#usesfirefoxsync

b) Show mobile card only if user has zero total devices connected https://github.com/mozilla/activity-stream/blob/master/content-src/asrouter/docs/targeting-attributes.md#sync

With dynamic triplet branch we can remove "payoff", "multidevice", "privacy" branches support https://searchfox.org/mozilla-central/source/browser/components/newtab/lib/ASRouter.jsm#74 . Supercharge should be removed with static branch and should be a separate bug https://bugzilla.mozilla.org/show_bug.cgi?id=1592671#c0

Iteration: 72.1 - Oct 21 - Nov 3 → 72.2 - Nov 4 - 17
Summary: Add dynamic triplet branch that shows sync and mobile cards based off targeting → Add dynamic triplet branch that shows sync, monitor and mobile cards based off targeting

QA Steps:

  1. Create a new firefox profile
  2. Set trailhead.firstrun.branches to join-dynamic
  3. Restart and open a new tab
  4. Verify dynamic triplets shown are - Sync, Monitor and Browse Privately
  5. Click on Sync and complete sync signin using email address that's not enrolled with Firefox Monitor
  6. Open or reload new tab and verify Sync card is no longer visible. Cards displayed are Monitor, Browse Privately and Send Tabs
  7. Enroll Account used in Step 5 in Firefox Monitor.
  8. Open or reload new tab and verify Monitor card is no longer visible. Cards displayed are Browse Privately, Send Tabs and Multidevice
  9. Login to Firefox Mobile browser and turn on sync
  10. Open or reload new tab on desktop and verify multidevice card (Get Firefox on Phone) is no longer visible. Cards displayed are Browse Privately, Send Tabs and Lockwise

Verify above steps with trailhead.firstrun.branches set to ' full_page_d-dynamic' to verify triplets shown on full page interrupt. Repeat above steps with trailhead.firstrun.branches set to 'join-dynamic' and verify triplets shown on follow up new tab.

  1. Start with a new profile by setting trailhead.firstrun.branches to join-supercharge
  2. Verify cards displayed in triplets shown are Sync, Monitor and Multidevice and are not changed by signing into sync or Firefox Monitor.
Pushed by pdahiya@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9fad2a91f180
Add dynamic triplet branch that shows sync and multidevice cards based off targeting r=k88hudson
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72
Regressed by: 1548404

Sorry I don't know a better place to ask this question, so reaching out here in the bug. Please redirect me as appropriate.

If I understand correctly, this adds a new message that will call attachedFxAOAuthClients in order to decide whether the message should be shown to users. As discussed in Bug 1547120 Comment 18, this is a potentially expensive call - it hits the FxA servers to get a list of all the things on the users account, and we don't do any caching internally because we expect it to be called infrequently.

Could you please help me understand the potential impact of this new message on Firefox Accounts server load, so I can help surface this to our operations folks? For what users will the attachedFxAOAuthClients call be triggered, and how frequently?

Flags: needinfo?(pdahiya)

(In reply to Ryan Kelly [:rfkelly] from comment #8)

Sorry I don't know a better place to ask this question, so reaching out here in the bug. Please redirect me as appropriate.

If I understand correctly, this adds a new message that will call attachedFxAOAuthClients in order to decide whether the message should be shown to users. As discussed in Bug 1547120 Comment 18, this is a potentially expensive call - it hits the FxA servers to get a list of all the things on the users account, and we don't do any caching internally because we expect it to be called infrequently.

Could you please help me understand the potential impact of this new message on Firefox Accounts server load, so I can help surface this to our operations folks? For what users will the attachedFxAOAuthClients call be triggered, and how frequently?

That’s valid concern, thanks for raising it.

attachedFxAOAuthClients (https://searchfox.org/mozilla-central/source/browser/components/newtab/lib/ASRouterTargeting.jsm#476)
is behind usesFirefoxSync (“services.sync.username” pref has value), and will be accessed only by users who are logged into sync.

It will be used to show ’Firefox Monitor’ Message Card in dynamic triplets first run experiment planned for 72. This experiment is only visible to new profiles ( profile age < 7 days) and for 5 total impression that is up to 5 opened new tab.

So my understanding a new user who has logged into sync can hit this API max 5 times. How many users will be enrolled in dynamic triplets experiment is still to be determined.

Having said that we should look into caching for future usage of this API as this is exposed as targeting parameter and open to use by future messages. Also we found value returned by fxAccounts.listAttachedOAuthClients() have lot of null ids which is unexpected and if possible will be good to optimize. Thanks!

Flags: needinfo?(pdahiya)

I have tried to verify this issue on Firefox Nightly (Build ID: 20191120234543) on Windows 10 x64, Mac 10.14.6 and Arch Linux.
However, in order to be able to verify the issue with all the steps provided in Comment 4 I had to change the browser.newtab.preload preference to false due to the fact that I encountered a preloading issue which I filled here Bug 1598292.

Having said that we should look into caching for future usage of this API as this is exposed as targeting parameter and open to use by future messages.

Is this best added in the asrouter code, or is it something you'd like to see handled automagically by the FxA infrastructure in the browser?

I have verified that the Sync and Monitor cards are correctly replaced on the "join-dynamic" and " full_page_d-dynamic" branches, and verified that the same cards remain unchanged on the "join-supercharge" branch using the steps from Comment 4 on Firefox 72.0a1 (Build-ID 20191124230652) using Windows 10 x64, macOS 10.15.1, and Arch Linux x64.

However during verification I have observed that the Multidevice card is not replaced on the first new tab and it is only replaced on the second new tab, I've logged bug 1599812 to track this behavior.

Status: RESOLVED → VERIFIED

(In reply to Ryan Kelly [:rfkelly] from comment #11)

Having said that we should look into caching for future usage of this API as this is exposed as targeting parameter and open to use by future messages.

Is this best added in the asrouter code, or is it something you'd like to see handled automagically by the FxA infrastructure in the browser?

We can put basic caching in ASRouter Code, opened https://bugzilla.mozilla.org/show_bug.cgi?id=1600779. Thanks!

You need to log in before you can comment on or make changes to this bug.