Add delete-redirect for deleting single-rev redirects during moves

A new user right, `delete-redirect`, is added (not given to anyone
by default). At Special:MovePage, if attempting to move to a single
revision redirect that would otherwise be an invalid target (i.e.
doesn't point to the source page), the user is able to delete the
target.

Deletions are logged as `delete/delete_redir2`, and the move is
then logged normally as `move/move`, mirroring current delete and
move logging.

To allow for separate handling by Special:MovePage,
MovePage::isValidMove now returns a fatal status `redirectexists` if
the target isn't valid but passes Title::isSingleRevRedirect.
Otherwise, `articleexists` is returned (as previously). Other callers
that don't intend to treat single revision redirects differently
should treat `redirectexists` the same as `articleexists`.

Currently, this deletion (like normal delete and move) cannot be
done through the move api. Since the deletion is only valid when
moving a page, unlike for normal deletion, deleting redirects with
this right cannot be done via the delete api either.

Bug: T239277
Change-Id: I36c8df0a12d326ae07018046541bd00103936144
This commit is contained in:
DannyS712 2019-12-19 23:13:31 +00:00
parent d8ae5a03a7
commit 77781b08c7
11 changed files with 150 additions and 42 deletions

View file

@ -6286,6 +6286,7 @@ $wgGrantPermissions['editsiteconfig']['editsitejs'] = true;
$wgGrantPermissions['createeditmovepage'] = $wgGrantPermissions['editpage'];
$wgGrantPermissions['createeditmovepage']['createpage'] = true;
$wgGrantPermissions['createeditmovepage']['createtalk'] = true;
$wgGrantPermissions['createeditmovepage']['delete-redirect'] = true;
$wgGrantPermissions['createeditmovepage']['move'] = true;
$wgGrantPermissions['createeditmovepage']['move-rootuserpages'] = true;
$wgGrantPermissions['createeditmovepage']['move-subpages'] = true;
@ -8250,6 +8251,7 @@ $wgLogActionsHandlers = [
'contentmodel/new' => ContentModelLogFormatter::class,
'delete/delete' => DeleteLogFormatter::class,
'delete/delete_redir' => DeleteLogFormatter::class,
'delete/delete_redir2' => DeleteLogFormatter::class,
'delete/event' => DeleteLogFormatter::class,
'delete/restore' => DeleteLogFormatter::class,
'delete/revision' => DeleteLogFormatter::class,
@ -8301,7 +8303,7 @@ $wgActionFilteredLogs = [
],
'delete' => [
'delete' => [ 'delete' ],
'delete_redir' => [ 'delete_redir' ],
'delete_redir' => [ 'delete_redir', 'delete_redir2' ],
'restore' => [ 'restore' ],
'event' => [ 'event' ],
'revision' => [ 'revision' ],

View file

@ -222,8 +222,11 @@ class MovePage {
} elseif ( $this->newTitle->getArticleID() && !$this->isValidMoveTarget() ) {
// The move is allowed only if (1) the target doesn't exist, or (2) the target is a
// redirect to the source, and has no history (so we can undo bad moves right after
// they're done).
$status->fatal( 'articleexists', $this->newTitle->getPrefixedText() );
// they're done). If the target is a single revision redirect to a different page,
// it can be deleted with just `delete-redirect` rights (i.e. without needing
// `delete`) - see T239277
$fatal = $this->newTitle->isSingleRevRedirect() ? 'redirectexists' : 'articleexists';
$status->fatal( $fatal, $this->newTitle->getPrefixedText() );
}
// @todo If the old title is invalid, maybe we should check if it somehow exists in the

View file

@ -127,6 +127,7 @@ class PermissionManager {
'createpage',
'createtalk',
'delete',
'delete-redirect',
'deletechangetags',
'deletedhistory',
'deletedtext',
@ -998,7 +999,7 @@ class PermissionManager {
} elseif ( !$title->isMovable() ) {
$errors[] = [ 'immobile-target-page' ];
}
} elseif ( $action == 'delete' ) {
} elseif ( $action == 'delete' || $action == 'delete-redirect' ) {
$tempErrors = $this->checkPageRestrictions( 'edit', $user, [], $rigor, true, $title );
if ( !$tempErrors ) {
$tempErrors = $this->checkCascadingSourcesRestrictions( 'edit',
@ -1008,7 +1009,7 @@ class PermissionManager {
// If protection keeps them from editing, they shouldn't be able to delete.
$errors[] = [ 'deleteprotected' ];
}
if ( $rigor !== self::RIGOR_QUICK && $wgDeleteRevisionsLimit
if ( $rigor !== self::RIGOR_QUICK && $action == 'delete' && $wgDeleteRevisionsLimit
&& !$this->userCan( 'bigdelete', $user, $title ) && $title->isBigDeletion()
) {
$errors[] = [ 'delete-toobig', $wgLang->formatNum( $wgDeleteRevisionsLimit ) ];

View file

@ -841,6 +841,7 @@ class RecentChange implements Taggable {
switch ( $type . '-' . $action ) {
case 'delete-delete':
case 'delete-delete_redir':
case 'delete-delete_redir2':
$pageStatus = 'deleted';
break;
case 'move-move':

View file

@ -156,6 +156,7 @@ class DeleteLogFormatter extends LogFormatter {
switch ( $this->entry->getSubtype() ) {
case 'delete': // Show undelete link
case 'delete_redir':
case 'delete_redir2':
if ( $permissionManager->userHasRight( $user, 'undelete' ) ) {
$message = 'undeletelink';
} else {

View file

@ -205,29 +205,37 @@ class MovePageForm extends UnlistedSpecialPage {
}
}
if ( count( $err ) == 1 && isset( $err[0][0] ) && $err[0][0] == 'articleexists'
&& $this->permManager->quickUserCan( 'delete', $user, $newTitle )
) {
$out->wrapWikiMsg(
"<div class='warningbox'>\n$1\n</div>\n",
[ 'delete_and_move_text', $newTitle->getPrefixedText() ]
);
$deleteAndMove = true;
$err = [];
}
if ( count( $err ) == 1 && isset( $err[0][0] ) && $err[0][0] == 'file-exists-sharedrepo'
&& $this->permManager->userHasRight( $user, 'reupload-shared' )
) {
$out->wrapWikiMsg(
"<div class='warningbox'>\n$1\n</div>\n",
[
'move-over-sharedrepo',
$newTitle->getPrefixedText()
]
);
$moveOverShared = true;
$err = [];
if ( count( $err ) == 1 && isset( $err[0][0] ) ) {
if ( $err[0][0] == 'articleexists'
&& $this->permManager->quickUserCan( 'delete', $user, $newTitle )
) {
$out->wrapWikiMsg(
"<div class='warningbox'>\n$1\n</div>\n",
[ 'delete_and_move_text', $newTitle->getPrefixedText() ]
);
$deleteAndMove = true;
$err = [];
} elseif ( $err[0][0] == 'redirectexists' && (
// Any user that can delete normally can also delete a redirect here
$this->permManager->quickUserCan( 'delete-redirect', $user, $newTitle ) ||
$this->permManager->quickUserCan( 'delete', $user, $newTitle ) )
) {
$out->wrapWikiMsg(
"<div class='warningbox'>\n$1\n</div>\n",
[ 'delete_redirect_and_move_text', $newTitle->getPrefixedText() ]
);
$deleteAndMove = true;
$err = [];
} elseif ( $err[0][0] == 'file-exists-sharedrepo'
&& $this->permManager->userHasRight( $user, 'reupload-shared' )
) {
$out->wrapWikiMsg(
"<div class='warningbox'>\n$1\n</div>\n",
[ 'move-over-sharedrepo', $newTitle->getPrefixedText() ]
);
$moveOverShared = true;
$err = [];
}
}
$oldTalk = $this->oldTitle->getTalkPage();
@ -387,8 +395,7 @@ class MovePageForm extends UnlistedSpecialPage {
);
}
if ( $this->permManager->userHasRight( $user, 'suppressredirect' )
) {
if ( $this->permManager->userHasRight( $user, 'suppressredirect' ) ) {
if ( $handlerSupportsRedirects ) {
$isChecked = $this->leaveRedirect;
$isDisabled = false;
@ -565,12 +572,30 @@ class MovePageForm extends UnlistedSpecialPage {
# Delete to make way if requested
if ( $this->deleteAndMove ) {
$permErrors = $this->permManager->getPermissionErrors( 'delete', $user, $nt );
if ( count( $permErrors ) ) {
# Only show the first error
$this->showForm( $permErrors, true );
$redir2 = $nt->isSingleRevRedirect();
return;
$permErrors = $this->permManager->getPermissionErrors(
$redir2 ? 'delete-redirect' : 'delete',
$user, $nt
);
if ( count( $permErrors ) ) {
if ( $redir2 ) {
if ( count( $this->permManager->getPermissionErrors( 'delete', $user, $nt ) ) ) {
// Cannot delete-redirect, or delete normally
// Only show the first error
$this->showForm( $permErrors, true );
return;
} else {
// Cannot delete-redirect, but can delete normally,
// so log as a normal deletion
$redir2 = false;
}
} else {
// Cannot delete normally
// Only show first error
$this->showForm( $permErrors, true );
return;
}
}
$page = WikiPage::factory( $nt );
@ -593,10 +618,11 @@ class MovePageForm extends UnlistedSpecialPage {
}
}
$error = ''; // passed by ref
$deletionLog = $redir2 ? 'delete_redir2' : 'delete';
$deleteStatus = $page->doDeleteArticleReal(
$reason,
$user,
/* suppress */ false
$reason, $user, false, null, $error,
null, [], $deletionLog
);
if ( !$deleteStatus->isGood() ) {
$this->showForm( $deleteStatus->getErrorsArray() );

View file

@ -1277,6 +1277,7 @@
"right-apihighlimits": "Use higher limits in API queries",
"right-writeapi": "Use of the write API",
"right-delete": "Delete pages",
"right-delete-redirect": "Delete single revision redirects",
"right-bigdelete": "Delete pages with large histories",
"right-deletelogentry": "Delete and undelete specific log entries",
"right-deleterevision": "Delete and undelete specific revisions of pages",
@ -1389,6 +1390,7 @@
"action-upload_by_url": "upload this file from a URL",
"action-writeapi": "use the write API",
"action-delete": "delete this page",
"action-delete-redirect": "overwrite single revision redirects",
"action-deleterevision": "delete revisions",
"action-deletelogentry": "delete log entries",
"action-deletedhistory": "view a page's deleted history",
@ -2869,6 +2871,7 @@
"movepage-moved-noredirect": "The creation of a redirect has been suppressed.",
"movepage-delete-first": "The target page has too many revisions to delete as part of a page move. Please first delete the page manually, then try again.",
"articleexists": "A page already exists at [[:$1]], or the page name you have chosen is not valid.\nPlease choose another name.",
"redirectexists": "A redirect already exists at [[:$1]], and it cannot be deleted automatically.\nPlease choose another name.",
"cantmove-titleprotected": "You cannot move a page to this location because the new title has been protected from creation.",
"movetalk": "Move associated talk page",
"move-subpages": "Move subpages (up to $1)",
@ -2889,6 +2892,7 @@
"category-move-redirect-override": "-",
"revertmove": "revert",
"delete_and_move_text": "The destination page \"[[:$1]]\" already exists.\nDo you want to delete it to make way for the move?",
"delete_redirect_and_move_text": "The destination page \"[[:$1]]\" already exists as a redirect.\nDo you want to delete it to make way for the move?",
"delete_and_move_confirm": "Yes, delete the page",
"delete_and_move_reason": "Deleted to make way for move from \"[[$1]]\"",
"selfmove": "The title is the same;\ncannot move a page over itself.",
@ -3899,6 +3903,7 @@
"rawmessage": "$1",
"logentry-delete-delete": "$1 {{GENDER:$2|deleted}} page $3",
"logentry-delete-delete_redir": "$1 {{GENDER:$2|deleted}} redirect $3 by overwriting",
"logentry-delete-delete_redir2": "$1 {{GENDER:$2|deleted}} redirect $3 by overwriting",
"logentry-delete-restore": "$1 {{GENDER:$2|restored}} page $3 ($4)",
"logentry-delete-restore-nocount": "$1 {{GENDER:$2|restored}} page $3",
"restore-count-revisions": "{{PLURAL:$1|1 revision|$1 revisions}}",
@ -4193,6 +4198,7 @@
"log-action-filter-contentmodel-new": "Creation of page with non-default content model",
"log-action-filter-delete-delete": "Page deletion",
"log-action-filter-delete-delete_redir": "Redirect overwrite",
"log-action-filter-delete-delete_redir2": "Redirect overwrite",
"log-action-filter-delete-restore": "Page undeletion",
"log-action-filter-delete-event": "Log deletion",
"log-action-filter-delete-revision": "Revision deletion",

View file

@ -1493,6 +1493,7 @@
"right-apihighlimits": "{{doc-right|apihighlimits}}",
"right-writeapi": "{{doc-right|writeapi}}",
"right-delete": "{{doc-right|delete}}\n{{Identical|Delete page}}",
"right-delete-redirect": "{{doc-right|delete-redirect}}",
"right-bigdelete": "{{doc-right|bigdelete}}",
"right-deletelogentry": "{{doc-right|deletelogentry}}\nThis user right is part of the [[mw:RevisionDelete|RevisionDelete]] feature.\nIt can be given to the group {{msg-mw|group-sysop}}, although this right is disabled by default.\n\nSee also:\n* {{msg-mw|right-suppressionlog}}\n* {{msg-mw|right-hideuser}}\n* {{msg-mw|right-suppressrevision}}\n* {{msg-mw|right-deleterevision}}",
"right-deleterevision": "{{doc-right|deleterevision}}\nThis user right is part of the [[mw:RevisionDelete|RevisionDelete]] feature.\nIt can be given to the group {{msg-mw|group-sysop}}, although this right is disabled by default.\n\nSee also\n* {{msg-mw|right-suppressionlog}}\n* {{msg-mw|right-hideuser}}\n* {{msg-mw|right-suppressrevision}}\n* {{msg-mw|right-deletelogentry}}",
@ -1605,6 +1606,7 @@
"action-upload_by_url": "{{Doc-action|upload_by_url}}",
"action-writeapi": "{{Doc-action|writeapi}}\n\nAPI is an abbreviation for [[w:API|application programming interface]].",
"action-delete": "{{Doc-action|delete}}",
"action-delete-redirect": "{{doc-action|delete-redirect}}",
"action-deleterevision": "{{Doc-action|deleterevision}}",
"action-deletelogentry": "{{Doc-action|deletelogentry}}",
"action-deletedhistory": "{{Doc-action|deletedhistory}}",
@ -3084,7 +3086,8 @@
"movepage-moved-redirect": "See also:\n* {{msg-mw|Movepage-moved}}\n* {{msg-mw|Movepage-moved-noredirect}}",
"movepage-moved-noredirect": "The message is shown after pagemove if checkbox \"{{int:move-leave-redirect}}\" was unselected before moving.\n\nSee also:\n* {{msg-mw|Movepage-moved}}\n* {{msg-mw|Movepage-moved-redirect}}",
"movepage-delete-first": "Error message shown when trying to move a page and delete the existing page by that name, but the existing page has too many revisions.",
"articleexists": "Used as error message when moving a page. Parameters:\n* $1 - target page title\n\nSee also:\n* {{msg-mw|Badarticleerror}}\n* {{msg-mw|Bad-target-model}}",
"articleexists": "Used as error message when moving a page. Parameters:\n* $1 - target page title\n\nSee also:\n* {{msg-mw|Redirectexists}}\n* {{msg-mw|Badarticleerror}}\n* {{msg-mw|Bad-target-model}}",
"redirectexists": "Used as error message when moving a page. Parameters:\n* $1 - target page title\n\nSee also:\n* {{msg-mw|Articleexists}}\n* {{msg-mw|Badarticleerror}}\n* {{msg-mw|Bad-target-model}}",
"cantmove-titleprotected": "Used as error message when moving a page.",
"movetalk": "The text of the checkbox to watch the associated talk page to the page you are moving. This only appears when the talk page is not empty. Used in [[Special:MovePage]].\n\nSee also:\n* {{msg-mw|Move-page-legend|legend for the form}}\n* {{msg-mw|newtitle|label for new title}}\n* {{msg-mw|Movereason|label for textarea}}\n* {{msg-mw|Move-leave-redirect|label for checkbox}}\n* {{msg-mw|Fix-double-redirects|label for checkbox}}\n* {{msg-mw|Move-subpages|label for checkbox}}\n* {{msg-mw|Move-talk-subpages|label for checkbox}}\n* {{msg-mw|Move-watch|label for checkbox}}",
"move-subpages": "The text of an option on the special page [[Special:MovePage|MovePage]]. If this option is ticked, any subpages will be moved with the main page to a new title.\n\nParameters:\n* $1 - ...\nSee also:\n* {{msg-mw|Move-page-legend|legend for the form}}\n* {{msg-mw|newtitle|label for new title}}\n* {{msg-mw|Movereason|label for textarea}}\n* {{msg-mw|Movetalk|label for checkbox}}\n* {{msg-mw|Move-leave-redirect|label for checkbox}}\n* {{msg-mw|Fix-double-redirects|label for checkbox}}\n* {{msg-mw|Move-talk-subpages|label for checkbox}}\n* {{msg-mw|Move-watch|label for checkbox}}",
@ -3104,8 +3107,9 @@
"move-redirect-text": "{{ignored}}The text that's added to a redirected page when that redirect is created.",
"category-move-redirect-override": "{{ignored}}The text that's added to a redirected category page when that redirect is created.",
"revertmove": "{{Identical|Revert}}",
"delete_and_move_text": "Used when moving a page, but the destination page already exists and needs deletion.\n\nThis message is to confirm that you really want to delete the page.\n\nParameters:\n* $1 - the destination page title\n\nSee also:\n* {{msg-mw|Delete and move confirm}}",
"delete_and_move_confirm": "Used when moving a page, but the destination page already exists and needs deletion.\n\nThis message is for a checkbox to confirm that you really want to delete the page.\n\nSee also:\n* {{msg-mw|Delete and move text}}",
"delete_and_move_text": "Used when moving a page, but the destination page already exists and needs deletion.\n\nThis message is to confirm that you really want to delete the page.\n\nParameters:\n* $1 - the destination page title\n\nSee also:\n* {{msg-mw|Delete redirect and move confirm}}\n* {{msg-mw|Delete and move confirm}}",
"delete_redirect_and_move_text": "Used when moving a page, but the destination page already exists as a redirect and needs deletion.\n\nThis message is to confirm that you really want to delete the page.\n\nParameters:\n* $1 - the destination page title\n\nSee also:\n* {{msg-mw|Delete and move confirm}}\n* {{msg-mw|Delete and move confirm}}",
"delete_and_move_confirm": "Used when moving a page, but the destination page already exists and needs deletion.\n\nThis message is for a checkbox to confirm that you really want to delete the page.\n\nSee also:\n* {{msg-mw|Delete and move text}}\n* {{msg-mw|Delete and move confirm}}\n* {{msg-mw|Delete redirect and move confirm}}",
"delete_and_move_reason": "Shown as reason in content language in the deletion log. Parameter:\n* $1 - The page name for which this page was deleted.",
"selfmove": "Used as error message when moving page.\n\nSee also:\n* {{msg-mw|badtitletext}}\n* {{msg-mw|immobile-source-namespace}}\n* {{msg-mw|immobile-target-namespace-iw}}\n* {{msg-mw|immobile-target-namespace}}",
"immobile-source-namespace": "Used as error message. Parameters:\n* $1 - source namespace name\nSee also:\n* {{msg-mw|Immobile-source-page}}\n* {{msg-mw|Immobile-target-namespace}}\n* {{msg-mw|Immobile-target-page}}",
@ -4115,6 +4119,7 @@
"rawmessage": "{{notranslate}} Used to pass arbitrary text as a message specifier array",
"logentry-delete-delete": "{{Logentry|[[Special:Log/delete]]}}",
"logentry-delete-delete_redir": "{{Logentry|[[Special:Log/delete]]}}",
"logentry-delete-delete_redir2": "{{Logentry|[[Special:Log/delete]]}}",
"logentry-delete-restore": "{{Logentry|[[Special:Log/delete]]}}\n* $4 - {{msg-mw|restore-count-revisions}} or {{msg-mw|restore-count-files}}, or a combination with both (e.g. \"3 revision and 1 file\")\n\n'''A note for RTL languages''': if $3 is a name of a page or a file written in a different language, the number in the beginning of $4 may be displayed incorrectly. Consider inserting a word or an RLM between $3 and $4.",
"logentry-delete-restore-nocount": "{{Logentry|[[Special:Log/delete]]}}",
"restore-count-revisions": "Used as parameter in {{msg-mw|logentry-delete-restore}}\n{{Identical|Revision}}",
@ -4409,6 +4414,7 @@
"log-action-filter-contentmodel-new": "{{doc-log-action-filter-action|contentmodel|new}}",
"log-action-filter-delete-delete": "{{doc-log-action-filter-action|delete|delete}}",
"log-action-filter-delete-delete_redir": "{{doc-log-action-filter-action|delete|delete_redir}}",
"log-action-filter-delete-delete_redir2": "{{doc-log-action-filter-action|delete|delete_redir2}}",
"log-action-filter-delete-restore": "{{doc-log-action-filter-action|delete|restore}}",
"log-action-filter-delete-event": "{{doc-log-action-filter-action|delete|event}}",
"log-action-filter-delete-revision": "{{doc-log-action-filter-action|delete|revision}}",

View file

@ -307,7 +307,8 @@ class UppercaseTitlesForUnicodeTransition extends Maintenance {
} else {
$mp = new MovePage( $oldTitle, $newTitle );
$status = $mp->isValidMove();
if ( !$status->isOK() && $status->hasMessage( 'articleexists' ) ) {
if ( !$status->isOK() && (
$status->hasMessage( 'articleexists' ) || $status->hasMessage( 'redirectexists' ) ) ) {
$munge = 'Target title exists';
}
}

View file

@ -494,4 +494,44 @@ class MovePageTest extends MediaWikiIntegrationTestCase {
$this->assertSame( $id, $toTitle->getArticleID() );
}
/**
* Test redirect handling
*
* @covers MovePage::isValidMove
*/
public function testRedirects() {
$this->editPage( 'ExistentRedirect', '#REDIRECT [[Existent]]' );
$mp = $this->newMovePage(
Title::newFromText( 'Existent' ),
Title::newFromText( 'ExistentRedirect' )
);
$this->assertSame(
[],
$mp->isValidMove()->getErrorsArray(),
'Sanity check - can move over normal redirect'
);
$this->editPage( 'ExistentRedirect3', '#REDIRECT [[Existent]]' );
$mp = $this->newMovePage(
Title::newFromText( 'Existent2' ),
Title::newFromText( 'ExistentRedirect3' )
);
$this->assertSame(
[ [ 'redirectexists', 'ExistentRedirect3' ] ],
$mp->isValidMove()->getErrorsArray(),
'Cannot move over redirect with a different target'
);
$this->editPage( 'ExistentRedirect3', '#REDIRECT [[Existent2]]' );
$mp = $this->newMovePage(
Title::newFromText( 'Existent' ),
Title::newFromText( 'ExistentRedirect3' )
);
$this->assertSame(
[ [ 'articleexists', 'ExistentRedirect3' ] ],
$mp->isValidMove()->getErrorsArray(),
'Multi-revision redirects count as articles'
);
}
}

View file

@ -1935,4 +1935,25 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
$pm = MediaWikiServices::getInstance()->getPermissionManager();
$this->assertNotSame( $pm->getUserPermissions( $user1 ), $pm->getUserPermissions( $user2 ) );
}
/**
* Test delete-redirect checks for Special:MovePage
*/
public function testDeleteRedirect() {
$this->editPage( 'ExistentRedirect3', '#REDIRECT [[Existent]]' );
$page = Title::newFromText( 'ExistentRedirect3' );
$pm = MediaWikiServices::getInstance()->getPermissionManager();
$user = $this->getMockBuilder( User::class )
->setMethods( [ 'getEffectiveGroups' ] )
->getMock();
$user->method( 'getEffectiveGroups' )->willReturn( [ '*', 'user' ] );
$this->assertFalse( $pm->quickUserCan( 'delete-redirect', $user, $page ) );
$pm->overrideUserRightsForTesting( $user, 'delete-redirect' );
$this->assertTrue( $pm->quickUserCan( 'delete-redirect', $user, $page ) );
$this->assertArrayEquals( [], $pm->getPermissionErrors( 'delete-redirect', $user, $page ) );
}
}