Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Set $status->value in EditFilterMergedContentHookConstraint::checkConstraint() properly to display error message

Browse source 
When hook handler return false and use $status->fatal() to put up a error,
we should set $status->value to self::AS_HOOK_ERROR_EXPECTED to show edit form
and display error message.

Bug: T273354
Change-Id: I02d643c5cb1978da8ab749856493d75137b6cb02
This commit is contained in:
Func 2021-02-17 04:11:46 +00:00 committed by Bartosz Dziewoński
parent 94d14ef42e
commit 278e195ab7
4 changed files with 8 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
RELEASE-NOTES-1.37
View file

@ -81,6 +81,8 @@ because of Phabricator reports.
- findVariantLink()
- mConverter
- updateConversionTable()
* (T273354) When an edit is prevented by an 'EditFilterMergedContent' hook
handler without changing the status, the edit form will now be displayed.
=== Deprecations in 1.37 ===
* JobQueue::getWiki, deprecated in 1.33, now emits deprecation warnings.

3
includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php
View file

@ -102,7 +102,8 @@ class EditFilterMergedContentHookConstraint implements IEditConstraint {
}
// Use the existing $status->value if the hook set it
if ( !$this->status->value ) {
$this->status->value = self::AS_HOOK_ERROR;
// T273354: Should be AS_HOOK_ERROR_EXPECTED to display error message
$this->status->value = self::AS_HOOK_ERROR_EXPECTED;
}
return self::CONSTRAINT_FAILED;
}

4
tests/phpunit/includes/EditPageConstraintsTest.php
View file

@ -376,13 +376,13 @@ class EditPageConstraintsTest extends MediaWikiLangTestCase {
public function provideTestEditFilterMergedContentHookConstraint() {
yield 'Hook returns false, status is good, no value set' => [
false, null, false, EditPage::AS_HOOK_ERROR, 'AS_HOOK_ERROR'
false, null, false, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'
];
yield 'Hook returns false, status is good, value set' => [
false, 1234567, false, 1234567, 'custom value 1234567'
];
yield 'Hook returns false, status is not good' => [
false, null, true, EditPage::AS_HOOK_ERROR, 'AS_HOOK_ERROR'
false, null, true, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'
];
yield 'Hook returns true, status is not ok' => [
true, null, true, EditPage::AS_HOOK_ERROR_EXPECTED, 'AS_HOOK_ERROR_EXPECTED'

4
tests/phpunit/unit/includes/editpage/Constraint/EditFilterMergedContentHookConstraintTest.php
View file

@ -66,9 +66,9 @@ class EditFilterMergedContentHookConstraintTest extends MediaWikiUnitTestCase {
public function testFailure_goodStatus() {
// Code path 1: Hook returns false, but status is still good
// Status has no value set, falls back to AS_HOOK_ERROR
// Status has no value set, falls back to AS_HOOK_ERROR_EXPECTED
$constraint = $this->getConstraint( false );
$this->assertConstraintFailed( $constraint, IEditConstraint::AS_HOOK_ERROR );
$this->assertConstraintFailed( $constraint, IEditConstraint::AS_HOOK_ERROR_EXPECTED );
}
public function testFailure_badStatus() {