From 727dd9f90bf4892e7be19c816729ffd09a2499a1 Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Fri, 19 May 2017 21:57:06 +0500 Subject: [PATCH] Success Message for Account Activation for logged out users. --- .../student/tests/test_activate_account.py | 27 +++++++++++++++++++ common/djangoapps/student/views.py | 14 +++++----- .../pages/lms/login_and_register.py | 2 +- lms/djangoapps/student_account/views.py | 8 ++++++ .../js/student_account/views/AccessView.js | 8 ++++++ .../js/student_account/views/LoginView.js | 15 +++++++++++ lms/static/sass/multicourse/_dashboard.scss | 6 ++--- lms/static/sass/views/_login-register.scss | 24 +++++++++++++++++ .../student_account/form_status.underscore | 4 +-- 9 files changed, 95 insertions(+), 13 deletions(-) diff --git a/common/djangoapps/student/tests/test_activate_account.py b/common/djangoapps/student/tests/test_activate_account.py index 8d4bcffa15..42f6caf697 100644 --- a/common/djangoapps/student/tests/test_activate_account.py +++ b/common/djangoapps/student/tests/test_activate_account.py @@ -6,6 +6,8 @@ from django.conf import settings from django.test import TestCase, override_settings from django.core.urlresolvers import reverse +from uuid import uuid4 + from edxmako.shortcuts import render_to_string from student.models import Registration from student.tests.factories import UserFactory @@ -159,3 +161,28 @@ class TestActivateAccount(TestCase): ) response = self.client.get(reverse('dashboard')) self.assertNotContains(response, expected_message, html=True) + + def test_account_activation_notification_on_logistration(self): + """ + Verify that logistration page displays success/error/info messages + about account activation. + """ + login_page_url = "{login_url}?next={redirect_url}".format( + login_url=reverse('signin_user'), + redirect_url=reverse('dashboard'), + ) + # Access activation link, message should say that account has been activated. + response = self.client.get(reverse('activate', args=[self.registration.activation_key]), follow=True) + self.assertRedirects(response, login_page_url) + self.assertContains(response, 'You have activated your account.') + + # Access activation link again, message should say that account is already active. + response = self.client.get(reverse('activate', args=[self.registration.activation_key]), follow=True) + self.assertRedirects(response, login_page_url) + self.assertContains(response, 'This account has already been activated.') + + # Open account activation page with an invalid activation link, + # there should be an error message displayed. + response = self.client.get(reverse('activate', args=[uuid4().hex]), follow=True) + self.assertRedirects(response, login_page_url) + self.assertContains(response, 'Your account could not be activated') diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 78d9f1311b..3850ef7b3d 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -90,7 +90,7 @@ from openedx.core.djangoapps.external_auth.login_and_register import ( register as external_auth_register ) from openedx.core.djangoapps import monitoring_utils -from openedx.core.djangolib.markup import HTML, Text +from openedx.core.djangolib.markup import HTML import track.views @@ -2302,7 +2302,7 @@ def activate_account(request, key): except (Registration.DoesNotExist, Registration.MultipleObjectsReturned): messages.error( request, - Text(_( + HTML(_( '{html_start}Your account could not be activated{html_end}' 'Something went wrong, please contact support to resolve this issue.' )).format( @@ -2310,7 +2310,7 @@ def activate_account(request, key): html_start=HTML('

'), html_end=HTML('

'), ), - extra_tags='account-activation icon' + extra_tags='account-activation aa-icon' ) else: if not registration.user.is_active: @@ -2318,20 +2318,20 @@ def activate_account(request, key): # Add account activation success message for display later messages.success( request, - Text(_('{html_start}Success{html_end} You have activated your account.')).format( + HTML(_('{html_start}Success{html_end} You have activated your account.')).format( html_start=HTML('

'), html_end=HTML('

'), ), - extra_tags='account-activation icon', + extra_tags='account-activation aa-icon', ) else: messages.info( request, - Text(_('{html_start}This account has already been activated.{html_end}')).format( + HTML(_('{html_start}This account has already been activated.{html_end}')).format( html_start=HTML('

'), html_end=HTML('

