When sending to yourself use 'one-off' rather than 'test' routes

There is no real reason to have to support both 'one-off' steps and also
'test' steps when sending a one off notification. It's a lot of complex
code, just to now set the one of the placeholders in the session.

We make our code much simpler but no longer using the 'test' routes but
instead adding a new endpoint to set the notification recipient when
sending to yourself before continuing on with the rest of the 'one-off'
flow.

After this is deployed for a day then we can completely remove the
'test' routes and this will help remove a lot of code complexity.
This commit is contained in:
David McDonald
2020-10-05 16:16:11 +01:00
parent 229d5eb716
commit 4aaf47aaa1
3 changed files with 102 additions and 15 deletions

View File

@@ -885,14 +885,6 @@ def get_send_test_page_title(template_type, entering_recipient, name=None):
return 'Personalise this message'
def is_current_user_the_recipient():
if 'recipient' not in session:
return False
if hasattr(current_user, 'mobile_number'):
return session['recipient'] in {current_user.email_address, current_user.mobile_number}
return session['recipient'] == current_user.email_address
def get_back_link(service_id, template, step_index, placeholders=None):
if step_index == 0:
if should_skip_template_page(template.template_type):
@@ -906,14 +898,14 @@ def get_back_link(service_id, template, step_index, placeholders=None):
service_id=service_id,
template_id=template.id,
)
elif is_current_user_the_recipient() and step_index > 1:
elif request.endpoint == 'main.send_test_step' and step_index > 1:
return url_for(
'main.send_test_step',
service_id=service_id,
template_id=template.id,
step_index=step_index - 1,
)
elif is_current_user_the_recipient() and step_index == 1:
elif request.endpoint == 'main.send_test_step' and step_index == 1:
return url_for(
'main.send_one_off_step',
service_id=service_id,
@@ -947,16 +939,40 @@ def get_skip_link(step_index, template):
if (
request.endpoint == 'main.send_one_off_step'
and step_index == 0
and template.template_type != 'letter'
and template.template_type in ("sms", "email")
and not (template.template_type == 'sms' and current_user.mobile_number is None)
and current_user.has_permissions('manage_templates', 'manage_service')
):
return (
'Use my {}'.format(first_column_headings[template.template_type][0]),
url_for('.send_test', service_id=current_service.id, template_id=template.id),
url_for('.send_one_off_to_myself', service_id=current_service.id, template_id=template.id),
)
@main.route("/services/<uuid:service_id>/template/<uuid:template_id>/one-off/send-to-myself", methods=['GET'])
@user_has_permissions('send_messages', restrict_admin_usage=True)
def send_one_off_to_myself(service_id, template_id):
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if db_template['template_type'] not in ("sms", "email"):
abort(404)
# We aren't concerned with creating the exact template (for example adding recipient and sender names)
# we just want to create enough to use `fields_to_fill_in`
template = get_template(
db_template,
current_service,
)
fields_to_fill_in(template, prefill_current_user=True)
return redirect(url_for(
'main.send_one_off_step',
service_id=service_id,
template_id=template_id,
step_index=1,
))
@main.route("/services/<uuid:service_id>/template/<uuid:template_id>/notification/check", methods=['GET'])
@user_has_permissions('send_messages', restrict_admin_usage=True)
def check_notification(service_id, template_id):

View File

@@ -265,6 +265,7 @@ class HeaderNavigation(Navigation):
'send_one_off',
'send_one_off_letter_address',
'send_one_off_step',
'send_one_off_to_myself',
'send_test',
'no_cookie.send_test_preview',
'send_test_step',
@@ -414,6 +415,7 @@ class MainNavigation(Navigation):
'send_one_off',
'send_one_off_letter_address',
'send_one_off_step',
'send_one_off_to_myself',
'send_test',
'no_cookie.send_test_preview',
'send_test_step',
@@ -726,6 +728,7 @@ class CaseworkNavigation(Navigation):
'send_one_off',
'send_one_off_letter_address',
'send_one_off_step',
'send_one_off_to_myself',
'send_test',
'send_test_step',
},
@@ -1232,6 +1235,7 @@ class OrgNavigation(Navigation):
'send_one_off',
'send_one_off_letter_address',
'send_one_off_step',
'send_one_off_to_myself',
'send_test',
'no_cookie.send_test_preview',
'send_test_step',

View File

@@ -1504,13 +1504,13 @@ def test_send_one_off_or_test_shows_placeholders_in_correct_order(
create_active_user_with_permissions(),
'sms',
'Use my phone number',
partial(url_for, 'main.send_test')
partial(url_for, 'main.send_one_off_to_myself')
),
(
create_active_user_with_permissions(),
'email',
'Use my email address',
partial(url_for, 'main.send_test')
partial(url_for, 'main.send_one_off_to_myself')
),
(
create_active_user_with_permissions(),
@@ -1720,7 +1720,7 @@ def test_send_one_off_has_link_to_use_existing_list(
(
'Use my phone number',
url_for(
'main.send_test',
'main.send_one_off_to_myself',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
),
@@ -4506,3 +4506,70 @@ def test_send_from_contact_list(
mock_set_metadata.assert_called_once_with(
SERVICE_ONE_ID, new_uuid, example_key='example value'
)
def test_send_to_myself_sets_placeholder_and_redirects_for_email(
mocker, client_request, fake_uuid, mock_get_service_email_template
):
with client_request.session_transaction() as session:
session['recipient'] = None
session['placeholders'] = {}
client_request.get(
'main.send_one_off_to_myself',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
_expected_status=302,
_expected_url=url_for(
'main.send_one_off_step',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
step_index=1,
_external=True,
)
)
with client_request.session_transaction() as session:
assert session['recipient'] == 'test@user.gov.uk'
assert session['placeholders'] == {'email address': 'test@user.gov.uk'}
def test_send_to_myself_sets_placeholder_and_redirects_for_sms(
mocker, client_request, fake_uuid, mock_get_service_template
):
with client_request.session_transaction() as session:
session['recipient'] = None
session['placeholders'] = {}
client_request.get(
'main.send_one_off_to_myself',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
_expected_status=302,
_expected_url=url_for(
'main.send_one_off_step',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
step_index=1,
_external=True,
)
)
with client_request.session_transaction() as session:
assert session['recipient'] == '07700 900762'
assert session['placeholders'] == {'phone number': '07700 900762'}
def test_send_to_myself_404s_for_letter(
mocker, client_request, fake_uuid, mock_get_service_letter_template
):
with client_request.session_transaction() as session:
session['recipient'] = None
session['placeholders'] = {}
client_request.get(
'main.send_one_off_to_myself',
service_id=SERVICE_ONE_ID,
template_id=fake_uuid,
_expected_status=404,
)