Merge "Improve wording for throttled filter warnings"

i18n/en.json

@@ -145,7 +145,7 @@
 	"abusefilter-enabled": "Enabled",
 	"abusefilter-deleted": "Deleted",
 	"abusefilter-disabled": "Disabled",
-	"abusefilter-throttled": "throttled",
+	"abusefilter-throttled": "High rate of matches",
 	"abusefilter-hitcount": "$1 {{PLURAL:$1|hit|hits}}",
 	"abusefilter-new": "Create a new filter",
 	"abusefilter-import-button": "Import filter",
@@ -192,7 +192,8 @@
 	"abusefilter-edit-oldwarning-view": "<strong>You are viewing an old version of this filter.\nThe statistics quoted are for the most recent version of the filter.</strong> &bull;\n[[Special:AbuseFilter/history/$2|Return to this filter's history]].",
 	"abusefilter-edit-status-label": "Statistics:",
 	"abusefilter-edit-status": "Of the last $1 {{PLURAL:$1|action|actions}}, this filter has matched $2 ($3%).\nOn average, its run time is $4 ms, and it consumes $5 {{PLURAL:$5|condition|conditions}} of the condition limit.",
-	"abusefilter-edit-throttled-warning": "'''Warning:''' This filter was automatically flagged as harmful. As a safety measure, the following actions will not execute ($1). Please review and [[mw:Extension:AbuseFilter/Conditions|optimize]] your conditions to remove this restriction",
+	"abusefilter-edit-throttled-warning": "'''Warning:''' This filter was automatically flagged as harmful. As a safety measure, the following {{PLURAL:$2|action|actions}} will not execute: $1. Please check whether the high rate of matches is expected. If so, you can save the filter again to remove this restriction.",
+	"abusefilter-edit-throttled-warning-no-actions": "'''Warning:''' This filter was automatically flagged as harmful. No actions were automatically disabled, but please check whether the high rate of matches is expected. If so, you can ignore this warning.",
 	"abusefilter-edit-new": "New filter",
 	"abusefilter-edit-save": "Save filter",
 	"abusefilter-edit-id": "Filter ID:",
@@ -573,6 +574,8 @@
 	"tag-abusefilter-condition-limit": "condition limit reached",
 	"tag-abusefilter-condition-limit-description": "Edits or other events that couldn't be checked by all active [[Special:AbuseFilter|abuse filters]] ([[mw:Extension:AbuseFilter/Conditions|help]]).",
 	"notification-header-throttle-filter": "Abuse filter $2 {{GENDER:$1|you}} recently edited was throttled.",
+	"notification-header-throttle-filter-actions": "The abuse filter $2 {{GENDER:$1|you}} recently edited had a high rate of matches and the following {{PLURAL:$4|action was|actions were}} automatically disabled: $3.",
+	"notification-header-throttle-filter-no-actions": "The abuse filter $2 {{GENDER:$1|you}} recently edited had a high rate of matches but no actions were automatically disabled.",
 	"notification-subject-throttle-filter": "An abuse filter {{GENDER:$1|you}} edited was throttled on {{SITENAME}}",
 	"notification-link-text-show-filter": "Show filter"
 }

i18n/qqq.json

@@ -186,7 +186,7 @@
 	"abusefilter-enabled": "Abuse filter status.\n{{Identical|Enabled}}",
 	"abusefilter-deleted": "Abuse filter status.\n{{Identical|Deleted}}",
 	"abusefilter-disabled": "Abuse filter status.\n{{Identical|Disabled}}",
-	"abusefilter-throttled": "Abuse filter status where some actions have been automatically disabled. See {{msg-mw|abusefilter-edit-throttled-warning}}",
+	"abusefilter-throttled": "Abuse filter status where the filter is matching many edits. See {{msg-mw|abusefilter-edit-throttled-warning}} and {{msg-mw|abusefilter-edit-throttled-warning-no-actions}}.",
 	"abusefilter-hitcount": "Indicates number of times an abuse filter was triggered. Parameters:\n* $1 is the number of hits.",
 	"abusefilter-new": "Button text for creating a new abuse filter.",
 	"abusefilter-import-button": "Used as link text in the navigation toolbar.\n\nThe link points to [[Special:AbuseLog]].",
