diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index ada8dcb16..dd976d3d0 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -19,12 +19,13 @@ from app import ( from app.main import main from app.main.forms import ( ChangeEmailForm, + ChangeMobileNumberForm, InviteUserForm, PermissionsForm, SearchUsersForm, ) from app.models.user import permissions -from app.utils import user_has_permissions +from app.utils import redact_mobile_number, user_has_permissions @main.route("/services//users") @@ -78,7 +79,10 @@ def invite_user(service_id): def edit_user_permissions(service_id, user_id): service_has_email_auth = current_service.has_permission('email_auth') user = current_service.get_team_member(user_id) - user_has_no_mobile_number = user.mobile_number is None + + mobile_number = None + if user.mobile_number: + mobile_number = redact_mobile_number(user.mobile_number, " ") form = PermissionsForm.from_user(user, service_id) @@ -96,7 +100,7 @@ def edit_user_permissions(service_id, user_id): user=user, form=form, service_has_email_auth=service_has_email_auth, - user_has_no_mobile_number=user_has_no_mobile_number + mobile_number=mobile_number ) @@ -178,15 +182,9 @@ def confirm_edit_user_email(service_id, user_id): try: user_api_client.update_user_attribute(str(user_id), email_address=new_email) except HTTPError as e: - if e.status_code == 403: - flash("You don't have permission to edit users emails for this service", 'info') - return redirect(url_for( - '.manage_users', - service_id=service_id)) - else: - abort(500, e) + abort(500, e) finally: - session.pop("team_member_email_change", None) + session.pop('team_member_email_change', None) return redirect(url_for( '.manage_users', @@ -200,6 +198,65 @@ def confirm_edit_user_email(service_id, user_id): ) +@main.route("/services//users//edit-mobile-number", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_service') +def edit_user_mobile_number(service_id, user_id): + user = current_service.get_team_member(user_id) + user_mobile_number = redact_mobile_number(user.mobile_number) + + form = ChangeMobileNumberForm(mobile_number=user_mobile_number) + if form.mobile_number.data == user_mobile_number and request.method == 'POST': + return redirect(url_for( + '.manage_users', + service_id=service_id + )) + if form.validate_on_submit(): + session['team_member_mobile_change'] = form.mobile_number.data + + return redirect(url_for('.confirm_edit_user_mobile_number', user_id=user.id, service_id=service_id)) + return render_template( + 'views/manage-users/edit-user-mobile.html', + user=user, + form=form, + service_id=service_id + ) + + +@main.route("/services//users//edit-mobile-number/confirm", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_service') +def confirm_edit_user_mobile_number(service_id, user_id): + user = current_service.get_team_member(user_id) + if 'team_member_mobile_change' in session: + new_number = session['team_member_mobile_change'] + else: + return redirect(url_for( + '.edit_user_mobile_number', + service_id=service_id, + user_id=user_id + )) + if request.method == 'POST': + try: + user_api_client.update_user_attribute(str(user_id), mobile_number=new_number) + except HTTPError as e: + abort(500, e) + finally: + session.pop('team_member_mobile_change', None) + + return redirect(url_for( + '.manage_users', + service_id=service_id + )) + + return render_template( + 'views/manage-users/confirm-edit-user-mobile-number.html', + user=user, + service_id=service_id, + new_mobile_number=new_number + ) + + @main.route("/services//cancel-invited-user/", methods=['GET']) @user_has_permissions('manage_service') def cancel_invited_user(service_id, invited_user_id): diff --git a/app/navigation.py b/app/navigation.py index 125c81bc3..e19042980 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -136,6 +136,7 @@ class HeaderNavigation(Navigation): 'choose_template_to_copy', 'confirm_edit_organisation_name', 'confirm_edit_user_email', + 'confirm_edit_user_mobile_number', 'confirm_redact_template', 'conversation', 'conversation_reply', @@ -159,6 +160,7 @@ class HeaderNavigation(Navigation): 'edit_template_postage', 'edit_user_org_permissions', 'edit_user_email', + 'edit_user_mobile_number', 'edit_user_permissions', 'email_not_received', 'email_template', @@ -327,7 +329,9 @@ class MainNavigation(Navigation): }, 'team-members': { 'confirm_edit_user_email', + 'confirm_edit_user_mobile_number', 'edit_user_email', + 'edit_user_mobile_number', 'edit_user_permissions', 'invite_user', 'manage_users', @@ -584,6 +588,7 @@ class CaseworkNavigation(Navigation): 'clear_cache', 'confirm_edit_organisation_name', 'confirm_edit_user_email', + 'confirm_edit_user_mobile_number', 'confirm_redact_template', 'conversation', 'conversation_reply', @@ -609,6 +614,7 @@ class CaseworkNavigation(Navigation): 'edit_service_template', 'edit_template_postage', 'edit_user_email', + 'edit_user_mobile_number', 'edit_user_org_permissions', 'edit_user_permissions', 'email_branding', @@ -823,6 +829,7 @@ class OrgNavigation(Navigation): 'choose_template_to_copy', 'clear_cache', 'confirm_edit_user_email', + 'confirm_edit_user_mobile_number', 'confirm_redact_template', 'conversation', 'conversation_reply', @@ -847,6 +854,7 @@ class OrgNavigation(Navigation): 'edit_service_template', 'edit_template_postage', 'edit_user_email', + 'edit_user_mobile_number', 'edit_user_permissions', 'email_branding', 'email_not_received', diff --git a/app/templates/views/edit-user-permissions.html b/app/templates/views/edit-user-permissions.html index 4bd585d96..17a4ef88f 100644 --- a/app/templates/views/edit-user-permissions.html +++ b/app/templates/views/edit-user-permissions.html @@ -16,7 +16,11 @@

{{ user.email_address }} Change

- + {% if mobile_number %} +

+ {{ mobile_number }} Change +

+ {% endif %}
{% call form_wrapper(class="column-three-quarters") %} diff --git a/app/templates/views/manage-users/confirm-edit-user-mobile-number.html b/app/templates/views/manage-users/confirm-edit-user-mobile-number.html new file mode 100644 index 000000000..99265137e --- /dev/null +++ b/app/templates/views/manage-users/confirm-edit-user-mobile-number.html @@ -0,0 +1,30 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% block per_page_title %} + Confirm change of mobile number +{% endblock %} + +{% block maincolumn_content %} + +

