EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/150083
Change subject: Newly created topics display as unwatched
......................................................................
Newly created topics display as unwatched
When submitting a new topic it would occasionally not display that
you were watching the title. This was due to trying to read our own
writes, it works on a dev box but as soon as you get a slave db in there
we get intermittent errors.
Solve this by adding a new listener. This new listener waits for PostRevision
instances to be created with appropriate change types and subscribes the user
to that change type. It additionally informs the WatchedtopicItems object
that it should override this title. The combination of the two allows the
TopicListQuery to appropriatly mark the title as watched without getting
a slave db involved.
This is also a little cleaner in terms of how the subscription gets triggered,
its centralized now rather than spread through the various Block instances.
Change-Id: I6e188544477156655ef5457b587fb20c06cd74ba
---
M Flow.php
M container.php
M includes/Block/Topic.php
M includes/Block/TopicList.php
A includes/Data/WatchTopicListener.php
M includes/Notifications/Controller.php
M includes/WatchedTopicItems.php
7 files changed, 92 insertions(+), 28 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/83/150083/1
diff --git a/Flow.php b/Flow.php
index c396f06..815ddd5 100755
--- a/Flow.php
+++ b/Flow.php
@@ -163,6 +163,7 @@
$wgAutoloadClasses['Flow\Data\Merger'] = $dir . 'includes/Data/Merger.php';
$wgAutoloadClasses['Flow\Data\RawSql'] = $dir . 'includes/Data/RawSql.php';
$wgAutoloadClasses['Flow\Data\UrlGenerationListener'] = $dir .
'includes/Data/UrlGenerationListener.php';
+$wgAutoloadClasses['Flow\Data\WatchTopicListener'] = $dir .
'includes/Data/WatchTopicListener.php';
$wgAutoloadClasses['Flow\Log\Logger'] = $dir . 'includes/Log/Logger.php';
$wgAutoloadClasses['Flow\Log\Formatter'] = $dir . 'includes/Log/Formatter.php';
$wgAutoloadClasses['Flow\Log\PostModerationLogger'] = $dir .
'includes/Log/PostModerationLogger.php';
diff --git a/container.php b/container.php
index 0e3c672..85d36e9 100644
--- a/container.php
+++ b/container.php
@@ -408,6 +408,10 @@
// using TreeRepository for extra information and stuffing it
into topic_root while indexing
$c['storage.topic_history.index'],
$c['reference.recorder'],
+ new Flow\Data\WatchTopicListener( $c['user'],
$c['watched_items'], array(
+ // list of revision types that trigger watching the
workflow
+ 'new-topic', 'reply', 'edit', 'edit-title'
+ ) ),
);
} );
$c['storage.post.mapper'] = $c->share( function( $c ) {
diff --git a/includes/Block/Topic.php b/includes/Block/Topic.php
index cf52b44..b8422a7 100644
--- a/includes/Block/Topic.php
+++ b/includes/Block/Topic.php
@@ -208,7 +208,6 @@
}
$this->setNotification( 'flow-topic-renamed' );
- $this->notificationController->subscribeToWorkflow(
$this->user, $this->workflow );
}
protected function validateReply() {
@@ -235,7 +234,6 @@
}
$this->setNotification( 'flow-post-reply', array( 'reply-to' =>
$post ) );
- $this->notificationController->subscribeToWorkflow(
$this->user, $this->workflow );
}
protected function validateModerateTopic() {
@@ -383,7 +381,6 @@
}
$this->setNotification( 'flow-post-edited' );
- $this->notificationController->subscribeToWorkflow(
$this->user, $this->workflow );
}
public function commit() {
diff --git a/includes/Block/TopicList.php b/includes/Block/TopicList.php
index d503f34..f517d16 100644
--- a/includes/Block/TopicList.php
+++ b/includes/Block/TopicList.php
@@ -156,7 +156,6 @@
// stuff needs to be in cache at least.
$storage->put( $this->topicWorkflow, $metadata );
- $this->notificationController->subscribeToWorkflow(
$this->user, $this->topicWorkflow );
$this->notificationController->notifyNewTopic( array(
'board-workflow' => $this->workflow,
'topic-workflow' => $this->topicWorkflow,
diff --git a/includes/Data/WatchTopicListener.php
b/includes/Data/WatchTopicListener.php
new file mode 100644
index 0000000..405742a
--- /dev/null
+++ b/includes/Data/WatchTopicListener.php
@@ -0,0 +1,70 @@
+<?php
+
+namespace Flow\Data;
+
+use Flow\Model\PostRevision;
+use Flow\WatchedTopicItems;
+use User;
+use WatchedItem;
+
+class WatchTopicListener {
+
+ /**
+ * @var User
+ */
+ protected $user;
+
+ /**
+ * @var WatchedTopicItems
+ */
+ protected $watchedTopicItems;
+
+ /**
+ * @var array List of revision change types to trigger watchlist on
+ */
+ protected $changeTypes;
+
+ /**
+ * @param User $user The user that will be subscribed to workflows
+ * @param WatchedTopicItems $watchedTopicItems Helper class for
watching titles
+ * @param array $changeTypes List of revision change types to trigger
watch on
+ */
+ public function __construct( User $user, WatchedTopicItems
$watchedTopicItems, array $changeTypes ) {
+ $this->user = $user;
+ $this->watchedTopicItems = $watchedTopicItems;
+ $this->changeTypes = $changeTypes;
+ }
+
+ public function onAfterInsert( $object, array $row, array $metadata ) {
+ if ( !$object instanceof PostRevision ) {
+ // @todo should this be an exception?
+ return;
+ }
+ if ( !in_array( $row['rev_change_type'], $this->changeTypes ) )
{
+ return;
+ }
+ if ( !isset( $metadata['workflow'] ) ) {
+ throw new FlowException( 'Missing required metadata:
workflow' );
+ }
+ if ( $metadata['workflow']->getType() !== 'topic' ) {
+ throw new FlowException( 'Expected `topic` workflow but
received `' . $metadata['workflow']->getType() . '`' );
+ }
+
+
+ $title = $metadata['workflow']->getArticleTitle();
+ if ( $title ) {
+ WatchedItem::fromUserTitle( $this->user, $title )
+ ->addWatch();
+ $this->watchedTopicItems->addOverrideWatched( $title );
+ }
+
+ if ( !isset( $metadata['title'] ) ) {
+ throw new FlowException( 'Missing metadata: title' );
+ }
+ }
+
+ // do nothing
+ public function onAfterLoad( $object, array $new ) {}
+ public function onAfterUpdate( $object, array $old, array $new, array
$metadata ) {}
+ public function onAfterRemove( $object, array $old, array $metadata ) {}
+}
diff --git a/includes/Notifications/Controller.php
b/includes/Notifications/Controller.php
index 5cde0c8..58978a0 100644
--- a/includes/Notifications/Controller.php
+++ b/includes/Notifications/Controller.php
@@ -556,27 +556,4 @@
}
return $talkUser;
}
-
- /**
- * @todo - This is not a good place to put auto-subscription, but I am
not sure
- * yet where the best place to put this
- */
- public function subscribeToWorkflow( User $user, Workflow $workflow ) {
- // Only topic is subscribable for now
- if ( $user->isAnon() || $workflow->getType() !== 'topic' ) {
- return;
- }
- $title = $workflow->getArticleTitle();
- // There is really nothing to do if UUID is reported as invalid
title text,
- // maybe just log it
- if ( !$title ) {
- return;
- }
- $watchedItem = WatchedItem::fromUserTitle(
- $user,
- $title
- );
- $watchedItem->addWatch();
- }
-
}
diff --git a/includes/WatchedTopicItems.php b/includes/WatchedTopicItems.php
index 3a27a38..4fd9543 100644
--- a/includes/WatchedTopicItems.php
+++ b/includes/WatchedTopicItems.php
@@ -14,10 +14,20 @@
protected $user;
protected $watchListDb;
+ protected $overrides = array();
public function __construct( User $user, DatabaseBase $watchListDb ) {
$this->user = $user;
$this->watchListDb = $watchListDb;
+ }
+
+ /**
+ * Helps prevent reading our own writes. If we have explicitly
+ * watched this title in this request set it here instead of
+ * querying a slave and possibly not noticing due to slave lag.
+ */
+ public function addOverrideWatched( Title $title ) {
+ $this->overrides[$title->getNamespace()][$title->getDBkey()] =
true;
}
/**
@@ -34,7 +44,13 @@
foreach ( $titles as $key => $id ) {
$obj = Title::makeTitleSafe( NS_TOPIC, $id );
if ( $obj ) {
- $titles[$key] = $obj->getDBkey();
+ $key = $obj->getDBkey();
+ if ( isset(
$this->overrides[$obj->getNamespace()][$key] ) ) {
+ $result[$key] = true;
+ unset( $titles[$key] );
+ } else {
+ $titles[$key] = $obj->getDBkey();
+ }
} else {
unset( $titles[$key] );
}
--
To view, visit https://gerrit.wikimedia.org/r/150083
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e188544477156655ef5457b587fb20c06cd74ba
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
