add tests for new check and send notification endpoints

app/main/views/send.py

@@ -601,12 +601,6 @@ def get_back_link(service_id, template_id, step_index):
 @login_required
 @user_has_permissions('manage_templates')
 def check_notification(service_id, template_id):
-    # go back to start of process
-    back_link = get_back_link(service_id, template_id, 0)
-
-    if {'recipient', 'placeholders'} - set(session.keys()) and back_link:
-        return redirect(back_link)
-
     db_template = service_api_client.get_service_template(service_id, template_id)['data']
 
     template = get_template(
@@ -614,6 +608,19 @@ def check_notification(service_id, template_id):
         current_service,
         show_recipient=True
     )
+
+    # go back to start of process
+    back_link = get_back_link(service_id, template_id, 0)
+
+    if (
+        (
+            not session.get('recipient') or
+            not all_placeholders_in_session(template.placeholders)
+        )
+        and back_link
+    ):
+        return redirect(back_link)
+
     template.values = get_receipient_and_placeholders_from_session(template.template_type)
 
     return render_template(

app/notify_client/notification_api_client.py

@@ -55,7 +55,7 @@ class NotificationApiClient(NotifyAdminAPIClient):
                 params=params
             )
 
-    def send_notification(self, service_id, *, template_id, recipient, personalisation=None):
+    def send_notification(self, service_id, *, template_id, recipient, personalisation):
         data = {
             'template_id': template_id,
             'to': recipient,

tests/app/main/views/test_send.py

@@ -561,7 +561,7 @@ def _redirects_with_help_argument(
     )
 
 
-def test_send_test_email_message_without_placeholders(
+def test_send_test_email_message_without_placeholders_redirects_to_check_page(
     logged_in_client,
     mocker,
     service_one,
@@ -571,19 +571,16 @@ def test_send_test_email_message_without_placeholders(
     mock_get_detailed_service_for_today,
     fake_uuid,
 ):
-
+    with logged_in_client.session_transaction() as session:
-    mocker.patch('app.main.views.send.s3download', return_value='email address\r\ntest@user.gov.uk')
+        session['recipient'] = 'foo@bar.com'
 
     response = logged_in_client.get(
-        url_for('main.send_test', service_id=service_one['id'], template_id=fake_uuid),
+        url_for('main.send_test', step_index=0, service_id=service_one['id'], template_id=fake_uuid),
         follow_redirects=True
     )
     assert response.status_code == 200
-    mock_s3_upload.assert_called_with(
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-        service_one['id'],
+    assert page.select('h1')[0].text.strip() == 'Preview of Two week reminder'
-        {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Report'},
-        'eu-west-1'
-    )
 
 
 def test_send_test_sms_message_with_placeholders_shows_first_field(
@@ -778,6 +775,7 @@ def test_send_test_sms_message_puts_submitted_data_in_session(
     fake_uuid,
 ):
     with logged_in_client.session_transaction() as session:
+        session['recipient'] = '07700 900762'
         session['placeholders'] = {}
 
     response = logged_in_client.post(
@@ -802,7 +800,6 @@ def test_send_test_sms_message_puts_submitted_data_in_session(
         assert session['placeholders']['name'] == 'Jo'
 
 
-
 @pytest.mark.parametrize('filetype', ['pdf', 'png'])
 def test_send_test_works_as_letter_preview(
     filetype,
@@ -866,7 +863,7 @@ def test_send_test_clears_session(
     assert response.status_code == 302
 
     with logged_in_client.session_transaction() as session:
-        assert session['recipient'] == None
+        assert session['recipient'] is None
         assert session['placeholders'] == {}
 
 
@@ -1589,3 +1586,143 @@ def test_check_messages_back_from_help_handles_unexpected_templates(client, mock
         'main.choose_template',
         service_id='1234',
     )
+
+
+@pytest.mark.parametrize('existing_session_items', [
+    {},
+    {'recipient': '07700900001'},
+    {'name': 'Jo'}
+])
+def test_check_notification_redirects_if_session_not_populated(
+    logged_in_client,
+    service_one,
+    fake_uuid,
+    existing_session_items,
+    mock_get_service_template_with_placeholders
+):
+    with logged_in_client.session_transaction() as session:
+        session.update(existing_session_items)
+
+    resp = logged_in_client.get(url_for(
+        'main.check_notification',
+        service_id=service_one['id'],
+        template_id=fake_uuid
+    ))
+
+    assert resp.location == url_for(
+        'main.view_template',
+        service_id=service_one['id'],
+        template_id=fake_uuid,
+        _external=True
+    )
+
+
+def test_check_notification_shows_preview(
+    client_request,
+    service_one,
+    fake_uuid,
+    mock_get_service_template
+):
+    with client_request.session_transaction() as session:
+        session['recipient'] = '07700900001'
+        session['placeholders'] = {}
+
+    page = client_request.get(
+        'main.check_notification',
+        service_id=service_one['id'],
+        template_id=fake_uuid
+    )
+
+    assert page.h1.text.strip() == 'Preview of Two week reminder'
+    assert (
+        page.findAll('a', {'class': 'page-footer-back-link'})[0]['href']
+    ) == url_for('main.view_template', service_id=service_one['id'], template_id=fake_uuid)
+
+
+def test_send_notification_submits_data(
+    client_request,
+    service_one,
+    fake_uuid,
+    mock_send_notification,
+):
+    with client_request.session_transaction() as session:
+        session['recipient'] = '07700900001'
+        session['placeholders'] = {'a': 'b'}
+
+    client_request.post(
+        'main.send_notification',
+        service_id=service_one['id'],
+        template_id=fake_uuid
+    )
+
+    mock_send_notification.assert_called_once_with(
+        service_one['id'],
+        template_id=fake_uuid,
+        recipient='07700900001',
+        personalisation={'a': 'b'}
+    )
+
+
+def test_send_notification_clears_session(
+    client_request,
+    service_one,
+    fake_uuid,
+    mock_send_notification,
+):
+    with client_request.session_transaction() as session:
+        session['recipient'] = '07700900001'
+        session['placeholders'] = {'a': 'b'}
+
+    client_request.post(
+        'main.send_notification',
+        service_id=service_one['id'],
+        template_id=fake_uuid
+    )
+
+    with client_request.session_transaction() as session:
+        assert 'recipient' not in session
+        assert 'placeholders' not in session
+
+
+def test_send_notification_redirects_if_missing_data(
+    logged_in_client,
+    service_one,
+    fake_uuid,
+):
+    with logged_in_client.session_transaction() as session:
+        session['placeholders'] = {'a': 'b'}
+
+    resp = logged_in_client.post(
+        url_for('main.send_notification', service_id=service_one['id'], template_id=fake_uuid)
+    )
+
+    assert resp.status_code == 302
+    assert resp.location == url_for(
+        '.send_one_off',
+        service_id=service_one['id'],
+        template_id=fake_uuid,
+        _external=True
+    )
+
+
+def test_send_notification_redirects_to_view_page(
+    logged_in_client,
+    service_one,
+    fake_uuid,
+    mock_send_notification,
+):
+    with logged_in_client.session_transaction() as session:
+        session['recipient'] = '07700900001'
+        session['placeholders'] = {'a': 'b'}
+
+    resp = logged_in_client.post(
+        url_for('main.send_notification', service_id=service_one['id'], template_id=fake_uuid)
+    )
+
+    assert resp.status_code == 302
+    assert resp.location == url_for(
+        '.view_notification',
+        service_id=service_one['id'],
+        notification_id=fake_uuid,
+        _external=True
+    )

tests/app/notify_client/test_notification_client.py

@@ -37,7 +37,7 @@ def test_client_gets_notifications_for_service_and_job_by_page(mocker, arguments
 
 def test_send_notification(mocker, logged_in_client, active_user_with_permissions):
     mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post')
-    NotificationApiClient().send_notification('foo', template_id='bar', recipient='07700900001')
+    NotificationApiClient().send_notification('foo', template_id='bar', recipient='07700900001', personalisation=None)
     mock_post.assert_called_once_with(
         url='/service/foo/send-notification',
         data={

tests/conftest.py

@@ -1,4 +1,4 @@
-
+from contextlib import contextmanager
 import os
 from datetime import date, datetime, timedelta
 from unittest.mock import Mock
@@ -61,11 +61,6 @@ def service_with_reply_to_addresses(api_user_active):
     )
 
 
-@pytest.fixture(scope='function')
-def mock_send_sms(request, mocker):
-    return mocker.patch("app.service_api_client.send_sms")
-
-
 @pytest.fixture(scope='function')
 def fake_uuid():
     return sample_uuid()
@@ -1665,6 +1660,19 @@ def mock_get_notification(mocker, fake_uuid, notification_status='delivered'):
     )
 
 
+@pytest.fixture
+def mock_send_notification(mocker, fake_uuid):
+    def _send_notification(
+        service_id, *, template_id, recipient, personalisation
+    ):
+        return {'id': fake_uuid}
+
+    return mocker.patch(
+        'app.notification_api_client.send_notification',
+        side_effect=_send_notification
+    )
+
+
 @pytest.fixture(scope='function')
 def client(app_):
     with app_.test_request_context(), app_.test_client() as client:
@@ -1712,6 +1720,12 @@ def os_environ():
 def client_request(logged_in_client):
     class ClientRequest:
 
+        @staticmethod
+        @contextmanager
+        def session_transaction():
+            with logged_in_client.session_transaction() as session:
+                yield session
+
         @staticmethod
         def get(endpoint, _expected_status=200, _follow_redirects=False, **endpoint_kwargs):
             resp = logged_in_client.get(