jenkins-bot has submitted this change and it was merged.

Change subject: Mentions are the primary notification
......................................................................


Mentions are the primary notification

There was code in locateMentionedUsers to exclude users who already
got the other type of motification. Since we want mentions to be the
primary notification, we no longer need that.

In addition, we want those other messages to no longer go out if the
mention notification is being sent.

Bug: T125428
Depends-on: Ifa0e2d3283f57624af4c5ec264f9f66223508e83
Change-Id: I5cd29f475cadf86d12d67f8d3bd63407e761afc2
---
M includes/Notifications/Controller.php
M includes/Notifications/Notifications.php
M includes/Notifications/UserLocator.php
3 files changed, 29 insertions(+), 41 deletions(-)

Approvals:
  Sbisson: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/Notifications/Controller.php 
b/includes/Notifications/Controller.php
index dd6ad0d..d191b93 100644
--- a/includes/Notifications/Controller.php
+++ b/includes/Notifications/Controller.php
@@ -97,16 +97,18 @@
                $events = array();
                switch( $eventName ) {
                        case 'flow-post-reply':
-                               $extraData += array(
-                                       'reply-to' => $revision->getReplyToId(),
-                                       'content' => Utils::htmlToPlaintext( 
$revision->getContent(), 200, $this->language ),
-                                       'topic-title' => 
Utils::htmlToPlaintext( $topicRevision->getContent( 'topic-title-html' ), 200, 
$this->language ),
-                               );
-
                                $mentionEvent = $this->generateMentionEvent( 
$revision, $topicRevision, $topicWorkflow, $user );
                                if ( $mentionEvent ) {
                                        $events[] = $mentionEvent;
                                }
+
+                               $extraData += array(
+                                       'reply-to' => $revision->getReplyToId(),
+                                       'content' => Utils::htmlToPlaintext( 
$revision->getContent(), 200, $this->language ),
+                                       'topic-title' => 
Utils::htmlToPlaintext( $topicRevision->getContent( 'topic-title-html' ), 200, 
$this->language ),
+                                       // pass along mentioned users to other 
notification, so it knows who to ignore
+                                       'mentioned-users' => $mentionEvent ? 
$mentionEvent->getExtraParam( 'mentioned-users' ) : array(),
+                               );
 
                                // if we're looking at the initial post 
(submitted along with the topic
                                // title), we don't want to send the 
flow-post-reply notification,
@@ -184,6 +186,11 @@
                        throw new FlowException( 'Expected Workflow but 
received ' . get_class( $boardWorkflow ) );
                }
 
+               $mentionEvent = $this->generateMentionEvent( $topicTitle, 
$topicTitle, $topicWorkflow, $user );
+               if ( $mentionEvent ) {
+                       $events[] = $mentionEvent;
+               }
+
                $events = array();
                $events[] = EchoEvent::create( array(
                        'type' => 'flow-new-topic',
@@ -202,13 +209,10 @@
                                        
$topicWorkflow->getOwnerTitle()->getArticleID( Title::GAID_FOR_UPDATE ),
                                        
$topicWorkflow->getArticleTitle()->getArticleID( Title::GAID_FOR_UPDATE ),
                                ),
+                               // pass along mentioned users to other 
notification, so it knows who to ignore
+                               'mentioned-users' => $mentionEvent ? 
$mentionEvent->getExtraParam( 'mentioned-users' ) : array(),
                        )
                ) );
-
-               $mentionEvent = $this->generateMentionEvent( $topicTitle, 
$topicTitle, $topicWorkflow, $user );
-               if ( $mentionEvent ) {
-                       $events[] = $mentionEvent;
-               }
 
                return $events;
        }
diff --git a/includes/Notifications/Notifications.php 
b/includes/Notifications/Notifications.php
index fe37a07..ea56d12 100644
--- a/includes/Notifications/Notifications.php
+++ b/includes/Notifications/Notifications.php
@@ -16,6 +16,9 @@
                        'EchoUserLocator::locateUsersWatchingTitle',
                        'EchoUserLocator::locateTalkPageOwner'
                ),
+               'user-filters' => array(
+                       'Flow\\NotificationsUserLocator::locateMentionedUsers',
+               ),
                'primary-link' => array(
                        'message' => 'flow-notification-link-text-view-topic',
                        'destination' => 'flow-new-topics'
@@ -43,6 +46,9 @@
                        
'Flow\\NotificationsUserLocator::locateUsersWatchingTopic',
                        'EchoUserLocator::locateTalkPageOwner'
                ),
+               'user-filters' => array(
+                       'Flow\\NotificationsUserLocator::locateMentionedUsers',
+               ),
                'primary-link' => array(
                        'message' => 'flow-notification-link-text-view-post',
                        'destination' => 'flow-post'
@@ -69,6 +75,9 @@
                        'Flow\\NotificationsUserLocator::locatePostAuthors',
                        'EchoUserLocator::locateTalkPageOwner'
                ),
+               'user-filters' => array(
+                       'Flow\\NotificationsUserLocator::locateMentionedUsers',
+               ),
                'primary-link' => array(
                        'message' => 'flow-notification-link-text-view-post',
                        'destination' => 'flow-post'
@@ -95,6 +104,9 @@
                        
'Flow\\NotificationsUserLocator::locateUsersWatchingTopic',
                        'EchoUserLocator::locateTalkPageOwner'
                ),
+               'user-filters' => array(
+                       'Flow\\NotificationsUserLocator::locateMentionedUsers',
+               ),
                'primary-link' => array(
                        'message' => 'flow-notification-link-text-view-post',
                        'destination' => 'flow-post'
diff --git a/includes/Notifications/UserLocator.php 
b/includes/Notifications/UserLocator.php
index 79649a1..7bb4142 100644
--- a/includes/Notifications/UserLocator.php
+++ b/includes/Notifications/UserLocator.php
@@ -75,36 +75,8 @@
         * @return array
         */
        public static function locateMentionedUsers( EchoEvent $event ) {
-               $mentionedUserIds = $event->getExtraParam( 'mentioned-users' );
-               if ( !$mentionedUserIds ) {
-                       return array();
-               }
-
-               $notifications = require __DIR__ . "/Notifications.php";
-               $extra = $event->getExtra();
-
-               /*
-                * Figure out which users may already receive a notification 
for this
-                * (e.g. because they're watching this)
-                * Mention notifications are triggered for new topics & for 
replies:
-                * let's fetch their locators & see which those events will 
already
-                * notify, so that we can discard those users here.
-                */
-               $conflict = isset( $extra['reply-to'] ) ? 'flow-post-reply' : 
'flow-new-topic';
-               $locators = $notifications[$conflict]['user-locators'];
-               $notifiedUsers = array();
-               foreach ( $locators as $callable ) {
-                       $locator = call_user_func( $callable, $event );
-                       /** @var User $user */
-                       foreach ( $locator as $user ) {
-                               $notifiedUsers[$user->getId()] = $user;
-                       }
-               }
-
-               // now subtract those that already receive another notification
-               $mentionedUserIds = array_diff( $mentionedUserIds, array_keys( 
$notifiedUsers ) );
-
-               return array_map( array( 'User', 'newFromId' ), 
$mentionedUserIds );
+               $userIds = $event->getExtraParam( 'mentioned-users', array() );
+               return array_map( array( 'User', 'newFromId' ), $userIds );
        }
 
        /**

-- 
To view, visit https://gerrit.wikimedia.org/r/267877
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5cd29f475cadf86d12d67f8d3bd63407e761afc2
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[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

Reply via email to