Introduce MovePage::moveSubpages(IfAllowed)

Title::moveSubpages() is now deprecated, and the one caller in core and
extensions was ported.

Change-Id: Ic1dc5a6f1a1bef89a35c13f0a72f6740c6a01c50
This commit is contained in:
Aryeh Gregor 2019-04-16 16:51:04 +03:00 committed by Bill Pirkle
parent 6071231e90
commit 9510dbc4ce
5 changed files with 263 additions and 49 deletions

View file

@ -215,6 +215,8 @@ because of Phabricator reports.
should be used when creating any temporary blocks.
* Parser::$mConf is deprecated. It will be removed entirely in a later version.
* Constructing Parser directly is deprecated. Obtain one from ParserFactory.
* Title::moveSubpages is deprecated. Use MovePage::moveSubpages or
MovePage::moveSubpagesIfAllowed.
=== Other changes in 1.34 ===
* …

View file

@ -286,6 +286,132 @@ class MovePage {
return $this->moveUnsafe( $user, $reason, $createRedirect, $changeTags );
}
/**
* Move the source page's subpages to be subpages of the target page, without checking user
* permissions. The caller is responsible for moving the source page itself. We will still not
* do moves that are inherently not allowed, nor will we move more than $wgMaximumMovedPages.
*
* @param User $user
* @param string|null $reason The reason for the move
* @param bool|null $createRedirect Whether to create redirects from the old subpages to
* the new ones
* @param string[] $changeTags Applied to entries in the move log and redirect page revision
* @return Status Good if no errors occurred. Ok if at least one page succeeded. The "value"
* of the top-level status is an array containing the per-title status for each page. For any
* move that succeeded, the "value" of the per-title status is the new page title.
*/
public function moveSubpages(
User $user, $reason = null, $createRedirect = true, array $changeTags = []
) {
return $this->moveSubpagesInternal( false, $user, $reason, $createRedirect, $changeTags );
}
/**
* Move the source page's subpages to be subpages of the target page, with user permission
* checks. The caller is responsible for moving the source page itself.
*
* @param User $user
* @param string|null $reason The reason for the move
* @param bool|null $createRedirect Whether to create redirects from the old subpages to
* the new ones. Ignored if the user doesn't have the 'suppressredirect' right.
* @param string[] $changeTags Applied to entries in the move log and redirect page revision
* @return Status Good if no errors occurred. Ok if at least one page succeeded. The "value"
* of the top-level status is an array containing the per-title status for each page. For any
* move that succeeded, the "value" of the per-title status is the new page title.
*/
public function moveSubpagesIfAllowed(
User $user, $reason = null, $createRedirect = true, array $changeTags = []
) {
return $this->moveSubpagesInternal( true, $user, $reason, $createRedirect, $changeTags );
}
/**
* @param bool $checkPermissions
* @param User $user
* @param string $reason
* @param bool $createRedirect
* @param array $changeTags
* @return Status
*/
private function moveSubpagesInternal(
$checkPermissions, User $user, $reason, $createRedirect, array $changeTags
) {
global $wgMaximumMovedPages;
$services = MediaWikiServices::getInstance();
if ( $checkPermissions ) {
if ( !$services->getPermissionManager()->userCan(
'move-subpages', $user, $this->oldTitle )
) {
return Status::newFatal( 'cant-move-subpages' );
}
}
$nsInfo = $services->getNamespaceInfo();
// Do the source and target namespaces support subpages?
if ( !$nsInfo->hasSubpages( $this->oldTitle->getNamespace() ) ) {
return Status::newFatal( 'namespace-nosubpages',
$nsInfo->getCanonicalName( $this->oldTitle->getNamespace() ) );
}
if ( !$nsInfo->hasSubpages( $this->newTitle->getNamespace() ) ) {
return Status::newFatal( 'namespace-nosubpages',
$nsInfo->getCanonicalName( $this->newTitle->getNamespace() ) );
}
// Return a status for the overall result. Its value will be an array with per-title
// status for each subpage. Merge any errors from the per-title statuses into the
// top-level status without resetting the overall result.
$topStatus = Status::newGood();
$perTitleStatus = [];
$subpages = $this->oldTitle->getSubpages( $wgMaximumMovedPages + 1 );
$count = 0;
foreach ( $subpages as $oldSubpage ) {
$count++;
if ( $count > $wgMaximumMovedPages ) {
$status = Status::newFatal( 'movepage-max-pages', $wgMaximumMovedPages );
$perTitleStatus[$oldSubpage->getPrefixedText()] = $status;
$topStatus->merge( $status );
$topStatus->setOk( true );
break;
}
// We don't know whether this function was called before or after moving the root page,
// so check both titles
if ( $oldSubpage->getArticleID() == $this->oldTitle->getArticleID() ||
$oldSubpage->getArticleID() == $this->newTitle->getArticleID()
) {
// When moving a page to a subpage of itself, don't move it twice
continue;
}
$newPageName = preg_replace(
'#^' . preg_quote( $this->oldTitle->getDBkey(), '#' ) . '#',
StringUtils::escapeRegexReplacement( $this->newTitle->getDBkey() ), # T23234
$oldSubpage->getDBkey() );
if ( $oldSubpage->isTalkPage() ) {
$newNs = $this->newTitle->getTalkPage()->getNamespace();
} else {
$newNs = $this->newTitle->getSubjectPage()->getNamespace();
}
// T16385: we need makeTitleSafe because the new page names may be longer than 255
// characters.
$newSubpage = Title::makeTitleSafe( $newNs, $newPageName );
$mp = new MovePage( $oldSubpage, $newSubpage );
$method = $checkPermissions ? 'moveIfAllowed' : 'move';
$status = $mp->$method( $user, $reason, $createRedirect, $changeTags );
if ( $status->isOK() ) {
$status->setResult( true, $newSubpage->getPrefixedText() );
}
$perTitleStatus[$oldSubpage->getPrefixedText()] = $status;
$topStatus->merge( $status );
$topStatus->setOk( true );
}
$topStatus->value = $perTitleStatus;
return $topStatus;
}
/**
* Moves *without* any sort of safety or sanity checks. Hooks can still fail the move, however.
*

View file

@ -3480,6 +3480,7 @@ class Title implements LinkTarget, IDBAccessObject {
/**
* Move this page's subpages to be subpages of $nt
*
* @deprecated since 1.34, use MovePage instead
* @param Title $nt Move target
* @param bool $auth Whether $wgUser's permissions should be checked
* @param string $reason The reason for the move
@ -3494,7 +3495,6 @@ class Title implements LinkTarget, IDBAccessObject {
public function moveSubpages( $nt, $auth = true, $reason = '', $createRedirect = true,
array $changeTags = []
) {
global $wgMaximumMovedPages;
// Check permissions
if ( !$this->userCan( 'move-subpages' ) ) {
return [
@ -3514,46 +3514,21 @@ class Title implements LinkTarget, IDBAccessObject {
];
}
$subpages = $this->getSubpages( $wgMaximumMovedPages + 1 );
global $wgUser;
$mp = new MovePage( $this, $nt );
$method = $auth ? 'moveSubpagesIfAllowed' : 'moveSubpages';
$result = $mp->$method( $wgUser, $reason, $createRedirect, $changeTags );
if ( !$result->isOk() ) {
return $result->getErrorsArray();
}
$retval = [];
$count = 0;
foreach ( $subpages as $oldSubpage ) {
$count++;
if ( $count > $wgMaximumMovedPages ) {
$retval[$oldSubpage->getPrefixedText()] = [
[ 'movepage-max-pages', $wgMaximumMovedPages ],
];
break;
}
// We don't know whether this function was called before
// or after moving the root page, so check both
// $this and $nt
if ( $oldSubpage->getArticleID() == $this->getArticleID()
|| $oldSubpage->getArticleID() == $nt->getArticleID()
) {
// When moving a page to a subpage of itself,
// don't move it twice
continue;
}
$newPageName = preg_replace(
'#^' . preg_quote( $this->mDbkeyform, '#' ) . '#',
StringUtils::escapeRegexReplacement( $nt->getDBkey() ), # T23234
$oldSubpage->getDBkey() );
if ( $oldSubpage->isTalkPage() ) {
$newNs = $nt->getTalkPage()->getNamespace();
foreach ( $result->getValue() as $key => $status ) {
if ( $status->isOK() ) {
$retval[$key] = $status->getValue();
} else {
$newNs = $nt->getSubjectPage()->getNamespace();
}
# T16385: we need makeTitleSafe because the new page names may
# be longer than 255 characters.
$newSubpage = self::makeTitleSafe( $newNs, $newPageName );
$success = $oldSubpage->moveTo( $newSubpage, $auth, $reason, $createRedirect, $changeTags );
if ( $success === true ) {
$retval[$oldSubpage->getPrefixedText()] = $newSubpage->getPrefixedText();
} else {
$retval[$oldSubpage->getPrefixedText()] = $success;
$retval[$key] = $status->getErrorsArray();
}
}
return $retval;

View file

@ -201,22 +201,22 @@ class ApiMove extends ApiBase {
public function moveSubpages( $fromTitle, $toTitle, $reason, $noredirect, $changeTags = [] ) {
$retval = [];
$success = $fromTitle->moveSubpages( $toTitle, true, $reason, !$noredirect, $changeTags );
if ( isset( $success[0] ) ) {
$status = $this->errorArrayToStatus( $success );
return [ 'errors' => $this->getErrorFormatter()->arrayFromStatus( $status ) ];
$mp = new MovePage( $fromTitle, $toTitle );
$result =
$mp->moveSubpagesIfAllowed( $this->getUser(), $reason, !$noredirect, $changeTags );
if ( !$result->isOk() ) {
// This means the whole thing failed
return [ 'errors' => $this->getErrorFormatter()->arrayFromStatus( $result ) ];
}
// At least some pages could be moved
// Report each of them separately
foreach ( $success as $oldTitle => $newTitle ) {
foreach ( $result->getValue() as $oldTitle => $status ) {
$r = [ 'from' => $oldTitle ];
if ( is_array( $newTitle ) ) {
$status = $this->errorArrayToStatus( $newTitle );
$r['errors'] = $this->getErrorFormatter()->arrayFromStatus( $status );
if ( $status->isOK() ) {
$r['to'] = $status->getValue();
} else {
// Success
$r['to'] = $newTitle;
$r['errors'] = $this->getErrorFormatter()->arrayFromStatus( $status );
}
$retval[] = $r;
}

View file

@ -5,6 +5,13 @@
*/
class MovePageTest extends MediaWikiTestCase {
public function setUp() {
parent::setUp();
$this->tablesUsed[] = 'page';
$this->tablesUsed[] = 'revision';
$this->tablesUsed[] = 'comment';
}
/**
* @dataProvider provideIsValidMove
* @covers MovePage::isValidMove
@ -83,4 +90,108 @@ class MovePageTest extends MediaWikiTestCase {
$status = $mp->move( $user, 'Reason', true );
$this->assertTrue( $status->hasMessage( $error ) );
}
/**
* Test moving subpages from one page to another
* @covers MovePage::moveSubpages
*/
public function testMoveSubpages() {
$name = ucfirst( __FUNCTION__ );
$subPages = [ "Talk:$name/1", "Talk:$name/2" ];
$ids = [];
$pages = [
$name,
"Talk:$name",
"$name 2",
"Talk:$name 2",
];
foreach ( array_merge( $pages, $subPages ) as $page ) {
$ids[$page] = $this->createPage( $page );
}
$oldTitle = Title::newFromText( "Talk:$name" );
$newTitle = Title::newFromText( "Talk:$name 2" );
$mp = new MovePage( $oldTitle, $newTitle );
$status = $mp->moveSubpages( $this->getTestUser()->getUser(), 'Reason', true );
$this->assertTrue( $status->isGood(),
"Moving subpages from Talk:{$name} to Talk:{$name} 2 was not completely successful." );
foreach ( $subPages as $page ) {
$this->assertMoved( $page, str_replace( $name, "$name 2", $page ), $ids[$page] );
}
}
/**
* Test moving subpages from one page to another
* @covers MovePage::moveSubpagesIfAllowed
*/
public function testMoveSubpagesIfAllowed() {
$name = ucfirst( __FUNCTION__ );
$subPages = [ "Talk:$name/1", "Talk:$name/2" ];
$ids = [];
$pages = [
$name,
"Talk:$name",
"$name 2",
"Talk:$name 2",
];
foreach ( array_merge( $pages, $subPages ) as $page ) {
$ids[$page] = $this->createPage( $page );
}
$oldTitle = Title::newFromText( "Talk:$name" );
$newTitle = Title::newFromText( "Talk:$name 2" );
$mp = new MovePage( $oldTitle, $newTitle );
$status = $mp->moveSubpagesIfAllowed( $this->getTestUser()->getUser(), 'Reason', true );
$this->assertTrue( $status->isGood(),
"Moving subpages from Talk:{$name} to Talk:{$name} 2 was not completely successful." );
foreach ( $subPages as $page ) {
$this->assertMoved( $page, str_replace( $name, "$name 2", $page ), $ids[$page] );
}
}
/**
* Shortcut function to create a page and return its id.
*
* @param string $name Page to create
* @return int ID of created page
*/
protected function createPage( $name ) {
return $this->editPage( $name, 'Content' )->value['revision']->getPage();
}
/**
* @param string $from Prefixed name of source
* @param string $to Prefixed name of destination
* @param string $id Page id of the page to move
* @param array|string|null $opts Options: 'noredirect' to expect no redirect
*/
protected function assertMoved( $from, $to, $id, $opts = null ) {
$opts = (array)$opts;
Title::clearCaches();
$fromTitle = Title::newFromText( $from );
$toTitle = Title::newFromText( $to );
$this->assertTrue( $toTitle->exists(),
"Destination {$toTitle->getPrefixedText()} does not exist" );
if ( in_array( 'noredirect', $opts ) ) {
$this->assertFalse( $fromTitle->exists(),
"Source {$fromTitle->getPrefixedText()} exists" );
} else {
$this->assertTrue( $fromTitle->exists(),
"Source {$fromTitle->getPrefixedText()} does not exist" );
$this->assertTrue( $fromTitle->isRedirect(),
"Source {$fromTitle->getPrefixedText()} is not a redirect" );
$target = Revision::newFromTitle( $fromTitle )->getContent()->getRedirectTarget();
$this->assertSame( $toTitle->getPrefixedText(), $target->getPrefixedText() );
}
$this->assertSame( $id, $toTitle->getArticleID() );
}
}