From 3683f99c3bef135a0f50ca0d12e7b176de08295e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 2 Jul 2018 09:08:21 +0100 Subject: [PATCH 1/3] =?UTF-8?q?Guess=20people=E2=80=99s=20names=20when=20t?= =?UTF-8?q?hey=E2=80=99re=20invited?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most people’s names, especially in government are in the format firstname.lastname@department.gov.uk. This means that you can pretty reliably guess that their name is ‘Firstname Lastname’. When users are invited to Notify we know their email address already. So this commit pre-populates the registration form based on this guess. This is a nice little detail, but it should also stop the browser pre-filling the name field with someone’s email address (which I think happens because the browser assumes a registration form will have an email field). --- app/main/forms.py | 11 ++++----- app/utils.py | 11 +++++++++ tests/app/main/views/test_register.py | 35 +++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 55faafbcc..826080376 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -41,6 +41,7 @@ from app.main.validators import ( ValidGovEmail, ) from app.notify_client.models import roles +from app.utils import guess_name_from_email_address def get_time_value_and_label(future_time): @@ -215,20 +216,18 @@ class RegisterUserForm(StripWhitespaceForm): auth_type = HiddenField('auth_type', default='sms_auth') -class RegisterUserFromInviteForm(StripWhitespaceForm): +class RegisterUserFromInviteForm(RegisterUserForm): def __init__(self, invited_user): super().__init__( service=invited_user['service'], email_address=invited_user['email_address'], auth_type=invited_user['auth_type'], + name=guess_name_from_email_address( + invited_user['email_address'] + ), ) - name = StringField( - 'Full name', - validators=[DataRequired(message='Can’t be empty')] - ) mobile_number = InternationalPhoneNumber('Mobile number', validators=[]) - password = password() service = HiddenField('service') email_address = HiddenField('email_address') auth_type = HiddenField('auth_type', validators=[DataRequired()]) diff --git a/app/utils.py b/app/utils.py index 4cb3a1d2a..b57732bd4 100644 --- a/app/utils.py +++ b/app/utils.py @@ -25,6 +25,7 @@ from flask import ( url_for, ) from flask_login import current_user +from notifications_utils.formatters import make_quotes_smart from notifications_utils.recipients import RecipientCSV from notifications_utils.template import ( EmailPreviewTemplate, @@ -613,3 +614,13 @@ class GovernmentEmailDomain(AgreementInfo): def unicode_truncate(s, length): encoded = s.encode('utf-8')[:length] return encoded.decode('utf-8', 'ignore') + + +def guess_name_from_email_address(email_address): + + possible_name = re.split(r'[\@\+]', email_address)[0] + + if '.' not in possible_name: + return '' + + return make_quotes_smart(possible_name.replace('.', ' ').title()) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 8341f23f9..45c4aa714 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -170,6 +170,34 @@ def test_register_with_existing_email_sends_emails( assert response.location == url_for('main.registration_continue', _external=True) +@pytest.mark.parametrize('email_address, expected_value', [ + ("example123@example.com", ""), + ("first.last@example.com", "First Last"), + ("first.middle.last@example.com", "First Middle Last"), + ("first.last-last@example.com", "First Last-Last"), + ("first.o'last@example.com", "First O’Last"), + ("first.last+testing@example.com", "First Last"), + ("first.last+testing+testing@example.com", "First Last"), +]) +def test_shows_registration_page_from_invite( + client_request, + fake_uuid, + email_address, + expected_value, +): + with client_request.session_transaction() as session: + session['invited_user'] = InvitedUser( + fake_uuid, fake_uuid, "", + email_address, + ["manage_users"], + "pending", + datetime.utcnow(), + 'sms_auth', + ).serialize() + page = client_request.get('main.register_from_invite') + assert page.select_one('input[name=name]')['value'] == expected_value + + def test_register_from_invite( client, fake_uuid, @@ -199,6 +227,13 @@ def test_register_from_invite( ) assert response.status_code == 302 assert response.location == url_for('main.verify', _external=True) + mock_register_user.assert_called_once_with( + 'Registered in another Browser', + invited_user.email_address, + '+4407700900460', + 'somreallyhardthingtoguess', + 'sms_auth', + ) def test_register_from_invite_when_user_registers_in_another_browser( From 5c5e0bac021244c3b3f0dee34e49b330266cf8c3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 2 Jul 2018 09:16:48 +0100 Subject: [PATCH 2/3] Stop email address wrapping --- app/assets/stylesheets/app.scss | 4 ++++ app/templates/views/register-from-invite.html | 5 ++++- tests/app/main/views/test_accept_invite.py | 6 ++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 0e64b1e23..ccc81098c 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -257,3 +257,7 @@ details .arrow { outline: 2px solid $black; max-width: 100%; } + +.nowrap { + white-space: nowrap; +} diff --git a/app/templates/views/register-from-invite.html b/app/templates/views/register-from-invite.html index 4051b8086..466b2f276 100644 --- a/app/templates/views/register-from-invite.html +++ b/app/templates/views/register-from-invite.html @@ -11,7 +11,10 @@ Create an account

Create an account

-

Your account will be created with this email: {{invited_user.email_address}}

+

+ Your account will be created with this email address: + {{invited_user.email_address}} +

{{ textbox(form.name, width='3-4') }} {% if invited_user.auth_type == 'sms_auth' %} diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index f8686f9d0..8e5513322 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -264,8 +264,10 @@ def test_new_user_accept_invite_calls_api_and_views_registration_page( page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Create an account' - email_in_page = page.find('main').find('p') - assert email_in_page.text.strip() == 'Your account will be created with this email: invited_user@test.gov.uk' # noqa + assert normalize_spaces(page.select_one('main p').text) == ( + 'Your account will be created with this email address: ' + 'invited_user@test.gov.uk' + ) form = page.find('form') name = form.find('input', id='name') From 660fadbce7771f2844187ffe1fbb1917a03f73ea Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 11 Jul 2018 13:31:38 +0100 Subject: [PATCH 3/3] Make the guessing a bit more sophisticated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Things we’ve noticed from looking at real data that we could handle in a smarter way: - removing numbers (there might be a tom.smith2@dept.gov.uk if tom.smith is already taken) - removing middle initials (again, these tend to be used for disambiguation and aren’t included when we ask people for their names) - ignoring email addresses which only have someone’s initial, not their first name (because we can’t make a decent guess in this case) --- app/utils.py | 35 +++++++++++++++++++++++++-- tests/app/main/views/test_register.py | 12 ++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/app/utils.py b/app/utils.py index b57732bd4..2c57a5d9c 100644 --- a/app/utils.py +++ b/app/utils.py @@ -27,6 +27,7 @@ from flask import ( from flask_login import current_user from notifications_utils.formatters import make_quotes_smart from notifications_utils.recipients import RecipientCSV +from notifications_utils.take import Take from notifications_utils.template import ( EmailPreviewTemplate, LetterImageTemplate, @@ -616,11 +617,41 @@ def unicode_truncate(s, length): return encoded.decode('utf-8', 'ignore') +def starts_with_initial(name): + return bool(re.match(r'^.\.', name)) + + +def remove_middle_initial(name): + return re.sub(r'\s+.\s+', ' ', name) + + +def remove_digits(name): + return ''.join(c for c in name if not c.isdigit()) + + +def normalize_spaces(name): + return ' '.join(name.split()) + + def guess_name_from_email_address(email_address): possible_name = re.split(r'[\@\+]', email_address)[0] - if '.' not in possible_name: + if '.' not in possible_name or starts_with_initial(possible_name): return '' - return make_quotes_smart(possible_name.replace('.', ' ').title()) + return Take( + possible_name + ).then( + str.replace, '.', ' ' + ).then( + remove_digits + ).then( + remove_middle_initial + ).then( + str.title + ).then( + make_quotes_smart + ).then( + normalize_spaces + ) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 45c4aa714..d5cd1847c 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -171,13 +171,23 @@ def test_register_with_existing_email_sends_emails( @pytest.mark.parametrize('email_address, expected_value', [ - ("example123@example.com", ""), ("first.last@example.com", "First Last"), ("first.middle.last@example.com", "First Middle Last"), + ("first.m.last@example.com", "First Last"), ("first.last-last@example.com", "First Last-Last"), ("first.o'last@example.com", "First O’Last"), ("first.last+testing@example.com", "First Last"), ("first.last+testing+testing@example.com", "First Last"), + ("first.last6@example.com", "First Last"), + ("first.last.212@example.com", "First Last"), + ("first.2.last@example.com", "First Last"), + ("first.2b.last@example.com", "First Last"), + ("first.1.2.3.last@example.com", "First Last"), + ("first.last.1.2.3@example.com", "First Last"), + # Instances where we can’t make a good-enough guess: + ("example123@example.com", ""), + ("f.last@example.com", ""), + ("f.m.last@example.com", ""), ]) def test_shows_registration_page_from_invite( client_request,