@@ -233,7 +233,8 @@
 	"abusefilter-edit-oldwarning-view": "Warning displayed when viewing an older version of a filter. Parameters:\n* $1 - (Unused) history ID\n* $2 - filter ID\nSee also {{msg-mw|AbuseFilter-edit-oldwarning}}",
 	"abusefilter-edit-status-label": "Field label for abuse filter statistics.\n{{Identical|Statistics}}",
 	"abusefilter-edit-status": "Parameters:\n* $1 - number of actions\n* $2 - matched count\n* $3 - matched percentage\n* $4 - time (in milliseconds)\n* $5 - number of conditions",
-	"abusefilter-edit-throttled-warning": "Used as warning message when the filter is throttled and actions will not execute. Parameters:\n* $1 - is a string containing the actions that will not execute",
+	"abusefilter-edit-throttled-warning": "Used as warning message when the filter is throttled and actions will not execute. Parameters:\n* $1 - is a string containing the actions that will not execute\n* $2 - number of disabled actions, for PLURAL",
+	"abusefilter-edit-throttled-warning-no-actions": "Used as warning message when the filter is throttled but no actions were disabled.",
 	"abusefilter-edit-new": "Field value in case an edited filter is new.",
 	"abusefilter-edit-save": "Submit button text to save a filter.",
 	"abusefilter-edit-id": "Field label for filter identifier.\n{{Identical|Filter ID}}",
@@ -614,6 +615,8 @@
 	"tag-abusefilter-condition-limit": "Change tag for edits that reached the condition limit",
 	"tag-abusefilter-condition-limit-description": "Description for \"condition limit reached\" change tag",
 	"notification-header-throttle-filter": "Header text for a notification when an abuse filter was throttled after the user edited it. Parameters:\n* $1 - the username of the viewing user, for use in GENDER\n* $2 - filter ID",
+	"notification-header-throttle-filter-actions": "Header text for a notification when an abuse filter was throttled after the user edited it and some actions were automatically disabled. Parameters:\n* $1 - the username of the viewing user, for use in GENDER\n* $2 - filter ID\n* $3 - list of actions that were disabled\n* $4 - number of disabled actions, for PLURAL",
+	"notification-header-throttle-filter-no-actions": "Header text for a notification when an abuse filter was throttled after the user edited it but no actions were automatically disabled. Parameters:\n* $1 - the username of the viewing user, for use in GENDER\n* $2 - filter ID",
 	"notification-subject-throttle-filter": "Email subject line for email notice when an abuse filter was throttled after the user edited it. Parameters:\n* $1 - the username of the receiving user, for use in GENDER",
 	"notification-link-text-show-filter": "Label for button that links to the filter that was throttled."
 }

includes/EchoNotifier.php

@@ -3,6 +3,8 @@
 namespace MediaWiki\Extension\AbuseFilter;
 
 use EchoEvent;
+use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
+use MediaWiki\Extension\AbuseFilter\Filter\ExistingFilter;
 use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseFilter;
 use Title;
 
