From 2ea9685552d5e3bc5286d6a13a0cb0395c5bacd4 Mon Sep 17 00:00:00 2001 From: AlasdairSwan Date: Fri, 24 Oct 2014 16:38:31 -0400 Subject: [PATCH] ECOM-369 Added handling of model save errors. Updated FormView to add preRender function to limit amount of duplicate code. Updated PasswordResetModel to return error object when errors occur. --- .../js/spec_helpers/edx.utils.validate.js | 3 +- .../models/PasswordResetModel.js | 4 +- .../js/student_account/views/AccessView.js | 23 ++++- .../js/student_account/views/FormView.js | 83 ++++++++++++------- .../js/student_account/views/LoginView.js | 23 ++--- .../views/PasswordResetView.js | 21 ++--- .../js/student_account/views/RegisterView.js | 13 +-- .../student_account/access.underscore | 4 +- .../student_account/login.underscore | 2 +- .../student_account/password_reset.underscore | 3 - 10 files changed, 95 insertions(+), 84 deletions(-) diff --git a/common/static/js/spec_helpers/edx.utils.validate.js b/common/static/js/spec_helpers/edx.utils.validate.js index b17ed04b24..1c0d1c4834 100644 --- a/common/static/js/spec_helpers/edx.utils.validate.js +++ b/common/static/js/spec_helpers/edx.utils.validate.js @@ -28,7 +28,6 @@ var edx = edx || {}; isBlank = _fn.validate.isBlank( $el ); if ( _fn.validate.isRequired( $el ) ) { -console.log('is required'); if ( isBlank ) { required = false; } else { @@ -41,7 +40,7 @@ console.log('is required'); } response.isValid = required && min && max && email; -console.log(response.isValid); + if ( !response.isValid ) { response.message = _fn.validate.getMessage( $el, { required: required, diff --git a/lms/static/js/student_account/models/PasswordResetModel.js b/lms/static/js/student_account/models/PasswordResetModel.js index 00958667b2..bfc1997fa9 100644 --- a/lms/static/js/student_account/models/PasswordResetModel.js +++ b/lms/static/js/student_account/models/PasswordResetModel.js @@ -29,8 +29,8 @@ var edx = edx || {}; .done(function() { model.trigger('success'); }) - .fail( function() { - model.trigger('error'); + .fail( function( error ) { + model.trigger( 'error', error ); }); } }); diff --git a/lms/static/js/student_account/views/AccessView.js b/lms/static/js/student_account/views/AccessView.js index dc2a45953f..f29450b7d0 100644 --- a/lms/static/js/student_account/views/AccessView.js +++ b/lms/static/js/student_account/views/AccessView.js @@ -126,8 +126,8 @@ var edx = edx || {}; }, resetPassword: function() { - this.$header.addClass('hidden'); - $(this.el).find('.form-type').addClass('hidden'); + this.element.hide( this.$header ); + this.element.hide( $(this.el).find('.form-type') ); this.loadForm('reset'); }, @@ -139,14 +139,29 @@ var edx = edx || {}; this.loadForm( type ); } - $(this.el).find('.form-wrapper').addClass('hidden'); - $form.removeClass('hidden'); + this.element.hide( $(this.el).find('.form-wrapper') ); + this.element.show( $form ); }, form: { isLoaded: function( $form ) { return $form.html().length > 0; } + }, + + /* Helper method ot toggle display + * including accessibility considerations + */ + element: { + hide: function( $el ) { + $el.addClass('hidden') + .attr('aria-hidden', true); + }, + + show: function( $el ) { + $el.removeClass('hidden') + .attr('aria-hidden', false); + } } }); diff --git a/lms/static/js/student_account/views/FormView.js b/lms/static/js/student_account/views/FormView.js index 18460a9626..ce9aa17a71 100644 --- a/lms/static/js/student_account/views/FormView.js +++ b/lms/static/js/student_account/views/FormView.js @@ -35,10 +35,20 @@ var edx = edx || {}; this.buildForm( data.fields ); this.model = data.model; - this.listenTo( this.model, 'error', this.modelError ); + this.listenTo( this.model, 'error', this.saveError ); + + this.preRender( data ); + }, + + /* Allows extended views to add custom + * init steps without needing to repeat + * default init steps + */ + preRender: function( data ) { + /* custom code goes here */ + return data; }, - // Renders the form. render: function( html ) { var fields = html || ''; @@ -76,6 +86,27 @@ var edx = edx || {}; this.render( html.join('') ); }, + /* Helper method ot toggle display + * including accessibility considerations + */ + element: { + hide: function( $el ) { + $el.addClass('hidden') + .attr('aria-hidden', true); + }, + + show: function( $el ) { + $el.removeClass('hidden') + .attr('aria-hidden', false); + } + }, + + forgotPassword: function( event ) { + event.preventDefault(); + + this.trigger('password-help'); + }, + getFormData: function() { var obj = {}, @@ -116,10 +147,25 @@ var edx = edx || {}; return obj; }, - forgotPassword: function( event ) { - event.preventDefault(); + saveError: function( error ) { + this.errors = ['
  • ' + error.responseText + '
    • ']; + this.setErrors(); + }, - this.trigger('password-help'); + setErrors: function() { + var $msg = this.$errors.find('.message-copy'), + html = [], + errors = this.errors, + i, + len = errors.length; + + for ( i=0; i' + error.responseText + '']; /* If we've gotten a 403 error, it means that we've successfully * authenticated with a third-party provider, but we haven't @@ -90,14 +83,14 @@ var edx = edx || {}; * to complete the registration process. */ if (error.status === 403 && error.responseText === "third-party-auth" && this.currentProvider) { - this.$alreadyAuthenticatedMsg.removeClass("hidden"); - } - else { - this.$alreadyAuthenticatedMsg.addClass("hidden"); + this.element.show( this.$alreadyAuthenticatedMsg ); + } else { + this.element.hide( this.$alreadyAuthenticatedMsg ); // TODO -- display the error } + this.setErrors(); } }); -})(jQuery, _, Backbone, gettext); \ No newline at end of file +})(jQuery, _, gettext); diff --git a/lms/static/js/student_account/views/PasswordResetView.js b/lms/static/js/student_account/views/PasswordResetView.js index 48b4480998..3474e7e3c5 100644 --- a/lms/static/js/student_account/views/PasswordResetView.js +++ b/lms/static/js/student_account/views/PasswordResetView.js @@ -1,6 +1,6 @@ var edx = edx || {}; -(function($, _, Backbone, gettext) { +(function($, _, gettext) { 'use strict'; edx.student = edx.student || {}; @@ -24,34 +24,25 @@ var edx = edx || {}; this.$form = $container.find('form'); - this.$resetFail = $container.find('.js-reset-fail'); this.$errors = $container.find('.submission-error'); this.listenTo( this.model, 'success', this.resetComplete ); - this.listenTo( this.model, 'error', this.resetError ); + this.listenTo( this.model, 'error', this.saveError ); }, toggleErrorMsg: function( show ) { if ( show ) { this.setErrors(); } else { - this.$errors - .addClass('hidden') - .attr('aria-hidden', true); + this.element.hide( this.$errors ); } }, resetComplete: function() { var $el = $(this.el); - $el.find('#password-reset-form').addClass('hidden'); - $el.find('.js-reset-success').removeClass('hidden'); - - this.$resetFail.addClass('hidden'); - }, - - resetError: function() { - this.$resetFail.removeClass('hidden'); + this.element.hide( $el.find('#password-reset-form') ); + this.element.show( $el.find('.js-reset-success') ); }, submitForm: function( event ) { @@ -73,4 +64,4 @@ var edx = edx || {}; } }); -})(jQuery, _, Backbone, gettext); +})(jQuery, _, gettext); diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index 697c19cf7c..c455741b19 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -1,6 +1,6 @@ var edx = edx || {}; -(function($, _, Backbone, gettext) { +(function($, _, gettext) { 'use strict'; edx.student = edx.student || {}; @@ -18,13 +18,7 @@ var edx = edx || {}; formType: 'register', - initialize: function( data ) { - this.tpl = $(this.tpl).html(); - this.fieldTpl = $(this.fieldTpl).html(); - - this.buildForm( data.fields ); - this.model = data.model; - + preRender: function( data ) { this.providers = data.thirdPartyAuth.providers || []; this.currentProvider = data.thirdPartyAuth.currentProvider || ''; }, @@ -45,6 +39,7 @@ var edx = edx || {}; thirdPartyAuth: function( event ) { var providerUrl = $(event.target).data('provider-url') || ''; + if (providerUrl) { window.location.href = providerUrl; } else { @@ -54,4 +49,4 @@ var edx = edx || {}; } }); -})(jQuery, _, Backbone, gettext); \ No newline at end of file +})(jQuery, _, gettext); diff --git a/lms/templates/student_account/access.underscore b/lms/templates/student_account/access.underscore index 6106cbc857..a920aaed56 100644 --- a/lms/templates/student_account/access.underscore +++ b/lms/templates/student_account/access.underscore @@ -8,7 +8,7 @@ checked<% } %> > -
    +
    @@ -16,7 +16,7 @@ checked<% } %>> -
    +
    diff --git a/lms/templates/student_account/login.underscore b/lms/templates/student_account/login.underscore index 604d847fe8..456c8c9982 100644 --- a/lms/templates/student_account/login.underscore +++ b/lms/templates/student_account/login.underscore @@ -3,7 +3,7 @@

    We couldn't log you in.

    - -