diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d2f090ef8..c9e0227f5 100644 --- a/app/celery/tasks.py +++ b/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 diff --git a/app/models.py b/app/models.py index b61809f89..f5f064615 100644 --- a/app/models.py +++ b/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) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 62de55140..4c7d985de 100644 --- a/app/notifications/process_notifications.py +++ b/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: diff --git a/app/user/rest.py b/app/user/rest.py index cfce893a6..0bca01a4c 100644 --- a/app/user/rest.py +++ b/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: diff --git a/migrations/versions/0228_notification_postage.py b/migrations/versions/0228_notification_postage.py new file mode 100644 index 000000000..26b592504 --- /dev/null +++ b/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') diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 2dea9b865..6f9082408 100644 --- a/tests/app/celery/test_tasks.py +++ b/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) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 029d41e34..0c3eafd11 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/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.mark.skip('+44(0)77009001', reason='No easy way to normalise this'), + pytest.param('+44077009001', marks=pytest.mark.skip(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, diff --git a/tests/app/db.py b/tests/app/db.py index bb5eaedce..5c4e5f674 100644 --- a/tests/app/db.py +++ b/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) diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 59e1244eb..256c504f2 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/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, diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py index af82d78d7..08117a2b2 100644 --- a/tests/app/notifications/test_process_letter_notifications.py +++ b/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): diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 18e2b72dc..426e5e273 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/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', [ - sample_notification, - sample_email_notification, -]) -def test_get_notification_doesnt_have_delivery_estimate_for_non_letters( - client, - notify_db, - notify_db_session, - notification_mock, -): - mocked_notification = notification_mock(notify_db, notify_db_session) +def test_get_notification_by_id_returns_postage_class_for_letters(client, sample_letter_notification): + auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) + response = client.get( + path='/v2/notifications/{}'.format(sample_letter_notification.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + assert response.json['postage'] == 'second' + + +@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), diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 72afe5214..a984d5e78 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/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'} diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 707ab2a04..2ea9582da 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/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