EBernhardson has uploaded a new change for review. https://gerrit.wikimedia.org/r/201596
Change subject: Allow moving flow boards ...................................................................... Allow moving flow boards * move added back to board menu, still removed from topic pages * moves all related topics to the new location as well * performs null edit to header with flow talkpage manager * Requires flow-create-board to move to a place that was not a flow board previously Bug: T90063 Change-Id: I4bdc665e21b16e4048c3c78c7083add803491dc7 --- M Flow.php M Hooks.php M autoload.php M container.php M i18n/en.json M i18n/qqq.json A includes/BoardMover.php M includes/Model/Workflow.php M includes/WorkflowLoaderFactory.php 9 files changed, 233 insertions(+), 6 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow refs/changes/96/201596/1 diff --git a/Flow.php b/Flow.php index e66bd23..487dd06 100644 --- a/Flow.php +++ b/Flow.php @@ -132,6 +132,8 @@ $wgHooks['CanonicalNamespaces'][] = 'FlowHooks::onCanonicalNamespaces'; $wgHooks['MovePageIsValidMove'][] = 'FlowHooks::onMovePageIsValidMove'; $wgHooks['AbortMove'][] = 'FlowHooks::onAbortMove'; +$wgHooks['TitleMove'][] = 'FlowHooks::onTitleMove'; +$wgHooks['TitleMoveComplete'][] = 'FlowHooks::onTitleMoveComplete'; $wgHooks['TitleSquidURLs'][] = 'FlowHooks::onTitleSquidURLs'; $wgHooks['WatchlistEditorBuildRemoveLine'][] = 'FlowHooks::onWatchlistEditorBuildRemoveLine'; $wgHooks['WatchlistEditorBeforeFormRender'][] = 'FlowHooks::onWatchlistEditorBeforeFormRender'; @@ -343,7 +345,7 @@ $wgFlowAbuseFilterEmergencyDisableAge = 86400; // One day. // Actions that must pass through to MediaWiki on flow enabled pages -$wgFlowCoreActionWhitelist = array( 'info', 'protect', 'unprotect', 'unwatch', 'watch', 'history', 'wikilove' ); +$wgFlowCoreActionWhitelist = array( 'info', 'protect', 'unprotect', 'unwatch', 'watch', 'history', 'wikilove', 'move' ); // When set to true Flow will compile templates into their intermediate forms // on every run. When set to false Flow will use the versions already written diff --git a/Hooks.php b/Hooks.php index 8151b58..1b0cf4f 100644 --- a/Hooks.php +++ b/Hooks.php @@ -1041,8 +1041,28 @@ } public static function onMovePageIsValidMove( Title $oldTitle, Title $newTitle, Status $status ) { - if ( self::$occupationController->isTalkpageOccupied( $oldTitle ) ) { + global $wgFlowOccupyNamespaces, $wgUser; + + // We only care about moving flow boards + if ( $oldTitle->getContentModel() !== CONTENT_MODEL_FLOW_BOARD ) { + return true; + } + + // pages within the Topic namespace are not movable + if ( $oldTitle->getNamespace() === NS_TOPIC ) { $status->fatal( 'flow-error-move' ); + return false; + } + + $occupationController = Container::get( 'occupation_controller' ); + if ( + // If the destination is not already a valid flow location + !$occupationController->isTalkpageOccupied( $newTitle, false ) + && + // and the user is not allowed to create new flow boards + !$wgUser->isAllowed( 'flow-create-board' ) + ) { + $status->fatal( 'flow-error-move-no-create-permissions' ); return false; } @@ -1328,4 +1348,29 @@ return true; } + + /** + * Occurs at the begining of the MovePage process. Perhaps ContentModel should be + * extended to be notified about moves explicitly. + */ + public static function onTitleMove( Title $oldTitle, Title $newTitle, User $user ) { + if ( $oldTitle->getContentModel() === CONTENT_MODEL_FLOW_BOARD ) { + // complete hack to make sure that when the page is saved to new + // location and rendered it doesn't throw an error about the wrong title + Container::get( 'factory.loader.workflow' )->pageMoveInProgress(); + // open a database transaction and prepare everything for the move, but + // don't commit yet. That is done below in self::onTitleMoveComplete + Container::get( 'board_mover' )->prepareMove( $oldTitle, $newTitle ); + } + + return true; + } + + public static function onTitleMoveComplete( Title $oldTitle, Title $newTitle, User $user, $pageid, $redirid, $reason ) { + if ( $newTitle->getContentModel() === CONTENT_MODEL_FLOW_BOARD ) { + Container::get( 'board_mover' )->commit(); + } + + return true; + } } diff --git a/autoload.php b/autoload.php index 1be2835..938a479 100644 --- a/autoload.php +++ b/autoload.php @@ -40,6 +40,7 @@ 'Flow\\Block\\TopicBlock' => __DIR__ . '/includes/Block/Topic.php', 'Flow\\Block\\TopicListBlock' => __DIR__ . '/includes/Block/TopicList.php', 'Flow\\Block\\TopicSummaryBlock' => __DIR__ . '/includes/Block/TopicSummary.php', + 'Flow\\BoardMover' => __DIR__ . '/includes/BoardMover.php', 'Flow\\Collection\\AbstractCollection' => __DIR__ . '/includes/Collection/AbstractCollection.php', 'Flow\\Collection\\CollectionCache' => __DIR__ . '/includes/Collection/CollectionCache.php', 'Flow\\Collection\\HeaderCollection' => __DIR__ . '/includes/Collection/HeaderCollection.php', diff --git a/container.php b/container.php index 7af6cae..199f82d 100644 --- a/container.php +++ b/container.php @@ -1199,4 +1199,13 @@ ); }; +$c['board_mover'] = function( $c ) { + return new Flow\BoardMover( + $c['db.factory'], + $c['memcache.buffered'], + $c['storage'], + $c['occupation_controller']->getTalkpageManager() + ); +}; + return $c; diff --git a/i18n/en.json b/i18n/en.json index 1584875..f37e112 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -175,7 +175,8 @@ "flow-error-no-commit": "The specified action could not be saved.", "flow-error-fetch-after-lock": "An error was encountered when requesting the new data. The lock/unlock operation succeeded just fine, though. The error message was: $1", "flow-error-content-too-long": "The content is too large. Content after expansion is limited to $1 {{PLURAL:$1|byte|bytes}}.", - "flow-error-move": "Moving a discussion board is currently not supported.", + "flow-error-move": "Moving a topic page is currently not supported.", + "flow-error-move-no-create-permissions": "The `flow-create-board` permission is required to move a flow board.", "flow-error-invalid-topic-uuid-title": "Bad title", "flow-error-invalid-topic-uuid": "The requested page title was invalid. Pages in the Topic namespace are automatically created by Flow.", "flow-error-unknown-workflow-id-title": "Unknown topic", diff --git a/i18n/qqq.json b/i18n/qqq.json index 4006924..093ccef 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -178,7 +178,8 @@ "flow-error-no-commit": "Error message when nothing was able to commit the request (data was submitted but it could not be processed).", "flow-error-fetch-after-lock": "Error message to be displayed when failing to request the new data after successfully performing lock/unlock topic. This is meant to indicate to the user that some error was encountered, but that the lock/unlock actually succeeded just fine - we just failed to get the new data to display the new status. Parameters:\n* $1 - The error message received.", "flow-error-content-too-long": "Error message when the expanded(html) output of a post is too large.\n\nParameters:\n* $1 - post content lengh limit in byte, could be used for plural support.", - "flow-error-move": "Error message when attempting to move a flow board (which is not yet supported)", + "flow-error-move": "Error message when attempting to move a flow topic", + "flow-error-move-no-create-permissions": "Error message when attempting to move a flow board to a page that the current user is not allowed to.", "flow-error-invalid-topic-uuid-title": "Title displayed at top of page and in browser title bar when the user requests a page within the Topic namespace that is not a valid UUID", "flow-error-invalid-topic-uuid": "Body of page displayed when the user requests a page within the Topic namespace that is not a valid UUID", "flow-error-unknown-workflow-id-title": "Title displayed at top of page and in browser title bar when the user requests a page within the Topic namespace that is not a known topic", diff --git a/includes/BoardMover.php b/includes/BoardMover.php new file mode 100644 index 0000000..4430019 --- /dev/null +++ b/includes/BoardMover.php @@ -0,0 +1,135 @@ +<?php + +namespace Flow; + +use DatabaseBase; +use Flow\Data\BufferedCache; +use Flow\Data\ManagerGroup; +use Flow\Exception\FlowException; +use Flow\Model\Header; +use Flow\Model\Workflow; +use Title; +use User; + +/** + * + */ +class BoardMover { + /** + * @var DbFactory + */ + protected $dbFactory; + + /** + * @var ManagerGroup + */ + protected $storage; + + /** + * @var User + */ + protected $nullEditUser; + + /** + * @var DatabaseBase|null + */ + protected $dbw; + + public function __construct( DbFactory $dbFactory, BufferedCache $cache, ManagerGroup $storage, User $nullEditUser ) { + $this->dbFactory = $dbFactory; + $this->cache = $cache; + $this->storage = $storage; + $this->nullEditUser = $nullEditUser; + } + + /** + * Collects the workflow and header (if it exists) and puts them into the database. Does + * not commit yet. It is intended for prepareMove to be called from the TitleMove hook, + * and commited from TitleMoveComplete hook. This ensures that if some error prevents the + * core transaction from commiting this transaction is also not commited. + */ + public function prepareMove( Title $oldTitle, Title $newTitle ) { + if ( $this->dbw !== null ) { + throw new FlowException( "Already prepared for move from {$oldTitle} to {$newTitle}" ); + } + if ( $oldTitle->getContentModel() !== CONTENT_MODEL_FLOW_BOARD ) { + throw new FlowException( "$oldTitle is not a flow board" ); + } + // @todo someday NS_TOPIC should be made CONTENT_MODEL_FLOW_TOPIC instead of approximating + // like this. + if ( $oldTitle->getNamespace() === NS_TOPIC ) { + throw new FlowException( "$oldTitle is a topic, not a board" ); + } + + // All reads must go through master to help ensure consistency + $this->dbFactory->forceMaster(); + + // Open a transaction, this will be closed or rolled back from self::commit or self::rollback + $this->dbw = $this->dbFactory->getDB( DB_MASTER ); + $this->dbw->begin(); + $this->cache->begin(); + + // @todo this loads every topic workflow this board has ever seen, + // would prefer to update db directly but that won't work due to + // the caching layer not getting updated. After dropping Flow\Data\Index\* + // revisit this. + $found = $this->storage->find( 'Workflow', array( + 'workflow_wiki' => wfWikiId(), + 'workflow_namespace' => $oldTitle->getNamespace(), + 'workflow_title_text' => $oldTitle->getDBkey() + ) ); + if ( !$found ) { + throw new FlowException( "Could not locate workflow for $oldTitle" ); + } + + $discussionWorkflow = null; + foreach ( $found as $workflow ) { + if ( $workflow->getType() === 'discussion' ) { + $discussionWorkflow = $workflow; + } + $workflow->updateFromTitle( $oldTitle, $newTitle ); + $this->storage->put( $workflow, array() ); + } + if ( $discussionWorkflow === null ) { + throw new FlowException( "Main discussion workflow for $oldTitle not found" ); + } + + $found = $this->storage->find( + 'Header', + array( 'rev_type_id' => $discussionWorkflow->getId() ), + array( 'sort' => 'rev_id', 'order' => 'DESC', 'limit' => 1 ) + ); + + if ( $found ) { + $this->header = reset( $found ); + $nextHeader = $this->header->newNullRevision( + $this->nullEditUser, + $this->header->getContentRaw(), + $this->header->getContentFormat(), + 'edit-header', + $newTitle + ); + $this->storage->put( $nextHeader, array( + 'workflow' => $discussionWorkflow, + ) ); + } + } + + /** + * @throws Exception\FlowException + */ + public function commit() { + if ( $this->dbw === null ) { + throw new FlowException( 'Board move not prepared.'); + } + + try { + $this->dbw->commit(); + $this->cache->commit(); + } catch ( \Exception $e ) { + $this->dbw->rollback(); + $this->cache->rollback(); + throw $e; + } + } +} diff --git a/includes/Model/Workflow.php b/includes/Model/Workflow.php index bb09926..6a66ba8 100644 --- a/includes/Model/Workflow.php +++ b/includes/Model/Workflow.php @@ -88,7 +88,7 @@ $obj->isNew = false; $obj->type = $row['workflow_type']; $obj->wiki = $row['workflow_wiki']; - $obj->pageId = $row['workflow_page_id']; + $obj->pageId = (int)$row['workflow_page_id']; $obj->namespace = (int) $row['workflow_namespace']; $obj->titleText = $row['workflow_title_text']; $obj->lastModified = $row['workflow_last_update_timestamp']; @@ -146,6 +146,30 @@ } /** + * Update the workflow after a page move. + * + * @param Title $oldTitle The title the workflow is currently located at + * @param Title $newTitle The title the workflow is moving to. During the page move + * process this has not yet been written out to DB and does not have an articleId() + * @throws DataModelException + */ + public function updateFromTitle( Title $oldTitle, Title $newTitle ) { + // temporary hack. @todo write maintenance script to fix all of these + if ( $this->pageId === 0 ) { + if ( $this->getOwnerTitle()->equals( $oldTitle ) ) { + $this->pageId = $oldTitle->getArticleID(); + } else { + throw new DataModelException( "The provided oldTitle ({$oldTitle}) does not match this workflow." ); + } + } elseif ( $oldTitle->getArticleID() !== $this->pageId ) { + throw new DataModelException( 'Must update from title with same page id. ' . $this->pageId . ' !== ' . $title->getArticleID() ); + } + + $this->namespace = $newTitle->getNamespace(); + $this->titleText = $newTitle->getDBkey(); + } + + /** * Return the title this workflow responds at * * @return Title diff --git a/includes/WorkflowLoaderFactory.php b/includes/WorkflowLoaderFactory.php index 32598f9..d335f10 100644 --- a/includes/WorkflowLoaderFactory.php +++ b/includes/WorkflowLoaderFactory.php @@ -34,6 +34,11 @@ protected $defaultWorkflowName; /** + * @var bool + */ + protected $pageMoveInProgress = false; + + /** * @param ManagerGroup $storage * @param BlockFactory $blockFactory * @param SubmissionHandler $submissionHandler @@ -49,6 +54,10 @@ $this->blockFactory = $blockFactory; $this->submissionHandler = $submissionHandler; $this->defaultWorkflowName = $defaultWorkflowName; + } + + public function pageMoveInProgress() { + $this->pageMoveInProgress = true; } /** @@ -118,7 +127,7 @@ if ( !$workflow ) { throw new UnknownWorkflowIdException( 'Invalid workflow requested by id', 'invalid-input' ); } - if ( $title !== false && !$workflow->matchesTitle( $title ) ) { + if ( $title !== false && $this->pageMoveInProgress === false && !$workflow->matchesTitle( $title ) ) { throw new InvalidInputException( 'Flow workflow is for different page', 'invalid-input' ); } -- To view, visit https://gerrit.wikimedia.org/r/201596 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4bdc665e21b16e4048c3c78c7083add803491dc7 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: master Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits