Hello Mattflaschen, Sbisson, jenkins-bot,
I'd like you to do a code review. Please visit
https://gerrit.wikimedia.org/r/405754
to review the following change.
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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/54/405754/1
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: newchange
Gerrit-Change-Id: Ie2635b1f9cf32d04ffe4bd2437c605e191507414
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: wmf/1.31.0-wmf.17
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Mattflaschen <[email protected]>
Gerrit-Reviewer: Sbisson <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
