Page MenuHomePhabricator
Create Task
Maniphest T191608

markasread param doesn't work for log entry thanks
Closed, ResolvedPublic5 Estimated Story Points
Actions

Description

Steps to reproduce:

  • Get thanked for a log entry (ie. page deletion).
  • Click notifications badge to see new notices.
  • Click the notification body, which redirects you to the log entry (the link includes markasread=123456789).

Expected result:

  • The notification is marked as read.

Actual result:

  • The notification is not marked as read, only as seen (indicated by the grey number).

Details

ProjectBranchLines +/-Subject
mediawiki/coreREL1_27+31 -71Add 'logid' parameter to Special:Log
mediawiki/coreREL1_29+31 -71Add 'logid' parameter to Special:Log
mediawiki/coreREL1_31+31 -77Add 'logid' parameter to Special:Log
mediawiki/coreREL1_30+31 -71Add 'logid' parameter to Special:Log
mediawiki/coremaster+30 -77Add 'logid' parameter to Special:Log
mediawiki/extensions/Thanksmaster+3 -3Use new Special:Log?logid=xxx URL for notification link
Customize query in gerrit

Related Objects
Search...

Event Timeline

matej_suchanek created this task.Apr 6 2018, 9:12 AM
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptApr 6 2018, 9:12 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
matej_suchanek added a parent task: T60485: [Epic] Allow thanks of log entry.Apr 6 2018, 9:13 AM
Samwilson added projects: Thanks, Community-Tech.Apr 9 2018, 2:21 AM
• TBolliger moved this task from New & TBD Tickets to To Be Estimated/Discussed on the Community-Tech board.Apr 9 2018, 6:45 PM
• TBolliger moved this task from To Be Estimated/Discussed to Estimated on the Community-Tech board.Apr 11 2018, 11:25 PM
• TBolliger set the point value for this task to 5.
Niharika edited projects, added Community-Tech-Sprint; removed Community-Tech.Apr 11 2018, 11:56 PM
Etonkovidova mentioned this in T192041: Logs 'Thank' notifications appear to bundle on different actions.Apr 12 2018, 12:01 AM
Etonkovidova added a subscriber: Etonkovidova.

Confirmed in beta - clicking on the notification body for Thank notifications that came from Logs pages won't make them markasread - the counter won't go down.

There is another issue with Logs 'Thank' notifications - T192041: Logs 'Thank' notifications appear to bundle on different actions

Samwilson claimed this task.Apr 16 2018, 4:22 AM
Samwilson moved this task from Ready to In Development on the Community-Tech-Sprint board.
Samwilson added a comment.Apr 16 2018, 8:00 AM

This seems to be because we're using the Special:Redirect/logid/123 method to link to a log entry, and the markasread parameter isn't forwarded with the redirect.

The problem is that there's no way to link directly to one log entry (it'd be nicer to just do that). The redirection only sort-of does it by limiting the Special:Log list to a specific user, offset, and log type (which in most cases resolves to just a single entry).

Echo marks things as read via the PersonalUrls hook, which is never called for Special:Redirect. Perhaps this there is a better hook... what is the most appropriate hook that's called both for Special:Redirect and action=diff? Is it BeforeInitialize? Or should we just do it specifically for Special:Redirect, perhaps in SpecialPageBeforeFormDisplay?

Or should we look at adding a single-log-entry view to core?

JJMC89 added subscribers: Ricordisamoa, Legoktm, JJMC89.Apr 17 2018, 3:57 PM
In T191608#4132792, @Samwilson wrote:

Or should we look at adding a single-log-entry view to core?

The below would be my suggestion if it would help here.

In T71107#2067250, @Legoktm wrote:
In T71107#1959153, @Ricordisamoa wrote:

Actually, it might have been wiser to add id-based paging to Special:Log so as to allow permanent links to lists of entries.

Yeah, I agree with this.

