jenkins-bot has submitted this change and it was merged.
Change subject: Fix insert order issue with recent change listeners
......................................................................
Fix insert order issue with recent change listeners
These listeners previously had to be run after the workflow
had been inserted to ensure things work. Remove this fragility
and depend on the new commit metadata instead.
Fixes bug where new topics did not show up in recent changes because
their workflow did not yet exist.
Change-Id: I44dfbb7896464d9b34326fe452fa921633b873e6
---
M container.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
M includes/Data/HeaderRecentChanges.php
M includes/Data/PostRevisionRecentChanges.php
M includes/Data/PostSummaryRecentChanges.php
6 files changed, 39 insertions(+), 65 deletions(-)
Approvals:
Bsitu: Looks good to me, approved
jenkins-bot: Verified
diff --git a/container.php b/container.php
index fc446ad..33fd573 100644
--- a/container.php
+++ b/container.php
@@ -231,7 +231,6 @@
new Flow\Data\HeaderRecentChanges(
$c['flow_actions'],
$c['repository.username'],
- $c['storage'],
$wgContLang
),
$c['storage.board_history.index'],
@@ -291,7 +290,6 @@
new Flow\Data\PostSummaryRecentChanges(
$c['flow_actions'],
$c['repository.username'],
- $c['storage'],
$wgContLang
),
new Flow\Data\UserNameListener(
@@ -392,7 +390,11 @@
$handlers = array(
new Flow\Log\PostModerationLogger( $c['logger'] ),
- new Flow\Data\PostRevisionRecentChanges( $c['flow_actions'],
$c['repository.username'], $c['storage'], $c['repository.tree'], $wgContLang ),
+ new Flow\Data\PostRevisionRecentChanges(
+ $c['flow_actions'],
+ $c['repository.username'],
+ $wgContLang
+ ),
$c['storage.board_history.index'],
new Flow\Data\UserNameListener(
$c['repository.username'],
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index b8422a7..4071773 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -400,7 +400,17 @@
$metadata = array(
'workflow' => $this->workflow,
+ 'topic-title' => $this->loadTopicTitle(),
);
+ if ( !$metadata['topic-title'] ) {
+ // permissions failure, should never have
gotten this far
+ throw new PermissionException( 'Not Allowed',
'insufficient-permission' );
+ }
+ if ( $this->newRevision->getPostId()->equals(
$metadata['topic-title']->getPostId() ) ) {
+ // When performing actions against the
topic-title self::loadTopicTitle
+ // returns the previous revision.
+ $metadata['topic-title'] = $this->newRevision;
+ }
$this->storage->put( $this->newRevision, $metadata );
$this->storage->put( $this->workflow, $metadata );
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index d106514..3065589 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -144,6 +144,7 @@
$storage = $this->storage;
$metadata = array(
'workflow' => $this->topicWorkflow,
+ 'topic-title' => $this->topicPost,
);
$storage->put( $this->topicListEntry, $metadata );
diff --git a/includes/Data/HeaderRecentChanges.php
b/includes/Data/HeaderRecentChanges.php
index a926ae4..b42f279 100644
--- a/includes/Data/HeaderRecentChanges.php
+++ b/includes/Data/HeaderRecentChanges.php
@@ -2,6 +2,7 @@
namespace Flow\Data;
+use Flow\Exception\FlowException;
use Flow\FlowActions;
use Flow\Model\Header;
use Flow\Parsoid\Utils;
@@ -9,18 +10,12 @@
class HeaderRecentChanges extends RecentChanges {
/**
- * @var ManagerGroup
- */
- protected $storage;
-
- /**
* @var Language Content Language
*/
protected $contLang;
- public function __construct( FlowActions $actions, UserNameBatch
$usernames, ManagerGroup $storage, Language $contLang ) {
+ public function __construct( FlowActions $actions, UserNameBatch
$usernames, Language $contLang ) {
parent::__construct( $actions, $usernames );
- $this->storage = $storage;
$this->contLang = $contLang;
}
@@ -29,12 +24,8 @@
* @param string[] $row
*/
public function onAfterInsert( $object, array $row, array $metadata ) {
- $workflowId = $object->getWorkflowId();
- $workflow = $this->storage->get( 'Workflow', $workflowId );
- if ( !$workflow ) {
- // unless in unit test, write to log
- wfDebugLog( 'Flow', __METHOD__ . ": could not locate
workflow for header " . $object->getRevisionId()->getAlphadecimal() );
- return;
+ if ( !isset( $metadata['workflow'] ) ) {
+ throw new FlowException( 'Missing required metadata:
workflow' );
}
$this->insert(
@@ -42,7 +33,7 @@
'header',
'Header',
$row,
- $workflow,
+ $metadata['workflow'],
array(
'content' => Utils::htmlToPlaintext(
$object->getContent(),
diff --git a/includes/Data/PostRevisionRecentChanges.php
b/includes/Data/PostRevisionRecentChanges.php
index 5a78969..57a47f5 100644
--- a/includes/Data/PostRevisionRecentChanges.php
+++ b/includes/Data/PostRevisionRecentChanges.php
@@ -2,31 +2,19 @@
namespace Flow\Data;
+use Flow\Exception\FlowException;
use Flow\FlowActions;
use Flow\Model\PostRevision;
-use Flow\Repository\TreeRepository;
use Language;
class PostRevisionRecentChanges extends RecentChanges {
- /**
- * @var ManagerGroup
- */
- protected $storage;
-
- /**
- * @var TreeRepository
- */
- protected $tree;
-
/**
* @var Language
*/
protected $contLang;
- public function __construct( FlowActions $actions, UserNameBatch
$usernames, ManagerGroup $storage, TreeRepository $tree, Language $contLang ) {
+ public function __construct( FlowActions $actions, UserNameBatch
$usernames, Language $contLang ) {
parent::__construct( $actions, $usernames );
- $this->storage = $storage;
- $this->tree = $tree;
$this->contLang = $contLang;
}
@@ -35,36 +23,28 @@
* @param string[] $row
*/
public function onAfterInsert( $object, array $row, array $metadata ) {
- // The workflow id is the same as the root's post id
- $workflowId = $object->getRootPost()->getPostId();
- // These are likely already in the in-process cache
- $workflow = $this->storage->get( 'Workflow', $workflowId );
- if ( !$workflow ) {
- // unless in unit test, write to log
- wfDebugLog( 'Flow', __METHOD__ . ": could not locate
workflow " . $workflowId->getAlphadecimal() );
- return;
+ if ( !isset( $metadata['workflow'] ) ) {
+ throw new FlowException( 'Missing required metadata:
workflow' );
}
+ if ( !isset( $metadata['topic-title'] ) ) {
+ throw new FlowException( 'Missing required metadata:
topic-title' );
+ }
+
+ $topic = $this->contLang->truncate(
+ $metadata['topic-title']->getContent( 'wikitext' ),
+ self::TRUNCATE_LENGTH
+ );
$this->insert(
$object,
'topic',
'PostRevision',
$row,
- $workflow,
+ $metadata['workflow'],
array(
'post' =>
$object->getPostId()->getAlphadecimal(),
- 'topic' => $this->getTopicTitle( $object ),
+ 'topic' => $topic,
)
);
- }
-
- protected function getTopicTitle( PostRevision $rev ) {
- $content = $rev->getRootPost()->getContent( 'wikitext' );
- if ( is_object( $content ) ) {
- // moderated
- return null;
- }
-
- return $this->contLang->truncate( $content,
self::TRUNCATE_LENGTH );
}
}
diff --git a/includes/Data/PostSummaryRecentChanges.php
b/includes/Data/PostSummaryRecentChanges.php
index 973d5f0..9b07618 100644
--- a/includes/Data/PostSummaryRecentChanges.php
+++ b/includes/Data/PostSummaryRecentChanges.php
@@ -8,18 +8,12 @@
class PostSummaryRecentChanges extends RecentChanges {
/**
- * @var ManagerGroup
- */
- protected $storage;
-
- /**
* @var Language Content Language
*/
protected $contLang;
- public function __construct( FlowActions $actions, UserNameBatch
$usernames, ManagerGroup $storage, Language $contLang ) {
+ public function __construct( FlowActions $actions, UserNameBatch
$usernames, Language $contLang ) {
parent::__construct( $actions, $usernames );
- $this->storage = $storage;
$this->contLang = $contLang;
}
@@ -28,12 +22,8 @@
* @param string[] $row
*/
public function onAfterInsert( $object, array $row, array $metadata ) {
- $workflowId = $object->getCollection()->getWorkflowId();
- $workflow = $this->storage->get( 'Workflow', $workflowId );
- if ( !$workflow ) {
- // unless in unit test, write to log
- wfDebugLog( 'Flow', __METHOD__ . ": could not locate
workflow for post summary " . $object->getRevisionId()->getAlphadecimal() );
- return;
+ if ( !isset( $metadata['workflow'] ) ) {
+ throw new FlowException( 'Missing required metadata:
workflow' );
}
$this->insert(
@@ -41,7 +31,7 @@
'topicsummary',
'PostSummary',
$row,
- $workflow,
+ $metadata['workflow'],
array(
'content' => $this->contLang->truncate(
$object->getContent(), self::TRUNCATE_LENGTH ),
'rev_type_id' =>
$object->getCollectionId()->getAlphadecimal()
--
To view, visit https://gerrit.wikimedia.org/r/150256
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I44dfbb7896464d9b34326fe452fa921633b873e6
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: SG <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
