Edit mobile page allows users to proceed without changing mobile number

Also:
- change formatting of mobile number redact characters

- redirect if session empty

- update a test with new mock after rebase

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',

app/templates/views/manage-users/edit-user-mobile.html

@@ -13,7 +13,7 @@
   <p id="user_name">This will change the mobile number for {{ user.name }}.</p>
   <div class="grid-row">
     <div class="column-three-quarters">
-      {% call form_wrapper() %}
+      {% call form_wrapper(class="extra-tracking") %}
         {{ textbox(form.mobile_number) }}
         {{ page_footer(
           'Save',

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)

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