Tweak methods related to global filters

To make the switch to afl_filter_id and afl_global easier.

Bug: T227095
Depends-On: Ie550889495232b534c0f9aec31039cf21b2135b1
Change-Id: If557bad8f5c1a6d15e3556e4bfbd0330d7d49c59

i18n/en.json

@@ -510,6 +510,7 @@
 	"abusefilter-log-header": "This log shows a summary of changes made to filters.\nFor full details, see [[Special:AbuseFilter/history|the list]] of recent filter changes.",
 	"abusefilter-logentry-create": "$1 {{GENDER:$2|created}} $4 ($5)",
 	"abusefilter-logentry-modify": "$1 {{GENDER:$2|modified}} $4 ($5)",
+	"abusefilter-log-invalid-filter": "Some of the specified filter IDs are invalid.",
 	"abusefilter-log-noresults": "No results",
 	"abusefilter-diff-title": "Differences between versions",
 	"abusefilter-diff-item": "Item",

i18n/qqq.json

@@ -544,6 +544,7 @@
 	"abusefilter-log-header": "Used as description on [[Special:Log/abusefilter]]",
 	"abusefilter-logentry-create": "Parameters:\n* $1 - a link to a user page with a user name as link text, followed by a series of related links\n* $2 - raw username, for GENDER support\n* $3 - (unused)\n* $4 - text {{msg-mw|abusefilter-log-detailedentry-local}} linked to the filter created\n* $5 - text {{msg-mw|abusefilter-log-detailslink}} linked to the filter change details\n{{Identical|Created}}",
 	"abusefilter-logentry-modify": "Parameters:\n* $1 - a link to a user page with a user name as link text, followed by a series of related links\n* $2 - raw username, for GENDER support\n* $3 - (unused)\n* $4 - text {{msg-mw|abusefilter-log-detailedentry-local}} linked to the modified filter\n* $5 - text {{msg-mw|abusefilter-log-detailslink}} linked to the filter change details",
+	"abusefilter-log-invalid-filter": "Warning message shown above search results in the AbuseLog.",
 	"abusefilter-log-noresults": "{{Identical|No result}}",
 	"abusefilter-diff-title": "Similar to {{msg-mw|Difference}}",
 	"abusefilter-diff-item": "{{Identical|Item}}",

includes/AbuseFilter.php

@@ -363,28 +363,24 @@ class AbuseFilter {
 	}
 
 	/**
-	 * @param string|int $filter
+	 * @param int $filterID The ID of the filter
+	 * @param bool|int $global Whether the filter is global
 	 * @return bool
 	 */
