diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index bcaf87853..ffd7239ab 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -82,7 +82,7 @@ def edit_user_permissions(service_id, user_id): mobile_number = None if user.mobile_number: - mobile_number = redact_mobile_number(user.mobile_number) + mobile_number = redact_mobile_number(user.mobile_number, " ") form = PermissionsForm.from_user(user, service_id) @@ -182,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', @@ -212,11 +206,15 @@ def edit_user_mobile_number(service_id, 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, @@ -230,18 +228,21 @@ def edit_user_mobile_number(service_id, user_id): @user_has_permissions('manage_service') def confirm_edit_user_mobile_number(service_id, user_id): user = user_api_client.get_user(user_id) - new_number = session['team_member_mobile_change'] + 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(user_id, mobile_number=new_number) except HTTPError as e: - if e.status_code == 403: - flash("You don't have permission to edit users' mobile numbers 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_mobile_change', None) return redirect(url_for( '.manage_users', diff --git a/app/templates/views/manage-users/edit-user-mobile.html b/app/templates/views/manage-users/edit-user-mobile.html index 860ea416e..08cadbc61 100644 --- a/app/templates/views/manage-users/edit-user-mobile.html +++ b/app/templates/views/manage-users/edit-user-mobile.html @@ -13,7 +13,7 @@

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

- {% call form_wrapper() %} + {% call form_wrapper(class="extra-tracking") %} {{ textbox(form.mobile_number) }} {{ page_footer( 'Save', diff --git a/app/utils.py b/app/utils.py index d29f21f63..d0da4e6c3 100644 --- a/app/utils.py +++ b/app/utils.py @@ -673,9 +673,10 @@ def printing_today_or_tomorrow(): return 'tomorrow' -def redact_mobile_number(mobile_number): +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] = "*" + 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 79f86eb0e..f27e52982 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -1014,6 +1014,7 @@ def test_confirm_edit_user_email_with_no_permission_aborts(): 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 ): @@ -1028,7 +1029,7 @@ def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_l 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 + 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 @@ -1052,7 +1053,7 @@ def test_edit_user_mobile_number_page( 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('input[name=mobile_number]')[0].attrs["value"] == "0770••••762" assert page.select('button[type=submit]')[0].text == "Save" @@ -1079,6 +1080,25 @@ def test_edit_user_mobile_number_redirects_to_confirmation( ) +def test_edit_user_mobile_number_redirects_to_manage_users_if_number_not_changed( + logged_in_client, + active_user_with_permissions, + 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, @@ -1106,6 +1126,22 @@ def test_confirm_edit_user_mobile_number_page( 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, + 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, @@ -1126,7 +1162,3 @@ def test_confirm_edit_user_mobile_number_changes_user_mobile_number( 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_with_no_permission_aborts(): - pass