Matthias Mullie has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/387258 )
Change subject: Simplify error handling & make it possible to perform different
kinds of checks
......................................................................
Simplify error handling & make it possible to perform different kinds of checks
It seems that the handling of errors in the controllers could
be greatly simplified.
I've also added a `thorough` param to the error/warning methods,
which will allow for different kinds of error/warning handling:
a simple check triggered by change events, and a more thorough
(asking user input, performing api calls, whatever you wouldn't
want to happen for every change event) on-submit version.
Change-Id: Iafcb8a839abbcea1dce38aa5ab30f11313b0dd9e
---
M resources/controller/uw.controller.Deed.js
M resources/controller/uw.controller.Details.js
M resources/mw.UploadWizardDetails.js
M resources/uw.ValidationMessageElement.js
4 files changed, 76 insertions(+), 110 deletions(-)
Approvals:
MarkTraceur: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/controller/uw.controller.Deed.js
b/resources/controller/uw.controller.Deed.js
index 884467e..eb31b94 100644
--- a/resources/controller/uw.controller.Deed.js
+++ b/resources/controller/uw.controller.Deed.js
@@ -53,19 +53,20 @@
if ( valid ) {
fields = this.deedChooser.deed.getFields();
- // Update any error/warning messages
- fields.forEach( function ( fieldLayout ) {
- fieldLayout.checkValidity();
- } );
-
- // TODO Handle warnings with a confirmation dialog
$.when.apply( $, fields.map( function ( fieldLayout ) {
- return fieldLayout.fieldWidget.getErrors();
- } ) ).done( function () {
+ // Update any error/warning messages
+ return fieldLayout.checkValidity( true );
+ } ) ).then( function () {
+ // `arguments` will be an array of all fields,
with their warnings & errors
+ // e.g. `[[something], []], [[], [something]]`
for 2 fields, where the first one has
+ // a warning and the last one an error
+
+ // TODO Handle warnings with a confirmation
dialog
+
var i;
for ( i = 0; i < arguments.length; i++ ) {
- if ( arguments[ i ].length ) {
- // One of the fields has errors
+ if ( arguments[ i ][ 1 ].length ) {
+ // One of the fields has
errors; refuse to proceed!
return;
}
}
diff --git a/resources/controller/uw.controller.Details.js
b/resources/controller/uw.controller.Details.js
index 99355d5..eac65f8 100644
--- a/resources/controller/uw.controller.Details.js
+++ b/resources/controller/uw.controller.Details.js
@@ -164,39 +164,27 @@
* @return {jQuery.Promise}
*/
uw.controller.Details.prototype.valid = function () {
- var
- detailsController = this,
- validityPromises = [],
- warningValidityPromises = [],
- titles = {};
+ var detailsController = this,
+ // validityPromises will hold all promises for all
uploads;
+ // prefilling with a bogus promise (no warnings &
errors) to
+ // ensure $.when always resolves with an array of
multiple
+ // results (if there's just 1, it would otherwise have
just
+ // that one's arguments, instead of a multi-dimensional
array
+ // of upload warnings & failures)
+ validityPromises = [ $.Deferred().resolve( [], []
).promise() ],
+ titles = [];
$.each( this.uploads, function ( i, upload ) {
// Update any error/warning messages about all
DetailsWidgets
- upload.details.checkValidity();
+ var promise = upload.details.checkValidity( true
).then( function () {
+ var warnings = [],
+ errors = [],
+ title;
- warningValidityPromises.push(
upload.details.getWarnings().then( function () {
- // iterate all arguments (which is an array of
arrays of
- // warnings) and turn it into a one-dimensional
warnings array
- var args = Array.prototype.slice.call(
arguments ),
- warnings = args.reduce( function (
result, warnings ) {
- return result.concat( warnings
);
- }, [] );
-
- if ( warnings.length ) {
- // One of the DetailsWidgets has
warnings
- return $.Deferred().reject( warnings );
- }
- } ) );
-
- validityPromises.push( upload.details.getErrors().then(
function () {
- var i, title, hasErrors = false;
-
- for ( i = 0; i < arguments.length; i++ ) {
- if ( arguments[ i ].length ) {
- // One of the DetailsWidgets
has errors
- hasErrors = true;
- }
- }
+ $.each( arguments, function ( i, result ) {
+ warnings = warnings.concat( result[ 0 ]
);
+ errors = errors.concat( result[ 1 ] );
+ } );
// Seen this title before?
title = upload.details.getTitle();
@@ -205,79 +193,42 @@
if ( titles[ title ] ) {
// Don't submit. Instead, set
an error in details step.
upload.details.setDuplicateTitleError();
- hasErrors = true;
+ errors.push( mw.message(
'mwe-upwiz-error-title-duplicate' ) );
} else {
titles[ title ] = true;
}
}
- if ( hasErrors ) {
- return $.Deferred().reject();
- }
- } ) );
+ return $.Deferred().resolve( warnings, errors
).promise();
+ } );
+
+ // Will hold an array of validation promises, one for
each upload
+ validityPromises.push( promise );
} );
- return $.when.apply( $, validityPromises ).then(
- function () {
- var i,
- warningPromises = [],
- combinedWarningPromise = $.Deferred();
+ // validityPromises is an array of promises that each resolve
with [warnings, errors]
+ // for each upload - now iterate them all to figure out if we
can proceed
+ return $.when.apply( $, validityPromises ).then( function () {
+ var warnings = [],
+ errors = [];
- /*
- * warningValidityPromises will be fail as soon
as one of the
- * promises is rejected. However, we want to
know about *all*
- * rejected promises, since they include the
warning messages,
- * and we'll want to show all of those warnings
at once.
- * Since we can't use warningValidityPromises's
failure callback,
- * we'll create other promises that will always
resolve (with
- * the rejected's warning messages).
- */
- for ( i = 0; i <
warningValidityPromises.length; i++ ) {
- warningPromises[ i ] = $.Deferred();
- warningValidityPromises[ i ].always(
warningPromises[ i ].resolve );
- }
+ $.each( arguments, function ( i, result ) {
+ warnings = warnings.concat( result[ 0 ] );
+ errors = errors.concat( result[ 1 ] );
+ } );
- /*
- * warningPromises will now always resolve (see
comment above)
- * with a bunch of warnings (or undefined, for
successful
- * uploads)
- * Now we can just wait for all of these to
resolve, combine all
- * warnings, and display the warning dialog!
- */
- combinedWarningPromise = $.when.apply( $,
warningPromises ).then( function () {
- // iterate all arguments (which is an
array of arrays of
- // warnings) and turn it into a
one-dimensional warnings array
- var args = Array.prototype.slice.call(
arguments ),
- // args also includes
`undefined`s, from uploads that
- // successfully resolved - we
don't need those!
- filtered = args.filter(
Array.isArray ),
- warnings = filtered.reduce(
function ( result, warnings ) {
- return result.concat(
warnings );
- }, [] );
-
- if ( warnings.length > 0 ) {
- // Update warning count before
dialog
- detailsController.showErrors();
- return
detailsController.confirmationDialog( warnings );
- }
- } );
-
- return $.when.apply( $, warningValidityPromises
).then(
- function () {
- // All uploads valid, no
warnings
- return $.Deferred().resolve(
true );
- },
- function () {
- // There were issues & they are
being handled in this
- // other promise :)
- return combinedWarningPromise;
- }
- );
- },
- function () {
+ if ( errors.length > 0 ) {
return $.Deferred().resolve( false );
}
- );
+
+ if ( warnings.length > 0 ) {
+ // Update warning count before dialog
+ detailsController.showErrors();
+ return detailsController.confirmationDialog(
warnings );
+ }
+
+ return $.Deferred().resolve( true );
+ } );
};
uw.controller.Details.prototype.confirmationDialog = function (
warnings ) {
diff --git a/resources/mw.UploadWizardDetails.js
b/resources/mw.UploadWizardDetails.js
index b657cd8..a375c1b 100644
--- a/resources/mw.UploadWizardDetails.js
+++ b/resources/mw.UploadWizardDetails.js
@@ -333,11 +333,17 @@
/**
* Check all the fields for errors and warnings and display
them in the UI.
+ *
+ * @param {boolean} thorough True to perform a thorough
validity check. Defaults to false for a fast on-change check.
+ * @return {jQuery.Promise} Combined promise of all fields'
validation results.
*/
- checkValidity: function () {
- this.getAllFields().forEach( function ( fieldLayout ) {
- fieldLayout.checkValidity();
- } );
+ checkValidity: function ( thorough ) {
+ var fields = this.getAllFields();
+
+ return $.when.apply( $, fields.map( function (
fieldLayout ) {
+ // Update any error/warning messages
+ return fieldLayout.checkValidity( thorough );
+ } ) );
},
/**
diff --git a/resources/uw.ValidationMessageElement.js
b/resources/uw.ValidationMessageElement.js
index 2f941c0..b039238 100644
--- a/resources/uw.ValidationMessageElement.js
+++ b/resources/uw.ValidationMessageElement.js
@@ -36,9 +36,14 @@
/**
* Check the field's widget for errors and warnings and display them in
the UI.
+ *
+ * @param {boolean} thorough True to perform a thorough validity check.
Defaults to false for a fast on-change check.
+ * @return {jQuery.Promise}
*/
- uw.ValidationMessageElement.prototype.checkValidity = function () {
+ uw.ValidationMessageElement.prototype.checkValidity = function (
thorough ) {
var element = this;
+ thorough = thorough || false;
+
if ( !this.validatedWidget.getWarnings ||
!this.validatedWidget.getErrors ) {
// Don't do anything for non-Details widgets
return;
@@ -46,13 +51,16 @@
if ( this.validatedWidget.pushPending ) {
this.validatedWidget.pushPending();
}
- $.when(
- this.validatedWidget.getWarnings(),
- this.validatedWidget.getErrors()
- ).done( function ( warnings, errors ) {
+
+ return $.when(
+ this.validatedWidget.getWarnings( thorough ),
+ this.validatedWidget.getErrors( thorough )
+ ).then( function ( warnings, errors ) {
// this.notices and this.errors are arrays of
mw.Messages and not strings in this subclass
element.setNotices( warnings );
element.setErrors( errors );
+
+ return $.Deferred().resolve( warnings, errors
).promise();
} ).always( function () {
if ( element.validatedWidget.popPending ) {
element.validatedWidget.popPending();
--
To view, visit https://gerrit.wikimedia.org/r/387258
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iafcb8a839abbcea1dce38aa5ab30f11313b0dd9e
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/UploadWizard
Gerrit-Branch: master
Gerrit-Owner: Matthias Mullie <[email protected]>
Gerrit-Reviewer: MarkTraceur <[email protected]>
Gerrit-Reviewer: Matthias Mullie <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
