1470170 - Implement priority system for AS Router messages

Status: RESOLVED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Ursula Sarracini (:ursula)]

Once the targeting lands, as a follow up to bug 1465553, we have to have a way to have "priorities" in messages. That means: messages that are shown based on a trigger of sorts should have a higher priority (like onboarding for example - where the "trigger" is first new tab opened for first run users), whereas messages that are sort of just always there (like showing a snippet for example) would have a lower priority

Comment 1

[GitHub Bugzilla PR Linker]

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

Comment 2

[[github robot]]

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

https://github.com/mozilla/activity-stream/commit/1a74d11862da63152216a215f8f8bf21349fb73c
Fix Bug 1470170 - Implement priority system for AS Router messages

https://github.com/mozilla/activity-stream/commit/77f75f09737b56e0817cf6e891614cbafbdf3ff5
Merge pull request #4212 from sarracini/bug_1470170

Fix Bug 1470170 - Implement priority system for AS Router messages

Comment 3

[Ed Lee :Mardak]

[Tracking Requested - why for this release]: about:welcome will be on in 62, and this allows messages that trigger on first run

Comment 4

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/0ef1451e9411

Comment 5

[Ed Lee :Mardak]

Does this need the changes from bug 1463836? Uplifting the one commit to 62 doesn't apply without the other bug.

Comment 6

[Ursula Sarracini (:ursula)]

These changes should be independent from the Remote Provider Message Previewing patch.. but I can see how in the export commit the changes are mixed in with the other patch. I can try to do a patch manually or we could uplift bug 1463836 too?

Comment 7

[Ed Lee :Mardak]

Let's get PR4212 on activity-stream firefox-62 git branch so that we can just export the one commit cleanly.

Comment 8

[Ursula Sarracini (:ursula)]

Yup, sounds good. I'll do that

Comment 9

[GitHub Bugzilla PR Linker]

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

Comment 10

[Ursula Sarracini (:ursula)]

Attachment: Bug 1470170 - Implement priority system for AS Router messages

Review commit: https://reviewboard.mozilla.org/r/254554/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254554/

Comment 11

[Ed Lee :Mardak]

Comment on attachment 8989517 [details]
Bug 1470170 - Implement priority system for AS Router messages

https://reviewboard.mozilla.org/r/254554/#review261374

::: browser/extensions/activity-stream/install.rdf.in:11
(Diff revision 1)
>    <Description about="urn:mozilla:install-manifest">
>      <em:id>[e-mail]</em:id>
>      <em:type>2</em:type>
>      <em:bootstrap>true</em:bootstrap>
>      <em:unpack>false</em:unpack>
> -    <em:version>2018.06.29.1011-6fefff6e</em:version>
> +    <em:version>2018.07.03.1125-79c5b431</em:version>

`2018.06.29.1011-6fefff6e` was from the indexeddb uplift. There were some beta patches from yesterday that were uplifted just today ending, so it looks like your beta branch is slightly out of date.

However, there's bug 1472297 that is just waiting on approval (and is just before this exported commit on our firefox-62 branch). So I think we can just edit this patch to assume the original version is that of the other bug, i.e., "2018.07.02.1290-7ff2cf8e"

Comment 12

[Ursula Sarracini (:ursula)]

Comment on attachment 8989517 [details]
Bug 1470170 - Implement priority system for AS Router messages

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/254554/diff/1-2/

Comment 13

[Ursula Sarracini (:ursula)]

Comment on attachment 8989517 [details]
Bug 1470170 - Implement priority system for AS Router messages

Approval Request Comment
[Feature/Bug causing the regression]: Showing the new simplified onboarding experience on about:welcome for the SHIELD experiment in Firefox 62
[User impact if declined]: The user will not see the new onboarding experience and the experiment cannot run in Firefox 62
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes (63.0a1, 20180630100552)
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Bug 1472297 has a patch that needs to be uplifted if we want to also uplift this one. 
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: Adds some logic to show some UI based on a certain criteria, but is behind a pref, and for an experiment
[String changes made/needed]: None

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8989517 [details]
Bug 1470170 - Implement priority system for AS Router messages

Support for a planned experiment in 62. Let's uplift for beta 7.

Comment 15

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

The patch from bug 1472297 applied cleanly on beta, but this one had conflicts.

Hunk #1 FAILED at 0
1 out of 1 hunks FAILED -- saving rejects to file browser/extensions/activity-stream/data/content/activity-stream.bundle.js.map.rej
patching file browser/extensions/activity-stream/install.rdf.in
Hunk #1 FAILED at 2
1 out of 1 hunks FAILED -- saving rejects to file browser/extensions/activity-stream/install.rdf.in.rej

Comment 16

[Ed Lee :Mardak]

Bug 1472297 can be uplifted first without this one. It was just noted in comment 13 because of the install.rdf.in version matching. Although I do see that the patch in mozreview doesn't seem quite right for revision 1 or revision 2:

revision 1:
- 2018.06.29.1011-6fefff6e
+ 2018.07.03.1125-79c5b431

revision 2:
- 2018.06.29.1011-6fefff6e
+ 2018.07.02.1290-7ff2cf8e

It should look more like:
- 2018.07.02.1290-7ff2cf8e
+ 2018.07.03.1125-79c5b431

ursula, looks like my suggestion of fixing up install.rdf.in won't be sufficient as there's bundle map issues too. If bug 1472297 isn't on mozilla-beta yet when you update the patch, I would:

1) update beta branch to latest mozilla-beta
2) apply patch from bug 1472297 and commit a dummy commit
3) run buildmc with 79c5b431
4) commit and mozreview this one commit

Comment 17

[Ed Lee :Mardak]

Attachment: Bug 1470170 - Implement priority system for AS Router messages. a=lizzard

Review commit: https://reviewboard.mozilla.org/r/255662/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/255662/

Comment 18

[Ed Lee :Mardak]

aryx, I made a new mozreview attachment 8990602 [details] that is updated to be applied on top of bug 1472297.

Our team was wondering if it would be acceptable and easier if we just pushed a patch series to mozreview or somewhere (as mozreview only supports one bug at a time…) even though the changes aren't actually related but happen to modify built artifact files, e.g., install.rdf.in and bundle.js.map?

Comment 19

[Ed Lee :Mardak]

Comment on attachment 8990602 [details]
Bug 1470170 - Implement priority system for AS Router messages.  a=lizzard

https://reviewboard.mozilla.org/r/255662/#review262378

Comment 20

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

https://hg.mozilla.org/releases/mozilla-beta/rev/5dc16f73a43f

Comment 21

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

(In reply to Ed Lee :Mardak (PTO July 7-22) from comment #18)
> Our team was wondering if it would be acceptable and easier if we just
> pushed a patch series to mozreview or somewhere (as mozreview only supports
> one bug at a time…) even though the changes aren't actually related but
> happen to modify built artifact files, e.g., install.rdf.in and
> bundle.js.map?
Hi Ed, are there commands which sheriffs could use to generate bundle.js.map and install.rdf.in?

Maybe a Github repo owned by the Mozilla org could workaround this issue (as long as there is a patch file which can be applied to a beta repository, this would "fix" it).