Merge branch 'master' into reduce-updates-to-notification

app/celery/tasks.py

@@ -87,7 +87,7 @@ def process_job(job_id):
     if not service.active:
         job.job_status = JOB_STATUS_CANCELLED
         dao_update_job(job)
-        current_app.logger.warn(
+        current_app.logger.warning(
             "Job {} has been cancelled, service {} is inactive".format(job_id, service.id))
         return
 

app/models.py

@@ -212,7 +212,7 @@ class EmailBranding(db.Model):
         db.String(255),
         db.ForeignKey('branding_type.name'),
         index=True,
-        nullable=True,
+        nullable=False,
         default=BRANDING_ORG
     )
 
@@ -1182,6 +1182,9 @@ class Notification(db.Model):
 
     reply_to_text = db.Column(db.String, nullable=True)
 
+    postage = db.Column(db.String, nullable=True)
+    CheckConstraint("notification_type != 'letter' or postage in ('first', 'second')")
+
     __table_args__ = (
         db.ForeignKeyConstraint(
             ['template_id', 'template_version'],
@@ -1373,7 +1376,8 @@ class Notification(db.Model):
                 ).strftime(DATETIME_FORMAT)
                 if self.scheduled_notification
                 else None
-            )
+            ),
+            "postage": self.postage
         }
 
         if self.notification_type == LETTER_TYPE:
@@ -1432,6 +1436,9 @@ class NotificationHistory(db.Model, HistoryModel):
     created_by = db.relationship('User')
     created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True)
 