Confirm change of mobile number

+ +
+
+ {% call form_wrapper() %} +

New mobile number:

+
+

{{ new_mobile_number }}

+
+

We will send {{ user.name }} an email to tell them about the change.

+ {{ page_footer( + 'Confirm', + destructive=destructive, + back_link=url_for('.edit_user_mobile_number', service_id=service_id, user_id=user.id) + ) }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/app/templates/views/manage-users/edit-user-mobile.html b/app/templates/views/manage-users/edit-user-mobile.html new file mode 100644 index 000000000..08cadbc61 --- /dev/null +++ b/app/templates/views/manage-users/edit-user-mobile.html @@ -0,0 +1,27 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/form.html" import form_wrapper %} + +{% block per_page_title %} + Change team member’s mobile number +{% endblock %} + +{% block maincolumn_content %} + +

Change team member’s mobile number

+

This will change the mobile number for {{ user.name }}.

+
+
+ {% call form_wrapper(class="extra-tracking") %} + {{ textbox(form.mobile_number) }} + {{ page_footer( + 'Save', + back_link=url_for('.edit_user_permissions', service_id=service_id, user_id=user.id), + back_link_text="Back" + ) }} + {% endcall %} +
+
+ +{% endblock %} diff --git a/app/templates/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index 2c3414caf..fab879e4f 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -15,7 +15,7 @@

{% if service_has_email_auth %} - {% if user_has_no_mobile_number %} + {% if not mobile_number %} {{ radios( form.login_authentication, disable=['sms_auth'], diff --git a/app/utils.py b/app/utils.py index e9615c725..d0da4e6c3 100644 --- a/app/utils.py +++ b/app/utils.py @@ -650,9 +650,9 @@ def guess_name_from_email_address(email_address): def should_skip_template_page(template_type): return ( - current_user.has_permissions('send_messages') and - not current_user.has_permissions('manage_templates', 'manage_api_keys') and - template_type != 'letter' + current_user.has_permissions('send_messages') + and not current_user.has_permissions('manage_templates', 'manage_api_keys') + and template_type != 'letter' ) @@ -671,3 +671,12 @@ def printing_today_or_tomorrow(): return 'today' else: return 'tomorrow' + + +def redact_mobile_number(mobile_number, spacing=""): + indices = [-4, -5, -6, -7] + redact_character = spacing + "•" + spacing + mobile_number_list = list(mobile_number.replace(" ", "")) + for i in indices: + mobile_number_list[i] = redact_character + return "".join(mobile_number_list) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 11e87b737..d50910c62 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -1005,3 +1005,176 @@ def test_confirm_edit_user_email_doesnt_change_user_email_for_non_team_member( user_id=USER_ONE_ID, _expected_status=404, ) + + +def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_link( + client_request, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker +): + user = active_user_with_permissions + mocker.patch('app.user_api_client.get_user', return_value=user) + + page = client_request.get( + 'main.edit_user_permissions', + service_id=service_one['id'], + user_id=user.id + ) + + assert user.name in page.find('h1').text + mobile_number_paragraph = page.select('p[id=user_mobile_number]')[0] + assert '0770 • • • • 762' in mobile_number_paragraph.text + change_link = mobile_number_paragraph.findChild() + assert change_link.attrs['href'] == '/services/{}/users/{}/edit-mobile-number'.format( + service_one['id'], user.id + ) + + +def test_edit_user_mobile_number_page( + client_request, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker +): + user = active_user_with_permissions + mocker.patch('app.user_api_client.get_user', return_value=user) + + page = client_request.get( + 'main.edit_user_mobile_number', + service_id=service_one['id'], + user_id=user.id + ) + + assert page.find('h1').text == "Change team member’s mobile number" + assert page.select('p[id=user_name]')[0].text == "This will change the mobile number for {}.".format(user.name) + assert page.select('input[name=mobile_number]')[0].attrs["value"] == "0770••••762" + assert page.select('button[type=submit]')[0].text == "Save" + + +def test_edit_user_mobile_number_redirects_to_confirmation( + logged_in_client, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker, + mock_get_user, +): + + data = {'mobile_number': '07554080636'} + response = logged_in_client.post( + url_for( + 'main.edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id), data=data) + assert response.status_code == 302 + assert response.location == url_for( + 'main.confirm_edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id, + _external=True + ) + + +def test_edit_user_mobile_number_redirects_to_manage_users_if_number_not_changed( + logged_in_client, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker, + mock_get_user, +): + + data = {'mobile_number': '0770••••762'} + response = logged_in_client.post( + url_for( + 'main.edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id), data=data) + assert response.status_code == 302 + assert response.location == url_for( + 'main.manage_users', service_id=service_one['id'], _external=True) + + +def test_confirm_edit_user_mobile_number_page( + logged_in_client, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker, + mock_get_user, +): + new_number = '07554080636' + with logged_in_client.session_transaction() as session: + session['team_member_mobile_change'] = new_number + response = logged_in_client.get(url_for( + 'main.confirm_edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id + )) + + assert response.status_code == 200 + assert 'Confirm change of mobile number' in response.get_data(as_text=True) + for text in [ + 'New mobile number:', + new_number, + 'We will send {} an email to tell them about the change.'.format(active_user_with_permissions.name) + ]: + assert text in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + + +def test_confirm_edit_user_mobile_number_page_redirects_if_session_empty( + logged_in_client, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker, + mock_get_user, +): + response = logged_in_client.get(url_for( + 'main.confirm_edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id + )) + assert response.status_code == 302 + assert 'Confirm change of mobile number' not in response.get_data(as_text=True) + + +def test_confirm_edit_user_mobile_number_changes_user_mobile_number( + logged_in_client, + active_user_with_permissions, + mock_get_users_by_service, + service_one, + mocker, + mock_get_user, + mock_update_user_attribute +): + new_number = '07554080636' + with logged_in_client.session_transaction() as session: + session['team_member_mobile_change'] = new_number + response = logged_in_client.post( + url_for( + 'main.confirm_edit_user_mobile_number', + service_id=service_one['id'], + user_id=active_user_with_permissions.id)) + assert response.status_code == 302 + assert response.location == url_for( + 'main.manage_users', service_id=service_one['id'], _external=True) + mock_update_user_attribute.assert_called_once_with(active_user_with_permissions.id, mobile_number=new_number) + + +def test_confirm_edit_user_mobile_number_doesnt_change_user_mobile_for_non_team_member( + client_request, + mock_get_users_by_service, +): + with client_request.session_transaction() as session: + session['team_member_mobile_change'] = '07554080636' + client_request.post( + 'main.confirm_edit_user_mobile_number', + service_id=SERVICE_ONE_ID, + user_id=USER_ONE_ID, + _expected_status=404, + )