jenkins-bot has submitted this change and it was merged.
Change subject: Generalize a couple implementations of
EchoGetDefaultNotifiedUsers
......................................................................
Generalize a couple implementations of EchoGetDefaultNotifiedUsers
There are a variety of generic strategies you could define to choose
which users should be notified about an event, such as 'users watching
the title' or 'talk page owner' (User_talk only).
This adds a new way to generically implement these in Echo and expose them
to other extensions rather than having each extension implement these
generic strategies themselves.
This patch only converts one notification, edit-user-talk. The remaining
notifications will be converted in future patches. The first user of this
will be Flow for notifying all users watching a particular talk page in
I4e46a9c003fbdde274b20ac7aef8455eab4a5222
The users watching title implementation provided here is minimalist, a larger
refactor to accomidate pages with thousands of watchers is being handled
in I3d3fa9328f348bb48682d3658622952ce82d3925
Change-Id: I19bb6a794d22565f3bb5421de92426d390197796
---
M Echo.php
M Hooks.php
M controller/NotificationController.php
M includes/AttributeManager.php
A includes/UserLocator.php
A tests/NotificationControllerTest.php
M tests/includes/AttributeManagerTest.php
7 files changed, 211 insertions(+), 20 deletions(-)
Approvals:
Bsitu: Looks good to me, approved
Jdlrobson: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/Echo.php b/Echo.php
index 32c4c01..9b30c97 100644
--- a/Echo.php
+++ b/Echo.php
@@ -89,6 +89,7 @@
// Internal stuff
$wgAutoloadClasses['EchoNotifier'] = $dir . 'Notifier.php';
+$wgAutoloadClasses['EchoUserLocator'] = $dir . 'includes/UserLocator.php';
$wgAutoloadClasses['EchoNotificationController'] = $dir .
'controller/NotificationController.php';
$wgAutoloadClasses['EchoDiscussionParser'] = $dir .
'includes/DiscussionParser.php';
$wgAutoloadClasses['EchoDiffParser'] = $dir . 'includes/DiffParser.php';
@@ -442,6 +443,9 @@
'icon' => 'site',
),
'edit-user-talk' => array(
+ 'user-locators' => array(
+ 'EchoUserLocator::locateTalkPageOwner',
+ ),
'primary-link' => array( 'message' =>
'notification-link-text-view-message', 'destination' => 'section' ),
'secondary-link' => array( 'message' =>
'notification-link-text-view-changes', 'destination' => 'diff' ),
'category' => 'edit-user-talk',
diff --git a/Hooks.php b/Hooks.php
index ae38e57..e1257a2 100644
--- a/Hooks.php
+++ b/Hooks.php
@@ -134,19 +134,6 @@
*/
public static function getDefaultNotifiedUsers( $event, &$users ) {
switch ( $event->getType() ) {
- // Everyone deserves to know when something happens
- // on their user talk page
- case 'edit-user-talk':
- if ( !$event->getTitle() ||
!$event->getTitle()->getNamespace() == NS_USER_TALK ) {
- break;
- }
-
- $username = $event->getTitle()->getText();
- $user = User::newFromName( $username );
- if ( $user && $user->getId() ) {
- $users[$user->getId()] = $user;
- }
- break;
case 'add-comment':
case 'add-talkpage-topic':
// Handled by EchoDiscussionParser
diff --git a/controller/NotificationController.php
b/controller/NotificationController.php
index c391659..49abd94 100644
--- a/controller/NotificationController.php
+++ b/controller/NotificationController.php
@@ -87,7 +87,7 @@
continue;
}
if ( $blacklisted &&
!self::isWhitelistedByUser( $event, $user ) ) {
- continue;
+ continue;
}
wfRunHooks( 'EchoGetNotificationTypes', array(
$user, $event, &$notifyTypes ) );
@@ -190,13 +190,27 @@
/**
* Retrieves an array of User objects to be notified for an EchoEvent.
*
- * @param $event EchoEvent to retrieve users to be notified for.
- * @return Array of User objects
+ * @param EchoEvent $event
+ * @return array keys are user ids, values are User objects
*/
- protected static function getUsersToNotifyForEvent( $event ) {
- $users = $notifyList = array();
+ public static function getUsersToNotifyForEvent( EchoEvent $event ) {
+ $type = $event->getType();
+ // Key notifyList by user id to ensure there are no duplicated
users
+ $notifyList = array();
+
+ $attributeManager = EchoAttributeManager::newFromGlobalVars();
+ foreach ( $attributeManager->getUserLocators( $type ) as
$callable ) {
+ if ( is_callable( $callable ) ) {
+ $notifyList += call_user_func( $callable,
$event );
+ } else {
+ wfDebugLog( __CLASS__, __FUNCTION__ . ":
Invalid user-locator returned for $type" );
+ }
+ }
+
+ // hook for injecting more users.
+ // @deprecated
+ $users = array();
wfRunHooks( 'EchoGetDefaultNotifiedUsers', array( $event,
&$users ) );
- // Make sure there is no duplicated users
foreach ( $users as $user ) {
$notifyList[$user->getId()] = $user;
}
@@ -259,7 +273,7 @@
/**
* INTERNAL. Must be public to be callable by the php error handling
methods.
*
- * Converts E_RECOVERABLE_ERROR, such as passing null to a method
expecting
+ * Converts E_RECOVERABLE_ERROR, such as passing null to a method
expecting
* a non-null object, into exceptions.
*/
public static function formatterErrorHandler( $errno, $errstr,
$errfile, $errline ) {
diff --git a/includes/AttributeManager.php b/includes/AttributeManager.php
index c7b85fe..4aac0a3 100644
--- a/includes/AttributeManager.php
+++ b/includes/AttributeManager.php
@@ -56,6 +56,21 @@
}
/**
+ * Get the user-locators related to the provided event type
+ *
+ * @param string $type
+ * @return array
+ */
+ public function getUserLocators( $type ) {
+ if ( isset( $this->notifications[$type]['user-locators'] ) ) {
+ return
(array)$this->notifications[$type]['user-locators'];
+ } else {
+ wfDebugLog( 'Echo', __METHOD__ . ": No user-locators
configured for $type" );
+ return array();
+ }
+ }
+
+ /**
* Get the enabled events for a user, which excludes user-dismissed
events
* from the general enabled events
* @param User
diff --git a/includes/UserLocator.php b/includes/UserLocator.php
new file mode 100644
index 0000000..9a7294c
--- /dev/null
+++ b/includes/UserLocator.php
@@ -0,0 +1,49 @@
+<?php
+
+class EchoUserLocator {
+
+ /**
+ * The echo job queue must be enabled to prevent timeouts submitting to
+ * heavily watched pages when this is used.
+ */
+ public static function locateUsersWatchingTitle( EchoEvent $event,
$batchSize = 500 ) {
+ $title = $event->getTitle();
+ if ( !$title ) {
+ return array();
+ }
+
+ $dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+ $res = $dbr->select(
+ array( 'watchlist' ),
+ array( 'wl_user' ),
+ array(
+ 'wl_namespace' => $title->getNamespace(),
+ 'wl_title' => $title->getDBkey(),
+ ),
+ __METHOD__
+ );
+
+ $users = array();
+ if ( $res ) {
+ foreach ( $res as $row ) {
+ $users[$row->wl_user] = User::newFromId(
$row->wl_user );
+ }
+ }
+
+ return $users;
+ }
+
+ public static function locateTalkPageOwner( EchoEvent $event ) {
+ $title = $event->getTitle();
+ if ( !$title || $title->getNamespace() !== NS_USER_TALK ) {
+ return array();
+ }
+
+ $user = User::newFromName( $title->getDBkey() );
+ if ( $user && !$user->isAnon() ) {
+ return array( $user->getId() => $user );
+ } else {
+ return array();
+ }
+ }
+}
diff --git a/tests/NotificationControllerTest.php
b/tests/NotificationControllerTest.php
new file mode 100644
index 0000000..30db70a
--- /dev/null
+++ b/tests/NotificationControllerTest.php
@@ -0,0 +1,67 @@
+<?php
+
+class NotificationControllerTest extends MediaWikiTestCase {
+
+ public function getUsersToNotifyForEventProvider() {
+ return array(
+ array(
+ 'With no options no users are notified',
+ // expected result
+ array(),
+ // event user locator config
+ array(),
+ ),
+
+ array(
+ 'Does not error when given non-existant
user-locator',
+ // expected result
+ array(),
+ // event user locator config
+ array( 'not-callable' ),
+ ),
+
+ array(
+ 'Calls selected locator and returns result',
+ // expected result
+ array( 123 ),
+ // event user locator config
+ function() { return array( 123 => 123 ); }
+ ),
+
+ array(
+ 'merges results of multiple locators',
+ // expected result
+ array( 123, 456 ),
+ // event user locator config
+ array(
+ function() { return array( 123 => 123
); },
+ function() { return array( 456 => 456
); },
+ ),
+ ),
+
+ );
+ }
+
+ /**
+ * @dataProvider getUsersToNotifyForEventProvider
+ */
+ public function testGetUsersToNotifyForEvent( $message, $expect,
$locatorConfigForEventType ) {
+ $this->setMwGlobals( array(
+ 'wgEchoNotifications' => array(
+ 'unit-test' => array(
+ 'user-locators' =>
$locatorConfigForEventType
+ ),
+ ),
+ ) );
+
+ $event = $this->getMockBuilder( 'EchoEvent' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $event->expects( $this->any() )
+ ->method( 'getType' )
+ ->will( $this->returnValue( 'unit-test' ) );
+
+ $users = EchoNotificationController::getUsersToNotifyForEvent(
$event );
+ $this->assertEquals( $expect, array_keys( $users ), $message );
+ }
+}
diff --git a/tests/includes/AttributeManagerTest.php
b/tests/includes/AttributeManagerTest.php
index dc0157d..b180afd 100644
--- a/tests/includes/AttributeManagerTest.php
+++ b/tests/includes/AttributeManagerTest.php
@@ -6,6 +6,61 @@
$this->assertInstanceOf( 'EchoAttributeManager',
EchoAttributeManager::newFromGlobalVars() );
}
+ public static function getUserLocatorsProvider() {
+ return array(
+ array(
+ 'No errors when requesting unknown type',
+ // expected result
+ array(),
+ // event type
+ 'foo',
+ // notification configuration
+ array(),
+ ),
+
+ array(
+ 'Returns selected notification configuration',
+ // expected result
+ array( 'woot!' ),
+ // event type
+ 'magic',
+ // notification configuration
+ array(
+ 'foo' => array(
+ 'user-locators' => array(
'frown' ),
+ ),
+ 'magic' => array(
+ 'user-locators' => array(
'woot!' ),
+ ),
+ ),
+ ),
+
+ array(
+ 'Accepts user-locators as string and returns
array',
+ // expected result
+ array( 'sagen' ),
+ // event type
+ 'challah',
+ // notification configuration
+ array(
+ 'challah' => array(
+ 'user-locators' => 'sagen',
+ ),
+ ),
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider getUserLocatorsProvider
+ */
+ public function testGetUserLocators( $message, $expect, $type,
$notifications) {
+ $manager = new EchoAttributeManager( $notifications, array() );
+
+ $result = $manager->getUserLocators( $type );
+ $this->assertEquals( $expect, $result, $message );
+ }
+
public function testGetCategoryEligibility() {
$notif = array(
'event_one' => array (
--
To view, visit https://gerrit.wikimedia.org/r/150437
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I19bb6a794d22565f3bb5421de92426d390197796
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Bsitu <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
