Automatic topic subscriptions (on any edit)

Bug: T284836
Change-Id: Ia42ad087218fd91a0cdd1664157d1049738e3c01
This commit is contained in:
Bartosz Dziewoński 2021-08-17 22:23:27 +02:00
parent 9fbf2b3177
commit 0d57aa9762
8 changed files with 126 additions and 17 deletions

View file

@ -497,6 +497,7 @@
"discussiontools-replytool": 1, "discussiontools-replytool": 1,
"discussiontools-sourcemodetoolbar": 1, "discussiontools-sourcemodetoolbar": 1,
"discussiontools-topicsubscription": 1, "discussiontools-topicsubscription": 1,
"discussiontools-autotopicsub": 1,
"discussiontools-abtest": "" "discussiontools-abtest": ""
}, },
"config": { "config": {
@ -532,6 +533,10 @@
"value": "default", "value": "default",
"description": "Override availability of DiscussionTools topic subscription feature. 'default', 'available', or 'unavailable'." "description": "Override availability of DiscussionTools topic subscription feature. 'default', 'available', or 'unavailable'."
}, },
"DiscussionTools_autotopicsub": {
"value": "default",
"description": "Override availability of DiscussionTools automatic topic subscription feature. 'default', 'available', or 'unavailable'."
},
"DiscussionToolsEnableTopicSubscriptionBackend": { "DiscussionToolsEnableTopicSubscriptionBackend": {
"value": true, "value": true,
"description": "Enable the topic subscription backend. This controls whether the feature can be tried out using the query parameter, and whether the Echo events will be created. Do not enable this unless the database tables exist. Do enable it before making the 'topicsubscription' feature available." "description": "Enable the topic subscription backend. This controls whether the feature can be tried out using the query parameter, and whether the Echo events will be created. Do not enable this unless the database tables exist. Do enable it before making the 'topicsubscription' feature available."

View file

@ -23,6 +23,8 @@ class HookUtils {
public const NEWTOPICTOOL = 'newtopictool'; public const NEWTOPICTOOL = 'newtopictool';
public const SOURCEMODETOOLBAR = 'sourcemodetoolbar'; public const SOURCEMODETOOLBAR = 'sourcemodetoolbar';
public const TOPICSUBSCRIPTION = 'topicsubscription'; public const TOPICSUBSCRIPTION = 'topicsubscription';
public const AUTOTOPICSUB = 'autotopicsub';
/** /**
* @var string[] List of all sub-features. Will be used to generate: * @var string[] List of all sub-features. Will be used to generate:
* - Feature override global: $wgDiscussionTools_FEATURE * - Feature override global: $wgDiscussionTools_FEATURE
@ -34,6 +36,7 @@ class HookUtils {
self::NEWTOPICTOOL, self::NEWTOPICTOOL,
self::SOURCEMODETOOLBAR, self::SOURCEMODETOOLBAR,
self::TOPICSUBSCRIPTION, self::TOPICSUBSCRIPTION,
self::AUTOTOPICSUB,
]; ];
/** /**
@ -52,7 +55,7 @@ class HookUtils {
return false; return false;
} }
if ( $feature === self::TOPICSUBSCRIPTION && !$user->isRegistered() ) { if ( ( $feature === self::TOPICSUBSCRIPTION || $feature === self::AUTOTOPICSUB ) && !$user->isRegistered() ) {
// Users must be logged in to use topic subscription // Users must be logged in to use topic subscription
return false; return false;
} }
@ -229,7 +232,10 @@ class HookUtils {
// Topic subscription is not available on your own talk page, as you will // Topic subscription is not available on your own talk page, as you will
// get 'edit-user-talk' notifications already. (T276996) // get 'edit-user-talk' notifications already. (T276996)
if ( $feature === self::TOPICSUBSCRIPTION && $title->equals( $output->getUser()->getTalkPage() ) ) { if (
( $feature === self::TOPICSUBSCRIPTION || $feature === self::AUTOTOPICSUB ) &&
$title->equals( $output->getUser()->getTalkPage() )
) {
return false; return false;
} }
@ -250,7 +256,7 @@ class HookUtils {
RequestContext::getMain()->getRequest()->getRawVal( 'dtenable' ); RequestContext::getMain()->getRequest()->getRawVal( 'dtenable' );
if ( if (
$feature === self::TOPICSUBSCRIPTION && ( $feature === self::TOPICSUBSCRIPTION || $feature === self::AUTOTOPICSUB ) &&
!$dtConfig->get( 'DiscussionToolsEnableTopicSubscriptionBackend' ) !$dtConfig->get( 'DiscussionToolsEnableTopicSubscriptionBackend' )
) { ) {
// Can't be enabled via query, because the tables may not exist yet (T280082) // Can't be enabled via query, because the tables may not exist yet (T280082)
@ -328,4 +334,27 @@ class HookUtils {
self::isFeatureEnabledForOutput( $out, self::NEWTOPICTOOL ) self::isFeatureEnabledForOutput( $out, self::NEWTOPICTOOL )
); );
} }
/**
* Check if we should be adding automatic topic subscriptions for this user on this page.
*
* @param UserIdentity $user
* @param Title $title
* @return bool
*/
public static function shouldAddAutoSubscription( UserIdentity $user, Title $title ): bool {
// This duplicates the logic from isFeatureEnabledForOutput(),
// because we don't have access to the request or the output here.
// Topic subscription is not available on your own talk page, as you will
// get 'edit-user-talk' notifications already. (T276996)
// (can't use User::getTalkPage() to check because this is a UserIdentity)
if ( $title->inNamespace( NS_USER_TALK ) && $title->getText() === $user->getName() ) {
return false;
}
// Check if the user has automatic subscriptions enabled, and the tools are enabled on the page.
return static::isAvailableForTitle( $title ) &&
static::isFeatureEnabledForUser( $user, self::AUTOTOPICSUB );
}
} }

View file

@ -95,6 +95,13 @@ class PreferenceHooks implements
]; ];
} }
if ( isset( $preferences['discussiontools-' . HookUtils::AUTOTOPICSUB] ) ) {
// Hide automatic subscriptions when subscriptions are disabled
$preferences['discussiontools-' . HookUtils::AUTOTOPICSUB]['hide-if'] = [
'===', 'discussiontools-' . HookUtils::TOPICSUBSCRIPTION, ''
];
}
$preferences['discussiontools-showadvanced'] = [ $preferences['discussiontools-showadvanced'] = [
'type' => 'api', 'type' => 'api',
]; ];

