1831397 - Update documentation to clarify targeting context of frequentVisits trigger

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement, P3)

Description

[Jason Prickett]

The documentation explaining the usage of the frequentVisits trigger is slightly ambiguous, and can be misinterpreted as providing timestamps of all recent visits, but in reality it only provides visits for the current host. It would be beneficial to update this documentation to clarify that the context only provides visit timestamps for the current host.

Comment 1

[buglinking]

Hello, as a beginner, I would like to fix this bug as my first contribution. Could I be assigned to work on this bug?

Comment 2

[Ivan]

D177211 submitted for review

Comment 3

[Ivan]

Attachment: Bug 1831397 - Added language to frequentVisits trigger to clarify context. r?jprickett

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:lsmith, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 5

[Louis Mascari]

Attachment: Bug 1831397 - Update documentation to clarify targeting context of frequentVisits trigger. r?jprickett!

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 7

[Mimi Jung]

This seems to have been assigned to me from phabricator automation after setting the patch as obsolete.

Comment 8

[Chris Shiohama]

It looks like this patch slipped through the cracks and was never landed. Assuming the changes are still valid, I can commandeer the patch, rebase on the current mozilla-central and re-post for review.

Comment 9

[Chris Shiohama]

Turns out I'm not capable of commandeering, but I'm still interested in this one.

Comment 10

[Meshach]

Hi, Pls I would like to work on this task

Comment 11

[Meg Viar [:mviar]]

Hi @meshach, thanks for for offering to pick this one up. I'm happy to review when the updated patch is ready, let me know if you need anything else in the meantime.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:Louismascari, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 13

[Meg Viar [:mviar]]

Passing to Louis to land - this have been approved by OMC.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:pdahiya, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 15

[Punam Dahiya [:pdahiya]]

Left comment for patch author to help rebase on latest central. Removing priority so that it can be picked in next triage Thanks

Comment 16

[Dan Mosedale (:dmosedale, :dmose)]

Chris or Meschach, if either of you would like to pick up this patch and rebase it against the current git main, or just submit a new patch, we'd be happy to review. Sorry for the delay!

Comment 17

[shwetank.tewari.87]

Hi, I can work on this and will be updating a patch soon.

Comment 18

[shwetank.tewari.87]

Hi, I tried to do the following steps:

1. Tried to create a local branch and do mom-phab patch D216724 --apply-to HEAD. It failed because there were significant changes in the main.
2. Then incorporated the required changes manually in the local branch, committed with mentioning the phab link in the commit message.
3. Then did moz-phab and it gave the following error:

(D216724) c367447df829 Bug 1831397 - Update documentation to clarify targeting context of frequentVisits trigger. r?jprickett!
!! You don't own this revision. Normally, you should only
   update revisions you own. You can "Commandeer" this
   revision from the web interface if you want to become
   the owner.
Submit to https://phabricator.services.mozilla.com (YES/No/Always)? YES

Updating revision D216724:
c367447df829 Bug 1831397 - Update documentation to clarify targeting context of frequentVisits trigger. r?jprickett!
You didn't specify a valid command, so we ran `submit` for you, and it failed.
Phabricator Error: [Access Denied: D216724] (Can Edit) You do not have permission to edit this object. // Members of the project "Restricted Project" can take this action. The owner of a revision can always view and edit it.

I think, since I do not own the patch I can not make revisions either. I guess I need something like "commandeering the revision" - I am not much sure because I am a first time contributor.

Please let me know what should I do to fix this.

Thanks

Comment 19

[Punam Dahiya [:pdahiya]]

I think, since I do not own the patch I can not make revisions either. I guess I need something like "commandeering the revision" - I am not much sure because I am a first time contributor.

@shwetank.tewari.87 you should see Commandeer Revision as one of the option under Add Action drop down at the bottom of patch https://phabricator.services.mozilla.com/D216724 . I agree that the main branch has diverged from the patch. Please feel free to abandon D216724 and attach a new patch with your fix to the bug. Thanks

Comment 20

[shwetank.tewari.87]

Hi, I need to be added as a reviewer to select Commandeer Revision option so that I can do the above. Thanks.

Comment 21

[shwetank.tewari.87]

I got the bug assigned but still can not see the option for Commandeer Revision. Thanks (In reply to shwetank.tewari.87 from comment #20)

Hi, I need to be added as a reviewer to select Commandeer Revision option so that I can do the above. Thanks.

Comment 22

[Punam Dahiya [:pdahiya]]

(In reply to shwetank.tewari.87 from comment #21)

I got the bug assigned but still can not see the option for Commandeer Revision. Thanks (In reply to shwetank.tewari.87 from comment #20)

@shwetank.tewari.87 if D216724 diff is not showing revision actions Commandeer Revision , to unblock simplest will be to abandon D216724 and submit a new patch with the fix ( Create a new commit with fix on branch based off latest central with commit message as 'Bug 1831397 - <commit description>' and submit using moz-phab submit HEAD~). I can help close D216724 once new patch is posted to the bug. Thanks

Comment 23

[shwetank.tewari.87]

Attachment: Bug 1831397 - Update documentation to clarify targeting context of frequentVisits trigger. r?jprickett!

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/875339ec5b07
https://hg.mozilla.org/integration/autoland/rev/475ab60c42f3
Update documentation to clarify targeting context of frequentVisits trigger. r=omc-reviewers,pdahiya

Comment 25

[amarc]

https://hg.mozilla.org/mozilla-central/rev/475ab60c42f3