jenkins-bot has submitted this change and it was merged.
Change subject: List creation and editing validation
......................................................................
List creation and editing validation
Add validation to CollectionContentOverlay:
* Checking title is less or equal to 90 characters
** Will need improvement for other languages
* Duplicate collection title handling already done
** Page is added to existing collection vs added to duplicate
*** Exception same title with case difference
Add validation to CollectionEditOverlay:
* Validates title is less or equal to 90 characters
* Validates description is less or equal to 280 characters
** Again this needs to consider other languages
* Shows edit failed toast
** Probably could have a different message but used existing
for now
* Added QUnit tests
bug: T92779
Change-Id: I1a755ffa5d459ed552ed2c102795f76cce5ddcc3
---
M includes/Gather.hooks.php
M resources/Resources.php
M resources/ext.gather.collection.editor/CollectionEditOverlay.js
M resources/ext.gather.watchstar/CollectionsContentOverlay.js
A tests/qunit/ext.gather.collection.editor/test_CollectionEditOverlay.js
5 files changed, 125 insertions(+), 28 deletions(-)
Approvals:
Jdlrobson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Gather.hooks.php b/includes/Gather.hooks.php
index cb05071..84a69e7 100644
--- a/includes/Gather.hooks.php
+++ b/includes/Gather.hooks.php
@@ -121,6 +121,12 @@
),
'dependencies' => array( 'ext.gather.watchstar' ),
);
+ $modules['qunit']['ext.gather.collection.editor.tests'] =
$boilerplate + array(
+ 'scripts' => array(
+
'ext.gather.collection.editor/test_CollectionEditOverlay.js',
+ ),
+ 'dependencies' => array( 'ext.gather.collection.editor'
),
+ );
return true;
}
diff --git a/resources/Resources.php b/resources/Resources.php
index f0f15e4..a63810f 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -133,6 +133,7 @@
'gather-add-to-existing',
'gather-watchlist-title',
'gather-add-toast',
+ 'gather-add-failed-toast',
'gather-remove-toast',
'gather-anon-cta',
'gather-collection-member',
diff --git a/resources/ext.gather.collection.editor/CollectionEditOverlay.js
b/resources/ext.gather.collection.editor/CollectionEditOverlay.js
index f617995..5213b63 100644
--- a/resources/ext.gather.collection.editor/CollectionEditOverlay.js
+++ b/resources/ext.gather.collection.editor/CollectionEditOverlay.js
@@ -16,6 +16,8 @@
CollectionEditOverlay = Overlay.extend( {
/** @inheritdoc */
className: 'collection-editor-overlay overlay position-fixed',
+ titleMaxLength: 90,
+ descriptionMaxLength: 180,
/** @inheritdoc */
defaults: $.extend( {}, Overlay.prototype.defaults, {
editFailedError: mw.msg(
'gather-edit-collection-failed-error' ),
@@ -55,30 +57,53 @@
* Event handler when the save button is clicked.
*/
onSaveClick: function () {
- var self = this;
- // disable button and inputs
- this.showSpinner();
- this.$( '.mw-ui-input, .save' ).prop( 'disabled', true
);
- this.api.editCollection(
- this.id, this.$( '.title' ).val(), this.$(
'.description' ).val()
- ).done( function () {
- // Go back to the page we were and reload to
avoid having to update the
- // JavaScript state.
- schema.log( {
- eventName: 'edit-collection'
- } ).done( function () {
- router.navigate( '/' );
- window.location.reload();
+ var title = this.$( '.title' ).val(),
+ description = this.$( '.description' ).val();
+
+ if ( this.isTitlevalid( title ) &&
this.isDescriptionValid( description ) ) {
+ // disable button and inputs
+ this.showSpinner();
+ this.$( '.mw-ui-input, .save' ).prop(
'disabled', true );
+ this.api.editCollection( this.id, title,
description ).done( function () {
+ // Go back to the page we were and
reload to avoid having to update the
+ // JavaScript state.
+ schema.log( {
+ eventName: 'edit-collection'
+ } ).done( function () {
+ router.navigate( '/' );
+ window.location.reload();
+ } );
+ } ).fail( function ( errMsg ) {
+ toast.show(
this.options.editFailedError, 'toast error' );
+ // Make it possible to try again.
+ this.$( '.mw-ui-input, .save' ).prop(
'disabled', false );
+ schema.log( {
+ eventName:
'edit-collection-error',
+ errorMessage: errMsg
+ } );
} );
- } ).fail( function ( errMsg ) {
- toast.show( self.options.editFailedError,
'toast error' );
- // Make it possible to try again.
- self.$( '.mw-ui-input, .save' ).prop(
'disabled', false );
- schema.log( {
- eventName: 'edit-collection-error',
- errorMessage: errMsg
- } );
- } );
+ } else {
+ toast.show( this.options.editFailedError,
'toast error' );
+ }
+
+ },
+ /**
+ * Tests if title is valid
+ * @param {[type]} title Proposed collection title
+ * @returns {Boolean}
+ */
+ isTitleValid: function ( title ) {
+ // FIXME: Need to consider other languages
+ return title.length <= this.titleMaxLength;
+ },
+ /**
+ * Tests if description is valid
+ * @param {[type]} description Proposed collection description
+ * @returns {Boolean}
+ */
+ isDescriptionValid: function ( description ) {
+ // FIXME: Need to consider other languages
+ return description.length <= this.descriptionMaxLength;
}
} );
diff --git a/resources/ext.gather.watchstar/CollectionsContentOverlay.js
b/resources/ext.gather.watchstar/CollectionsContentOverlay.js
index f578a5e..2f08bc6 100644
--- a/resources/ext.gather.watchstar/CollectionsContentOverlay.js
+++ b/resources/ext.gather.watchstar/CollectionsContentOverlay.js
@@ -86,6 +86,15 @@
this.hideSpinner();
},
/**
+ * Tests if title is valid
+ * FIXME: This is duplicating code from CollectionEditOverlay.js
+ * @param {String} title Proposed collection title
+ * @returns {Boolean}
+ */
+ isTitleValid: function ( title ) {
+ return title.length <= 90;
+ },
+ /**
* Event handler for focusing input
* @param {jQuery.Event} ev
*/
@@ -121,11 +130,17 @@
title = $( ev.target ).find( 'input' ).val();
ev.preventDefault();
- this.showSpinner();
- this.addCollection( title, page );
- schema.log( {
- eventName: 'new-collection'
- } );
+ if ( this.isTitleValid( title ) ) {
+ this.showSpinner();
+ this.addCollection( title, page );
+ schema.log( {
+ eventName: 'new-collection'
+ } );
+
+ this.addCollection( title, page );
+ } else {
+ toast.show( mw.msg( 'gather-add-failed-toast',
title ), 'toast' );
+ }
},
/**
* Event handler for all clicks inside overlay.
@@ -239,6 +254,7 @@
var self = this,
api = this.api;
+ this.showSpinner();
return api.addCollection( title ).done( function (
collection ) {
api.addPageToCollection( collection.id, page
).done(
$.proxy( self,
'_collectionStateChange', collection, true )
diff --git
a/tests/qunit/ext.gather.collection.editor/test_CollectionEditOverlay.js
b/tests/qunit/ext.gather.collection.editor/test_CollectionEditOverlay.js
new file mode 100644
index 0000000..9c8f03e
--- /dev/null
+++ b/tests/qunit/ext.gather.collection.editor/test_CollectionEditOverlay.js
@@ -0,0 +1,49 @@
+( function ( M ) {
+ var CollectionEditOverlay = M.require(
'ext.gather.edit/CollectionEditOverlay' ),
+ collection,
+ overlay;
+
+ QUnit.module( 'Gather', {
+ setup: function () {
+ collection = {
+ id: 1,
+ title: 'Cool title',
+ description: 'Hey, I\'m a collection
description.'
+ };
+ overlay = new CollectionEditOverlay( {
+ collection: collection
+ } );
+ this.validTitle = randomString( overlay.titleMaxLength
);
+ this.invalidTitle = randomString(
overlay.titleMaxLength + 1 );
+ this.validDescription = randomString(
overlay.descriptionMaxLength );
+ this.invalidDescription = randomString(
overlay.descriptionMaxLength + 1 );
+
+ }
+ } );
+
+ /**
+ * Use base 36 method to generate a random string with specified length
+ * @param {Number} length length of desired string
+ * @returns {String} randomly generated string
+ */
+ function randomString( length ) {
+ return Math.round(
+ ( Math.pow( 36, length + 1 ) - Math.random() *
Math.pow( 36, length ) )
+ ).toString( 36 ).slice( 1 );
+ }
+
+ QUnit.test( 'Collection title validation', 2, function ( assert ) {
+ assert.strictEqual( overlay.isTitleValid(
this.validTitle ), true,
+ 'Check that a valid title is correctly
evaluated' );
+ assert.strictEqual( overlay.isTitleValid(
this.invalidTitle ), false,
+ 'Check that an invalid title is correctly
evaluated' );
+ } );
+
+ QUnit.test( 'Collection description validation', 2, function ( assert )
{
+ assert.strictEqual( overlay.isDescriptionValid(
this.validDescription ), true,
+ 'Check that a valid description is correctly
evaluated' );
+ assert.strictEqual( overlay.isDescriptionValid(
this.invalidDescription ), false,
+ 'Check that an invalid description is correctly
evaluated' );
+ } );
+
+}( mw.mobileFrontend ) );
--
To view, visit https://gerrit.wikimedia.org/r/198430
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a755ffa5d459ed552ed2c102795f76cce5ddcc3
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/Gather
Gerrit-Branch: master
Gerrit-Owner: Robmoen <rm...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Robmoen <rm...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
