Implement priority system for AS Router messages

RESOLVED FIXED in Firefox 62

Status

()

defect
P1
normal
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: ursula, Assigned: ursula)

Tracking

(Blocks 1 bug)

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

Firefox Tracking Flags

(firefox62+ fixed, firefox63+ fixed)

Details

User Story

https://github.com/mozilla/activity-stream/compare/7ff2cf8e9be924ddcad5b837b9a45542b2a9889c...79c5b431a35852d56ee52fc9044de896465e252b

Attachments

(4 attachments)

Assignee

Description

11 months ago
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

Updated

11 months ago
Iteration: --- → 63.1 - July 9
Priority: -- → P2
Assignee

Updated

11 months ago
Assignee: nobody → usarracini
Priority: P2 → P1

Comment 2

11 months ago
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

Updated

11 months ago
Status: NEW → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → FIXED

Comment 3

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

Updated

11 months ago
Blocks: 1472302

Comment 5

11 months ago
Does this need the changes from bug 1463836? Uplifting the one commit to 62 doesn't apply without the other bug.
Flags: needinfo?(usarracini)
Assignee

Comment 6

11 months ago
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?
Flags: needinfo?(usarracini)

Comment 7

11 months ago
Let's get PR4212 on activity-stream firefox-62 git branch so that we can just export the one commit cleanly.
Flags: needinfo?(usarracini)
Assignee

Comment 8

11 months ago
Yup, sounds good. I'll do that
Flags: needinfo?(usarracini)
Comment hidden (mozreview-request)

Comment 11

11 months ago
mozreview-review
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>activity-stream@mozilla.org</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"
Attachment #8989517 - Flags: review?(edilee) → review+

Updated

11 months ago
User Story: (updated)
Depends on: 1472297
Comment hidden (mozreview-request)
Assignee

Comment 13

11 months ago
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
Attachment #8989517 - Flags: approval-mozilla-beta?
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.
Attachment #8989517 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
Flags: needinfo?(usarracini)

Comment 16

11 months ago
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 hidden (mozreview-request)

Comment 18

11 months ago
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?
Flags: needinfo?(usarracini) → needinfo?(aryx.bugmail)

Comment 19

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

https://reviewboard.mozilla.org/r/255662/#review262378
Attachment #8990602 - Flags: review?(edilee) → review+

Updated

11 months ago
Blocks: 1472209
(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).
Flags: needinfo?(aryx.bugmail)
You need to log in before you can comment on or make changes to this bug.