From d8ebcdce22e10a7c0fcc31a5ea08bae7d4e3da24 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 10 Dec 2021 16:56:08 +0000 Subject: [PATCH] Stop errors when changing an email address to an invalid one We use the `ChangeEmailForm` if you want to change your own email address or someone else's email address. This has various validators which get run. We check if the email address is valid (by using a function from utils) and if the email address is already in use (by calling API). If the email address is not valid, we should not call API to see if it's already in use because this will cause an exception in API leading to a `500` in admin. We now only call API if there were no other errors with the email address. (The `test_should_redirect_after_name_change` test didn't need the `mock_email_is_not_already_in_use` fixture, so this has been removed.) --- app/main/forms.py | 6 ++++++ tests/app/main/views/test_user_profile.py | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/main/forms.py b/app/main/forms.py index 3b05c7b9e..4b1deece3 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1535,6 +1535,12 @@ class ChangeEmailForm(StripWhitespaceForm): email_address = email_address() def validate_email_address(self, field): + # The validate_email_func can be used to call API to check if the email address is already in + # use. We don't want to run that check for invalid email addresses, since that will cause an error. + # If there are any other validation errors on the email_address, we should skip this check. + if self.email_address.errors: + return + is_valid = self.validate_email_func(field.data) if is_valid: raise ValidationError("The email address is already in use") diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index c9dfcef7b..0368f8fca 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -74,7 +74,6 @@ def test_should_show_name_page( def test_should_redirect_after_name_change( client_request, mock_update_user_attribute, - mock_email_is_not_already_in_use ): client_request.post( 'main.user_profile_name', @@ -109,6 +108,8 @@ def test_should_redirect_after_email_change( ) ) + assert mock_email_is_not_already_in_use.called + @pytest.mark.parametrize('email_address,error_message', [ ('me@example.com', 'Enter a public sector email address or find out who can use Notify'), @@ -128,6 +129,8 @@ def test_should_show_errors_if_new_email_address_does_not_validate( ) assert normalize_spaces(page.find('span', class_='govuk-error-message').text) == f'Error: {error_message}' + # We only call API to check if the email address is already in use if there are no other errors + assert not mock_email_is_not_already_in_use.called def test_should_show_authenticate_after_email_change(