Fix bug with reply to email addresses

https://www.pivotaltracker.com/story/show/180026726

There was a bug where if you enter an invalid email address in
to the edit reply to email address form and click save, the
form you get shown with your error message will always contain
the field to set as default the reply to and also delete. This should
not have been the case. If you make an error on the form when
changing a reply to that is already a default, then you should not
be given the chance to change it to not default, nor should you
be able to delete it.

This commit fixes that bug by making sure the additional form fields
are only shown if the reply to being changed is not the default.
This commit is contained in:
David McDonald
2021-11-04 11:21:58 +00:00
parent 1c2b65356f
commit b4b124d681
3 changed files with 67 additions and 2 deletions

View File

@@ -617,9 +617,13 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id):
def service_edit_email_reply_to(service_id, reply_to_email_id):
form = ServiceReplyToEmailForm()
reply_to_email_address = current_service.get_email_reply_to_address(reply_to_email_id)
if request.method == 'GET':
form.email_address.data = reply_to_email_address['email_address']
form.is_default.data = reply_to_email_address['is_default']
show_choice_of_default_checkbox = not reply_to_email_address['is_default']
if form.validate_on_submit():
if form.email_address.data == reply_to_email_address["email_address"] or current_user.platform_admin:
service_api_client.update_reply_to_email_address(
@@ -653,6 +657,7 @@ def service_edit_email_reply_to(service_id, reply_to_email_id):
'views/service-settings/email-reply-to/edit.html',
form=form,
reply_to_email_address_id=reply_to_email_id,
show_choice_of_default_checkbox=show_choice_of_default_checkbox
)

View File

@@ -23,7 +23,7 @@
},
error_message_with_html=True
) }}
{% if form.is_default.data %}
{% if not show_choice_of_default_checkbox %}
<p class="form-group">
This is the default reply-to address for {{ current_service.name }} emails
</p>

View File

@@ -2893,7 +2893,7 @@ def test_add_edit_reply_to_email_address_goes_straight_to_update_if_address_not_
None,
),
])
def test_shows_delete_link_for_email_reply_to_address(
def test_shows_delete_link_for_get_request_for_edit_email_reply_to_address(
mocker,
reply_to_address,
expected_link_text,
@@ -2923,6 +2923,66 @@ def test_shows_delete_link_for_email_reply_to_address(
assert not page.select('.page-footer a')
@pytest.mark.parametrize('reply_to_address, default_choice_and_delete_link_expected, default_checkbox_checked', [
(
create_reply_to_email_address(is_default=False),
True,
False
),
(
create_reply_to_email_address(is_default=False),
True,
True
),
(
create_reply_to_email_address(is_default=True),
False,
False # not expecting a checkbox to even be shown to be ticked
),
])
def test_shows_delete_link_for_error_on_post_request_for_edit_email_reply_to_address(
mocker,
reply_to_address,
default_choice_and_delete_link_expected,
default_checkbox_checked,
fake_uuid,
client_request,
):
mocker.patch('app.service_api_client.get_reply_to_email_address', return_value=reply_to_address)
data = {'email_address': "not a valid email address"}
if default_checkbox_checked:
data["is_default"] = "y"
page = client_request.post(
'main.service_edit_email_reply_to',
service_id=SERVICE_ONE_ID,
reply_to_email_id=sample_uuid(),
_data=data,
_expected_status=200
)
assert page.select_one('.govuk-back-link').text.strip() == 'Back'
assert page.select_one('.govuk-back-link')['href'] == url_for(
'.service_email_reply_to',
service_id=SERVICE_ONE_ID,
)
assert page.select_one('.govuk-error-message').text.strip() == 'Error: Enter a valid email address'
assert page.select_one('input#email_address').get('value') == 'not a valid email address'
if default_choice_and_delete_link_expected:
link = page.select_one('.page-footer a')
assert normalize_spaces(link.text) == "Delete"
assert link['href'] == url_for(
'main.service_confirm_delete_email_reply_to',
service_id=SERVICE_ONE_ID,
reply_to_email_id=sample_uuid()
)
assert page.select_one('input#is_default').has_attr('checked') == default_checkbox_checked
else:
assert not page.select('.page-footer a')
def test_confirm_delete_reply_to_email_address(
fake_uuid,
client_request,