-	public static function filterHidden( $filter ) {
-		if ( $filter === 'new' ) {
-			return false;
-		}
-		list( $filterID, $global ) = self::splitGlobalName( $filter );
+	public static function filterHidden( $filterID, $global = false ) {
 		if ( $global ) {
 			global $wgAbuseFilterCentralDB;
 			if ( !$wgAbuseFilterCentralDB ) {
 				return false;
 			}
 			$dbr = wfGetDB( DB_REPLICA, [], $wgAbuseFilterCentralDB );
-			$filter = $filterID;
 		} else {
 			$dbr = wfGetDB( DB_REPLICA );
 		}
 		$hidden = $dbr->selectField(
 			'abuse_filter',
 			'af_hidden',
-			[ 'af_id' => $filter ],
+			[ 'af_id' => $filterID ],
 			__METHOD__
 		);
 
@@ -823,6 +819,7 @@ class AbuseFilter {
 	 * @param int $id The filter ID
 	 * @param bool $global Whether the filter is global
 	 * @return string
+	 * @todo Calling this method should be avoided wherever possible
 	 */
 	public static function buildGlobalName( $id, $global = true ) {
 		$prefix = $global ? self::GLOBAL_FILTER_PREFIX : '';
@@ -1456,7 +1453,8 @@ class AbuseFilter {
 			}
 
 			if ( $wgAbuseFilterNotifications !== false ) {
-				if ( self::filterHidden( $data['afl_filter'] ) && !$wgAbuseFilterNotificationsPrivate ) {
+				list( $filterID, $global ) = self::splitGlobalName( $data['afl_filter'] );
+				if ( self::filterHidden( $filterID, $global ) && !$wgAbuseFilterNotificationsPrivate ) {
 					continue;
 				}
 				self::publishEntry( $dbw, $entry, $wgAbuseFilterNotifications );

includes/Views/AbuseFilterViewDiff.php

@@ -97,6 +97,11 @@ class AbuseFilterViewDiff extends AbuseFilterView {
 		$newSpec = $this->mParams[4];
 		$this->mFilter = $this->mParams[1];
 
+		if ( !is_numeric( $this->mFilter ) ) {
+			$this->getOutput()->addWikiMsg( 'abusefilter-diff-invalid' );
+			return false;
+		}
+
 		if ( AbuseFilter::filterHidden( $this->mFilter )
 			&& !$this->getUser()->isAllowedAny( 'abusefilter-modify', 'abusefilter-view-private' )
 		) {

includes/Views/AbuseFilterViewEdit.php

@@ -184,8 +184,9 @@ class AbuseFilterViewEdit extends AbuseFilterView {
 
 		// Hide hidden filters.
 		if ( ( ( isset( $row->af_hidden ) && $row->af_hidden ) ||
-				AbuseFilter::filterHidden( $filter ) )
-			&& !$this->canViewPrivate() ) {
+			( $filter !== 'new' && AbuseFilter::filterHidden( $filter ) ) ) &&
+			!$this->canViewPrivate()
+		) {
 			return $this->msg( 'abusefilter-edit-denied' )->escaped();
 		}
 

includes/Views/AbuseFilterViewExamine.php

@@ -165,7 +165,8 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 			return;
 		}
 
-		if ( !SpecialAbuseLog::canSeeDetails( $row->afl_filter ) ) {
+		list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
+		if ( !SpecialAbuseLog::canSeeDetails( $filterID, $global ) ) {
 			$out->addWikiMsg( 'abusefilter-log-cannot-see-details' );
 			return;
 		}

includes/api/ApiQueryAbuseLog.php

@@ -74,7 +74,8 @@ class ApiQueryAbuseLog extends ApiQueryBase {
 				$params['filter'] = [ $params['filter'] ];
 			}
 			foreach ( $params['filter'] as $filter ) {
-				if ( AbuseFilter::filterHidden( $filter ) ) {
+				list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filter );
+				if ( AbuseFilter::filterHidden( $filterID, $global ) ) {
 					$this->dieWithError(
 						[ 'apierror-permissiondenied', $this->msg( 'action-abusefilter-log-private' ) ]
 					);
@@ -172,7 +173,8 @@ class ApiQueryAbuseLog extends ApiQueryBase {
 					continue;
 				}
 			}
-			$canSeeDetails = SpecialAbuseLog::canSeeDetails( $row->afl_filter );
+			list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
+			$canSeeDetails = SpecialAbuseLog::canSeeDetails( $filterID, $global );
 
 			$entry = [];
 			if ( $fld_ids ) {
@@ -180,7 +182,6 @@ class ApiQueryAbuseLog extends ApiQueryBase {
 				$entry['filter_id'] = $canSeeDetails ? $row->afl_filter : '';
 			}
 			if ( $fld_filter ) {
-				list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
 				if ( $global ) {
 					$entry['filter'] = AbuseFilter::getGlobalFilterDescription( $filterID );
 				} else {

includes/pagers/AbuseFilterPager.php

@@ -211,7 +211,10 @@ class AbuseFilterPager extends TablePager {
 				$msg = $value ? 'abusefilter-hidden' : 'abusefilter-unhidden';
 				return $this->msg( $msg )->parse();
 			case 'af_hit_count':
-				if ( SpecialAbuseLog::canSeeDetails( $row->af_id, $row->af_hidden ) ) {
+				// Global here is used to determine whether the log entry is for an external, global
+				// filter, but all filters shown on Special:AbuseFilter are local.
+				$global = false;
+				if ( SpecialAbuseLog::canSeeDetails( $row->af_id, $global, $row->af_hidden ) ) {
 					$count_display = $this->msg( 'abusefilter-hitcount' )
 						->numParams( $value )->text();
 					$link = $this->linkRenderer->makeKnownLink(

includes/special/SpecialAbuseLog.php

@@ -314,16 +314,15 @@ class SpecialAbuseLog extends SpecialPage {
 
 		$dbr = wfGetDB( DB_REPLICA );
 
-		$row = $dbr->selectRow(
-			[ 'abuse_filter_log', 'abuse_filter' ],
+		$deleted = $dbr->selectField(
+			'abuse_filter_log',
 			'afl_deleted',
 			[ 'afl_id' => $id ],
-			__METHOD__,
-			[],
-			[ 'abuse_filter' => [ 'LEFT JOIN', 'af_id=afl_filter' ] ]
+			__METHOD__
 		);
 
-		if ( !$row ) {
+		if ( $deleted === false ) {
+			$output->addWikiMsg( 'abusefilter-log-nonexistent' );
 			return;
 		}
 
@@ -348,7 +347,7 @@ class SpecialAbuseLog extends SpecialPage {
 			],
 			'hidden' => [
 				'type' => 'toggle',
-				'default' => $row->afl_deleted,
+				'default' => $deleted,
 				'label-message' => 'abusefilter-log-hide-hidden',
 			],
 		];
@@ -455,16 +454,38 @@ class SpecialAbuseLog extends SpecialPage {
 
 		$searchFilters = [];
 		if ( $this->mSearchFilter ) {
-			$searchFilters = array_map( 'trim', explode( '|', $this->mSearchFilter ) );
+			$rawFilters = array_map( 'trim', explode( '|', $this->mSearchFilter ) );
+			// Map of [ [ id, global ], ... ]
+			$filtersList = [];
+			$foundInvalid = false;
+			foreach ( $rawFilters as $filter ) {
+				try {
+					$filtersList[] = AbuseFilter::splitGlobalName( $filter );
+				} catch ( InvalidArgumentException $e ) {
+					$foundInvalid = true;
+					continue;
+				}
+			}
+
+			if ( $foundInvalid ) {
+				$out->addHTML(
+					Html::rawElement(
+						'p',
+						[],
+						Html::warningBox( $this->msg( 'abusefilter-log-invalid-filter' )->escaped() )
+					)
+				);
+			}
+
 			// if a filter is hidden, users who can't view private filters should
 			// not be able to find log entries generated by it.
 			if ( !AbuseFilterView::canViewPrivate()
 				&& !$this->getUser()->isAllowed( 'abusefilter-log-private' )
 			) {
 				$searchedForPrivate = false;
-				foreach ( $searchFilters as $index => $filter ) {
-					if ( AbuseFilter::filterHidden( $filter ) ) {
-						unset( $searchFilters[$index] );
+				foreach ( $filtersList as $index => $filterData ) {
+					if ( AbuseFilter::filterHidden( ...$filterData ) ) {
+						unset( $filtersList[$index] );
 						$searchedForPrivate = true;
 					}
 				}
@@ -472,6 +493,10 @@ class SpecialAbuseLog extends SpecialPage {
 					$out->addWikiMsg( 'abusefilter-log-private-not-included' );
 				}
 			}
+
+			foreach ( $filtersList as $filterData ) {
+				$searchFilters[] = AbuseFilter::buildGlobalName( ...$filterData );
+			}
 		}
 
 		$searchIDs = null;
@@ -576,13 +601,14 @@ class SpecialAbuseLog extends SpecialPage {
 		if ( !$row ) {
 			$error = 'abusefilter-log-nonexistent';
 		} else {
-			if ( AbuseFilter::splitGlobalName( $row->afl_filter )[1] ) {
+			list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
+			if ( $global ) {
 				$filter_hidden = null;
 			} else {
 				$filter_hidden = $row->af_hidden;
 			}
 
-			if ( !self::canSeeDetails( $row->afl_filter, $filter_hidden ) ) {
+			if ( !self::canSeeDetails( $filterID, $global, $filter_hidden ) ) {
 				$error = 'abusefilter-log-cannot-see-details';
 			} elseif ( self::isHidden( $row ) === true && !self::canSeeHidden() ) {
 				$error = 'abusefilter-log-details-hidden';
@@ -711,13 +737,14 @@ class SpecialAbuseLog extends SpecialPage {
 		if ( !$row ) {
 			$error = 'abusefilter-log-nonexistent';
 		} else {
-			if ( AbuseFilter::splitGlobalName( $row->afl_filter )[1] ) {
+			list( $filterID, $global ) = AbuseFilter::splitGlobalName( $row->afl_filter );
+			if ( $global ) {
 				$filter_hidden = null;
 			} else {
 				$filter_hidden = $row->af_hidden;
 			}
 
-			if ( !self::canSeeDetails( $row->afl_filter, $filter_hidden ) ) {
+			if ( !self::canSeeDetails( $filterID, $global, $filter_hidden ) ) {
 				$error = 'abusefilter-log-cannot-see-details';
 			} elseif ( !self::canSeePrivate() ) {
 				$error = 'abusefilter-log-cannot-see-private-details';
@@ -906,18 +933,19 @@ class SpecialAbuseLog extends SpecialPage {
 	}
 
 	/**
-	 * @param string|int|null $filter_id
-	 * @param bool|int|null $filter_hidden
+	 * @param int|null $id The ID of the filter
+	 * @param bool|int|null $global Whether the filter is global
+	 * @param bool|int|null $hidden Whether the filter is hidden
 	 * @return bool
 	 */
-	public static function canSeeDetails( $filter_id = null, $filter_hidden = null ) {
+	public static function canSeeDetails( $id = null, $global = false, $hidden = null ) {
 		global $wgUser;
 
-		if ( $filter_id !== null ) {
-			if ( $filter_hidden === null ) {
-				$filter_hidden = AbuseFilter::filterHidden( $filter_id );
+		if ( $id !== null ) {
+			if ( $hidden === null ) {
+				$hidden = AbuseFilter::filterHidden( $id, $global );
 			}
-			if ( $filter_hidden ) {
+			if ( $hidden ) {
 				return $wgUser->isAllowed( 'abusefilter-log-detail' ) && (
 					AbuseFilterView::canViewPrivate() || $wgUser->isAllowed( 'abusefilter-log-private' )
 				);
@@ -1025,7 +1053,7 @@ class SpecialAbuseLog extends SpecialPage {
 			$filter_hidden = $row->af_hidden;
 		}
 
-		if ( self::canSeeDetails( $row->afl_filter, $filter_hidden ) ) {
+		if ( self::canSeeDetails( $filterID, $global, $filter_hidden ) ) {
 			if ( $isListItem ) {
 				$detailsLink = $linkRenderer->makeKnownLink(
 					$this->getPageTitle( $row->afl_id ),
@@ -1065,9 +1093,9 @@ class SpecialAbuseLog extends SpecialPage {
 					->numParams( $filterID )->text();
 				$filterLink = Linker::makeExternalLink( $globalURL, $linkText );
 			} else {
-				$title = SpecialPage::getTitleFor( 'AbuseFilter', $row->afl_filter );
+				$title = SpecialPage::getTitleFor( 'AbuseFilter', $filterID );
 				$linkText = $this->msg( 'abusefilter-log-detailedentry-local' )
-					->numParams( $row->afl_filter )->text();
+					->numParams( $filterID )->text();
 				$filterLink = $linkRenderer->makeKnownLink( $title, $linkText );
 			}
 			$description = $this->msg( 'abusefilter-log-detailedentry-meta' )->rawParams(