Thiemo Mättig (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/365581 )
Change subject: Never return undefined from RepoApi.normalizeMultiValue
......................................................................
Never return undefined from RepoApi.normalizeMultiValue
Note how "undefined" is strictly forbidden in the "_post" method, see
line #885:
if ( value === undefined … ) {
throw new Error( …
This was the error that made the browser tests fail in Ib1b9e30, even
if MediaWiki core and the Wikibase API module would have been fine with
unused parameters set to "undefined".
I would go so far and call this an actual bug in this class. The private
method "normalizeMultiValue" turns empty strings into "undefined", but this
is guaranteed to fail in the private "_post" method. The only way to work
around this was to provide empty arrays instead of empty strings (which is
what actually happens in production code). This should be consistent.
Change-Id: I84d360f7532c28353aff39baa791cbeffe7b6768
---
M src/RepoApi.js
M tests/RepoApi.tests.js
2 files changed, 13 insertions(+), 15 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseJavaScriptApi
refs/changes/81/365581/1
diff --git a/src/RepoApi.js b/src/RepoApi.js
index 414cece..8e52061 100644
--- a/src/RepoApi.js
+++ b/src/RepoApi.js
@@ -409,8 +409,8 @@
*
* @param {string} id `Entity` id.
* @param {number} baseRevId Revision id the edit shall be performed on.
- * @param {string|string[]} add Alias(es) to add.
- * @param {string|string[]} remove Alias(es) to remove.
+ * @param {string[]} add Aliases to add.
+ * @param {string[]} remove Aliases to remove.
* @param {string} language Language code of the language the aliases
should be added/removed
* in.
* @return {Object} jQuery.Promise
@@ -845,17 +845,15 @@
* @private
*
* @param {string[]|string|null} [value]
- * @return {string|undefined}
+ * @return {string}
*/
normalizeMultiValue: function ( value ) {
if ( Array.isArray( value ) ) {
- return value.map( function ( item ) {
- return '\x1f' + item;
- } ).join( '' );
+ value = value.join( '\x1f' );
}
// We must enforce the alternative separation character, see
ApiBase.php::explodeMultiValue.
- return value ? '\x1f' + value : undefined;
+ return value ? '\x1f' + value : '';
},
/**
diff --git a/tests/RepoApi.tests.js b/tests/RepoApi.tests.js
index ed0cb96..6124e04 100644
--- a/tests/RepoApi.tests.js
+++ b/tests/RepoApi.tests.js
@@ -200,8 +200,8 @@
assert.equal( getParam( mock.spy, 'languages', 1 ), '\x1flanguage code'
);
assert.equal( getParam( mock.spy, 'ids', 2 ), '\x1fentity id' );
- assert.strictEqual( getParam( mock.spy, 'props', 2 ), undefined );
- assert.strictEqual( getParam( mock.spy, 'languages', 2 ), undefined );
+ assert.strictEqual( getParam( mock.spy, 'props', 2 ), '' );
+ assert.strictEqual( getParam( mock.spy, 'languages', 2 ), '' );
} );
QUnit.test( 'getEntitiesByPage()', function( assert ) {
@@ -264,20 +264,20 @@
assert.equal( getParam( mock.spy, 'sites', 3 ), '\x1fsite id' );
assert.equal( getParam( mock.spy, 'titles', 3 ), '\x1ftitle' );
- assert.strictEqual( getParam( mock.spy, 'props', 3 ), undefined );
- assert.strictEqual( getParam( mock.spy, 'languages', 3 ), undefined );
+ assert.strictEqual( getParam( mock.spy, 'props', 3 ), '' );
+ assert.strictEqual( getParam( mock.spy, 'languages', 3 ), '' );
assert.strictEqual( getParam( mock.spy, 'normalize', 3 ), undefined );
assert.equal( getParam( mock.spy, 'sites', 4 ), '\x1fsite id' );
assert.equal( getParam( mock.spy, 'titles', 4 ), '\x1ftitle' );
- assert.strictEqual( getParam( mock.spy, 'props', 4 ), undefined );
- assert.strictEqual( getParam( mock.spy, 'languages', 4 ), undefined );
+ assert.strictEqual( getParam( mock.spy, 'props', 4 ), '' );
+ assert.strictEqual( getParam( mock.spy, 'languages', 4 ), '' );
assert.strictEqual( getParam( mock.spy, 'normalize', 4 ), undefined );
assert.equal( getParam( mock.spy, 'sites', 5 ), '\x1fsite id' );
assert.equal( getParam( mock.spy, 'titles', 5 ), '\x1ftitle' );
- assert.strictEqual( getParam( mock.spy, 'props', 5 ), undefined );
- assert.strictEqual( getParam( mock.spy, 'languages', 5 ), undefined );
+ assert.strictEqual( getParam( mock.spy, 'props', 5 ), '' );
+ assert.strictEqual( getParam( mock.spy, 'languages', 5 ), '' );
assert.strictEqual( getParam( mock.spy, 'normalize', 5 ), undefined );
} );
--
To view, visit https://gerrit.wikimedia.org/r/365581
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I84d360f7532c28353aff39baa791cbeffe7b6768
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseJavaScriptApi
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
