diff --git a/extension.json b/extension.json index 8d7f7c22a..ffe5351a4 100644 --- a/extension.json +++ b/extension.json @@ -160,7 +160,8 @@ }, "abusefilter-protected-vars": { "change-access": [ - "change-access" + "change-access-enable", + "change-access-disable" ] } }, diff --git a/includes/Special/SpecialAbuseLog.php b/includes/Special/SpecialAbuseLog.php index ec8d617c6..9ded5616f 100644 --- a/includes/Special/SpecialAbuseLog.php +++ b/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'; } diff --git a/tests/phpunit/integration/ProtectedVarsAccessLoggerTest.php b/tests/phpunit/integration/ProtectedVarsAccessLoggerTest.php index cd6657dec..0a58e2b8a 100644 --- a/tests/phpunit/integration/ProtectedVarsAccessLoggerTest.php +++ b/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();