ApiComparePages: Don't error with no prev/next rev
Prior to I700edfa76, torelative=prev on the first revision of a page would "diff" from an empty revision, and torelative=next on the latest revision would diff to that same latest revision. People were depending on that behavior, so restore it. Bug: T203433 Change-Id: Ie81b58c196998a8047322740fe1d1fa44eff8526
This commit is contained in:
Brad Jorsch
parent
093feda2c9
commit
cf4f985f22
4 changed files with 97 additions and 21 deletions
|
|
@ -64,16 +64,48 @@ class ApiComparePages extends ApiBase {
|
|||
switch ( $params['torelative'] ) {
|
||||
case 'prev':
|
||||
// Swap 'from' and 'to'
|
||||
list( $toRev, $toRelRev2, $toValsRev ) = [ $fromRev, $fromRelRev, $fromValsRev ];
|
||||
$fromRev = $this->revisionStore->getPreviousRevision( $fromRelRev );
|
||||
list( $toRev, $toRelRev, $toValsRev ) = [ $fromRev, $fromRelRev, $fromValsRev ];
|
||||
$fromRev = $this->revisionStore->getPreviousRevision( $toRelRev );
|
||||
$fromRelRev = $fromRev;
|
||||
$fromValsRev = $fromRev;
|
||||
if ( !$fromRev ) {
|
||||
$title = Title::newFromLinkTarget( $toRelRev->getPageAsLinkTarget() );
|
||||
$this->addWarning( [
|
||||
'apiwarn-compare-no-prev',
|
||||
wfEscapeWikiText( $title->getPrefixedText() ),
|
||||
$toRelRev->getId()
|
||||
] );
|
||||
|
||||
// (T203433) Create an empty dummy revision as the "previous".
|
||||
// The main slot has to exist, the rest will be handled by DifferenceEngine.
|
||||
$fromRev = $this->revisionStore->newMutableRevisionFromArray( [
|
||||
'title' => $title ?: Title::makeTitle( NS_SPECIAL, 'Badtitle/' . __METHOD__ )
|
||||
] );
|
||||
$fromRev->setContent(
|
||||
SlotRecord::MAIN,
|
||||
$toRelRev->getContent( SlotRecord::MAIN, RevisionRecord::RAW )
|
||||
->getContentHandler()
|
||||
->makeEmptyContent()
|
||||
);
|
||||
}
|
||||
break;
|
||||
|
||||
case 'next':
|
||||
$toRev = $this->revisionStore->getNextRevision( $fromRelRev );
|
||||
$toRelRev = $toRev;
|
||||
$toValsRev = $toRev;
|
||||
if ( !$toRev ) {
|
||||
$title = Title::newFromLinkTarget( $fromRelRev->getPageAsLinkTarget() );
|
||||
$this->addWarning( [
|
||||
'apiwarn-compare-no-next',
|
||||
wfEscapeWikiText( $title->getPrefixedText() ),
|
||||
$fromRelRev->getId()
|
||||
] );
|
||||
|
||||
// (T203433) The web UI treats "next" as "cur" in this case.
|
||||
// Avoid repeating metadata by making a MutableRevisionRecord with no changes.
|
||||
$toRev = MutableRevisionRecord::newFromParentRevision( $fromRelRev );
|
||||
}
|
||||
break;
|
||||
|
||||
case 'cur':
|
||||
|
|
@ -93,10 +125,12 @@ class ApiComparePages extends ApiBase {
|
|||
list( $toRev, $toRelRev, $toValsRev ) = $this->getDiffRevision( 'to', $params );
|
||||
}
|
||||
|
||||
// Handle missing from or to revisions
|
||||
// Handle missing from or to revisions (should never happen)
|
||||
// @codeCoverageIgnoreStart
|
||||
if ( !$fromRev || !$toRev ) {
|
||||
$this->dieWithError( 'apierror-baddiff' );
|
||||
}
|
||||
// @codeCoverageIgnoreEnd
|
||||
|
||||
// Handle revdel
|
||||
if ( !$fromRev->audienceCan(
|
||||
|
|
|
|||
|
|
@ -1887,6 +1887,8 @@
|
|||
"apiwarn-badurlparam": "Could not parse <var>$1urlparam</var> for $2. Using only width and height.",
|
||||
"apiwarn-badutf8": "The value passed for <var>$1</var> contains invalid or non-normalized data. Textual data should be valid, NFC-normalized Unicode without C0 control characters other than HT (\\t), LF (\\n), and CR (\\r).",
|
||||
"apiwarn-checktoken-percentencoding": "Check that symbols such as \"+\" in the token are properly percent-encoded in the URL.",
|
||||
"apiwarn-compare-no-next": "Revision $2 is the latest revision of $1, there is no revision for <kbd>torelative=next</kbd> to compare to.",
|
||||
"apiwarn-compare-no-prev": "Revision $2 is the earliest revision of $1, there is no revision for <kbd>torelative=prev</kbd> to compare to.",
|
||||
"apiwarn-compare-nocontentmodel": "No content model could be determined, assuming $1.",
|
||||
"apiwarn-deprecation-deletedrevs": "<kbd>list=deletedrevs</kbd> has been deprecated. Please use <kbd>prop=deletedrevisions</kbd> or <kbd>list=alldeletedrevisions</kbd> instead.",
|
||||
"apiwarn-deprecation-httpsexpected": "HTTP used when HTTPS was expected.",
|
||||
|
|
|
|||
|
|
@ -1774,6 +1774,8 @@
|
|||
"apiwarn-badurlparam": "{{doc-apierror}}\n\nParameters:\n* $1 - Module parameter prefix, e.g. \"bl\".\n* $2 - Image title.",
|
||||
"apiwarn-badutf8": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter name.\n{{doc-important|Do not translate \"\\t\", \"\\n\", and \"\\r\"}}",
|
||||
"apiwarn-checktoken-percentencoding": "{{doc-apierror}}",
|
||||
"apiwarn-compare-no-next": "{{doc-apierror}}\n\nParameters:\n* $1 - Title of the page.\n* $2 - Revision ID.",
|
||||
"apiwarn-compare-no-prev": "{{doc-apierror}}\n\nParameters:\n* $1 - Title of the page.\n* $2 - Revision ID.",
|
||||
"apiwarn-compare-nocontentmodel": "{{doc-apierror}}\n\nParameters:\n* $1 - Content model being assumed.",
|
||||
"apiwarn-deprecation-deletedrevs": "{{doc-apierror}}",
|
||||
"apiwarn-deprecation-httpsexpected": "{{doc-apierror}}",
|
||||
|
|
|
|||
|
|
@ -553,6 +553,62 @@ class ApiComparePagesTest extends ApiTestCase {
|
|||
]
|
||||
],
|
||||
],
|
||||
'Relative diff, no prev' => [
|
||||
[
|
||||
'fromrev' => '{{REPL:revA1}}',
|
||||
'torelative' => 'prev',
|
||||
'prop' => 'ids|rel|diff|title|user|comment',
|
||||
],
|
||||
[
|
||||
'warnings' => [
|
||||
[
|
||||
'code' => 'compare-no-prev',
|
||||
'module' => 'compare',
|
||||
],
|
||||
],
|
||||
'compare' => [
|
||||
'toid' => '{{REPL:pageA}}',
|
||||
'torevid' => '{{REPL:revA1}}',
|
||||
'tons' => 0,
|
||||
'totitle' => 'ApiComparePagesTest A',
|
||||
'touser' => '{{REPL:creator}}',
|
||||
'touserid' => '{{REPL:creatorid}}',
|
||||
'tocomment' => 'Test for ApiComparePagesTest: A 1',
|
||||
'toparsedcomment' => 'Test for ApiComparePagesTest: A 1',
|
||||
'next' => '{{REPL:revA2}}',
|
||||
'body' => '<tr><td colspan="2" class="diff-lineno" id="mw-diff-left-l1" >Line 1:</td>' . "\n"
|
||||
. '<td colspan="2" class="diff-lineno">Line 1:</td></tr>' . "\n"
|
||||
. '<tr><td class=\'diff-marker\'>−</td><td class=\'diff-deletedline\'><div> </div></td><td class=\'diff-marker\'>+</td><td class=\'diff-addedline\'><div><ins class="diffchange diffchange-inline">A 1</ins></div></td></tr>' . "\n",
|
||||
],
|
||||
],
|
||||
],
|
||||
'Relative diff, no next' => [
|
||||
[
|
||||
'fromrev' => '{{REPL:revA4}}',
|
||||
'torelative' => 'next',
|
||||
'prop' => 'ids|rel|diff|title|user|comment',
|
||||
],
|
||||
[
|
||||
'warnings' => [
|
||||
[
|
||||
'code' => 'compare-no-next',
|
||||
'module' => 'compare',
|
||||
],
|
||||
],
|
||||
'compare' => [
|
||||
'fromid' => '{{REPL:pageA}}',
|
||||
'fromrevid' => '{{REPL:revA4}}',
|
||||
'fromns' => 0,
|
||||
'fromtitle' => 'ApiComparePagesTest A',
|
||||
'fromuser' => '{{REPL:creator}}',
|
||||
'fromuserid' => '{{REPL:creatorid}}',
|
||||
'fromcomment' => 'Test for ApiComparePagesTest: A 4',
|
||||
'fromparsedcomment' => 'Test for ApiComparePagesTest: A 4',
|
||||
'prev' => '{{REPL:revA3}}',
|
||||
'body' => '',
|
||||
],
|
||||
],
|
||||
],
|
||||
'Diff for specific slots' => [
|
||||
// @todo Use a page with multiple slots here
|
||||
[
|
||||
|
|
@ -874,24 +930,6 @@ class ApiComparePagesTest extends ApiTestCase {
|
|||
[],
|
||||
'missingcontent'
|
||||
],
|
||||
'Error, Relative diff, no prev' => [
|
||||
[
|
||||
'fromrev' => '{{REPL:revA1}}',
|
||||
'torelative' => 'prev',
|
||||
'prop' => 'ids',
|
||||
],
|
||||
[],
|
||||
'baddiff'
|
||||
],
|
||||
'Error, Relative diff, no next' => [
|
||||
[
|
||||
'fromrev' => '{{REPL:revA4}}',
|
||||
'torelative' => 'next',
|
||||
'prop' => 'ids',
|
||||
],
|
||||
[],
|
||||
'baddiff'
|
||||
],
|
||||
'Error, section diff with no revision' => [
|
||||
[
|
||||
'fromtitle' => 'ApiComparePagesTest F',
|
||||
|
|
|
|||
Loading…
Reference in a new issue