From a9033efb28ca6e0e2e4ccb7bf9e9103b8d474b26 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Tue, 17 Jul 2018 15:23:25 -0400 Subject: [PATCH] MCR: Add temporary web UI mcrundo action Since SDC doesn't actually require the edit form to handle multi-slot editing, updating EditPage with its normal undo handling is being put off for later. But in the mean time we still want some sort of "undo" to work, hence this mcrundo that doesn't allow for editing. Bug: T200216 Change-Id: I1f11d8ed141cb11576d2df883856d03e8f64bd38 Depends-On: Iedd9bf6c057e8b396a575bab700b15bd38b32cc9 --- RELEASE-NOTES-1.32 | 7 + autoload.php | 1 + includes/DefaultSettings.php | 1 + includes/EditPage.php | 14 +- includes/Storage/PageUpdater.php | 9 + includes/actions/McrUndoAction.php | 376 ++++++++++++++++++++++++++++ includes/content/ContentHandler.php | 58 +++-- languages/i18n/en.json | 4 + languages/i18n/qqq.json | 4 + maintenance/tables.sql | 2 + 10 files changed, 455 insertions(+), 21 deletions(-) create mode 100644 includes/actions/McrUndoAction.php diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 0ad2e41b65..e3bb6d332c 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -75,6 +75,11 @@ production. render diffs between two Content objects, and DifferenceEngine::setRevisions() to render diffs between two custom (potentially multi-content) revisions. Added GetSlotDiffRenderer hook which works like GetDifferenceEngine for slots. +* Added a temporary action=mcrundo to the web UI, as the normal undo logic + can't yet handle MCR and deadlines are forcing is to put off fixing that. + This action should be considered deprecated and should not be used directly. +* Extensions overriding ContentHandler::getUndoContent() will need to be + updated for the changed method signature. === External library changes in 1.32 === * … @@ -377,6 +382,8 @@ because of Phabricator reports. MediaWikiServices. * mw.user.stickyRandomId was renamed to the more explicit mw.user.getPageviewToken to better capture its function. +* Passing Revision objects to ContentHandler::getUndoContent() is deprecated, + Content object should be passed instead. === Other changes in 1.32 === * (T198811) The following tables have had their UNIQUE indexes turned into diff --git a/autoload.php b/autoload.php index cef68b04ca..e6ae2bf1bb 100644 --- a/autoload.php +++ b/autoload.php @@ -851,6 +851,7 @@ $wgAutoloadLocalClasses = [ 'MappedIterator' => __DIR__ . '/includes/libs/MappedIterator.php', 'MarkpatrolledAction' => __DIR__ . '/includes/actions/MarkpatrolledAction.php', 'McTest' => __DIR__ . '/maintenance/mctest.php', + 'McrUndoAction' => __DIR__ . '/includes/actions/McrUndoAction.php', 'MediaHandler' => __DIR__ . '/includes/media/MediaHandler.php', 'MediaHandlerFactory' => __DIR__ . '/includes/media/MediaHandlerFactory.php', 'MediaStatisticsPage' => __DIR__ . '/includes/specials/SpecialMediaStatistics.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index fdac10a53e..fd943cc1ba 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -7998,6 +7998,7 @@ $wgActions = [ 'history' => true, 'info' => true, 'markpatrolled' => true, + 'mcrundo' => McrUndoAction::class, 'protect' => true, 'purge' => true, 'raw' => true, diff --git a/includes/EditPage.php b/includes/EditPage.php index d1f874ead7..e087a6e217 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -684,7 +684,10 @@ class EditPage { # checking, etc. if ( 'initial' == $this->formtype || $this->firsttime ) { if ( $this->initialiseForm() === false ) { - $this->noSuchSectionPage(); + $out = $this->context->getOutput(); + if ( $out->getRedirect() === '' ) { // mcrundo hack redirects, don't override it + $this->noSuchSectionPage(); + } return; } @@ -1220,8 +1223,13 @@ class EditPage { !$oldrev->isDeleted( Revision::DELETED_TEXT ) ) { if ( WikiPage::hasDifferencesOutsideMainSlot( $undorev, $oldrev ) ) { - // Cannot yet undo edits that involve anything other the main slot. - $undoMsg = 'main-slot-only'; + // Hack for undo while EditPage can't handle multi-slot editing + $this->context->getOutput()->redirect( $this->mTitle->getFullURL( [ + 'action' => 'mcrundo', + 'undo' => $undo, + 'undoafter' => $undoafter, + ] ) ); + return false; } else { $content = $this->page->getUndoContent( $undorev, $oldrev ); diff --git a/includes/Storage/PageUpdater.php b/includes/Storage/PageUpdater.php index c6795ea83a..838efcd7e2 100644 --- a/includes/Storage/PageUpdater.php +++ b/includes/Storage/PageUpdater.php @@ -351,6 +351,15 @@ class PageUpdater { $this->slotsUpdate->modifyContent( $role, $content ); } + /** + * Set the new slot for the given slot role + * + * @param SlotRecord $slot + */ + public function setSlot( SlotRecord $slot ) { + $this->slotsUpdate->modifySlot( $slot ); + } + /** * Explicitly inherit a slot from some earlier revision. * diff --git a/includes/actions/McrUndoAction.php b/includes/actions/McrUndoAction.php new file mode 100644 index 0000000000..90d1f686cd --- /dev/null +++ b/includes/actions/McrUndoAction.php @@ -0,0 +1,376 @@ +persist(); + + // Some stuff copied from EditAction + $this->useTransactionalTimeLimit(); + + $out = $this->getOutput(); + $out->setRobotPolicy( 'noindex,nofollow' ); + if ( $this->getContext()->getConfig()->get( 'UseMediaWikiUIEverywhere' ) ) { + $out->addModuleStyles( [ + 'mediawiki.ui.input', + 'mediawiki.ui.checkbox', + ] ); + } + + // IP warning headers copied from EditPage + // (should more be copied?) + if ( wfReadOnly() ) { + $out->wrapWikiMsg( + "
\n$1\n
", + [ 'readonlywarning', wfReadOnlyReason() ] + ); + } elseif ( $this->context->getUser()->isAnon() ) { + if ( !$this->getRequest()->getCheck( 'wpPreview' ) ) { + $out->wrapWikiMsg( + "
\n$1\n
", + [ 'anoneditwarning', + // Log-in link + SpecialPage::getTitleFor( 'Userlogin' )->getFullURL( [ + 'returnto' => $this->getTitle()->getPrefixedDBkey() + ] ), + // Sign-up link + SpecialPage::getTitleFor( 'CreateAccount' )->getFullURL( [ + 'returnto' => $this->getTitle()->getPrefixedDBkey() + ] ) + ] + ); + } else { + $out->wrapWikiMsg( "
\n$1
", + 'anonpreviewwarning' + ); + } + } + + parent::show(); + } + + protected function checkCanExecute( User $user ) { + parent::checkCanExecute( $user ); + + $this->undoafter = $this->getRequest()->getInt( 'undoafter' ); + $this->undo = $this->getRequest()->getInt( 'undo' ); + + if ( $this->undo == 0 || $this->undoafter == 0 ) { + throw new ErrorPageError( 'mcrundofailed', 'mcrundo-missingparam' ); + } + + $curRev = $this->page->getRevision(); + if ( !$curRev ) { + throw new ErrorPageError( 'mcrundofailed', 'nopagetext' ); + } + $this->curRev = $curRev->getRevisionRecord(); + $this->cur = $this->getRequest()->getInt( 'cur', $this->curRev->getId() ); + + $revisionLookup = MediaWikiServices::getInstance()->getRevisionLookup(); + + $undoRev = $revisionLookup->getRevisionById( $this->undo ); + $oldRev = $revisionLookup->getRevisionById( $this->undoafter ); + + if ( $undoRev === null || $oldRev === null || + $undoRev->isDeleted( RevisionRecord::DELETED_TEXT ) || + $oldRev->isDeleted( RevisionRecord::DELETED_TEXT ) + ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-norev' ); + } + + return true; + } + + /** + * @return MutableRevisionRecord + */ + private function getNewRevision() { + $revisionLookup = MediaWikiServices::getInstance()->getRevisionLookup(); + + $undoRev = $revisionLookup->getRevisionById( $this->undo ); + $oldRev = $revisionLookup->getRevisionById( $this->undoafter ); + $curRev = $this->curRev; + + $isLatest = $curRev->getId() === $undoRev->getId(); + + if ( $undoRev === null || $oldRev === null || + $undoRev->isDeleted( RevisionRecord::DELETED_TEXT ) || + $oldRev->isDeleted( RevisionRecord::DELETED_TEXT ) + ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-norev' ); + } + + if ( $isLatest ) { + // Short cut! Undoing the current revision means we just restore the old. + return MutableRevisionRecord::newFromParentRevision( $oldRev ); + } + + $newRev = MutableRevisionRecord::newFromParentRevision( $curRev ); + + // Figure out the roles that need merging by first collecting all roles + // and then removing the ones that don't. + $rolesToMerge = array_unique( array_merge( + $oldRev->getSlotRoles(), + $undoRev->getSlotRoles(), + $curRev->getSlotRoles() + ) ); + + // Any roles with the same content in $oldRev and $undoRev can be + // inherited because undo won't change them. + $rolesToMerge = array_intersect( + $rolesToMerge, $oldRev->getSlots()->getRolesWithDifferentContent( $undoRev->getSlots() ) + ); + if ( !$rolesToMerge ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-nochange' ); + } + + // Any roles with the same content in $oldRev and $curRev were already reverted + // and so can be inherited. + $rolesToMerge = array_intersect( + $rolesToMerge, $oldRev->getSlots()->getRolesWithDifferentContent( $curRev->getSlots() ) + ); + if ( !$rolesToMerge ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-nochange' ); + } + + // Any roles with the same content in $undoRev and $curRev weren't + // changed since and so can be reverted to $oldRev. + $diffRoles = array_intersect( + $rolesToMerge, $undoRev->getSlots()->getRolesWithDifferentContent( $curRev->getSlots() ) + ); + foreach ( array_diff( $rolesToMerge, $diffRoles ) as $role ) { + if ( $oldRev->hasSlot( $role ) ) { + $newRev->inheritSlot( $oldRev->getSlot( $role, RevisionRecord::RAW ) ); + } else { + $newRev->removeSlot( $role ); + } + } + $rolesToMerge = $diffRoles; + + // Any slot additions or removals not handled by the above checks can't be undone. + // There will be only one of the three revisions missing the slot: + // - !old means it was added in the undone revisions and modified after. + // Should it be removed entirely for the undo, or should the modified version be kept? + // - !undo means it was removed in the undone revisions and then readded with different content. + // Which content is should be kept, the old or the new? + // - !cur means it was changed in the undone revisions and then deleted after. + // Did someone delete vandalized content instead of undoing (meaning we should ideally restore + // it), or should it stay gone? + foreach ( $rolesToMerge as $role ) { + if ( !$oldRev->hasSlot( $role ) || !$undoRev->hasSlot( $role ) || !$curRev->hasSlot( $role ) ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-failure' ); + } + } + + // Try to merge anything that's left. + foreach ( $rolesToMerge as $role ) { + $oldContent = $oldRev->getSlot( $role, RevisionRecord::RAW )->getContent(); + $undoContent = $undoRev->getSlot( $role, RevisionRecord::RAW )->getContent(); + $curContent = $curRev->getSlot( $role, RevisionRecord::RAW )->getContent(); + $newContent = $undoContent->getContentHandler() + ->getUndoContent( $curContent, $undoContent, $oldContent, $isLatest ); + if ( !$newContent ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-failure' ); + } + $newRev->setSlot( SlotRecord::newUnsaved( $role, $newContent ) ); + } + + return $newRev; + } + + private function generateDiff() { + $newRev = $this->getNewRevision(); + if ( $newRev->hasSameContent( $this->curRev ) ) { + throw new ErrorPageError( 'mcrundofailed', 'undo-nochange' ); + } + + $diffEngine = new DifferenceEngine( $this->context ); + $diffEngine->setRevisions( $this->curRev, $newRev ); + + $oldtitle = $this->context->msg( 'currentrev' )->parse(); + $newtitle = $this->context->msg( 'yourtext' )->parse(); + + if ( $this->getRequest()->getCheck( 'wpPreview' ) ) { + $diffEngine->renderNewRevision(); + return ''; + } else { + $diffText = $diffEngine->getDiff( $oldtitle, $newtitle ); + $diffEngine->showDiffStyle(); + return '
' . $diffText . '
'; + } + } + + public function onSubmit( $data ) { + global $wgUseRCPatrol; + + if ( !$this->getRequest()->getCheck( 'wpSave' ) ) { + // Diff or preview + return false; + } + + $updater = $this->page->getPage()->newPageUpdater( $this->context->getUser() ); + $curRev = $updater->grabParentRevision(); + if ( !$curRev ) { + throw new ErrorPageError( 'mcrundofailed', 'nopagetext' ); + } + + if ( $this->cur !== $curRev->getId() ) { + return Status::newFatal( 'mcrundo-changed' ); + } + + $newRev = $this->getNewRevision(); + if ( !$newRev->hasSameContent( $curRev ) ) { + // Copy new slots into the PageUpdater, and remove any removed slots. + // TODO: This interface is awful, there should be a way to just pass $newRev. + // TODO: MCR: test this once we can store multiple slots + foreach ( $newRev->getSlots()->getSlots() as $slot ) { + $updater->setSlot( $slot ); + } + foreach ( $curRev->getSlotRoles() as $role ) { + if ( !$newRev->hasSlot( $role ) ) { + $updater->removeSlot( $role ); + } + } + + $updater->setOriginalRevisionId( false ); + $updater->setUndidRevisionId( $this->undo ); + + // TODO: Ugh. + if ( $wgUseRCPatrol && $this->getTitle()->userCan( 'autopatrol', $this->getUser() ) ) { + $updater->setRcPatrolStatus( RecentChange::PRC_AUTOPATROLLED ); + } + + $updater->saveRevision( + CommentStoreComment::newUnsavedComment( trim( $this->getRequest()->getVal( 'wpSummary' ) ) ), + EDIT_AUTOSUMMARY | EDIT_UPDATE + ); + + return $updater->getStatus(); + } + + return Status::newGood(); + } + + protected function usesOOUI() { + return true; + } + + protected function getFormFields() { + $request = $this->getRequest(); + $config = $this->context->getConfig(); + $oldCommentSchema = $config->get( 'CommentTableSchemaMigrationStage' ) === MIGRATION_OLD; + $ret = [ + 'diff' => [ + 'type' => 'info', + 'vertical-label' => true, + 'raw' => true, + 'default' => function () { + return $this->generateDiff(); + } + ], + 'summary' => [ + 'type' => 'text', + 'id' => 'wpSummary', + 'name' => 'wpSummary', + 'cssclass' => 'mw-summary', + 'label-message' => 'summary', + 'maxlength' => $oldCommentSchema ? 200 : CommentStore::COMMENT_CHARACTER_LIMIT, + 'value' => $request->getVal( 'wpSummary', '' ), + 'size' => 60, + 'spellcheck' => 'true', + ], + 'summarypreview' => [ + 'type' => 'info', + 'label-message' => 'summary-preview', + 'raw' => true, + ], + ]; + + if ( $request->getCheck( 'wpSummary' ) ) { + $ret['summarypreview']['default'] = Xml::tags( 'div', [ 'class' => 'mw-summary-preview' ], + Linker::commentBlock( trim( $request->getVal( 'wpSummary' ) ), $this->getTitle(), false ) + ); + } else { + unset( $ret['summarypreview'] ); + } + + return $ret; + } + + protected function alterForm( HTMLForm $form ) { + $form->setWrapperLegendMsg( 'confirm-mcrundo-title' ); + + $labelAsPublish = $this->context->getConfig()->get( 'EditSubmitButtonLabelPublish' ); + + $form->setSubmitName( 'wpSave' ); + $form->setSubmitTooltip( $labelAsPublish ? 'publish' : 'save' ); + $form->setSubmitTextMsg( $labelAsPublish ? 'publishchanges' : 'savechanges' ); + $form->showCancel( true ); + $form->setCancelTarget( $this->getTitle() ); + $form->addButton( [ + 'name' => 'wpPreview', + 'value' => '1', + 'label-message' => 'showpreview', + 'attribs' => Linker::tooltipAndAccesskeyAttribs( 'preview' ), + ] ); + $form->addButton( [ + 'name' => 'wpDiff', + 'value' => '1', + 'label-message' => 'showdiff', + 'attribs' => Linker::tooltipAndAccesskeyAttribs( 'diff' ), + ] ); + + $form->addHiddenField( 'undo', $this->undo ); + $form->addHiddenField( 'undoafter', $this->undoafter ); + $form->addHiddenField( 'cur', $this->curRev->getId() ); + } + + public function onSuccess() { + $this->getOutput()->redirect( $this->getTitle()->getFullURL() ); + } + + protected function preText() { + return '
'; + } +} diff --git a/includes/content/ContentHandler.php b/includes/content/ContentHandler.php index b3286a9a29..22fbe6feda 100644 --- a/includes/content/ContentHandler.php +++ b/includes/content/ContentHandler.php @@ -1,9 +1,4 @@ getContent(); + public function getUndoContent( $current, $undo, $undoafter, $undoIsLatest = false ) { + Assert::parameterType( Revision::class . '|' . Content::class, $current, '$current' ); + if ( $current instanceof Content ) { + Assert::parameter( $undo instanceof Content, '$undo', + 'Must be Content when $current is Content' ); + Assert::parameter( $undoafter instanceof Content, '$undoafter', + 'Must be Content when $current is Content' ); + $cur_content = $current; + $undo_content = $undo; + $undoafter_content = $undoafter; + } else { + Assert::parameter( $undo instanceof Revision, '$undo', + 'Must be Revision when $current is Revision' ); + Assert::parameter( $undoafter instanceof Revision, '$undoafter', + 'Must be Revision when $current is Revision' ); - if ( empty( $cur_content ) ) { - return false; // no page - } + $cur_content = $current->getContent(); - $undo_content = $undo->getContent(); - $undoafter_content = $undoafter->getContent(); + if ( empty( $cur_content ) ) { + return false; // no page + } + + $undo_content = $undo->getContent(); + $undoafter_content = $undoafter->getContent(); + + if ( !$undo_content || !$undoafter_content ) { + return false; // no content to undo + } - if ( !$undo_content || !$undoafter_content ) { - return false; // no content to undo + $undoIsLatest = $current->getId() === $undo->getId(); } try { $this->checkModelID( $cur_content->getModel() ); $this->checkModelID( $undo_content->getModel() ); - if ( $current->getId() !== $undo->getId() ) { + if ( !$undoIsLatest ) { // If we are undoing the most recent revision, // its ok to revert content model changes. However // if we are undoing a revision in the middle, then diff --git a/languages/i18n/en.json b/languages/i18n/en.json index 47747b6090..57b0cdedb2 100644 --- a/languages/i18n/en.json +++ b/languages/i18n/en.json @@ -3669,6 +3669,10 @@ "confirm-unwatch-top": "Remove this page from your watchlist?", "confirm-rollback-button": "OK", "confirm-rollback-top": "Revert edits to this page?", + "confirm-mcrundo-title": "Undo a change", + "mcrundofailed": "Undo failed", + "mcrundo-missingparam": "Missing required parameters on request.", + "mcrundo-changed": "The page has been changed since you viewed the diff. Please review the new change.", "semicolon-separator": "; ", "comma-separator": ", ", "colon-separator": ": ", diff --git a/languages/i18n/qqq.json b/languages/i18n/qqq.json index b6558b690d..6283aec1ff 100644 --- a/languages/i18n/qqq.json +++ b/languages/i18n/qqq.json @@ -3869,6 +3869,10 @@ "confirm-unwatch-top": "Used as confirmation message.", "confirm-rollback-button": "Used as Submit button text.\n{{Identical|OK}}", "confirm-rollback-top": "Used as confirmation message.", + "confirm-mcrundo-title": "Title for the editless undo form.", + "mcrundo-changed": "Message displayed when the page has been edited between the loading and submission of an editless undo.", + "mcrundo-missingparam": "Error displayed when parameters for action=mcrundo are missing", + "mcrundofailed": "Title of the error page when an editless undo fails.", "semicolon-separator": "{{optional}}", "comma-separator": "{{optional}}\n\nWarning: languages have different usages of punctuation, and sometimes they are swapped (e.g. openining and closing quotation marks, or full stop and colon in Armenian), or change their form (the full stop in Chinese and Japanese, the prefered \"colon\" in Armenian used in fact as the regular full stop, the comma in Arabic, Armenian, and Chinese...)\n\nTheir spacing (before or after) may also vary across languages (for example French requires a non-breaking space, preferably narrow if the browser supports NNBSP, on the inner side of some punctuations like quotation/question/exclamation marks, colon, and semicolons).", "colon-separator": "{{optional}}\nChange it only if your language uses another character for ':' or it needs an extra space before the colon.", diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 8cda4f9272..fe064f516a 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -687,6 +687,8 @@ CREATE TABLE /*_*/slots ( -- The revision ID of the revision that originated the slot's content. -- To find revisions that changed slots, look for slot_origin = slot_revision_id. + -- TODO: Is that actually true? Rollback seems to violate it by setting + -- slot_origin to an older rev_id. Undeletions could result in the same situation. slot_origin bigint unsigned NOT NULL, PRIMARY KEY ( slot_revision_id, slot_role_id ) -- 2.20.1