View file

@ -266,6 +266,9 @@ class EventDispatcher {
], ],
'agent' => $user, 'agent' => $user,
]; ];
$titleForSubscriptions = Title::castFromPageIdentity( $title )->createFragmentTarget( $heading->getText() );
static::addAutoSubscription( $user, $titleForSubscriptions, $heading->getName() );
} }
} }
@ -282,6 +285,26 @@ class EventDispatcher {
} ); } );
} }
/**
* Add an automatic subscription to the given item, assuming the user has automatic subscriptions
* enabled.
*
* @param UserIdentity $user
* @param Title $title
* @param string $itemName
*/
protected static function addAutoSubscription( UserIdentity $user, Title $title, string $itemName ) {
$dtConfig = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'discussiontools' );
if (
$dtConfig->get( 'DiscussionToolsEnableTopicSubscriptionBackend' ) &&
HookUtils::shouldAddAutoSubscription( $user, $title )
) {
$subscriptionStore = MediaWikiServices::getInstance()->getService( 'DiscussionTools.SubscriptionStore' );
$subscriptionStore->addAutoSubscriptionForUser( $user, $title, $itemName );
}
}
/** /**
* Return all users subscribed to a comment * Return all users subscribed to a comment
* *
@ -295,7 +318,7 @@ class EventDispatcher {
$subscriptionStore = MediaWikiServices::getInstance()->getService( 'DiscussionTools.SubscriptionStore' ); $subscriptionStore = MediaWikiServices::getInstance()->getService( 'DiscussionTools.SubscriptionStore' );
$subscriptionItems = $subscriptionStore->getSubscriptionItemsForTopic( $subscriptionItems = $subscriptionStore->getSubscriptionItemsForTopic(
$commentName, $commentName,
SubscriptionStore::STATE_SUBSCRIBED [ SubscriptionStore::STATE_SUBSCRIBED, SubscriptionStore::STATE_AUTOSUBSCRIBED ]
); );
// Update notified timestamps // Update notified timestamps

View file

@ -26,6 +26,7 @@ class SubscriptionStore {
*/ */
public const STATE_UNSUBSCRIBED = 0; public const STATE_UNSUBSCRIBED = 0;
public const STATE_SUBSCRIBED = 1; public const STATE_SUBSCRIBED = 1;
public const STATE_AUTOSUBSCRIBED = 2;
/** @var ILBFactory */ /** @var ILBFactory */
private $lbFactory; private $lbFactory;
@ -69,14 +70,14 @@ class SubscriptionStore {
* @param IDatabase $db * @param IDatabase $db
* @param UserIdentity|null $user * @param UserIdentity|null $user
* @param array|null $itemNames * @param array|null $itemNames
* @param int|null $state One of SubscriptionStore::STATE_* constants * @param int|int[]|null $state One of (or an array of) SubscriptionStore::STATE_* constants
* @return IResultWrapper|false * @return IResultWrapper|false
*/ */
private function fetchSubscriptions( private function fetchSubscriptions(
IDatabase $db, IDatabase $db,
?UserIdentity $user = null, ?UserIdentity $user = null,
?array $itemNames = null, ?array $itemNames = null,
?int $state = null $state = null
) { ) {
$conditions = []; $conditions = [];
@ -111,14 +112,14 @@ class SubscriptionStore {
/** /**
* @param UserIdentity $user * @param UserIdentity $user
* @param array|null $itemNames * @param array|null $itemNames
* @param int|null $state One of SubscriptionStore::STATE_* constants * @param int[]|null $state Array of SubscriptionStore::STATE_* constants
* @param array $options * @param array $options
* @return SubscriptionItem[] * @return SubscriptionItem[]
*/ */
public function getSubscriptionItemsForUser( public function getSubscriptionItemsForUser(
UserIdentity $user, UserIdentity $user,
?array $itemNames = null, ?array $itemNames = null,
?int $state = null, ?array $state = null,
array $options = [] array $options = []
): array { ): array {
// Only a registered user can be subscribed // Only a registered user can be subscribed
@ -151,13 +152,13 @@ class SubscriptionStore {
/** /**
* @param string $itemName * @param string $itemName
* @param int|null $state One of SubscriptionStore::STATE_* constants * @param int[]|null $state An array of SubscriptionStore::STATE_* constants
* @param array $options * @param array $options
* @return array * @return array
*/ */
public function getSubscriptionItemsForTopic( public function getSubscriptionItemsForTopic(
string $itemName, string $itemName,
?int $state = null, ?array $state = null,
array $options = [] array $options = []
): array { ): array {
$options += [ 'forWrite' => false ]; $options += [ 'forWrite' => false ];
@ -239,12 +240,14 @@ class SubscriptionStore {
* @param UserIdentity $user * @param UserIdentity $user
* @param LinkTarget $target * @param LinkTarget $target
* @param string $itemName * @param string $itemName
* @param int $state
* @return bool * @return bool
*/ */
public function addSubscriptionForUser( public function addSubscriptionForUser(
UserIdentity $user, UserIdentity $user,
LinkTarget $target, LinkTarget $target,
string $itemName string $itemName,
int $state = self::STATE_SUBSCRIBED
): bool { ): bool {
if ( $this->readOnlyMode->isReadOnly() ) { if ( $this->readOnlyMode->isReadOnly() ) {
return false; return false;
@ -265,12 +268,12 @@ class SubscriptionStore {
'sub_title' => $target->getDBkey(), 'sub_title' => $target->getDBkey(),
'sub_section' => $target->getFragment(), 'sub_section' => $target->getFragment(),
'sub_item' => $itemName, 'sub_item' => $itemName,
'sub_state' => self::STATE_SUBSCRIBED, 'sub_state' => $state,
'sub_created' => $dbw->timestamp(), 'sub_created' => $dbw->timestamp(),
], ],
[ [ 'sub_user', 'sub_item' ] ], [ [ 'sub_user', 'sub_item' ] ],
[ [
'sub_state' => self::STATE_SUBSCRIBED, 'sub_state' => $state,
], ],
__METHOD__ __METHOD__
); );
@ -306,6 +309,37 @@ class SubscriptionStore {
return (bool)$dbw->affectedRows(); return (bool)$dbw->affectedRows();
} }
/**
* @param UserIdentity $user
* @param LinkTarget $target
* @param string $itemName
* @return bool
*/
public function addAutoSubscriptionForUser(
UserIdentity $user,
LinkTarget $target,
string $itemName
): bool {
// Check for existing subscriptions.
// Note that this includes subscriptions with state=STATE_UNSUBSCRIBED.
$subscriptionItems = $this->getSubscriptionItemsForUser(
$user,
[ $itemName ],
null,
[ 'forWrite' => true ]
);
if ( $subscriptionItems ) {
return false;
}
return $this->addSubscriptionForUser(
$user,
$target,
$itemName,
self::STATE_AUTOSUBSCRIBED
);
}
/** /**
* @param string $field Timestamp field name * @param string $field Timestamp field name
* @param UserIdentity|null $user * @param UserIdentity|null $user

View file

@ -49,7 +49,7 @@
}, },
{ {
"name": "sub_state", "name": "sub_state",
"comment": "0: unsubscribed; 1: subscribed", "comment": "0: unsubscribed; 1: subscribed, 2: auto-subscribed",
"type": "integer", "type": "integer",
"options": { "options": {
"notnull": true, "notnull": true,

View file

@ -7,6 +7,7 @@ use MediaWiki\Extension\DiscussionTools\Notifications\EventDispatcher;
use MediaWiki\Page\PageIdentity; use MediaWiki\Page\PageIdentity;
use MediaWiki\Revision\RevisionRecord; use MediaWiki\Revision\RevisionRecord;
use MediaWiki\User\UserIdentity; use MediaWiki\User\UserIdentity;
use Title;
class MockEventDispatcher extends EventDispatcher { class MockEventDispatcher extends EventDispatcher {
@ -50,4 +51,14 @@ class MockEventDispatcher extends EventDispatcher {
public static function addCommentChangeTag( RevisionRecord $newRevRecord ) { public static function addCommentChangeTag( RevisionRecord $newRevRecord ) {
} }
/**
* No-op for testing
*
* @param UserIdentity $user
* @param Title $title
* @param string $itemName
*/
protected static function addAutoSubscription( UserIdentity $user, Title $title, string $itemName ) {
}
} }

View file

@ -16,14 +16,14 @@ class MockSubscriptionStore extends SubscriptionStore {
/** /**
* @param UserIdentity $user Unused, required for inheritance * @param UserIdentity $user Unused, required for inheritance
* @param array|null $itemNames Unused, required for inheritance * @param array|null $itemNames Unused, required for inheritance
* @param int|null $state Unused, required for inheritance * @param int[]|null $state Unused, required for inheritance
* @param array $options Unused, required for inheritance * @param array $options Unused, required for inheritance
* @return array * @return SubscriptionItem[]
*/ */
public function getSubscriptionItemsForUser( public function getSubscriptionItemsForUser(
UserIdentity $user, UserIdentity $user,
?array $itemNames = null, ?array $itemNames = null,
?int $state = null, ?array $state = null,
array $options = [] array $options = []
): array { ): array {
return []; return [];