Page MenuHomePhabricator

It should be possible to understand the reason of revision creation from RevisionRecordInserted hook
Closed, DeclinedPublic

Description

The parent ticket for this one indicates, that the RevisionRecordInserted hook is run for all the historical revisions of the page when it's undeleted. Since that hook is used to create revision-create events for Event-Platform used by ChangeProp and Analytics, this is not a desirable behavior - all the Parsoid HTML and dependent content types get re-rendered, the analytics edit history database gets updated etc etc.

For pages with a lot of history, this creates significant and completely unnecessary load. So it could be cool if the hook provided info on why exactly the revision was created - whether it's due to page undeletion, a redirect left on a page move or a normal edit, however right now I don't see a way to figure that out from the hook. That way Event-Platform could either filter the unnesessary events or indicate the reason of the event creation to the consumers.

Event Timeline

Pchelolo triaged this task as Normal priority.Feb 27 2018, 3:04 PM
Pchelolo raised the priority of this task from Normal to Needs Triage.
Pchelolo created this task.
Pchelolo added subscribers: Legoktm, Addshore.
Nuria moved this task from Incoming to Radar on the Analytics board.Feb 28 2018, 11:03 PM
daniel moved this task from Inbox to Epic on the Multi-Content-Revisions board.May 7 2018, 10:41 AM
daniel moved this task from Epic to Inbox on the Multi-Content-Revisions board.May 7 2018, 10:48 AM

Change 456037 had a related patch set uploaded (by Ppchelko; owner: Ppchelko):
[mediawiki/core@master] Add the revision record inserted reason to the hook.

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

Change 456037 abandoned by Ppchelko:
Add the revision record inserted reason to the hook.

Reason:
Hm, actually 'PageContentSaveComplete' should do the trick.

Initially, we've used 'PageContentSaveComplete', but it's also called for null edits, so we've switched to 'RevisionInsertComplete', that was deprecated, so we switched to 'RevisionRecordInserted'. Seems like it's time to circle back to 'PagecontentSaveComplete'

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

Pchelolo closed this task as Declined.Sep 7 2018, 9:17 PM
Pchelolo edited projects, added Services (done); removed Services (doing), Patch-For-Review.

We do not really need that to achieve the goal of the parent task - we can switch to PageContentSaveComplete hook that's only called on page edits, not restores.