gerritbot added a subscriber: gerritbot.Apr 23 2018, 4:16 AM

Change 428265 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/core@master] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/428265

gerritbot added a project: Patch-For-Review.Apr 23 2018, 4:16 AM
gerritbot added a comment.Apr 23 2018, 4:42 AM

Change 428266 had a related patch set uploaded (by Samwilson; owner: Samwilson):
[mediawiki/extensions/Thanks@master] Use new Special:Log?logid=xxx URL for notification link

https://gerrit.wikimedia.org/r/428266

Samwilson moved this task from In Development to Needs Review/Feedback on the Community-Tech-Sprint board.Apr 23 2018, 4:44 AM
Samwilson mentioned this in T187638: When a log event is (partially) hidden Special:Redirect/logid can link to the incorrect log and reveal hidden information (CVE-2018-0504).Apr 23 2018, 10:29 PM
Samwilson added a comment.May 3 2018, 1:00 AM

This is sorted now: no /123 ID parameter, because it clashes with the existing user parameter in that position (and usernames can be numeric). Ready for re-review: https://gerrit.wikimedia.org/r/#/c/428265/ (and then https://gerrit.wikimedia.org/r/#/c/428266/ ).

gerritbot added a comment.May 4 2018, 2:56 AM

Change 428265 merged by jenkins-bot:
[mediawiki/core@master] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/428265

gerritbot added a comment.May 4 2018, 5:59 AM

Change 428266 merged by jenkins-bot:
[mediawiki/extensions/Thanks@master] Use new Special:Log?logid=xxx URL for notification link

https://gerrit.wikimedia.org/r/428266

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-05-08 (1.32.0-wmf.3)).May 4 2018, 6:00 AM
Niharika added a subscriber: Niharika.EditedMay 5 2018, 1:15 AM

Fixed. Rolling out the week of May 7.

Niharika closed this task as Resolved.Jun 6 2018, 11:07 PM
Niharika moved this task from Needs Review/Feedback to Q1 2018-19 on the Community-Tech-Sprint board.
Niharika moved this task from Q1 2018-19 to Archive on the Community-Tech-Sprint board.Jul 3 2018, 7:37 PM
gerritbot added a comment.Jul 7 2018, 10:46 AM

Change 444351 had a related patch set uploaded (by Reedy; owner: Samwilson):
[mediawiki/core@REL1_31] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444351

Restricted Application added a project: Growth-Team. · View Herald TranscriptJul 7 2018, 10:46 AM
gerritbot added a comment.Jul 7 2018, 11:10 AM

Change 444356 had a related patch set uploaded (by Reedy; owner: Samwilson):
[mediawiki/core@REL1_30] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444356

gerritbot added a comment.Jul 7 2018, 11:11 AM

Change 444357 had a related patch set uploaded (by Reedy; owner: Samwilson):
[mediawiki/core@REL1_27] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444357

gerritbot added a comment.Jul 7 2018, 11:12 AM

Change 444358 had a related patch set uploaded (by Reedy; owner: Samwilson):
[mediawiki/core@REL1_29] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444358

gerritbot added a comment.Jul 7 2018, 12:19 PM

Change 444351 merged by jenkins-bot:
[mediawiki/core@REL1_31] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444351

gerritbot added a comment.Jul 7 2018, 12:19 PM

Change 444356 merged by jenkins-bot:
[mediawiki/core@REL1_30] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444356

gerritbot added a comment.Jul 7 2018, 12:26 PM

Change 444357 merged by jenkins-bot:
[mediawiki/core@REL1_27] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444357

gerritbot added a comment.Jul 7 2018, 12:26 PM

Change 444358 merged by jenkins-bot:
[mediawiki/core@REL1_29] Add 'logid' parameter to Special:Log

https://gerrit.wikimedia.org/r/444358

Niharika edited projects, added Community-Tech; removed Community-Tech-Sprint.Jul 9 2018, 5:30 PM
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL