AbuseFilterViewEdit: Expose status object to form builder

Why:

* For improvements to the protected filters workflow (T377765),
  a checkbox needs to be displayed conditionally on the status
  of a previously submitted form.

What:

* Pass a status object into ::buildFilterEditor, and build the
  HTML for any warnings or errors in that function, instead of
  first building the HTML and passing it.
* This also allows more than one error and/or warning to be
  shown if present, though this may not happen in practice yet.

Change-Id: I37da49711e7fc192856f013cd931c05379f8e8c6
This commit is contained in:
Thalia 2024-11-13 12:02:15 +00:00
parent 5c5d64fcb9
commit 6a14ea54fb
2 changed files with 79 additions and 20 deletions

View file

@ -26,6 +26,7 @@ use MediaWiki\Permissions\PermissionManager;
use MediaWiki\SpecialPage\SpecialPage;
use MediaWiki\Xml\Xml;
use OOUI;
use StatusValue;
use UnexpectedValueException;
use Wikimedia\Rdbms\IDBAccessObject;
use Wikimedia\Rdbms\IExpression;
@ -205,21 +206,20 @@ class AbuseFilterViewEdit extends AbuseFilterView {
if ( !$tokenMatches ) {
// Token invalid or expired while the page was open, warn to retry
$error = Html::warningBox( $this->msg( 'abusefilter-edit-token-not-match' )->parseAsBlock() );
$this->buildFilterEditor( $error, $newFilter, $filter, $history_id );
$status = StatusValue::newGood();
$status->warning( 'abusefilter-edit-token-not-match' );
$this->buildFilterEditor( $status, $newFilter, $filter, $history_id );
return;
}
$status = $this->filterStore->saveFilter( $user, $filter, $newFilter, $origFilter );
if ( !$status->isGood() ) {
$msg = $status->getMessages()[0];
if ( $status->isOK() ) {
// Fixable error, show the editing interface
$error = Html::errorBox( $this->msg( $msg )->parseAsBlock() );
$this->buildFilterEditor( $error, $newFilter, $filter, $history_id );
$this->buildFilterEditor( $status, $newFilter, $filter, $history_id );
} else {
$this->showUnrecoverableError( $msg );
$this->showUnrecoverableError( $status->getMessages()[0] );
}
} elseif ( $status->getValue() === false ) {
// No change
@ -263,13 +263,13 @@ class AbuseFilterViewEdit extends AbuseFilterView {
* 4 - Load an old version of an existing filter
* 5 - Show the user input again if saving fails after one of the steps above
*
* @param string|null $error An error message to show above the filter box (HTML).
* @param StatusValue|null $status A status for showing warnings or errors above the filter box
* @param Filter $filterObj
* @param int|null $filter The filter ID, or null for a new filter
* @param int|null $history_id The history ID of the filter, if applicable. Otherwise null
*/
private function buildFilterEditor(
$error,
?StatusValue $status,
Filter $filterObj,
?int $filter,
$history_id
@ -332,8 +332,14 @@ class AbuseFilterViewEdit extends AbuseFilterView {
$out->addWikiMsg( $oldWarningMessage, $history_id, $filter );
}
if ( $error !== null ) {
$out->addHTML( $error );
if ( $status !== null && !$status->isGood() ) {
foreach ( $status->getMessages( 'error' ) as $message ) {
$out->addHTML( Html::errorBox( $this->msg( $message )->parseAsBlock() ) );
}
foreach ( $status->getMessages( 'warning' ) as $message ) {
$out->addHTML( Html::warningBox( $this->msg( $message )->parseAsBlock() ) );
}
}
$fields = [];

View file

@ -44,12 +44,12 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
/**
* @var SimpleAuthority
*/
private $authorityCannotViewProtectedVar;
private $authorityCannotUseProtectedVar;
/**
* @var SimpleAuthority
*/
private $authorityCanViewProtectedVar;
private $authorityCanUseProtectedVar;
protected function setUp(): void {
parent::setUp();
@ -78,15 +78,24 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
$user = $this->getTestSysop()->getUser();
// Create an authority who can see private filters but not protected variables
$this->authorityCannotViewProtectedVar = new SimpleAuthority(
$this->authorityCannotUseProtectedVar = new SimpleAuthority(
$user,
[ 'abusefilter-log-private', 'abusefilter-view-private' ]
[
'abusefilter-log-private',
'abusefilter-view-private',
'abusefilter-modify',
]
);
// Create an authority who can see private and protected variables
$this->authorityCanViewProtectedVar = new SimpleAuthority(
$this->authorityCanUseProtectedVar = new SimpleAuthority(
$user,
[ 'abusefilter-access-protected-vars', 'abusefilter-log-private', 'abusefilter-view-private' ]
[
'abusefilter-access-protected-vars',
'abusefilter-log-private',
'abusefilter-view-private',
'abusefilter-modify',
]
);
}
@ -186,13 +195,57 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
->execute();
}
public function testViewEditTokenMismatch() {
[ $html, ] = $this->executeSpecialPage(
'new',
new FauxRequest(
[
'wpFilterDescription' => 'Test filter',
'wpFilterRules' => 'user_name = "1.2.3.4"',
'wpFilterNotes' => '',
],
// This was posted
true,
),
null,
$this->authorityCannotUseProtectedVar
);
$this->assertStringContainsString(
'abusefilter-edit-token-not-match',
$html,
'The token mismatch warning message was not present.'
);
}
public function testViewEditUnrecoverableError() {
[ $html, $response ] = $this->executeSpecialPage(
'new',
new FauxRequest(
[
'wpFilterDescription' => '',
'wpFilterRules' => 'user_name = "1.2.3.4"',
'wpFilterNotes' => '',
],
// This was posted
true,
)
);
$this->assertStringContainsString(
'abusefilter-edit-notallowed',
$html,
'The permission error message was not present.'
);
}
public function testViewTestBatchProtectedVarsFilterVisibility() {
// Assert that the user who cannot see protected variables cannot load the filter
[ $html, ] = $this->executeSpecialPage(
'test/1',
new FauxRequest(),
null,
$this->authorityCannotViewProtectedVar
$this->authorityCannotUseProtectedVar
);
$this->assertStringNotContainsString( '1.2.3.4', $html );
@ -201,7 +254,7 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
'test/1',
new FauxRequest(),
null,
$this->authorityCanViewProtectedVar
$this->authorityCanUseProtectedVar
);
$this->assertStringContainsString( '1.2.3.4', $html );
}
@ -226,7 +279,7 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
'',
$requestWithProtectedVar,
null,
$this->authorityCannotViewProtectedVar
$this->authorityCannotUseProtectedVar
);
$this->assertStringContainsString( 'table_pager_empty', $html );
@ -235,7 +288,7 @@ class SpecialAbuseFilterTest extends SpecialPageTestBase {
'',
$requestWithProtectedVar,
null,
$this->authorityCanViewProtectedVar
$this->authorityCanUseProtectedVar
);
$this->assertStringContainsString( '1.2.3.4', $html );
}