+    postage = db.Column(db.String, nullable=True)
+    CheckConstraint("notification_type != 'letter' or postage in ('first', 'second')")
+
     __table_args__ = (
         db.ForeignKeyConstraint(
             ['template_id', 'template_version'],
@@ -1783,7 +1790,7 @@ class FactBilling(db.Model):
     provider = db.Column(db.Text, nullable=True, primary_key=True)
     rate_multiplier = db.Column(db.Integer(), nullable=True, primary_key=True)
     international = db.Column(db.Boolean, nullable=False, primary_key=False)
-    rate = db.Column(db.Numeric(), nullable=True)
+    rate = db.Column(db.Numeric(), nullable=False)
     billable_units = db.Column(db.Integer(), nullable=True)
     notifications_sent = db.Column(db.Integer(), nullable=True)
     created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow)

app/notifications/process_notifications.py

@@ -18,6 +18,7 @@ from app.models import (
     EMAIL_TYPE,
     KEY_TYPE_TEST,
     SMS_TYPE,
+    LETTER_TYPE,
     NOTIFICATION_CREATED,
     Notification,
     ScheduledNotification
@@ -107,6 +108,8 @@ def persist_notification(
         notification.rate_multiplier = recipient_info.billable_units
     elif notification_type == EMAIL_TYPE:
         notification.normalised_to = format_email_address(notification.to)
+    elif notification_type == LETTER_TYPE:
+        notification.postage = service.postage
 
     # if simulated create a Notification model to return but do not persist the Notification to the dB
     if not simulated:

app/user/rest.py

@@ -164,7 +164,7 @@ def send_user_2fa_code(user_id, code_type):
 
     if count_user_verify_codes(user_to_send_to) >= current_app.config.get('MAX_VERIFY_CODE_COUNT'):
         # Prevent more than `MAX_VERIFY_CODE_COUNT` active verify codes at a time
-        current_app.logger.warn('Too many verify codes created for user {}'.format(user_to_send_to.id))
+        current_app.logger.warning('Too many verify codes created for user {}'.format(user_to_send_to.id))
     else:
         data = request.get_json()
         if code_type == SMS_TYPE:

migrations/versions/0228_notification_postage.py

@@ -0,0 +1,23 @@
+"""
+
+Revision ID: 0228_notification_postage
+Revises: 0227_postage_constraints
+Create Date: 2018-09-19 11:42:52.229430
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = '0228_notification_postage'
+down_revision = '0227_postage_constraints'
+
+
+def upgrade():
+    op.add_column('notification_history', sa.Column('postage', sa.String(), nullable=True))
+    op.add_column('notifications', sa.Column('postage', sa.String(), nullable=True))
+
+
+def downgrade():
+    op.drop_column('notifications', 'postage')
+    op.drop_column('notification_history', 'postage')

tests/app/celery/test_tasks.py

@@ -982,6 +982,32 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session):
     assert notification_db.reply_to_text == contact_block.contact_block
 
 
+@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
+
+    mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
+
+    notification_json = _notification_json(
+        template=sample_letter_job.template,
+        to='Foo',
+        personalisation={'addressline1': 'Foo', 'addressline2': 'Bar', 'postcode': 'Flob'},
+        job_id=sample_letter_job.id,
+        row_number=1
+    )
+    notification_id = uuid.uuid4()
+
+    save_letter(
+        sample_letter_job.service_id,
+        notification_id,
+        encryption.encrypt(notification_json),
+    )
+
+    notification_db = Notification.query.one()
+    assert notification_db.id == notification_id
+    assert notification_db.postage == postage
+
+
 def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_session):
     service = create_service()
     create_letter_contact(service=service, contact_block="Address contact", is_default=True)

tests/app/dao/notification_dao/test_notification_dao.py

@@ -1405,8 +1405,8 @@ def test_dao_get_notifications_by_to_field_escapes(
     '(0)7700 9001',
     '4477009001',
     '+4477009001',
-    pytest.mark.skip('+44077009001', reason='No easy way to normalise this'),
+    pytest.param('+44077009001', marks=pytest.mark.skip(reason='No easy way to normalise this')),
-    pytest.mark.skip('+44(0)77009001', reason='No easy way to normalise this'),
+    pytest.param('+44(0)77009001', marks=pytest.mark.skip(reason='No easy way to normalise this')),
 ])
 def test_dao_get_notifications_by_to_field_matches_partial_phone_numbers(
     sample_template,

tests/app/db.py

@@ -178,7 +178,8 @@ def create_notification(
         one_off=False,
         sms_sender_id=None,
         reply_to_text=None,
-        created_by_id=None
+        created_by_id=None,
+        postage=None
 ):
     if created_at is None:
         created_at = datetime.utcnow()
@@ -196,6 +197,9 @@ def create_notification(
         if not api_key:
             api_key = create_api_key(template.service, key_type=key_type)
 
+    if template.template_type == 'letter' and postage is None:
+        postage = 'second'
+
     data = {
         'id': uuid.uuid4(),
         'to': to_field,
@@ -224,7 +228,8 @@ def create_notification(
         'phone_prefix': phone_prefix,
         'normalised_to': normalised_to,
         'reply_to_text': reply_to_text,
-        'created_by_id': created_by_id
+        'created_by_id': created_by_id,
+        'postage': postage
     }
     notification = Notification(**data)
     dao_create_notification(notification)

tests/app/notifications/rest/test_send_notification.py

@@ -161,11 +161,12 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t
         ),
         '6',
     ),
-    pytest.mark.xfail((
+    pytest.param(
         None,
         ('we consider None equivalent to missing personalisation'),
         '',
-    )),
+        marks=pytest.mark.xfail
+    ),
 ])
 def test_send_notification_with_placeholders_replaced_with_unusual_types(
     client,

tests/app/notifications/test_process_letter_notifications.py

@@ -25,6 +25,7 @@ def test_create_letter_notification_creates_notification(sample_letter_template,
     assert notification.key_type == sample_api_key.key_type
     assert notification.reference is not None
     assert notification.client_reference is None
+    assert notification.postage == 'second'
 
 
 def test_create_letter_notification_sets_reference(sample_letter_template, sample_api_key):

tests/app/v2/notifications/test_get_notifications.py

@@ -9,11 +9,6 @@ from tests.app.db import (
     create_template,
 )
 
-from tests.app.conftest import (
-    sample_notification,
-    sample_email_notification,
-)
-
 
 @pytest.mark.parametrize('billable_units, provider', [
     (1, 'mmg'),
@@ -75,7 +70,8 @@ def test_get_notification_by_id_returns_200(
         "subject": None,
         'sent_at': sample_notification.sent_at,
         'completed_at': sample_notification.completed_at(),
-        'scheduled_for': '2017-05-12T14:15:00.000000Z'
+        'scheduled_for': '2017-05-12T14:15:00.000000Z',
+        'postage': None,
     }
 
     assert json_response == expected_response
@@ -126,7 +122,8 @@ def test_get_notification_by_id_with_placeholders_returns_200(
         "subject": "Bob",
         'sent_at': sample_notification.sent_at,
         'completed_at': sample_notification.completed_at(),
-        'scheduled_for': None
+        'scheduled_for': None,
+        'postage': None,
     }
 
     assert json_response == expected_response
@@ -267,17 +264,22 @@ def test_get_notification_adds_delivery_estimate_for_letters(
     assert json_response['estimated_delivery'] == estimated_delivery
 
 
-@pytest.mark.parametrize('notification_mock', [
+def test_get_notification_by_id_returns_postage_class_for_letters(client, sample_letter_notification):
-    sample_notification,
+    auth_header = create_authorization_header(service_id=sample_letter_notification.service_id)
-    sample_email_notification,
+    response = client.get(
-])
+        path='/v2/notifications/{}'.format(sample_letter_notification.id),
-def test_get_notification_doesnt_have_delivery_estimate_for_non_letters(
+        headers=[('Content-Type', 'application/json'), auth_header]
-    client,
+    )
-    notify_db,
+
-    notify_db_session,
+    assert response.status_code == 200
-    notification_mock,
+    assert response.json['postage'] == 'second'
-):
+
-    mocked_notification = notification_mock(notify_db, notify_db_session)
+
+@pytest.mark.parametrize('template_type', ['sms', 'email'])
+def test_get_notification_doesnt_have_delivery_estimate_for_non_letters(client, sample_service, template_type):
+    template = create_template(service=sample_service, template_type=template_type)
+    mocked_notification = create_notification(template=template)
+
     auth_header = create_authorization_header(service_id=mocked_notification.service_id)
     response = client.get(
         path='/v2/notifications/{}'.format(mocked_notification.id),

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -9,6 +9,7 @@ from app.config import TaskNames, QueueNames
 from app.models import (
     Job,
     Notification,
+    NotificationHistory,
     EMAIL_TYPE,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEAM,
@@ -76,12 +77,11 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo
     assert Job.query.count() == 0
     notification = Notification.query.one()
     assert notification.status == NOTIFICATION_CREATED
-    notification_id = notification.id
+    assert resp_json['id'] == str(notification.id)
-    assert resp_json['id'] == str(notification_id)
     assert resp_json['reference'] == reference
     assert resp_json['content']['subject'] == sample_letter_template.subject
     assert resp_json['content']['body'] == sample_letter_template.content
-    assert 'v2/notifications/{}'.format(notification_id) in resp_json['uri']
+    assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri']
     assert resp_json['template']['id'] == str(sample_letter_template.id)
     assert resp_json['template']['version'] == sample_letter_template.version
     assert (
@@ -95,6 +95,28 @@ 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('postage', ['first', 'second'])
+def test_post_letter_notification_sets_postage(client, sample_letter_template, mocker, postage):
+    sample_letter_template.service.postage = postage
+    mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async')
+    data = {
+        'template_id': str(sample_letter_template.id),
+        'personalisation': {
+            'address_line_1': 'Her Royal Highness Queen Elizabeth II',
+            'address_line_2': 'Buckingham Palace',
+            'address_line_3': 'London',
+            'postcode': 'SW1 1AA',
+            'name': 'Lizzie'
+        }
+    }
+
+    resp_json = letter_request(client, data, service_id=sample_letter_template.service_id)
+
+    assert validate(resp_json, post_letter_response) == resp_json
+    notification = Notification.query.one()
+    assert notification.postage == postage
+
+
 @pytest.mark.parametrize('env', [
     'staging',
     'live',
@@ -441,8 +463,10 @@ def test_post_precompiled_letter_with_invalid_base64(client, notify_user, mocker
     assert not Notification.query.first()
 
 
-def test_post_precompiled_letter_notification_returns_201(client, notify_user, mocker):
+@pytest.mark.parametrize('postage', ['first', 'second'])
+def test_post_precompiled_letter_notification_returns_201(client, notify_user, mocker, postage):
     sample_service = create_service(service_permissions=['letter', 'precompiled_letter'])
+    sample_service.postage = postage
     s3mock = mocker.patch('app.v2.notifications.post_notifications.upload_letter_pdf')
     mocker.patch('app.v2.notifications.post_notifications.pdf_page_count', return_value=5)
     mocker.patch("app.letters.rest.notify_celery.send_task")
@@ -460,10 +484,14 @@ def test_post_precompiled_letter_notification_returns_201(client, notify_user, m
 
     s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True)
 
-    notification = Notification.query.first()
+    notification = Notification.query.one()
 
     assert notification.billable_units == 3
     assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK
+    assert notification.postage == postage
+
+    notification_history = NotificationHistory.query.one()
+    assert notification_history.postage == postage
 
     resp_json = json.loads(response.get_data(as_text=True))
     assert resp_json == {'id': str(notification.id), 'reference': 'letter-reference'}

tests/app/v2/notifications/test_post_notifications.py

@@ -58,6 +58,7 @@ def test_post_sms_notification_returns_201(client, sample_template_with_placehol
     assert len(notifications) == 1
     assert notifications[0].status == NOTIFICATION_CREATED
     notification_id = notifications[0].id
+    assert notifications[0].postage is None
     assert resp_json['id'] == str(notification_id)
     assert resp_json['reference'] == reference
     assert resp_json['content']['body'] == sample_template_with_placeholders.content.replace("(( Name))", "Jo")
@@ -309,6 +310,7 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_
     assert validate(resp_json, post_email_response) == resp_json
     notification = Notification.query.one()
     assert notification.status == NOTIFICATION_CREATED
+    assert notification.postage is None
     assert resp_json['id'] == str(notification.id)
     assert resp_json['reference'] == reference
     assert notification.reference is None