1628688 - Update ASRouter to handle entrypoint parameters from snippets

Status: RESOLVED FIXED

(Firefox :: about:logins, enhancement, P1)

Description

[Andrei Oprea [:andreio]]

Follow up to bug 1622971 we want to handle query parameters for snippet links.

Comment 1

[Andrei Oprea [:andreio]]

Attachment: Bug 1628688 - Update ASRouter to handle endpoint parameters from snippets

Comment 2

[Andrei Oprea [:andreio]]

I've added a patch that allows passing in entrypoint information. Looking at current snippet capabilities it seems that opening about:logins would be possible through a <button> so I added the new button_action_entrypoint. It only needs to specify the value (eg snippet) not the query param name too.
Would this be required for links as well? It would make it a little more tricky to handle.
I did not add a default entrypoint value for all button actions because for some actions like OPEN_APPLICATIONS_MENU that wouldn't really make sense.

Comment 3

[Matthew N. [:MattN]]

So we really can't target snippets by Firefox version number ranges? That would remove the need for this IIUC

Comment 4

[Giorgos Logiotatidis [:giorgos]]

(In reply to Matthew N. [:MattN] (PM me if request are blocking you) from comment #3)

So we really can't target snippets by Firefox version number ranges? That would remove the need for this IIUC

We can, but we will have to create two versions for each snippet one the entrypoint and one without which then will require double admin and will increase complexity of metrics since we will have to gather numbers for multiple IDs. It's overall a snowflake situation that Snippet Editors must remember to handle properly. Adding this patch of the Fx level is best imo.

Comment 5

[Giorgos Logiotatidis [:giorgos]]

Snippets backend has been updated to serve entrypoints

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec2b47ef6989
Update ASRouter to handle endpoint parameters from snippets r=k88hudson

Comment 7

[Alexandru Michis [:malexandru]]

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

Comment 8

[Matthew N. [:MattN]]

This snippet which will run in a few days isn't passing the correct entry point as I get pwmgr open_management direct in about:telemetry#events-tab instead of pwmgr open_management snippet. Where does that problem lie?

about:newtab?theme=light&dir=ltr&endpoint=https://snippets-admin.mozilla.org/preview-asr/6cc9d112-3780-456d-afdf-6182b52173f0/

Comment 9

[Matthew N. [:MattN]]

action.data.entrypoint is undefined in ASRouter.jsm case ra.OPEN_ABOUT_PAGE: so maybe https://github.com/mozmeao/snippets-service/commit/ac7b37ae486c49258a35325d10e8b5ef961179f8 wasn't deployed yet. I also reviewed that commit now and realized it that the entrypoint value is incorrect anyways. It should be snippet, not snippets, see https://phabricator.services.mozilla.com/D67073.

Comment 10

[Giorgos Logiotatidis [:giorgos]]

Snippets service has code deployed and returns what it's supposed to return to Message Router. See the payload here

https://snippets-admin.mozilla.org/preview-asr/6cc9d112-3780-456d-afdf-6182b52173f0/

Indeed we used snippets because that's the value it has been used for FxA attribution in the past. It's an easy fix if we must change that, although keeping things the same is preferable.

I defer to Andrei to verify that things on his side as complete too.

Comment 11

[Andrei Oprea [:andreio]]

entrypoint_value should be snippet based on comment #9

Comment 12

[Giorgos Logiotatidis [:giorgos]]

Updated prod and verified with Andrei.

Comment 13

[Matthew N. [:MattN]]

Giorgos, the other problem is the names of the properties: entrypoint_name and entrypoint_value should be button_entrypoint_name and button_entrypoint_value respectively. See https://searchfox.org/mozilla-central/rev/9193635dca8cfdcb68f114306194ffc860456044/browser/components/newtab/content-src/asrouter/templates/SimpleSnippet/SimpleSnippet.jsx#29-33

Comment 14

[Giorgos Logiotatidis [:giorgos]]

Corrected and verified with Andrei.