Bugfix: Fix minor issues with protected vars logging

- Fix an issue where if a user didn't have view permissions they could
  get the preference check error (a preference they wouldn't have) on
  SpecialAbuseLog
- Fix an issue where the `change-access` hadn't been updated to the used
  disabled/enabled log types
- Fix an issue where a ProtectedVarsAccessLoggerTest test wasn't
  correctly using the data provider data
- Improve naming since ProtectedVarsAccessLogger exists in its own test
  file instead of being a subset of tests on AbuseLoggerTest

Bug: T371798
Change-Id: I53f22855e63d9e1339361a5c9ee7886e0f74714a

extension.json

@@ -160,7 +160,8 @@
 		},
 		"abusefilter-protected-vars": {
 			"change-access": [
-				"change-access"
+				"change-access-enable",
+				"change-access-disable"
 			]
 		}
 	},

includes/Special/SpecialAbuseLog.php

@@ -767,9 +767,12 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
 				}
 			}
 
+			// Only show the preference error if another error isn't already set
+			// as this error shouldn't take precedence over a view permission error
 			if (
 				FilterUtils::isProtected( $privacyLevel ) &&
-				!$this->afPermissionManager->canViewProtectedVariableValues( $performer )
+				!$this->afPermissionManager->canViewProtectedVariableValues( $performer ) &&
+				!$error
 			) {
 				$error = 'abusefilter-examine-protected-vars-permission';
 			}

tests/phpunit/integration/ProtectedVarsAccessLoggerTest.php

@@ -41,15 +41,16 @@ class ProtectedVarsAccessLoggerTest extends MediaWikiIntegrationTestCase {
 	/**
 	 * @dataProvider provideProtectedVarsLogTypes
 	 */
-	public function testProtectedVarsLoggerNoCUEnabled() {
+	public function testLogs_CUDisabled( $options, $expected ) {
 		$extensionRegistry = $this->createMock( ExtensionRegistry::class );
 		$extensionRegistry->method( 'isLoaded' )->with( 'CheckUser' )->willReturn( false );
 		$this->setService( 'ExtensionRegistry', $extensionRegistry );
 
 		$performer = $this->getTestSysop();
+		$logAction = $options['logAction'];
 		AbuseFilterServices::getAbuseLoggerFactory()
 			->getProtectedVarsAccessLogger()
-			->logAccessEnabled( $performer->getUserIdentity() );
+			->$logAction( $performer->getUserIdentity(), ...$options['params'] );
 
 		// Assert that the action wasn't inserted into CheckUsers' temp account logging table
 		$this->assertSame(
@@ -58,7 +59,7 @@ class ProtectedVarsAccessLoggerTest extends MediaWikiIntegrationTestCase {
 				->select( 'COUNT(*)' )
 				->from( 'logging' )
 				->where( [
-					'log_action' => 'af-change-access-enable',
+					'log_action' => $expected['expectedCULogType'],
 					'log_type' => TemporaryAccountLogger::LOG_TYPE,
 					] )
 				->fetchField()
@@ -70,7 +71,7 @@ class ProtectedVarsAccessLoggerTest extends MediaWikiIntegrationTestCase {
 				->select( 'COUNT(*)' )
 				->from( 'logging' )
 				->where( [
-					'log_action' => 'change-access-enable',
+					'log_action' => $expected['expectedAFLogType'],
 					'log_type' => ProtectedVarsAccessLogger::LOG_TYPE,
 					] )
 				->fetchField()
@@ -82,7 +83,7 @@ class ProtectedVarsAccessLoggerTest extends MediaWikiIntegrationTestCase {
 	/**
 	 * @dataProvider provideProtectedVarsLogTypes
 	 */
-	public function testProtectedVarsLoggerSendingLogsToCUTable( $options, $expected ) {
+	public function testLogs_CUEnabled( $options, $expected ) {
 		$this->markTestSkippedIfExtensionNotLoaded( 'CheckUser' );
 
 		$performer = $this->getTestSysop();