Merge pull request #2342 from alphagov/choosing_postage_goes_live

Set postage on basis of postage argument or template postage

app/notifications/process_notifications.py

@@ -23,8 +23,7 @@ from app.models import (
     LETTER_TYPE,
     NOTIFICATION_CREATED,
     Notification,
-    ScheduledNotification,
-    CHOOSE_POSTAGE
+    ScheduledNotification
 )
 from app.dao.notifications_dao import (
     dao_create_notification,
@@ -108,13 +107,7 @@ def persist_notification(
     elif notification_type == EMAIL_TYPE:
         notification.normalised_to = format_email_address(notification.to)
     elif notification_type == LETTER_TYPE:
-        if postage:
-            notification.postage = postage
-        else:
-            if service.has_permission(CHOOSE_POSTAGE) and template_postage:
-                notification.postage = template_postage
-            else:
-                notification.postage = service.postage
+        notification.postage = postage or template_postage
 
     # if simulated create a Notification model to return but do not persist the Notification to the dB
     if not simulated:

app/v2/notifications/post_notifications.py

@@ -28,6 +28,7 @@ from app.models import (
     NOTIFICATION_SENDING,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_PENDING_VIRUS_CHECK,
+    SECOND_CLASS
 )
 from app.notifications.process_letter_notifications import (
     create_letter_notification
@@ -355,6 +356,7 @@ def get_precompiled_letter_template(service_id):
         hidden=True,
         subject='Pre-compiled PDF',
         content='',
+        postage=SECOND_CLASS
     )
 
     dao_create_template(template)

tests/app/celery/test_tasks.py

@@ -40,7 +40,8 @@ from app.models import (
     JOB_STATUS_ERROR,
     JOB_STATUS_IN_PROGRESS,
     LETTER_TYPE,
-    SMS_TYPE
+    SMS_TYPE,
+    PRECOMPILED_LETTER
 )
 
 from tests.app import load_example_csv
@@ -50,6 +51,8 @@ from tests.app.conftest import (
     sample_job as create_sample_job,
     sample_email_template as create_sample_email_template,
     sample_notification as create_sample_notification,
+    sample_letter_template,
+    sample_letter_job
 )
 from tests.app.db import (
     create_inbound_sms,
@@ -1011,22 +1014,22 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session):
 
 
 @pytest.mark.parametrize('postage', ['first', 'second'])
-def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, sample_letter_job, postage):
-    sample_letter_job.service.postage = postage
+def test_save_letter_saves_letter_to_database_with_correct_postage(mocker, notify_db_session, postage):
+    service = create_service(service_permissions=[LETTER_TYPE, PRECOMPILED_LETTER])
+    template = sample_letter_template(service, postage=postage)
+    letter_job = sample_letter_job(template)
 
     mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
-
     notification_json = _notification_json(
-        template=sample_letter_job.template,
+        template=letter_job.template,
         to='Foo',
         personalisation={'addressline1': 'Foo', 'addressline2': 'Bar', 'postcode': 'Flob'},
-        job_id=sample_letter_job.id,
+        job_id=letter_job.id,
         row_number=1
     )
     notification_id = uuid.uuid4()
-
     save_letter(
-        sample_letter_job.service_id,
+        letter_job.service_id,
         notification_id,
         encryption.encrypt(notification_json),
     )

tests/app/conftest.py

@@ -253,6 +253,8 @@ def sample_template(
         data.update({
             'subject': subject_line
         })
+    if template_type == 'letter':
+        data['postage'] = 'second'
     template = Template(**data)
     dao_create_template(template)
 
@@ -317,8 +319,8 @@ def sample_template_without_email_permission(notify_db, notify_db_session):
 
 
 @pytest.fixture
-def sample_letter_template(sample_service_full_permissions):
-    return create_template(sample_service_full_permissions, template_type=LETTER_TYPE)
+def sample_letter_template(sample_service_full_permissions, postage="second"):
+    return create_template(sample_service_full_permissions, template_type=LETTER_TYPE, postage=postage)
 
 
 @pytest.fixture

tests/app/db.py

@@ -42,6 +42,7 @@ from app.models import (
     User,
     EMAIL_TYPE,
     SMS_TYPE,
+    LETTER_TYPE,
     KEY_TYPE_NORMAL,
     AnnualBilling,
     InvitedOrganisationUser,
@@ -159,9 +160,10 @@ def create_template(
         'created_by': service.created_by,
         'reply_to': reply_to,
         'hidden': hidden,
-        'folder': folder,
-        'postage': postage,
+        'folder': folder
     }
+    if template_type == LETTER_TYPE:
+        data["postage"] = postage or "second"
     if template_type != SMS_TYPE:
         data['subject'] = subject
     template = Template(**data)

tests/app/notifications/test_process_notification.py

@@ -12,8 +12,7 @@ from app.models import (
     NotificationHistory,
     ScheduledNotification,
     Template,
-    LETTER_TYPE,
-    CHOOSE_POSTAGE
+    LETTER_TYPE
 )
 from app.notifications.process_notifications import (
     create_content_for_notification,
@@ -466,22 +465,23 @@ def test_persist_email_notification_stores_normalised_email(
 
 
 @pytest.mark.parametrize(
-    "service_permissions, template_postage, expected_postage",
+    "postage_argument, template_postage, expected_postage",
     [
-        ([LETTER_TYPE], "first", "second"),
-        ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "first"),
-        ([LETTER_TYPE, CHOOSE_POSTAGE], None, "second"),
+        ("second", "first", "second"),
+        ("first", "first", "first"),
+        ("first", "second", "first"),
+        (None, "second", "second")
     ]
 )
 def test_persist_letter_notification_finds_correct_postage(
     mocker,
     notify_db,
     notify_db_session,
-    service_permissions,
+    postage_argument,
     template_postage,
     expected_postage
 ):
-    service = create_service(service_permissions=service_permissions, postage="second")
+    service = create_service(service_permissions=[LETTER_TYPE])
     api_key = create_api_key(notify_db, notify_db_session, service=service)
     template = create_template(service, template_type=LETTER_TYPE, postage=template_postage)
     mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template)
@@ -495,6 +495,7 @@ def test_persist_letter_notification_finds_correct_postage(
         notification_type=LETTER_TYPE,
         api_key_id=api_key.id,
         key_type=api_key.key_type,
+        postage=postage_argument
     )
     persisted_notification = Notification.query.all()[0]
 
@@ -502,18 +503,22 @@ def test_persist_letter_notification_finds_correct_postage(
 
 
 def test_persist_notification_with_billable_units_stores_correct_info(
-    sample_template,
+    mocker
 ):
+    service = create_service(service_permissions=[LETTER_TYPE])
+    template = create_template(service, template_type=LETTER_TYPE)
+    mocker.patch('app.dao.templates_dao.dao_get_template_by_id', return_value=template)
     persist_notification(
-        template_id=sample_template.id,
-        template_version=sample_template.version,
+        template_id=template.id,
+        template_version=template.version,
         recipient="123 Main Street",
-        service=sample_template.service,
+        service=template.service,
         personalisation=None,
-        notification_type='letter',
+        notification_type=template.template_type,
         api_key_id=None,
         key_type="normal",
-        billable_units=3
+        billable_units=3,
+        template_postage=template.postage
     )
     persisted_notification = Notification.query.all()[0]
 

tests/app/template/test_rest.py

@@ -18,8 +18,7 @@ from app.models import (
     LETTER_TYPE,
     SMS_TYPE,
     Template,
-    TemplateHistory,
-    CHOOSE_POSTAGE
+    TemplateHistory
 )
 from app.dao.templates_dao import dao_get_template_by_id, dao_redact_template
 
@@ -44,7 +43,7 @@ from tests.conftest import set_config_values
 def test_should_create_a_new_template_for_a_service(
     client, sample_user, template_type, subject
 ):
-    service = create_service(service_permissions=[template_type, CHOOSE_POSTAGE])
+    service = create_service(service_permissions=[template_type])
     data = {
         'name': 'my template',
         'template_type': template_type,
@@ -339,7 +338,7 @@ def test_must_have_a_subject_on_an_email_or_letter_template(client, sample_user,
 
 
 def test_update_should_update_a_template(client, sample_user):
-    service = create_service(service_permissions=[LETTER_TYPE, CHOOSE_POSTAGE])
+    service = create_service(service_permissions=[LETTER_TYPE])
     template = create_template(service, template_type="letter", postage="second")
     data = {
         'content': 'my template has new content, swell!',

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -19,8 +19,7 @@ from app.models import (
     NOTIFICATION_SENDING,
     NOTIFICATION_DELIVERED,
     NOTIFICATION_PENDING_VIRUS_CHECK,
-    SMS_TYPE,
-    CHOOSE_POSTAGE
+    SMS_TYPE
 )
 from app.schema_validation import validate
 from app.v2.errors import RateLimitError
@@ -96,20 +95,11 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo
     mock.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF)
 
 
-@pytest.mark.parametrize('service_permissions, service_postage, template_postage, expected_postage', [
-    ([LETTER_TYPE], "second", "first", "second"),
-    ([LETTER_TYPE], "first", "second", "first"),
-    ([LETTER_TYPE], "first", None, "first"),
-    ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "first", "first"),
-    ([LETTER_TYPE, CHOOSE_POSTAGE], "second", None, "second"),
-    ([LETTER_TYPE, CHOOSE_POSTAGE], "second", "second", "second"),
-    ([LETTER_TYPE, CHOOSE_POSTAGE], "first", "second", "second"),
-])
 def test_post_letter_notification_sets_postage(
-    client, notify_db_session, mocker, service_permissions, service_postage, template_postage, expected_postage
+    client, notify_db_session, mocker
 ):
-    service = create_service(service_permissions=service_permissions, postage=service_postage)
-    template = create_template(service, template_type="letter", postage=template_postage)
+    service = create_service(service_permissions=[LETTER_TYPE])
+    template = create_template(service, template_type="letter", postage="first")
     mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
     data = {
         'template_id': str(template.id),
@@ -126,7 +116,7 @@ def test_post_letter_notification_sets_postage(
 
     assert validate(resp_json, post_letter_response) == resp_json
     notification = Notification.query.one()
-    assert notification.postage == expected_postage
+    assert notification.postage == "first"
 
 
 @pytest.mark.parametrize('env', [
@@ -469,19 +459,15 @@ def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker
     assert not Notification.query.first()
 
 
-@pytest.mark.parametrize('service_postage, notification_postage, expected_postage', [
-    ('second', 'second', 'second'),
-    ('second', 'first', 'first'),
-    ('second', None, 'second'),
-    ('first', 'first', 'first'),
-    ('first', 'second', 'second'),
-    ('first', None, 'first'),
+@pytest.mark.parametrize('notification_postage, expected_postage', [
+    ('second', 'second'),
+    ('first', 'first'),
+    (None, 'second')
 ])
 def test_post_precompiled_letter_notification_returns_201(
-    client, notify_user, mocker, service_postage, notification_postage, expected_postage
+    client, notify_user, mocker, notification_postage, expected_postage
 ):
     sample_service = create_service(service_permissions=['letter', 'precompiled_letter'])
-    sample_service.postage = service_postage
     s3mock = mocker.patch('app.v2.notifications.post_notifications.upload_letter_pdf')
     mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task')
     data = {

tests/app/v2/template/test_get_template.py

@@ -10,13 +10,15 @@ from tests.app.db import create_template
 valid_version_params = [None, 1]
 
 
-@pytest.mark.parametrize("tmp_type, expected_name, expected_subject", [
-    (SMS_TYPE, 'sms Template Name', None),
-    (EMAIL_TYPE, 'email Template Name', 'Template subject'),
-    (LETTER_TYPE, 'letter Template Name', 'Template subject')
+@pytest.mark.parametrize("tmp_type, expected_name, expected_subject,postage", [
+    (SMS_TYPE, 'sms Template Name', None, None),
+    (EMAIL_TYPE, 'email Template Name', 'Template subject', None),
+    (LETTER_TYPE, 'letter Template Name', 'Template subject', "second")
 ])
 @pytest.mark.parametrize("version", valid_version_params)
-def test_get_template_by_id_returns_200(client, sample_service, tmp_type, expected_name, expected_subject, version):
+def test_get_template_by_id_returns_200(
+    client, sample_service, tmp_type, expected_name, expected_subject, version, postage
+):
     template = create_template(sample_service, template_type=tmp_type)
     auth_header = create_authorization_header(service_id=sample_service.id)
 
@@ -41,7 +43,7 @@ def test_get_template_by_id_returns_200(client, sample_service, tmp_type, expect
         "subject": expected_subject,
         'name': expected_name,
         'personalisation': {},
-        'postage': None,
+        'postage': postage,
     }
 
     assert json_response == expected_response