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

ApiOptions: fix resetting some preferences to default

Browse source 
For preferences like 'skin' that have a limited number of values, null
is not a valid value, thus attempts to reset them fail with
"Validation error for \"skin\": This value is required."

Bug: T65080

Change-Id: I86554a6d30c8ab970740d8682fb2261476de0677
This commit is contained in:
Max Semenik 2018-07-13 20:14:06 -07:00
parent 28dea19eae
commit 3ea7bba4d4
3 changed files with 129 additions and 145 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
RELEASE-NOTES-1.32
View file

@ -122,6 +122,7 @@ production.
* SpecialPage::execute() will now only call checkLoginSecurityLevel() if * SpecialPage::execute() will now only call checkLoginSecurityLevel() if
getLoginSecurityLevel() returns non-false. getLoginSecurityLevel() returns non-false.
* (T43720, T46197) Improved page display title handling for category pages * (T43720, T46197) Improved page display title handling for category pages
* (T65080) Fixed resetting options of some types via API action=options.
=== Action API changes in 1.32 === === Action API changes in 1.32 ===
* Added templated parameters. * Added templated parameters.

16
includes/api/ApiOptions.php
View file

@ -80,12 +80,18 @@ class ApiOptions extends ApiBase {
switch ( $prefsKinds[$key] ) { switch ( $prefsKinds[$key] ) {
case 'registered': case 'registered':
// Regular option. // Regular option.
if ( $htmlForm === null ) { if ( $value === null ) {
// We need a dummy HTMLForm for the validate callback... // Reset it
$htmlForm = new HTMLForm( [], $this ); $validation = true;
} else {
// Validate
if ( $htmlForm === null ) {
// We need a dummy HTMLForm for the validate callback...
$htmlForm = new HTMLForm( [], $this );
}
$field = HTMLForm::loadInputFromParameters( $key, $prefs[$key], $htmlForm );
$validation = $field->validate( $value, $user->getOptions() );
} }
$field = HTMLForm::loadInputFromParameters( $key, $prefs[$key], $htmlForm );
$validation = $field->validate( $value, $user->getOptions() );
break; break;
case 'registered-multiselect': case 'registered-multiselect':
case 'registered-checkmatrix': case 'registered-checkmatrix':

257
tests/phpunit/includes/api/ApiOptionsTest.php
View file

@ -61,6 +61,11 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
[ $this, 'hookGetPreferences' ] [ $this, 'hookGetPreferences' ]
] ]
] ); ] );
$this->mergeMwGlobalArrayValue( 'wgDefaultUserOptions', [
'testradio' => 'option1',
] );
// Workaround for static caching in User::getDefaultOptions()
$this->setContentLang( Language::factory( 'qqq' ) );
} }
public function hookGetPreferences( $user, &$preferences ) { public function hookGetPreferences( $user, &$preferences ) {
@ -90,7 +95,11 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
'default' => [], 'default' => [],
]; ];
return true; $preferences['testradio'] = [
'type' => 'radio',
'options' => [ 'Option 1' => 'option1', 'Option 2' => 'option2' ],
'section' => 'test',
];
} }
/** /**
@ -106,6 +115,7 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
'willBeNull' => 'registered', 'willBeNull' => 'registered',
'willBeEmpty' => 'registered', 'willBeEmpty' => 'registered',
'willBeHappy' => 'registered', 'willBeHappy' => 'registered',
'testradio' => 'registered',
'testmultiselect-opt1' => 'registered-multiselect', 'testmultiselect-opt1' => 'registered-multiselect',
'testmultiselect-opt2' => 'registered-multiselect', 'testmultiselect-opt2' => 'registered-multiselect',
'testmultiselect-opt3' => 'registered-multiselect', 'testmultiselect-opt3' => 'registered-multiselect',
@ -243,65 +253,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response ); $this->assertEquals( self::$Success, $response );
} }
public function testOptionWithValue() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
$this->mUserMock->expects( $this->once() )
->method( 'setOption' )
->with( $this->equalTo( 'name' ), $this->equalTo( 'value' ) );
$this->mUserMock->expects( $this->once() )
->method( 'saveSettings' );
$request = $this->getSampleRequest( [ 'optionname' => 'name', 'optionvalue' => 'value' ] );
$response = $this->executeQuery( $request );
$this->assertEquals( self::$Success, $response );
}
public function testOptionResetValue() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
$this->mUserMock->expects( $this->once() )
->method( 'setOption' )
->with( $this->equalTo( 'name' ), $this->identicalTo( null ) );
$this->mUserMock->expects( $this->once() )
->method( 'saveSettings' );
$request = $this->getSampleRequest( [ 'optionname' => 'name' ] );
$response = $this->executeQuery( $request );
$this->assertEquals( self::$Success, $response );
}
public function testChange() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
$this->mUserMock->expects( $this->exactly( 3 ) )
->method( 'setOption' )
->withConsecutive(
[ $this->equalTo( 'willBeNull' ), $this->identicalTo( null ) ],
[ $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) ],
[ $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) ]
);
$this->mUserMock->expects( $this->once() )
->method( 'saveSettings' );
$request = $this->getSampleRequest( [
'change' => 'willBeNull|willBeEmpty=|willBeHappy=Happy'
] );
$response = $this->executeQuery( $request );
$this->assertEquals( self::$Success, $response );
}
public function testResetChangeOption() { public function testResetChangeOption() {
$this->mUserMock->expects( $this->once() ) $this->mUserMock->expects( $this->once() )
->method( 'resetOptions' ); ->method( 'resetOptions' );
@ -328,95 +279,121 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response ); $this->assertEquals( self::$Success, $response );
} }
public function testMultiSelect() { /**
* @dataProvider provideOptionManupulation
* @param array $params
* @param array $setOptions
* @param array|null $result
*/
public function testOptionManupulation( array $params, array $setOptions, array $result = null,
$message = ''
) {
$this->mUserMock->expects( $this->never() ) $this->mUserMock->expects( $this->never() )
->method( 'resetOptions' ); ->method( 'resetOptions' );
$this->mUserMock->expects( $this->exactly( 4 ) ) $this->mUserMock->expects( $this->exactly( count( $setOptions ) ) )
->method( 'setOption' ) ->method( 'setOption' )
->withConsecutive( ->withConsecutive( ...$setOptions );
[ $this->equalTo( 'testmultiselect-opt1' ), $this->identicalTo( true ) ],
[ $this->equalTo( 'testmultiselect-opt2' ), $this->identicalTo( null ) ],
[ $this->equalTo( 'testmultiselect-opt3' ), $this->identicalTo( false ) ],
[ $this->equalTo( 'testmultiselect-opt4' ), $this->identicalTo( false ) ]
);
$this->mUserMock->expects( $this->once() ) if ( $setOptions ) {
->method( 'saveSettings' ); $this->mUserMock->expects( $this->once() )
->method( 'saveSettings' );
$request = $this->getSampleRequest( [ } else {
'change' => 'testmultiselect-opt1=1|testmultiselect-opt2|' $this->mUserMock->expects( $this->never() )
. 'testmultiselect-opt3=|testmultiselect-opt4=0' ->method( 'saveSettings' );
] ); }
$request = $this->getSampleRequest( $params );
$response = $this->executeQuery( $request ); $response = $this->executeQuery( $request );
$this->assertEquals( self::$Success, $response ); if ( !$result ) {
$result = self::$Success;
}
$this->assertEquals( $result, $response, $message );
} }
public function testSpecialOption() { public function provideOptionManupulation() {
$this->mUserMock->expects( $this->never() ) return [
->method( 'resetOptions' ); [
[ 'change' => 'userjs-option=1' ],
$this->mUserMock->expects( $this->never() ) [ [ 'userjs-option', '1' ] ],
->method( 'saveSettings' ); null,
'Setting userjs options',
$request = $this->getSampleRequest( [ ],
'change' => 'special=1' [
] ); [ 'change' => 'willBeNull|willBeEmpty=|willBeHappy=Happy' ],
[
$response = $this->executeQuery( $request ); [ 'willBeNull', null ],
[ 'willBeEmpty', '' ],
$this->assertEquals( [ [ 'willBeHappy', 'Happy' ],
'options' => 'success', ],
'warnings' => [ null,
'options' => [ 'Basic option setting',
'warnings' => "Validation error for \"special\": cannot be set by this module." ],
] [
] [ 'change' => 'testradio=option2' ],
], $response ); [ [ 'testradio', 'option2' ] ],
} null,
'Changing radio options',
public function testUnknownOption() { ],
$this->mUserMock->expects( $this->never() ) [
->method( 'resetOptions' ); [ 'change' => 'testradio' ],
[ [ 'testradio', null ] ],
$this->mUserMock->expects( $this->never() ) null,
->method( 'saveSettings' ); 'Resetting radio options',
],
$request = $this->getSampleRequest( [ [
'change' => 'unknownOption=1' [ 'change' => 'unknownOption=1' ],
] ); [],
[
$response = $this->executeQuery( $request ); 'options' => 'success',
'warnings' => [
$this->assertEquals( [ 'options' => [
'options' => 'success', 'warnings' => "Validation error for \"unknownOption\": not a valid preference."
'warnings' => [ ],
'options' => [ ],
'warnings' => "Validation error for \"unknownOption\": not a valid preference." ],
] 'Unrecognized options should be rejected',
] ],
], $response ); [
} [ 'change' => 'special=1' ],
[],
public function testUserjsOption() { [
$this->mUserMock->expects( $this->never() ) 'options' => 'success',
->method( 'resetOptions' ); 'warnings' => [
'options' => [
$this->mUserMock->expects( $this->once() ) 'warnings' => "Validation error for \"special\": cannot be set by this module."
->method( 'setOption' ) ]
->with( $this->equalTo( 'userjs-option' ), $this->equalTo( '1' ) ); ]
],
$this->mUserMock->expects( $this->once() ) 'Refuse setting special options',
->method( 'saveSettings' ); ],
[
$request = $this->getSampleRequest( [ [
'change' => 'userjs-option=1' 'change' => 'testmultiselect-opt1=1|testmultiselect-opt2|'
] ); . 'testmultiselect-opt3=|testmultiselect-opt4=0'
],
$response = $this->executeQuery( $request ); [
[ 'testmultiselect-opt1', true ],
$this->assertEquals( self::$Success, $response ); [ 'testmultiselect-opt2', null ],
[ 'testmultiselect-opt3', false ],
[ 'testmultiselect-opt4', false ],
],
null,
'Setting multiselect options',
],
[
[ 'optionname' => 'name', 'optionvalue' => 'value' ],
[ [ 'name', 'value' ] ],
null,
'Setting options via optionname/optionvalue'
],
[
[ 'optionname' => 'name' ],
[ [ 'name', null ] ],
null,
'Resetting options via optionname without optionvalue',
],
];
} }
} }