jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/348166 )
Change subject: TagMultiselectWidget: Redo data validation for Tag* and Menu*
......................................................................
TagMultiselectWidget: Redo data validation for Tag* and Menu*
The menu adds options to consider, but we still want to make
sure we are testing on the basics
(arbitrary/duplicates/allowed list/etc). Make sure that the
isAllowedData method is running properly in the base class
and also in the Menu class.
Also, add unit tests for data validation.
Change-Id: Id32f37d9bf1828cae616effabf3c2377697cd6fe
---
M src/widgets/MenuTagMultiselectWidget.js
M src/widgets/TagMultiselectWidget.js
M tests/index.php
A tests/widgets/MenuTagMultiselectWidget.test.js
M tests/widgets/TagMultiselectWidget.test.js
5 files changed, 102 insertions(+), 14 deletions(-)
Approvals:
jenkins-bot: Verified
VolkerE: Looks good to me, approved
Jforrester: Looks good to me, but someone else must approve
diff --git a/src/widgets/MenuTagMultiselectWidget.js
b/src/widgets/MenuTagMultiselectWidget.js
index 51a8986..9785221 100644
--- a/src/widgets/MenuTagMultiselectWidget.js
+++ b/src/widgets/MenuTagMultiselectWidget.js
@@ -222,11 +222,15 @@
};
/**
- * @inheritdoc
+ * Get the allowed values list
+ *
+ * @return {string[]} Allowed data values
*/
-OO.ui.MenuTagMultiselectWidget.prototype.isAllowedData = function ( data ) {
- return
OO.ui.MenuTagMultiselectWidget.parent.prototype.isAllowedData.call( this, data
) &&
- !!this.menu.getItemFromData( data );
+OO.ui.MenuTagMultiselectWidget.prototype.getAllowedValues = function () {
+ var menuDatas = this.menu.getItems().map( function ( menuItem ) {
+ return menuItem.getData();
+ } );
+ return this.allowedValues.concat( menuDatas );
};
/**
diff --git a/src/widgets/TagMultiselectWidget.js
b/src/widgets/TagMultiselectWidget.js
index 1aafda5..2a76ef1 100644
--- a/src/widgets/TagMultiselectWidget.js
+++ b/src/widgets/TagMultiselectWidget.js
@@ -473,11 +473,9 @@
* Check whether a given value is allowed to be added
*
* @param {string|Object} data Requested value
- * @return {boolean} Value exists in the allowed values list
+ * @return {boolean} Value is allowed
*/
OO.ui.TagMultiselectWidget.prototype.isAllowedData = function ( data ) {
- var hash = OO.getHash( data );
-
if ( this.allowArbitrary ) {
return true;
}
@@ -492,7 +490,7 @@
// Check with allowed values
if (
this.getAllowedValues().some( function ( value ) {
- return hash === OO.getHash( value );
+ return data === value;
} )
) {
return true;
diff --git a/tests/index.php b/tests/index.php
index e7b80f6..650e684 100644
--- a/tests/index.php
+++ b/tests/index.php
@@ -39,6 +39,7 @@
<script src="./Process.test.js"></script>
<script src="./mixins/FlaggedElement.test.js"></script>
<script src="./widgets/TagMultiselectWidget.test.js"></script>
+ <script src="./widgets/MenuTagMultiselectWidget.test.js"></script>
<!-- JS/PHP comparison tests -->
<script>OO.ui.JSPHPTestSuite = <?php echo $testSuiteJSON; ?></script>
<script src="./JSPHP.test.standalone.js"></script>
diff --git a/tests/widgets/MenuTagMultiselectWidget.test.js
b/tests/widgets/MenuTagMultiselectWidget.test.js
new file mode 100644
index 0000000..639cda9
--- /dev/null
+++ b/tests/widgets/MenuTagMultiselectWidget.test.js
@@ -0,0 +1,43 @@
+( function () {
+ QUnit.module( 'MenuTagMultiselectWidget' );
+
+ QUnit.test( 'isAllowedData', 4, function ( assert ) {
+ var widget;
+
+ widget = new OO.ui.MenuTagMultiselectWidget( {
+ options: [
+ { data: 'foo', label: 'Foo' },
+ { data: 'bar', label: 'Bar' },
+ { data: 'baz', label: 'Baz' }
+ ]
+ } );
+
+ assert.ok(
+ widget.isAllowedData( 'foo' ),
+ 'Data in menu items is allowed'
+ );
+
+ widget.addTag( 'foo' );
+ assert.ok(
+ !widget.isAllowedData( 'foo' ),
+ 'Data in menu but also duplicate is not allowed (for
allowDuplicates: false)'
+ );
+ assert.ok(
+ !widget.isAllowedData( 'blip' ),
+ 'Data not in menu is not allowed'
+ );
+
+ widget = new OO.ui.MenuTagMultiselectWidget( {
+ options: [
+ { data: 'foo', label: 'Foo' },
+ { data: 'bar', label: 'Bar' },
+ { data: 'baz', label: 'Baz' }
+ ],
+ allowedValues: [ 'something', 'else' ]
+ } );
+ assert.ok(
+ widget.isAllowedData( 'something' ),
+ 'Data from allowed values is allowed'
+ );
+ } );
+}() );
diff --git a/tests/widgets/TagMultiselectWidget.test.js
b/tests/widgets/TagMultiselectWidget.test.js
index a2557cd..c37c7bd 100644
--- a/tests/widgets/TagMultiselectWidget.test.js
+++ b/tests/widgets/TagMultiselectWidget.test.js
@@ -43,6 +43,50 @@
);
} );
+ QUnit.test( 'isAllowedData', 5, function ( assert ) {
+ var widget;
+
+ widget = new OO.ui.TagMultiselectWidget( {
+ allowArbitrary: true
+ } );
+ assert.ok(
+ widget.isAllowedData( '123foobar' ),
+ 'isAllowedData: allowArbitrary:true, random options are
valid'
+ );
+
+ widget = new OO.ui.TagMultiselectWidget( {
+ allowArbitrary: false,
+ allowedValues: [ 'foo', 'bar' ]
+ } );
+
+ assert.ok(
+ !widget.isAllowedData( '123foobar' ),
+ 'isAllowedData: allowArbitrary:false, data not in
allowedValues is invalid'
+ );
+ assert.ok(
+ widget.isAllowedData( 'foo' ),
+ 'isAllowedData: allowArbitrary:false, data in
allowedValues valid'
+ );
+
+ widget.addTag( 'foo' );
+ assert.ok(
+ !widget.isAllowedData( 'foo' ),
+ 'isAllowedData: allowDuplicates:false, duplicate values
are invalid even if they are in allowedValues'
+ );
+
+ widget = new OO.ui.TagMultiselectWidget( {
+ allowArbitrary: false,
+ allowedValues: [ 'foo', 'bar' ],
+ allowDuplicates: true
+ } );
+
+ widget.addTag( 'foo' );
+ assert.ok(
+ widget.isAllowedData( 'foo' ),
+ 'isAllowedData: allowDuplicates:true allows duplicate
values'
+ );
+ } );
+
QUnit.test( 'addTag', 7, function ( assert ) {
var widget,
getItemDatas = function ( items ) {
@@ -195,9 +239,8 @@
'Getting the next item from the first item.'
);
- assert.equalDomElement(
- widget.getNextItem( items[ items.length - 1 ] )[ 0 ],
- widget.input.$input[ 0 ],
+ assert.ok(
+ widget.getNextItem( items[ items.length - 1 ] ) ===
widget.input,
'Getting the next item from the last item, returns the
input.$input element (if inputPosition:inline or inputPosition:outline)'
);
@@ -219,9 +262,8 @@
widget.setValue( [ 'foo', 'bar', 'baz' ] );
items = widget.getItems();
- assert.equalDomElement(
- widget.getPreviousItem( items[ 0 ] )[ 0 ],
- widget.input.$input[ 0 ],
+ assert.ok(
+ widget.getPreviousItem( items[ 0 ] ) === widget.input,
'Getting the previous item from the first item returns
the input.$input element (if inputPosition:inline or inputPosition:outline)'
);
assert.strictEqual(
--
To view, visit https://gerrit.wikimedia.org/r/348166
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id32f37d9bf1828cae616effabf3c2377697cd6fe
Gerrit-PatchSet: 2
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: VolkerE <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
