Merge pull request #4166 from alphagov/allow_user_delete_mobile_number

Let users on email auth delete their mobile numbers

app/main/views/user_profile.py

@@ -118,22 +118,43 @@ def user_profile_email_confirm(token):
 
 
 @main.route("/user-profile/mobile-number", methods=['GET', 'POST'])
+@main.route(
+    "/user-profile/mobile-number/delete",
+    methods=['GET'],
+    endpoint="user_profile_confirm_delete_mobile_number"
+)
 @user_is_logged_in
 def user_profile_mobile_number():
 
+    user = User.from_id(current_user.id)
     form = ChangeMobileNumberForm(mobile_number=current_user.mobile_number)
 
     if form.validate_on_submit():
         session[NEW_MOBILE] = form.mobile_number.data
         return redirect(url_for('.user_profile_mobile_number_authenticate'))
 
+    if (request.endpoint == "main.user_profile_confirm_delete_mobile_number"):
+        flash("Are you sure you want to delete your mobile number from Notify?", 'delete')
+
     return render_template(
         'views/user-profile/change.html',
         thing='mobile number',
-        form_field=form.mobile_number
+        form_field=form.mobile_number,
+        user_auth=user.auth_type
     )
 
 
+@main.route("/user-profile/mobile-number/delete", methods=['POST'])
+@user_is_logged_in
+def user_profile_mobile_number_delete():
+    if current_user.auth_type != 'email_auth':
+        abort(403)
+
+    current_user.update(mobile_number=None)
+
+    return redirect(url_for('.user_profile'))
+
+
 @main.route("/user-profile/mobile-number/authenticate", methods=['GET', 'POST'])
 @user_is_logged_in
 def user_profile_mobile_number_authenticate():

app/navigation.py

@@ -67,12 +67,14 @@ class HeaderNavigation(Navigation):
         },
         'user-profile': {
             'user_profile',
+            'user_profile_confirm_delete_mobile_number',
             'user_profile_email',
             'user_profile_email_authenticate',
             'user_profile_email_confirm',
             'user_profile_mobile_number',
             'user_profile_mobile_number_authenticate',
             'user_profile_mobile_number_confirm',
+            'user_profile_mobile_number_delete',
             'user_profile_name',
             'user_profile_password',
             'user_profile_disable_platform_admin_view',

app/templates/views/user-profile/change.html

@@ -20,9 +20,20 @@
     <div class="govuk-grid-column-three-quarters">
       {% call form_wrapper() %}
         {{ form_field(error_message_with_html=True) }}
-        {{ page_footer('Save') }}
+        {% if current_user.auth_type == 'email_auth' and (current_user.mobile_number and thing == "mobile number") %}
+          {{ page_footer(
+            'Save',
+            delete_link=url_for(
+              'main.user_profile_mobile_number_delete',
+              user_id=current_user.id
+            ),
+            delete_link_text='Delete your number'
+            )
+          }}
+        {% else %}
+          {{ page_footer('Save')}}
+        {% endif %}
       {% endcall %}
     </div>
   </div>
-
 {% endblock %}

tests/app/main/views/test_user_profile.py

@@ -12,6 +12,8 @@ from app.models.webauthn_credential import (
 )
 from tests.conftest import (
     create_api_user_active,
+    create_user,
+    fake_uuid,
     normalize_spaces,
     url_for_endpoint_with_token,
 )
@@ -91,6 +93,8 @@ def test_should_show_email_page(
         'main.user_profile_email'
     )
     assert page.select_one('h1').text.strip() == 'Change your email address'
+    # template is shared with "Change your mobile number" but we don't want to show Delete mobile number link
+    assert 'Delete your number' not in page.text
 
 
 def test_should_redirect_after_email_change(
@@ -183,6 +187,75 @@ def test_should_show_mobile_number_page(
 ):
     page = client_request.get(('main.user_profile_mobile_number'))
     assert 'Change your mobile number' in page.text
+    assert 'Delete your number' not in page.text
+
+
+def test_change_your_mobile_number_page_shows_delete_link_if_user_on_email_auth(
+    client_request,
+    api_user_active_email_auth,
+    mocker
+):
+    mocker.patch('app.user_api_client.get_user', return_value=api_user_active_email_auth)
+    page = client_request.get(('main.user_profile_mobile_number'))
+    assert 'Change your mobile number' in page.text
+    assert 'Delete your number' in page.text
+
+
+def test_change_your_mobile_number_page_doesnt_show_delete_link_if_user_has_no_mobile_number(
+    client_request,
+    api_user_active_email_auth,
+    mocker
+):
+    user = create_user(
+        id=fake_uuid,
+        auth_type='email_auth',
+        mobile_number=None
+    )
+    mocker.patch('app.user_api_client.get_user', return_value=user)
+    page = client_request.get(('main.user_profile_mobile_number'))
+    assert 'Change your mobile number' in page.text
+    assert 'Delete your number' not in page.text
+
+
+def test_confirm_delete_mobile_number(
+    client_request,
+    api_user_active_email_auth,
+    mocker
+):
+    mocker.patch('app.user_api_client.get_user', return_value=api_user_active_email_auth)
+
+    page = client_request.get(
+        '.user_profile_confirm_delete_mobile_number',
+        _test_page_title=False,
+    )
+
+    assert normalize_spaces(page.select_one('.banner-dangerous').text) == (
+        'Are you sure you want to delete your mobile number from Notify? '
+        'Yes, delete'
+    )
+    assert 'action' not in page.select_one('.banner-dangerous form')
+    assert page.select_one('.banner-dangerous form')['method'] == 'post'
+
+
+def test_delete_mobile_number(
+    client_request,
+    api_user_active_email_auth,
+    mocker
+):
+    mocker.patch('app.user_api_client.get_user', return_value=api_user_active_email_auth)
+    mock_delete = mocker.patch('app.user_api_client.update_user_attribute')
+
+    client_request.post(
+        '.user_profile_mobile_number_delete',
+        _expected_redirect=url_for(
+            '.user_profile',
+            _external=True,
+        )
+    )
+    mock_delete.assert_called_once_with(
+        api_user_active_email_auth["id"],
+        mobile_number=None
+    )
 
 
 @pytest.mark.parametrize('phone_number_to_register_with', [

tests/app/test_navigation.py

@@ -317,6 +317,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, {
     'usage',
     'user_information',
     'user_profile',
+    'user_profile_confirm_delete_mobile_number',
     'user_profile_confirm_delete_security_key',
     'user_profile_delete_security_key',
     'user_profile_disable_platform_admin_view',
@@ -327,6 +328,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, {
     'user_profile_mobile_number',
     'user_profile_mobile_number_authenticate',
     'user_profile_mobile_number_confirm',
+    'user_profile_mobile_number_delete',
     'user_profile_name',
     'user_profile_password',
     'user_profile_security_keys',