Make notifyAgent a per-type property rather than per-event

Specify which notification types allow notifying the event agent in
$wgEchoNotifications, and stop specifying it in the event_extra data.

Putting 'notifyAgent' => true in event_extra will still work, but is
discouraged.

Change-Id: I4f558654ec23757dd4ecd6986eb3e9a5593f5386

extension.json

@@ -684,6 +684,7 @@
 					"user-locators": [
 						"EchoUserLocator::locateEventAgent"
 					],
+					"canNotifyAgent": true,
 					"category": "system",
 					"group": "positive",
 					"section": "message",
@@ -769,6 +770,7 @@
 							"EchoUserLocator::locateEventAgent"
 						]
 					],
+					"canNotifyAgent": true,
 					"category": "mention-failure",
 					"bundle": {
 						"web": true,
@@ -784,6 +786,7 @@
 							"EchoUserLocator::locateEventAgent"
 						]
 					],
+					"canNotifyAgent": true,
 					"category": "mention-failure",
 					"group": "negative",
 					"section": "alert",
@@ -795,6 +798,7 @@
 							"EchoUserLocator::locateEventAgent"
 						]
 					],
+					"canNotifyAgent": true,
 					"category": "mention-success",
 					"bundle": {
 						"web": true,
@@ -845,6 +849,7 @@
 					"user-locators": [
 						"EchoUserLocator::locateEventAgent"
 					],
+					"canNotifyAgent": true,
 					"category": "system",
 					"notify-type-availability": {
 						"email": false
@@ -857,6 +862,7 @@
 					"user-locators": [
 						"EchoUserLocator::locateEventAgent"
 					],
+					"canNotifyAgent": true,
 					"category": "article-reminder",
 					"group": "positive",
 					"presentation-model": "EchoArticleReminderPresentationModel",

includes/AttributeManager.php

@@ -374,4 +374,19 @@ class EchoAttributeManager {
 		return 'alert';
 	}
 
+	/**
+	 * Get notification types that allow their own agent to be notified.
+	 *
+	 * @return string[] Notification types
+	 */
+	public function getNotifyAgentEvents() {
+		$events = [];
+		foreach ( $this->notifications as $event => $attribs ) {
+			if ( $attribs['canNotifyAgent'] ?? false ) {
+				$events[] = $event;
+			}
+		}
+		return $events;
+	}
+
 }

includes/DiscussionParser.php

@@ -228,7 +228,6 @@ abstract class EchoDiscussionParser {
 					'extra' => [
 						'max-mentions' => $wgEchoMaxMentionsCount,
 						'section-title' => $header,
-						'notifyAgent' => true
 					],
 					'agent' => $agent,
 				] );
@@ -261,7 +260,6 @@ abstract class EchoDiscussionParser {
 						'subject-name' => User::newFromId( $mentionedUserId )->getName(),
 						'section-title' => $header,
 						'revid' => $revision->getId(),
-						'notifyAgent' => true
 					],
 					'agent' => $agent,
 				] );
@@ -278,7 +276,6 @@ abstract class EchoDiscussionParser {
 						'subject-name' => $anonymousUser,
 						'section-title' => $header,
 						'revid' => $revision->getId(),
-						'notifyAgent' => true
 					],
 					'agent' => $agent,
 				] );
@@ -295,7 +292,6 @@ abstract class EchoDiscussionParser {
 						'subject-name' => $unknownUser,
 						'section-title' => $header,
 						'revid' => $revision->getId(),
-						'notifyAgent' => true
 					],
 					'agent' => $agent,
 				] );

includes/EchoHooks.php

@@ -577,7 +577,6 @@ class EchoHooks {
 							'agent' => $user,
 							// Edit threshold notifications are sent to the agent
 							'extra' => [
-								'notifyAgent' => true,
 								'editCount' => $thresholdCount,
 							]
 						]
@@ -667,10 +666,6 @@ class EchoHooks {
 			EchoEvent::create( [
 				'type' => 'welcome',
 				'agent' => $user,
-				// Welcome notification is sent to the agent
-				'extra' => [
-					'notifyAgent' => true
-				]
 			] );
 		}
 

includes/api/ApiEchoArticleReminder.php

@@ -25,7 +25,6 @@ class ApiEchoArticleReminder extends ApiBase {
 			'agent' => $user,
 			'title' => $this->getTitleFromTitleOrPageId( $params ),
 			'extra' => [
-				'notifyAgent' => true,
 				'comment' => $params['comment'],
 			],
 		] );

includes/controller/NotificationController.php

@@ -449,9 +449,8 @@ class EchoNotificationController {
 			return true;
 		} );
 
