Page MenuHomePhabricator

EditPage::getBaseRevision returns a revision
Closed, ResolvedPublic

Description

It would be a breaking change to have it return a RevisionRecord instead, which is what it probably should be returning
$this->mBaseRevision = $revRecord ? new Revision( $revRecord ) : null;
The method also has a warning note

@warning this method is very poorly named. If the user opened the form with ?oldid=X, one might think of X as the "base revision", which is NOT what this returns, see oldid for that. One might further assume that this corresponds to the $baseRevId parameter of WikiPage::doEditContent, which is not the case either. getExpectedParentRevision() would perhaps be a better name.

Proposal:
Introduce EditPage::getExpectedParentRevision (or ...ParentRevisionRecord?) that returns a RevisionRecord instead
Have EditPage::getBaseRevision be a wrapper for the new method that returns a revision instead of a revision record
Replace uses in WMF deployed code, deprecate, etc

Event Timeline

DannyS712 triaged this task as Medium priority.Mar 27 2020, 6:59 AM
DannyS712 created this task.
DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.

Makes sense, killing two birds with one stone. There were talks about refactoring EditPage, but I guess the timeline of a major refactoring would be much longer

Change 585843 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Introduce EditPage::getBaseRevisionRecord, deprecate using Revision

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

DannyS712 moved this task from Next to In progress on the User-DannyS712 board.Apr 4 2020, 12:12 AM

https://codesearch.wmflabs.org/search/?q=-%3EgetBaseRevision%5C(%5C)&i=nope&files=&repos=

Note that while there are a number of calls to getBaseRevision in the WikiBase repository, they do not refer to EditPage::getBaseRevision. Outside of the EditPage class itself, the only usage is in the TwoColConflict extension.

  1. Merge patch soft deprecating getBaseRevision and adding getExpectedParentRevision
  2. Update TwoColConflict
  3. Hard deprecate getBaseRevision (hopefully in 1.35)
  4. Remove getBaseRevision (hopefully in 1.36)
Restricted Application added a project: archived--TCB-Team. · View Herald TranscriptApr 4 2020, 12:17 AM
DannyS712 claimed this task.Apr 4 2020, 3:52 AM

Change 585843 merged by jenkins-bot:
[mediawiki/core@master] Introduce EditPage::getExpectedParentRevision, deprecate using Revision

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

Change 588066 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/TwoColConflict@master] Use EditPage::getExpectedParentRevision when available

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

Change 588066 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Use EditPage::getExpectedParentRevision when available

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

Change 588451 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Hard deprecate EditPage::getBaseRevision

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

Change 588451 merged by jenkins-bot:
[mediawiki/core@master] Hard deprecate EditPage::getBaseRevision

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

DannyS712 closed this task as Resolved.Apr 13 2020, 7:21 PM
DannyS712 removed a project: Patch-For-Review.