'), ), - extra_tags='account-activation icon', + extra_tags='account-activation aa-icon', ) # Enroll student in any pending courses he/she may have if auto_enroll flag is set diff --git a/common/test/acceptance/pages/lms/login_and_register.py b/common/test/acceptance/pages/lms/login_and_register.py index 6544698a98..83f89a8380 100644 --- a/common/test/acceptance/pages/lms/login_and_register.py +++ b/common/test/acceptance/pages/lms/login_and_register.py @@ -353,7 +353,7 @@ class CombinedLoginAndRegisterPage(PageObject): """Wait for a status message to be visible following third_party registration, then return it.""" def _check_func(): """Return third party auth status notice message.""" - selector = '.js-auth-warning p' + selector = '.js-auth-warning div' msg_element = self.q(css=selector) if msg_element.visible: return (True, msg_element.text[0]) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index d9e51780a7..3a835a098f 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -117,6 +117,13 @@ def login_and_registration_form(request, initial_mode="login"): if ext_auth_response is not None: return ext_auth_response + # Account activation message + account_activation_messages = [ + { + 'message': message.message, 'tags': message.tags + } for message in messages.get_messages(request) if 'account-activation' in message.tags + ] + # Otherwise, render the combined login/registration page context = { 'data': { @@ -126,6 +133,7 @@ def login_and_registration_form(request, initial_mode="login"): 'third_party_auth_hint': third_party_auth_hint or '', 'platform_name': configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME), 'support_link': configuration_helpers.get_value('SUPPORT_SITE_LINK', settings.SUPPORT_SITE_LINK), + 'account_activation_messages': account_activation_messages, # Include form descriptions retrieved from the user API. # We could have the JS client make these requests directly, diff --git a/lms/static/js/student_account/views/AccessView.js b/lms/static/js/student_account/views/AccessView.js index 435cc9d3dd..0302d8024a 100644 --- a/lms/static/js/student_account/views/AccessView.js +++ b/lms/static/js/student_account/views/AccessView.js @@ -52,6 +52,9 @@ this.thirdPartyAuthHint = options.third_party_auth_hint || null; + // Account activation messages + this.accountActivationMessages = options.account_activation_messages || []; + if (options.login_redirect_url) { // Ensure that the next URL is internal for security reasons if (! window.isExternal(options.login_redirect_url)) { @@ -82,6 +85,10 @@ // Once the third party error message has been shown once, // there is no need to show it again, if the user changes mode: this.thirdPartyAuth.errorMessage = null; + + // Once the account activation messages have been shown once, + // there is no need to show it again, if the user changes mode: + this.accountActivationMessages = []; }, render: function() { @@ -119,6 +126,7 @@ model: model, resetModel: this.resetModel, thirdPartyAuth: this.thirdPartyAuth, + accountActivationMessages: this.accountActivationMessages, platformName: this.platformName, supportURL: this.supportURL, createAccountOption: this.createAccountOption diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 25587a2058..f66a7fc131 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -39,6 +39,7 @@ this.resetModel = data.resetModel; this.supportURL = data.supportURL; this.createAccountOption = data.createAccountOption; + this.accountActivationMessages = data.accountActivationMessages; this.listenTo(this.model, 'sync', this.saveSuccess); this.listenTo(this.resetModel, 'sync', this.resetEmail); @@ -85,6 +86,20 @@ */ this.model.save(); } + + // Display account activation success or error messages. + this.renderAccountActivationMessages(); + }, + + renderAccountActivationMessages: function() { + _.each(this.accountActivationMessages, this.renderAccountActivationMessage, this); + }, + + renderAccountActivationMessage: function(message) { + this.renderFormFeedback(this.formStatusTpl, { + jsHook: message.tags, + message: HtmlUtils.HTML(message.message) + }); }, forgotPassword: function(event) { diff --git a/lms/static/sass/multicourse/_dashboard.scss b/lms/static/sass/multicourse/_dashboard.scss index f002f310c8..1ab6e32810 100644 --- a/lms/static/sass/multicourse/_dashboard.scss +++ b/lms/static/sass/multicourse/_dashboard.scss @@ -1453,7 +1453,7 @@ a.fade-cover{ margin-bottom: 0; } - &.icon .message-copy:before { + &.aa-icon .message-copy:before { position: absolute; left: -1em; content: "\f05a"; // fa-info-circle @@ -1468,7 +1468,7 @@ a.fade-cover{ background-color: $palette-success-back; border: $palette-success-border 1px solid; - &.icon .message-copy:before { + &.aa-icon .message-copy:before { position: absolute; left: -1em; content: "\f00c"; // fa-check @@ -1483,7 +1483,7 @@ a.fade-cover{ background-color: $palette-error-back; border: $palette-error-border 1px solid; - &.icon .message-copy:before { + &.aa-icon .message-copy:before { position: absolute; left: -1em; content: "\f06a"; // fa-exclamation-circle diff --git a/lms/static/sass/views/_login-register.scss b/lms/static/sass/views/_login-register.scss index d99b80130e..e16374c49c 100644 --- a/lms/static/sass/views/_login-register.scss +++ b/lms/static/sass/views/_login-register.scss @@ -560,6 +560,30 @@ margin: 0 0 ($baseline/4) $baseline; } } + + &.account-activation { + .message-copy { + padding: 0 1em !important; + } + + &.info { + background-color: $palette-info-back; + border: $palette-info-border 1px solid; + color: $palette-info-text; + } + + &.success { + background-color: $palette-success-back; + border: $palette-success-border 1px solid; + color: $palette-success-text; + } + + &.error { + background-color: $palette-error-back; + border: $palette-error-border 1px solid; + color: $palette-error-text 1px solid; + } + } } .submission-error, .system-error { diff --git a/lms/templates/student_account/form_status.underscore b/lms/templates/student_account/form_status.underscore index 68dc4e4e05..59b74887b6 100644 --- a/lms/templates/student_account/form_status.underscore +++ b/lms/templates/student_account/form_status.underscore @@ -1,6 +1,6 @@
-

- <%- message %> +

+ <%= HtmlUtils.ensureHtml(message) %>