@@ -16,19 +18,23 @@ class EchoNotifier {
 
 	/** @var FilterLookup */
 	private $filterLookup;
-
+	/** @var ConsequencesRegistry */
+	private $consequencesRegistry;
 	/** @var bool */
 	private $isEchoLoaded;
 
 	/**
 	 * @param FilterLookup $filterLookup
+	 * @param ConsequencesRegistry $consequencesRegistry
 	 * @param bool $isEchoLoaded
 	 */
 	public function __construct(
 		FilterLookup $filterLookup,
+		ConsequencesRegistry $consequencesRegistry,
 		bool $isEchoLoaded
 	) {
 		$this->filterLookup = $filterLookup;
+		$this->consequencesRegistry = $consequencesRegistry;
 		$this->isEchoLoaded = $isEchoLoaded;
 	}
 
@@ -42,10 +48,10 @@ class EchoNotifier {
 
 	/**
 	 * @param int $filter
-	 * @return int
+	 * @return ExistingFilter
 	 */
-	private function getLastUserIDForFilter( int $filter ): int {
-		return $this->filterLookup->getFilter( $filter, false )->getUserID();
+	private function getFilterObject( int $filter ): ExistingFilter {
+		return $this->filterLookup->getFilter( $filter, false );
 	}
 
 	/**
@@ -54,11 +60,17 @@ class EchoNotifier {
 	 * @return array
 	 */
 	public function getDataForEvent( int $filter ): array {
+		$filterObj = $this->getFilterObject( $filter );
+		$throttledActionNames = array_intersect(
+			$filterObj->getActionsNames(),
+			$this->consequencesRegistry->getDangerousActionNames()
+		);
 		return [
 			'type' => self::EVENT_TYPE,
 			'title' => $this->getTitleForFilter( $filter ),
 			'extra' => [
-				'user' => $this->getLastUserIDForFilter( $filter ),
+				'user' => $filterObj->getUserID(),
+				'throttled-actions' => $throttledActionNames,
 			],
 		];
 	}

includes/ServiceWiring.php

@@ -150,6 +150,7 @@ return [
 	EchoNotifier::SERVICE_NAME => static function ( MediaWikiServices $services ): EchoNotifier {
 		return new EchoNotifier(
 			$services->getService( FilterLookup::SERVICE_NAME ),
+			$services->getService( ConsequencesRegistry::SERVICE_NAME ),
 			ExtensionRegistry::getInstance()->isLoaded( 'Echo' )
 		);
 	},

includes/SpecsFormatter.php

@@ -4,7 +4,9 @@ namespace MediaWiki\Extension\AbuseFilter;
 
 use Language;
 use MediaWiki\Extension\AbuseFilter\Filter\AbstractFilter;
+use Message;
 use MessageLocalizer;
+use RawMessage;
 
 /**
  * @todo Improve this once DI around Message objects is improved in MW core.
@@ -32,6 +34,7 @@ class SpecsFormatter {
 	/**
 	 * @param string $action
 	 * @return string HTML
+	 * @todo Replace usage with getActionMessage
 	 */
 	public function getActionDisplay( string $action ): string {
 		// Give grep a chance to find the usages:
@@ -42,6 +45,20 @@ class SpecsFormatter {
 		return $msg->isDisabled() ? htmlspecialchars( $action ) : $msg->escaped();
 	}
 
+	/**
+	 * @param string $action
+	 * @return Message
+	 */
+	public function getActionMessage( string $action ): Message {
+		// Give grep a chance to find the usages:
+		// abusefilter-action-tag, abusefilter-action-throttle, abusefilter-action-warn,
+		// abusefilter-action-blockautopromote, abusefilter-action-block, abusefilter-action-degroup,
+		// abusefilter-action-rangeblock, abusefilter-action-disallow
+		$msg = $this->messageLocalizer->msg( "abusefilter-action-$action" );
+		// XXX Why do we expect the message to be disabled?
+		return $msg->isDisabled() ? new RawMessage( $action ) : $msg;
+	}
+
 	/**
 	 * @param string $action
 	 * @param string[] $parameters

includes/ThrottleFilterPresentationModel.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Extension\AbuseFilter;
 
 use EchoEventPresentationModel;
+use Message;
 
 class ThrottleFilterPresentationModel extends EchoEventPresentationModel {
 
@@ -19,7 +20,27 @@ class ThrottleFilterPresentationModel extends EchoEventPresentationModel {
 	public function getHeaderMessage() {
 		$text = $this->event->getTitle()->getText();
 		list( , $filter ) = explode( '/', $text, 2 );
-		return $this->msg( 'notification-header-throttle-filter' )
+		$disabledActions = $this->event->getExtraParam( 'throttled-actions' );
+		if ( $disabledActions === null ) {
+			// BC for when we didn't include the actions here.
+			return $this->msg( 'notification-header-throttle-filter' )
+				->params( $this->getViewingUserForGender() )
+				->numParams( $filter );
+		}
+		if ( $disabledActions ) {
+			$specsFormatter = AbuseFilterServices::getSpecsFormatter();
+			$specsFormatter->setMessageLocalizer( $this );
+			$disabledActionsLocalized = [];
+			foreach ( $disabledActions as $action ) {
+				$disabledActionsLocalized[] = $specsFormatter->getActionMessage( $action )->text();
+			}
+			return $this->msg( 'notification-header-throttle-filter-actions' )
+				->params( $this->getViewingUserForGender() )
+				->numParams( $filter )
+				->params( Message::listParam( $disabledActionsLocalized ) )
+				->params( count( $disabledActionsLocalized ) );
+		}
+		return $this->msg( 'notification-header-throttle-filter-no-actions' )
 			->params( $this->getViewingUserForGender() )
 			->numParams( $filter );
 	}

includes/View/AbuseFilterViewEdit.php

@@ -395,26 +395,26 @@ class AbuseFilterViewEdit extends AbuseFilterView {
 		}
 
 		if ( $filterObj->isThrottled() ) {
-			$throttledActions = array_intersect(
+			$throttledActionNames = array_intersect(
 				$filterObj->getActionsNames(),
 				$this->consequencesRegistry->getDangerousActionNames()
 			);
 
-			if ( $throttledActions ) {
-				$throttledActions = array_map(
-					function ( $filterAction ) {
-						// TODO: This is SpecsFormatter::getActionDisplay, but not escaped
-						return $this->msg( 'abusefilter-action-' . $filterAction )->text();
-					},
-					$throttledActions
-				);
+			if ( $throttledActionNames ) {
+				$throttledActionsLocalized = [];
+				foreach ( $throttledActionNames as $actionName ) {
+					$throttledActionsLocalized[] = $this->specsFormatter->getActionMessage( $actionName )->text();
+				}
 
-				$flags .= Html::warningBox(
-					$this->msg( 'abusefilter-edit-throttled-warning' )
-					->plaintextParams( $lang->commaList( $throttledActions ) )
-					->parseAsBlock()
-				);
+				$throttledMsg = $this->msg( 'abusefilter-edit-throttled-warning' )
+					->plaintextParams( $lang->commaList( $throttledActionsLocalized ) )
+					->params( count( $throttledActionsLocalized ) )
+					->parseAsBlock();
+			} else {
+				$throttledMsg = $this->msg( 'abusefilter-edit-throttled-warning-no-actions' )
+					->parseAsBlock();
 			}
+			$flags .= Html::warningBox( $throttledMsg );
 		}
 
 		foreach ( $checkboxes as $checkboxId ) {

tests/phpunit/integration/EchoNotifierTest.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Extension\AbuseFilter\Tests\Integration;
 
 use EchoEvent;
+use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
 use MediaWiki\Extension\AbuseFilter\EchoNotifier;
 use MediaWiki\Extension\AbuseFilter\Filter\ExistingFilter;
 use MediaWiki\Extension\AbuseFilter\FilterLookup;
@@ -44,11 +45,16 @@ class EchoNotifierTest extends MediaWikiIntegrationTestCase {
 	 * @dataProvider provideDataForEvent
 	 * @covers ::__construct
 	 * @covers ::getDataForEvent
-	 * @covers ::getLastUserIDForFilter
+	 * @covers ::getFilterObject
 	 * @covers ::getTitleForFilter
 	 */
 	public function testGetDataForEvent( bool $loaded, int $filter, int $userID ) {
-		$notifier = new EchoNotifier( $this->getFilterLookup(), $loaded );
+		$expectedThrottledActions = [];
+		$notifier = new EchoNotifier(
+			$this->getFilterLookup(),
+			$this->createMock( ConsequencesRegistry::class ),
+			$loaded
+		);
 		[
 			'type' => $type,
 			'title' => $title,
@@ -60,7 +66,7 @@ class EchoNotifierTest extends MediaWikiIntegrationTestCase {
 		$this->assertSame( -1, $title->getNamespace() );
 		[ , $subpage ] = explode( '/', $title->getText(), 2 );
 		$this->assertSame( (string)$filter, $subpage );
-		$this->assertSame( [ 'user' => $userID ], $extra );
+		$this->assertSame( [ 'user' => $userID, 'throttled-actions' => $expectedThrottledActions ], $extra );
 	}
 
 	/**
@@ -70,7 +76,11 @@ class EchoNotifierTest extends MediaWikiIntegrationTestCase {
 		if ( !class_exists( EchoEvent::class ) ) {
 			$this->markTestSkipped( 'Echo not loaded' );
 		}
-		$notifier = new EchoNotifier( $this->getFilterLookup(), true );
+		$notifier = new EchoNotifier(
+			$this->getFilterLookup(),
+			$this->createMock( ConsequencesRegistry::class ),
+			true
+		);
 		$this->assertInstanceOf( EchoEvent::class, $notifier->notifyForFilter( 1 ) );
 	}
 
@@ -80,7 +90,11 @@ class EchoNotifierTest extends MediaWikiIntegrationTestCase {
 	public function testNotifyForFilter_EchoNotLoaded() {
 		$lookup = $this->createMock( FilterLookup::class );
 		$lookup->expects( $this->never() )->method( $this->anything() );
-		$notifier = new EchoNotifier( $lookup, false );
+		$notifier = new EchoNotifier(
+			$lookup,
+			$this->createMock( ConsequencesRegistry::class ),
+			false
+		);
 		$this->assertFalse( $notifier->notifyForFilter( 1 ) );
 	}