Page MenuHomePhabricator

Provide mechanism to allow dynamically tag log entries
Closed, ResolvedPublic

Description

One of Advanced Mobile Contributions project requirements is tag user actions with advanced mobile edit tag.

The AMC tag should be applied under two conditions:

  • the user acts in mobile web view
  • the AMC mode is enabled in Special:MobileOptions page

If those two conditions are fulfilled we want to log actions listed below:

  • blocking & unblocking
  • delete article entries
  • review changes (PageTriage extension)
  • thanks (Thanks extension)
  • moving pages
  • protecting pages
  • pending changes

The only place where we can define tags is the place where ManualLogEntry object is created and inserted to the database. This would cause a hard dependency between MediaWiki Core/PageTriage/Thanks extensions and MobileFrontend extension (where the Advanced Mobile Contributions mode lives).

Developer Notes

Most probably we should use Hooks system, and MobileFrontend should listen to InsertLog hook and inject tags. There are two possible ways of doing that:

  • call the hook inside the ManualLogEntry::insert()) method, most probably this will require a bit of refactoring
  • create hooks for each log entry point, and make MobileFrontend listen to all newly created hooks

Note: Some of the logs (like block log) can already contain the advanced mobile edit tag. Some actions during ManualLogEntry apply the set of tags applied to related RecentChange object. AMC is using the RecentChangeSave hook to apply advanced mobile edit tag, which then is applied to the Log entry. The tagging system should take care of that and apply only one tag.

Details

Related Gerrit Patches:

Event Timeline

pmiazga created this task.Feb 8 2019, 11:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 8 2019, 11:05 PM
pmiazga renamed this task from Provide mechanism to allow dynamicly tagging log entries to Provide mechanism to allow dynamically tag log entries.Feb 8 2019, 11:06 PM
ovasileva triaged this task as High priority.Feb 11 2019, 9:20 AM
Jdlrobson added a project: Epic.
Jdlrobson added a subscriber: Jdlrobson.

There are a lot of moving parts touching lots of code here so tagging as epic. T215477 seems like the first logical step to uncovering risk here.

Jdlrobson renamed this task from Provide mechanism to allow dynamically tag log entries to [EPIC] Provide mechanism to allow dynamically tag log entries.Feb 11 2019, 7:54 PM

Change 493464 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/core@master] Provide a Taggable interface

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

Change 493469 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/core@master] Define ManualLogEntry:beforeInsert hook

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

Change 493469 abandoned by Pmiazga:
Define ManualLogEntry:beforeInsert hook

Reason:
Reedy, thank you for reviews but looks like this is not required. I'll abandon this change.

Long story short, if we want to add tags to log entries -> the long entry has to be published to rcandudp or rc (logs do not have tags, only associated rc entries have those. If you have a log that is published to udp only, it won't store tags). If log is published to rcandudp we can listen to RecentChangeSave hook, and inject the tag just there (what we already do as we want to mark edits with the "mobile web edit" tag). Looks like there is no need to inject tags to log entries. I'm just so confused right now. Now I think that we don't need this patch, and most probably we won't need the Taggable interface - now the Taggable interface is only a good thing to have.

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

Looks like the log entries can be tagged indirectly by publishing them to rc|rcandupd and using RecentChangeSave hook to inject tags.
This epic is most probably not required anymore as code allows us to tag Log items - just in a kinda bit hacky way.

Jdlrobson reopened this task as Open.Mar 5 2019, 6:31 PM
Jdlrobson updated the task description. (Show Details)

Change 496257 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/core@master] Define ManualLogEntryBeforePublish hook

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

Change 493464 merged by jenkins-bot:
[mediawiki/core@master] Provide a Taggable interface

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

Jdlrobson renamed this task from [EPIC] Provide mechanism to allow dynamically tag log entries to Provide mechanism to allow dynamically tag log entries.Mar 18 2019, 11:49 PM
Jdlrobson moved this task from Epics/Goals to Needs Prioritization on the Readers-Web-Backlog board.

Wondering if this is really an epic now we have made progress :) Moving to needs analysis to remind myself to check what is remaining once the remaining patch is merged (pending a rebase)

Change 496257 merged by jenkins-bot:
[mediawiki/core@master] Define ManualLogEntryBeforePublish hook

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

I have to agree with you. It's not an epic, the task was relatively simple, the hardest part was finding people who can provide feedback on the changes to core repository. Also, the work is already done, and the tagging system works on production. There was a problem with some log entries (see T218940) but it's also tackled.

The only EPIC that comes to my mind is the overall rewrite of the Logs system, as the current implementation is very difficult to maintain/extend.

I believe this is done given conversation with Piotr, but if there is anything missing (And any remaining work we need to do) let's spit out a new task as part of sign off.

I think we're done here but want to check with @Tbayer to see if there's anything else we'd like to check in order to resolve this

I think we're done here but want to check with @Tbayer to see if there's anything else we'd like to check in order to resolve this

T215597: QA edit tags for moderation actions is still open; should it be considered a subtask of this?

ovasileva closed this task as Resolved.Apr 8 2019, 5:03 PM

I think we're done here but want to check with @Tbayer to see if there's anything else we'd like to check in order to resolve this

T215597: QA edit tags for moderation actions is still open; should it be considered a subtask of this?

I think we can make this a subtask of T215597: QA edit tags for moderation actions and resolve this one for now, with qa tracked in the other task