-		// Don't notify the person who initiated the event unless the event extra says to do so
-		$extra = $event->getExtra();
-		if ( ( !isset( $extra['notifyAgent'] ) || !$extra['notifyAgent'] ) && $event->getAgent() ) {
+		// Don't notify the person who initiated the event unless the event allows it
+		if ( !$event->canNotifyAgent() && $event->getAgent() ) {
 			$agentId = $event->getAgent()->getId();
 			$notify->addFilter( function ( $user ) use ( $agentId ) {
 				return $user->getId() != $agentId;

includes/model/Event.php

@@ -506,6 +506,21 @@ class EchoEvent extends EchoAbstractEntity implements Bundleable {
 		return $this->agent;
 	}
 
+	/**
+	 * Check whether this event allows its agent to be notified.
+	 *
+	 * Notifying the agent is only allowed if the event's type allows it, or if the event extra
+	 * explicity specifies 'notifyAgent' => true.
+	 *
+	 * @return bool
+	 */
+	public function canNotifyAgent() {
+		global $wgEchoNotifications;
+		$allowedInConfig = $wgEchoNotifications[$this->getType()]['canNotifyAgent'] ?? false;
+		$allowedInExtra = $this->getExtraParam( 'notifyAgent', false );
+		return $allowedInConfig || $allowedInExtra;
+	}
+
 	/**
 	 * @param bool $fromMaster
 	 * @return null|Title

maintenance/generateSampleNotifications.php

@@ -298,9 +298,6 @@ class GenerateSampleNotifications extends Maintenance {
 		EchoEvent::create( [
 			'type' => 'welcome',
 			'agent' => $user,
-			'extra' => [
-				'notifyAgent' => true
-			],
 			'timestamp' => $this->getTimestamp(),
 		] );
 	}

tests/phpunit/DiscussionParserTest.php

@@ -418,14 +418,12 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Hello Users',
-						'notifyAgent' => true,
 						'subject-name' => 'Ping',
 					],
 					[
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Hello Users',
-						'notifyAgent' => true,
 						'subject-name' => 'Po?ng',
 					],
 				],
@@ -442,14 +440,12 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'type' => 'mention',
 						'agent' => 'Admin',
 						'section-title' => 'Hello Users',
-						'notifyAgent' => null,
 						'subject-name' => null,
 					],
 					[
 						'type' => 'mention-success',
 						'agent' => 'Admin',
 						'section-title' => 'Hello Users',
-						'notifyAgent' => true,
 						'subject-name' => 'Test11',
 					],
 				],
@@ -466,7 +462,6 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
-						'notifyAgent' => true,
 						'subject-name' => 'NoUser',
 					],
 				],
@@ -483,7 +478,6 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
-						'notifyAgent' => true,
 						'subject-name' => 'NoUser',
 					],
 				],
@@ -510,14 +504,12 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => 'Section 1.5',
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 					[
 						'type' => 'mention-success',
 						'agent' => 'Admin',
 						'section-title' => 'Section 1.5',
 						'subject-name' => 'Test11',
-						'notifyAgent' => true,
 					],
 				],
 			],
@@ -534,14 +526,12 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => 'Section 1.5',
 						'subject-name' => 'NoUser1.5',
-						'notifyAgent' => true,
 					],
 					[
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
 						'subject-name' => 'NoUser2',
-						'notifyAgent' => true,
 					],
 				],
 			],
@@ -558,21 +548,18 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => 'Section 1',
 						'subject-name' => 'NoUser1',
-						'notifyAgent' => true,
 					],
 					[
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Section 1.75',
 						'subject-name' => 'NoUser1.75',
-						'notifyAgent' => true,
 					],
 					[
 						'type' => 'mention-failure',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
 						'subject-name' => 'NoUser2',
-						'notifyAgent' => true,
 					],
 				],
 				[
@@ -587,7 +574,6 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => false,
 						'subject-name' => null,
-						'notifyAgent' => null,
 					] ],
 				],
 				[
@@ -603,21 +589,18 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 							'agent' => 'Admin',
 							'section-title' => 'Section 1',
 							'subject-name' => null,
-							'notifyAgent' => null,
 						],
 						[
 							'type' => 'mention-success',
 							'agent' => 'Admin',
 							'section-title' => 'Section 1',
 							'subject-name' => 'Test11',
-							'notifyAgent' => true,
 						],
 						[
 							'type' => 'edit-user-talk',
 							'agent' => 'Admin',
 							'section-title' => 'Section 1',
 							'subject-name' => null,
-							'notifyAgent' => null,
 						],
 					],
 				],
@@ -635,21 +618,18 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => 'Section 1',
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 					[
 						'type' => 'mention-success',
 						'agent' => 'Admin',
 						'section-title' => 'Section 1',
 						'subject-name' => 'Test11',
-						'notifyAgent' => true,
 					],
 					[
 						'type' => 'edit-user-talk',
 						'agent' => 'Admin',
 						'section-title' => false,
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 				],
 			],
@@ -666,21 +646,18 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 					[
 						'type' => 'mention-success',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
 						'subject-name' => 'Test11',
-						'notifyAgent' => true,
 					],
 					[
 						'type' => 'edit-user-talk',
 						'agent' => 'Admin',
 						'section-title' => 'Section 2',
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 				],
 			],
@@ -697,14 +674,12 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 						'agent' => 'Admin',
 						'section-title' => false,
 						'subject-name' => null,
-						'notifyAgent' => null,
 					],
 					[
 						'type' => 'mention-success',
 						'agent' => 'Admin',
 						'section-title' => false,
 						'subject-name' => 'Test11',
-						'notifyAgent' => true,
 					],
 				],
 			],
@@ -729,7 +704,6 @@ class EchoDiscussionParserTest extends MediaWikiTestCase {
 					'type' => $event->getType(),
 					'agent' => $event->getAgent()->getName(),
 					'section-title' => $event->getExtraParam( 'section-title' ),
-					'notifyAgent' => $event->getExtraParam( 'notifyAgent' ),
 					'subject-name' => $event->getExtraParam( 'subject-name' ),
 				];
 				return false;