jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/405754 )
Change subject: Revert "Improve error-handling for OptInController" ...................................................................... Revert "Improve error-handling for OptInController" This reverts commit cbb86c9a06058af4b2d37e2c00bef0df69e3f41d. Change-Id: Ie2635b1f9cf32d04ffe4bd2437c605e191507414 --- M Hooks.php M container.php M extension.json M includes/Import/OptInController.php A includes/Import/OptInUpdate.php 5 files changed, 70 insertions(+), 111 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/Hooks.php b/Hooks.php index 052fe03..de887c1 100644 --- a/Hooks.php +++ b/Hooks.php @@ -8,7 +8,7 @@ use Flow\Exception\PermissionException; use Flow\Data\Listener\RecentChangesListener; use Flow\Formatter\CheckUserQuery; -use Flow\Import\OptInController; +use Flow\Import\OptInUpdate; use Flow\Model\UUID; use Flow\OccupationController; use Flow\SpamFilter\AbuseFilter; @@ -1754,20 +1754,20 @@ $after = $user->getBoolOption( BETA_FEATURE_FLOW_USER_TALK_PAGE ); $action = null; - $optInController = Flow\Container::get( 'controller.opt_in' ); if ( !$before && $after ) { - $action = OptInController::$ENABLE; + $action = OptInUpdate::$ENABLE; // Check if the user had a flow board - if ( !$optInController->hasFlowBoardArchive( $user ) ) { + $c = new Flow\Import\OptInController(); + if ( !$c->hasFlowBoardArchive( $user ) ) { // Enable the guided tour by setting the cookie RequestContext::getMain()->getRequest()->response()->setCookie( 'Flow_optIn_guidedTour', '1' ); } } elseif ( $before && !$after ) { - $action = OptInController::$DISABLE; + $action = OptInUpdate::$DISABLE; } if ( $action ) { - $optInController->initiateChange( $action, $user->getTalkPage(), $user ); + DeferredUpdates::addUpdate( new OptInUpdate( $action, $user->getTalkPage(), $user ) ); } return true; diff --git a/container.php b/container.php index ff79ba9..e7ab7df 100644 --- a/container.php +++ b/container.php @@ -781,22 +781,6 @@ // must always happen before calling flow code. $c['occupation_controller'] = FlowHooks::getOccupationController(); -$c['helper.archive_name'] = function ( $c ) { - return new Flow\Import\ArchiveNameHelper(); -}; - -$c['controller.opt_in'] = function ( $c ) { - return new Flow\Import\OptInController( - $c['occupation_controller'], - $c['controller.notification'], - $c['helper.archive_name'], - $c['db.factory'], - $c['default_logger'], - $c['occupation_controller']->getTalkpageManager() - - ); -}; - $c['controller.notification'] = function ( $c ) { global $wgContLang; return new Flow\NotificationController( $wgContLang, $c['repository.tree'] ); diff --git a/extension.json b/extension.json index e766273..ce0ab2d 100644 --- a/extension.json +++ b/extension.json @@ -1109,6 +1109,7 @@ "Flow\\Import\\LiquidThreadsApi\\ScriptedImportRevision": "includes/Import/LiquidThreadsApi/Objects.php", "Flow\\Import\\LiquidThreadsApi\\TopicIterator": "includes/Import/LiquidThreadsApi/Iterators.php", "Flow\\Import\\OptInController": "includes/Import/OptInController.php", + "Flow\\Import\\OptInUpdate": "includes/Import/OptInUpdate.php", "Flow\\Import\\PageImportState": "includes/Import/Importer.php", "Flow\\Import\\Plain\\ImportHeader": "includes/Import/Plain/ImportHeader.php", "Flow\\Import\\Plain\\ObjectRevision": "includes/Import/Plain/ObjectRevision.php", diff --git a/includes/Import/OptInController.php b/includes/Import/OptInController.php index 49d9e84..7fd1171 100644 --- a/includes/Import/OptInController.php +++ b/includes/Import/OptInController.php @@ -4,10 +4,7 @@ use DateTime; use DateTimeZone; -use DeferredUpdates; use DerivativeContext; -use Exception; -use Flow\DbFactory; use Flow\Collection\HeaderCollection; use Flow\Content\BoardContent; use Flow\Exception\InvalidDataException; @@ -17,9 +14,7 @@ use Flow\WorkflowLoader; use Flow\WorkflowLoaderFactory; use IContextSource; -use Psr\Log\LoggerInterface; use MovePage; -use MWExceptionHandler; use Parser; use ParserOptions; use RequestContext; @@ -34,8 +29,6 @@ * Entry point for enabling Flow on a page. */ class OptInController { - public static $ENABLE = 'enable'; - public static $DISABLE = 'disable'; /** * @var OccupationController @@ -53,16 +46,6 @@ private $archiveNameHelper; /** - * @var DbFactory - */ - private $dbFactory; - - /** - * @var LoggerInterface - */ - private $logger; - - /** * @var IContextSource */ private $context; @@ -72,80 +55,13 @@ */ private $user; - /** - * @param OccupationController $occupationController - * @param NotificationController $notificationController - * @param ArchiveNameHelper $archiveNameHelper - * @param DbFactory $dbFactory - * @param LoggerInterface $logger Logger for errors and exceptions - * @param User $scriptUser User that takes actions, such as creating the board or - * editing descriptions - */ - public function __construct( - OccupationController $occupationController, - NotificationController $notificationController, - ArchiveNameHelper $archiveNameHelper, - DbFactory $dbFactory, - LoggerInterface $logger, - User $scriptUser - ) { - $this->occupationController = $occupationController; - $this->notificationController = $notificationController; - $this->archiveNameHelper = $archiveNameHelper; - $this->dbFactory = $dbFactory; - $this->logger = $logger; - $this->user = $scriptUser; + public function __construct() { + $this->occupationController = Container::get( 'occupation_controller' ); + $this->notificationController = Container::get( 'controller.notification' ); + $this->archiveNameHelper = new ArchiveNameHelper(); + $this->user = $this->occupationController->getTalkpageManager(); $this->context = new DerivativeContext( RequestContext::getMain() ); $this->context->setUser( $this->user ); - } - - /** - * @param string $action Action to take, self::ENABLE or self::DISABLE - * @param Title $talkpage Title of user's talk page - * @param User $user User that owns the talk page - */ - public function initiateChange( $action, Title $talkpage, User $user ) { - $flowDbw = $this->dbFactory->getDB( DB_MASTER ); - $wikiDbw = $this->dbFactory->getWikiDB( DB_MASTER ); - - $outerMethod = __METHOD__; - $logger = $this->logger; - - // We need both since we use both databases. - DeferredUpdates::addCallableUpdate( - function () use ( $logger, $outerMethod, $wikiDbw, $action, $talkpage, $user ) { - DeferredUpdates::addCallableUpdate( - function () use ( $logger, $outerMethod, $action, $talkpage, $user ) { - try { - if ( $action === self::$ENABLE ) { - $this->enable( $talkpage, $user ); - } elseif ( $action === self::$DISABLE ) { - $this->disable( $talkpage ); - } else { - $logger->error( $outerMethod . ': unrecognized action: ' . $action ); - } - } catch ( Exception $e ) { - $logger->error( - $outerMethod . ' failed to {action} Flow on \'{talkpage}\' for user \'{user}\'. Exception: {exception}', - [ - 'action' => $action, - 'talkpage' => $talkpage, - 'user' => $user, - 'exception' => $e, - ] - ); - // rollback both Flow and Core DBs - MWExceptionHandler::rollbackMasterChangesAndLog( $e ); - $this->dbFactory->getDB( DB_MASTER )->rollback( $outerMethod ); - } - }, - $outerMethod, - $wikiDbw - ); - }, - __METHOD__, - $flowDbw - ); } /** diff --git a/includes/Import/OptInUpdate.php b/includes/Import/OptInUpdate.php new file mode 100644 index 0000000..3c1a0f7 --- /dev/null +++ b/includes/Import/OptInUpdate.php @@ -0,0 +1,58 @@ +<?php + +namespace Flow\Import; + +use DeferrableUpdate; +use MWExceptionHandler; +use Title; +use User; + +class OptInUpdate implements DeferrableUpdate { + + public static $ENABLE = 'enable'; + public static $DISABLE = 'disable'; + + /** + * @var string + */ + protected $action; + + /** + * @var Title + */ + protected $talkpage; + + /** + * @var User + */ + protected $user; + + /** + * @param string $action + * @param Title $talkpage + * @param User $user + */ + public function __construct( $action, Title $talkpage, User $user ) { + $this->action = $action; + $this->talkpage = $talkpage; + $this->user = $user; + } + + /** + * Enable or disable Flow on a talk page + */ + function doUpdate() { + $c = new OptInController(); + try { + if ( $this->action === self::$ENABLE ) { + $c->enable( $this->talkpage, $this->user ); + } elseif ( $this->action === self::$DISABLE ) { + $c->disable( $this->talkpage ); + } else { + wfLogWarning( 'OptInUpdate: unrecognized action: ' . $this->action ); + } + } catch ( \Exception $e ) { + MWExceptionHandler::logException( $e ); + } + } +} -- To view, visit https://gerrit.wikimedia.org/r/405754 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie2635b1f9cf32d04ffe4bd2437c605e191507414 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Flow Gerrit-Branch: wmf/1.31.0-wmf.17 Gerrit-Owner: Catrope <r...@wikimedia.org> Gerrit-Reviewer: Catrope <r...@wikimedia.org> Gerrit-Reviewer: Mattflaschen <mflasc...@wikimedia.org> Gerrit-Reviewer: Sbisson <sbis...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits