From 10af2bccf79e60afa5a171b8964704a90fefc19b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 12 Jan 2016 09:52:00 +0000 Subject: [PATCH 1/4] Extract user profile route into its own file --- app/main/__init__.py | 3 ++- app/main/views/index.py | 6 ------ app/main/views/user_profile.py | 7 +++++++ tests/app/main/views/test_user_profile.py | 4 ++++ 4 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 app/main/views/user_profile.py create mode 100644 tests/app/main/views/test_user_profile.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 578ea5e92..4a78540b1 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -4,5 +4,6 @@ main = Blueprint('main', __name__) from app.main.views import ( index, sign_in, sign_out, register, two_factor, verify, sms, add_service, - code_not_received, jobs, dashboard, templates, service_settings, forgot_password, new_password, styleguide + code_not_received, jobs, dashboard, templates, service_settings, forgot_password, + new_password, styleguide, user_profile ) diff --git a/app/main/views/index.py b/app/main/views/index.py index fb14d2b5d..eb41d0720 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -32,12 +32,6 @@ def checkemail(): return render_template('views/check-email.html') -@main.route("/user-profile") -@login_required -def userprofile(): - return render_template('views/user-profile.html') - - @main.route("/manage-users") @login_required def manageusers(): diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py new file mode 100644 index 000000000..60f88459d --- /dev/null +++ b/app/main/views/user_profile.py @@ -0,0 +1,7 @@ +from flask import render_template +from app.main import main + + +@main.route("/user-profile") +def userprofile(): + return render_template('views/user-profile.html') diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py new file mode 100644 index 000000000..ed185fc5f --- /dev/null +++ b/tests/app/main/views/test_user_profile.py @@ -0,0 +1,4 @@ +def test_should_show_overview_page(notifications_admin): + response = notifications_admin.test_client().get('/user-profile') + + assert response.status_code == 200 From ba50f132fcdf926dd8b88709a2ca6de606225169 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 12 Jan 2016 09:58:21 +0000 Subject: [PATCH 2/4] Add table to user profile page --- app/templates/views/user-profile.html | 34 +++++++++++++++++------ tests/app/main/views/test_user_profile.py | 1 + 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/app/templates/views/user-profile.html b/app/templates/views/user-profile.html index 3384c9ba5..05af98581 100644 --- a/app/templates/views/user-profile.html +++ b/app/templates/views/user-profile.html @@ -1,19 +1,35 @@ {% extends "withnav_template.html" %} -{% from "components/page-footer.html" import page_footer %} +{% from "components/table.html" import list_table, row, field %} {% block page_title %} -GOV.UK Notify | User settings + GOV.UK Notify | Your profile {% endblock %} {% block maincolumn_content %} -

User profile

+

Your profile

-

Here's where users can update their profile, password etc.

- - {{ page_footer( - back_link = url_for('.dashboard'), - back_link_text = 'Back to dashboard' - ) }} + {% call(item) list_table( + [ + {'label': 'Name', 'value': current_user.name, 'url': url_for('.userprofile_name')}, + {'label': 'Email address', 'value': current_user.email_address, 'url': url_for('.userprofile_email')}, + {'label': 'Mobile number', 'value': current_user.mobile_number, 'url': url_for('.userprofile_mobile_number')}, + {'label': 'Password', 'value': 'Last changed 1 January 2016, 10:00AM', 'url': url_for('.userprofile_password')}, + ], + caption='Account settings', + field_headings=['Setting', 'Value', 'Link to change'], + field_headings_visible=False, + caption_visible=False + ) %} + {% call field() %} + {{ item.label }} + {% endcall %} + {% call field() %} + {{ item.value }} + {% endcall %} + {% call field(align='right') %} + Change + {% endcall %} + {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index ed185fc5f..c4ef6ac23 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -1,4 +1,5 @@ def test_should_show_overview_page(notifications_admin): response = notifications_admin.test_client().get('/user-profile') + assert 'Your profile' in response.get_data(as_text=True) assert response.status_code == 200 From df79dc69f66cfbec8ba78b6cf519bf755407f18c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 12 Jan 2016 10:28:14 +0000 Subject: [PATCH 3/4] Add loops for changing each part of your profile This commit adds a page or series of pages for changing your: Name | Email address | Mobile number | Password ------------------|-------------------|-------------------|------------ Enter new value | Enter new value | Enter new value | Enter new value | Enter 2fa code | Enter 2fa code | Return to profile | Return to profile | Return to profile | Return to profile (each row is a page) --- app/main/forms.py | 20 ++++ app/main/views/user_profile.py | 103 +++++++++++++++++- app/templates/views/user-profile/change.html | 31 ++++++ app/templates/views/user-profile/confirm.html | 29 +++++ tests/app/main/views/test_user_profile.py | 86 +++++++++++++++ 5 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 app/templates/views/user-profile/change.html create mode 100644 app/templates/views/user-profile/confirm.html diff --git a/app/main/forms.py b/app/main/forms.py index e4559a164..0fbae1bbb 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -155,3 +155,23 @@ class NewPasswordForm(Form): class CsvUploadForm(Form): file = FileField('File to upload', validators=[DataRequired( message='Please pick a file'), CsvFileValidator()]) + + +class ChangeNameForm(Form): + new_name = StringField(u'Your name') + + +class ChangeEmailForm(Form): + email_address = email_address() + + +class ConfirmEmailForm(Form): + email_code = email_code() + + +class ChangeMobileNumberForm(Form): + mobile_number = mobile_number() + + +class ConfirmMobileNumberForm(Form): + sms_code = sms_code() diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index 60f88459d..19c56b10f 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -1,7 +1,108 @@ -from flask import render_template +from flask import request, render_template, redirect, url_for +from flask.ext.login import current_user from app.main import main +from app.main.forms import ( + NewPasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm, + ChangeMobileNumberForm, ConfirmMobileNumberForm +) @main.route("/user-profile") def userprofile(): return render_template('views/user-profile.html') + + +@main.route("/user-profile/name", methods=['GET', 'POST']) +def userprofile_name(): + + form = ChangeNameForm() + + if request.method == 'GET': + if current_user.is_authenticated(): + form.new_name.data = current_user.name + return render_template( + 'views/user-profile/change.html', + thing='name', + form_field=form.new_name + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile')) + + +@main.route("/user-profile/email", methods=['GET', 'POST']) +def userprofile_email(): + + form = ChangeEmailForm() + + if request.method == 'GET': + if current_user.is_authenticated(): + form.email_address.data = current_user.email_address + return render_template( + 'views/user-profile/change.html', + thing='email address', + form_field=form.email_address + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_email_confirm')) + + +@main.route("/user-profile/email/confirm", methods=['GET', 'POST']) +def userprofile_email_confirm(): + + form = ConfirmEmailForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/confirm.html', + form_field=form.email_code, + thing='email address' + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile')) + + +@main.route("/user-profile/mobile-number", methods=['GET', 'POST']) +def userprofile_mobile_number(): + + form = ChangeMobileNumberForm() + + if request.method == 'GET': + if current_user.is_authenticated(): + form.mobile_number.data = current_user.mobile_number + return render_template( + 'views/user-profile/change.html', + thing='mobile number', + form_field=form.mobile_number + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_mobile_number_confirm')) + + +@main.route("/user-profile/mobile-number/confirm", methods=['GET', 'POST']) +def userprofile_mobile_number_confirm(): + + form = ConfirmMobileNumberForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/confirm.html', + form_field=form.sms_code, + thing='mobile number' + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile')) + + +@main.route("/user-profile/password", methods=['GET', 'POST']) +def userprofile_password(): + + form = NewPasswordForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/change.html', + thing='password', + form_field=form.new_password + ) + elif request.method == 'POST': + return redirect(url_for('.userprofile')) diff --git a/app/templates/views/user-profile/change.html b/app/templates/views/user-profile/change.html new file mode 100644 index 000000000..27224d410 --- /dev/null +++ b/app/templates/views/user-profile/change.html @@ -0,0 +1,31 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your {{ thing }}

+ +
+ {% if preamble %} +

+ {{ preamble }} +

+ {% endif %} +
+
+ {{ textbox(form_field) }} + {{ page_footer( + 'Save', + back_link=url_for('.userprofile'), + back_link_text="Back to your profile" + ) }} +
+
+
+ +{% endblock %} diff --git a/app/templates/views/user-profile/confirm.html b/app/templates/views/user-profile/confirm.html new file mode 100644 index 000000000..4fa7a8fff --- /dev/null +++ b/app/templates/views/user-profile/confirm.html @@ -0,0 +1,29 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your {{ thing }}

+ +
+
+

+ We’ve sent a code to your new {{ thing }}. +

+
+ {{ textbox(form_field) }} + {{ page_footer( + 'Confirm', + destructive=destructive, + back_link=url_for('.service_settings') + ) }} +
+
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index c4ef6ac23..a292c271f 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -3,3 +3,89 @@ def test_should_show_overview_page(notifications_admin): assert 'Your profile' in response.get_data(as_text=True) assert response.status_code == 200 + + +def test_should_show_name_page(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/name') + + assert 'Change your name' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_name_change(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/name') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile' + + +def test_should_show_email_page(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/email') + + assert 'Change your email address' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_email_change(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/email') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/email/confirm' + + +def test_should_show_confirm_after_email_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/email/confirm') + + assert 'Change your email address' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_email_change_confirm(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/email/confirm') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile' + + +def test_should_show_mobile_number_page(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/mobile-number') + + assert 'Change your mobile number' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_mobile_number_change(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/email') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/email/confirm' + + +def test_should_show_confirm_after_mobile_number_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/mobile-number/confirm') + + assert 'Change your mobile number' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_mobile_number_confirm(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/mobile-number/confirm') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile' + + +def test_should_show_password_page(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/password') + + assert 'Change your password' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_password_change(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/password') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile' From c94ac4266c89fa9968c4650ea1fd78417a671cfe Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 12 Jan 2016 11:25:46 +0000 Subject: [PATCH 4/4] Add confirmation of password for important changes This commit adds an extra page or field for confirming your current password when making important changes Name | Email address | Mobile number | Password ---------------------|-------------------|-------------------|------------ No password required | As second page | As second page | On same page as new password --- app/main/forms.py | 9 +++- app/main/views/user_profile.py | 43 ++++++++++++++++--- app/templates/views/signin.html | 2 +- .../views/user-profile/authenticate.html | 26 +++++++++++ .../views/user-profile/change-password.html | 27 ++++++++++++ tests/app/main/views/test_user_profile.py | 32 +++++++++++++- 6 files changed, 129 insertions(+), 10 deletions(-) create mode 100644 app/templates/views/user-profile/authenticate.html create mode 100644 app/templates/views/user-profile/change-password.html diff --git a/app/main/forms.py b/app/main/forms.py index 0fbae1bbb..5ad1d04b5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -31,8 +31,8 @@ def mobile_number(): Regexp(regex=mobile_number_regex, message='Enter a +44 mobile number')]) -def password(): - return PasswordField('Create a password', +def password(label='Create a password'): + return PasswordField(label, validators=[DataRequired(message='Password can not be empty'), Length(10, 255, message='Password must be at least 10 characters'), Blacklist(message='That password is blacklisted, too common')]) @@ -152,6 +152,11 @@ class NewPasswordForm(Form): new_password = password() +class ChangePasswordForm(Form): + old_password = password('Current password') + new_password = password('New password') + + class CsvUploadForm(Form): file = FileField('File to upload', validators=[DataRequired( message='Please pick a file'), CsvFileValidator()]) diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index 19c56b10f..1b8db76cb 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -2,8 +2,8 @@ from flask import request, render_template, redirect, url_for from flask.ext.login import current_user from app.main import main from app.main.forms import ( - NewPasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm, - ChangeMobileNumberForm, ConfirmMobileNumberForm + ChangePasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm, + ChangeMobileNumberForm, ConfirmMobileNumberForm, ConfirmPasswordForm ) @@ -42,6 +42,22 @@ def userprofile_email(): thing='email address', form_field=form.email_address ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_email_authenticate')) + + +@main.route("/user-profile/email/authenticate", methods=['GET', 'POST']) +def userprofile_email_authenticate(): + + form = ConfirmPasswordForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/authenticate.html', + thing='email address', + form=form, + back_link=url_for('.userprofile_email') + ) elif request.method == 'POST': return redirect(url_for('.userprofile_email_confirm')) @@ -74,6 +90,22 @@ def userprofile_mobile_number(): thing='mobile number', form_field=form.mobile_number ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_mobile_number_authenticate')) + + +@main.route("/user-profile/mobile-number/authenticate", methods=['GET', 'POST']) +def userprofile_mobile_number_authenticate(): + + form = ConfirmPasswordForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/authenticate.html', + thing='mobile number', + form=form, + back_link=url_for('.userprofile_mobile_number_confirm') + ) elif request.method == 'POST': return redirect(url_for('.userprofile_mobile_number_confirm')) @@ -96,13 +128,12 @@ def userprofile_mobile_number_confirm(): @main.route("/user-profile/password", methods=['GET', 'POST']) def userprofile_password(): - form = NewPasswordForm() + form = ChangePasswordForm() if request.method == 'GET': return render_template( - 'views/user-profile/change.html', - thing='password', - form_field=form.new_password + 'views/user-profile/change-password.html', + form=form ) elif request.method == 'POST': return redirect(url_for('.userprofile')) diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index fe6210b0b..add765160 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -17,7 +17,7 @@ Sign in
{{ textbox(form.email_address) }} {{ textbox(form.password) }} - {{ page_footer("Continue", back_link="#", back_link_text="Forgotten password?") }} + {{ page_footer("Continue", back_link=url_for('.forgot_password'), back_link_text="Forgotten password?") }}
diff --git a/app/templates/views/user-profile/authenticate.html b/app/templates/views/user-profile/authenticate.html new file mode 100644 index 000000000..8587bcc85 --- /dev/null +++ b/app/templates/views/user-profile/authenticate.html @@ -0,0 +1,26 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your {{ thing }}

+ +
+
+ +
+ {{ textbox(form.password) }} + {{ page_footer( + 'Confirm', + back_link=back_link + ) }} +
+
+
+ +{% endblock %} diff --git a/app/templates/views/user-profile/change-password.html b/app/templates/views/user-profile/change-password.html new file mode 100644 index 000000000..108258bfb --- /dev/null +++ b/app/templates/views/user-profile/change-password.html @@ -0,0 +1,27 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your password

+ +
+
+
+ {{ textbox(form.old_password) }} + {{ textbox(form.new_password) }} + {{ page_footer( + 'Save', + back_link=url_for('.userprofile'), + back_link_text="Back to your profile" + ) }} +
+
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index a292c271f..ae052cdd0 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -29,6 +29,21 @@ def test_should_show_email_page(notifications_admin): def test_should_redirect_after_email_change(notifications_admin): response = notifications_admin.test_client().post('/user-profile/email') + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/email/authenticate' + + +def test_should_show_authenticate_after_email_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/email/authenticate') + + assert 'Change your email address' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_email_change_confirm(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/email/authenticate') + assert response.status_code == 302 assert response.location == 'http://localhost/user-profile/email/confirm' @@ -59,7 +74,22 @@ def test_should_redirect_after_mobile_number_change(notifications_admin): response = notifications_admin.test_client().post('/user-profile/email') assert response.status_code == 302 - assert response.location == 'http://localhost/user-profile/email/confirm' + assert response.location == 'http://localhost/user-profile/email/authenticate' + + +def test_should_show_authenticate_after_mobile_number_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/mobile-number/authenticate') + + assert 'Change your mobile number' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_mobile_number_authenticate(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/mobile-number/authenticate') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/mobile-number/confirm' def test_should_show_confirm_after_mobile_number_change(notifications_admin):