diff --git a/app/aws/s3.py b/app/aws/s3.py index a64be797c..4551c8c72 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -99,3 +99,6 @@ def upload_letters_pdf(reference, crown, filedata): bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], file_location=upload_file_name ) + + current_app.logger.info("Uploading letters PDF {} to {}".format( + upload_file_name, current_app.config['LETTERS_PDF_BUCKET_NAME'])) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py new file mode 100644 index 000000000..24c7347ae --- /dev/null +++ b/app/celery/letters_pdf_tasks.py @@ -0,0 +1,65 @@ +from flask import current_app +from requests import ( + post as requests_post, + RequestException +) + +from botocore.exceptions import ClientError as BotoClientError + +from app import notify_celery +from app.aws import s3 +from app.config import QueueNames +from app.dao.notifications_dao import get_notification_by_id, update_notification_status_by_id +from app.statsd_decorators import statsd + + +@notify_celery.task(bind=True, name="create-letters-pdf", max_retries=15, default_retry_delay=300) +@statsd(namespace="tasks") +def create_letters_pdf(self, notification_id): + try: + notification = get_notification_by_id(notification_id, _raise=True) + + pdf_data = get_letters_pdf( + notification.template, + contact_block=notification.reply_to_text, + org_id=notification.service.dvla_organisation.id, + values=notification.personalisation + ) + current_app.logger.info("PDF Letter {} reference {} created at {}, {} bytes".format( + notification.id, notification.reference, notification.created_at, len(pdf_data))) + s3.upload_letters_pdf(reference=notification.reference, crown=notification.service.crown, filedata=pdf_data) + except (RequestException, BotoClientError): + try: + current_app.logger.exception( + "Letters PDF notification creation for id: {} failed".format(notification_id) + ) + self.retry(queue=QueueNames.RETRY) + except self.MaxRetriesExceededError: + current_app.logger.exception( + "RETRY FAILED: task create_letters_pdf failed for notification {}".format(notification_id), + ) + update_notification_status_by_id(notification_id, 'technical-failure') + + +def get_letters_pdf(template, contact_block, org_id, values): + template_for_letter_print = { + "subject": template.subject, + "content": template.content + } + + data = { + 'letter_contact_block': contact_block, + 'template': template_for_letter_print, + 'values': values, + 'dvla_org_id': org_id, + } + resp = requests_post( + '{}/print.pdf'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'] + ), + json=data, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + resp.raise_for_status() + + return resp.content diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 697fd2eef..ed14be2e4 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -58,6 +58,8 @@ from app.celery.tasks import ( from app.config import QueueNames, TaskNames from app.utils import convert_utc_to_bst from app.v2.errors import JobIncompleteError +from app.dao.service_callback_api_dao import get_service_callback_api_for_service +from app.celery.service_callback_tasks import send_delivery_status_to_service @worker_process_shutdown.connect @@ -189,10 +191,18 @@ def delete_invitations(): @notify_celery.task(name='timeout-sending-notifications') @statsd(namespace="tasks") def timeout_notifications(): - updated = dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')) - if updated: + notifications = dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')) + + if notifications: + for notification in notifications: + # queue callback task only if the service_callback_api exists + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) + + if service_callback_api: + send_delivery_status_to_service.apply_async([str(id)], queue=QueueNames.NOTIFY) + current_app.logger.info( - "Timeout period reached for {} notifications, status has been updated.".format(updated)) + "Timeout period reached for {} notifications, status has been updated.".format(len(notifications))) @notify_celery.task(name='send-daily-performance-platform-stats') diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index 3bf033a80..19f322842 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -34,7 +34,7 @@ def send_delivery_status_to_service(self, notification_id): "to": notification.to, "status": notification.status, "created_at": notification.created_at.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request - "updated_at": notification.updated_at.strftime(DATETIME_FORMAT), # the last time the status was updated + "completed_at": notification.updated_at.strftime(DATETIME_FORMAT), # the last time the status was updated "sent_at": notification.sent_at.strftime(DATETIME_FORMAT), # the time the email was sent "notification_type": notification.notification_type } diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 53d714213..8657cfb5a 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -4,6 +4,7 @@ from collections import namedtuple from celery.signals import worker_process_shutdown from flask import current_app + from notifications_utils.recipients import ( RecipientCSV ) @@ -29,6 +30,7 @@ from app import ( ) from app.aws import s3 from app.celery import provider_tasks +from app.celery import letters_pdf_tasks from app.celery.service_callback_tasks import send_delivery_status_to_service from app.config import QueueNames from app.dao.inbound_sms_dao import dao_get_inbound_sms_by_id @@ -44,7 +46,7 @@ from app.dao.notifications_dao import ( dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id, - dao_get_notifications_by_references + dao_get_notifications_by_references, ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service @@ -121,7 +123,10 @@ def process_job(job_id): def job_complete(job, service, template_type, resumed=False, start=None): - if template_type == LETTER_TYPE: + if ( + template_type == LETTER_TYPE and + not service.has_permission('letters_as_pdf') + ): if service.research_mode: update_job_to_sent_to_dvla.apply_async([str(job.id)], queue=QueueNames.RESEARCH_MODE) else: @@ -136,7 +141,7 @@ def job_complete(job, service, template_type, resumed=False, start=None): if resumed: current_app.logger.info( - "Resumed Job {} completed at {}".format(job.id, job.created_at, start, finished) + "Resumed Job {} completed at {}".format(job.id, job.created_at) ) else: current_app.logger.info( @@ -312,6 +317,18 @@ def save_letter( reply_to_text=service.get_default_letter_contact() ) + if service.has_permission('letters_as_pdf'): + if not service.research_mode: + letters_pdf_tasks.create_letters_pdf.apply_async( + [str(saved_notification.id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + else: + update_letter_notifications_to_sent_to_dvla.apply_async( + kwargs={'notification_references': [saved_notification.reference]}, + queue=QueueNames.RESEARCH_MODE + ) + current_app.logger.info("Letter {} created at {}".format(saved_notification.id, saved_notification.created_at)) except SQLAlchemyError as e: handle_exception(self, notification, notification_id, e) @@ -580,4 +597,4 @@ def process_incomplete_job(job_id): if row_number > resume_from_row: process_row(row_number, recipient, personalisation, template, job, job.service) - job_complete(job, job.service, template, resumed=True) + job_complete(job, job.service, template.template_type, resumed=True) diff --git a/app/config.py b/app/config.py index d553041e3..4ec4c3f75 100644 --- a/app/config.py +++ b/app/config.py @@ -30,6 +30,7 @@ class QueueNames(object): RETRY = 'retry-tasks' NOTIFY = 'notify-internal-tasks' PROCESS_FTP = 'process-ftp-tasks' + CREATE_LETTERS_PDF = 'create-letters-pdf' @staticmethod def all_queues(): @@ -44,6 +45,7 @@ class QueueNames(object): QueueNames.JOBS, QueueNames.RETRY, QueueNames.NOTIFY, + QueueNames.CREATE_LETTERS_PDF, ] @@ -304,6 +306,9 @@ class Config(object): # {"dataset_1": "token_1", ...} PERFORMANCE_PLATFORM_ENDPOINTS = json.loads(os.environ.get('PERFORMANCE_PLATFORM_ENDPOINTS', '{}')) + TEMPLATE_PREVIEW_API_HOST = os.environ.get('TEMPLATE_PREVIEW_API_HOST', 'http://localhost:6013') + TEMPLATE_PREVIEW_API_KEY = os.environ.get('TEMPLATE_PREVIEW_API_KEY', 'my-secret-key') + ###################### # Config overrides ### diff --git a/app/dao/letter_rate_dao.py b/app/dao/letter_rate_dao.py new file mode 100644 index 000000000..e69001a44 --- /dev/null +++ b/app/dao/letter_rate_dao.py @@ -0,0 +1,23 @@ +from app import db +from app.dao.dao_utils import transactional +from app.models import LetterRate + + +@transactional +def dao_create_letter_rate(letter_rate): + db.session.add(letter_rate) + + +def get_letter_rates_for_daterange(date, crown, sheet_count, post_class='second'): + rates = LetterRate.query.filter( + LetterRate.start_date <= date + ).filter((LetterRate.end_date == None) | # noqa + (LetterRate.end_date > date) + ).filter( + LetterRate.crown == crown + ).filter( + LetterRate.sheet_count == sheet_count + ).filter( + LetterRate.post_class == post_class + ).all() + return rates diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 1f7b33167..29b49ba7c 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -26,6 +26,8 @@ from app.models import ( Notification, NotificationHistory, ScheduledNotification, + Service, + ServicePermission, Template, TemplateHistory, KEY_TYPE_NORMAL, @@ -224,8 +226,11 @@ def get_notification_with_personalisation(service_id, notification_id, key_type) @statsd(namespace="dao") -def get_notification_by_id(notification_id): - return Notification.query.filter_by(id=notification_id).first() +def get_notification_by_id(notification_id, _raise=False): + if _raise: + return Notification.query.filter_by(id=notification_id).one() + else: + return Notification.query.filter_by(id=notification_id).first() def get_notifications(filter_dict=None): @@ -328,17 +333,24 @@ def dao_delete_notifications_and_history_by_id(notification_id): def _timeout_notifications(current_statuses, new_status, timeout_start, updated_at): + + notifications = Notification.query.filter( + Notification.created_at < timeout_start, + Notification.status.in_(current_statuses), + Notification.notification_type != LETTER_TYPE + ).all() for table in [NotificationHistory, Notification]: q = table.query.filter( table.created_at < timeout_start, table.status.in_(current_statuses), table.notification_type != LETTER_TYPE ) - last_update_count = q.update( + q.update( {'status': new_status, 'updated_at': updated_at}, synchronize_session=False ) - return last_update_count + # return a list of q = notification_ids in Notification table for sending delivery receipts + return notifications def dao_timeout_notifications(timeout_period_in_seconds): @@ -359,14 +371,14 @@ def dao_timeout_notifications(timeout_period_in_seconds): timeout = functools.partial(_timeout_notifications, timeout_start=timeout_start, updated_at=updated_at) # Notifications still in created status are marked with a technical-failure: - updated = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE) + updated_ids = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE) # Notifications still in sending or pending status are marked with a temporary-failure: - updated += timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], NOTIFICATION_TEMPORARY_FAILURE) + updated_ids += timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], NOTIFICATION_TEMPORARY_FAILURE) db.session.commit() - return updated + return updated_ids def get_total_sent_notifications_in_date_range(start_date, end_date, notification_type): @@ -534,14 +546,23 @@ def dao_set_created_live_letter_api_notifications_to_pending(): this is used in the run_scheduled_jobs task, so we put a FOR UPDATE lock on the job table for the duration of the transaction so that if the task is run more than once concurrently, one task will block the other select from completing until it commits. + + Note - do not process services that have letters_as_pdf permission as they + will get processed when the letters PDF zip task is created """ notifications = db.session.query( Notification + ).join( + Service ).filter( Notification.notification_type == LETTER_TYPE, Notification.status == NOTIFICATION_CREATED, Notification.key_type == KEY_TYPE_NORMAL, - Notification.api_key != None # noqa + Notification.api_key != None, # noqa + # Ignore services that have letters_as_pdf permission + ~Service.permissions.any( + ServicePermission.permission == 'letters_as_pdf' + ) ).with_for_update( ).all() diff --git a/app/models.py b/app/models.py index 6eeda9a77..2f196c7a5 100644 --- a/app/models.py +++ b/app/models.py @@ -283,6 +283,9 @@ class Service(db.Model, Versioned): default_letter_contact = [x for x in self.letter_contacts if x.is_default] return default_letter_contact[0].contact_block if default_letter_contact else None + def has_permission(self, permission): + return permission in [p.permission for p in self.permissions] + class AnnualBilling(db.Model): __tablename__ = "annual_billing" @@ -1484,17 +1487,12 @@ class LetterRate(db.Model): __tablename__ = 'letter_rates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - valid_from = valid_from = db.Column(db.DateTime, nullable=False) - - -class LetterRateDetail(db.Model): - __tablename__ = 'letter_rate_details' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - letter_rate_id = db.Column(UUID(as_uuid=True), db.ForeignKey('letter_rates.id'), index=True, nullable=False) - letter_rate = db.relationship('LetterRate', backref='letter_rates') - page_total = db.Column(db.Integer, nullable=False) + start_date = db.Column(db.DateTime, nullable=False) + end_date = db.Column(db.DateTime, nullable=True) + sheet_count = db.Column(db.Integer, nullable=False) # double sided sheet rate = db.Column(db.Numeric(), nullable=False) + crown = db.Column(db.Boolean, nullable=False) + post_class = db.Column(db.String, nullable=False) class MonthlyBilling(db.Model): diff --git a/app/service/rest.py b/app/service/rest.py index d71cd2b90..9c1763af0 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -286,12 +286,11 @@ def get_service_provider_aggregate_statistics(service_id): # tables. This is so product owner can pass stories as done @service_blueprint.route('//history', methods=['GET']) def get_service_history(service_id): - from app.models import (Service, ApiKey, TemplateHistory, Event) + from app.models import (Service, ApiKey, TemplateHistory) from app.schemas import ( service_history_schema, api_key_history_schema, - template_history_schema, - event_schema + template_history_schema ) service_history = Service.get_history_model().query.filter_by(id=service_id).all() @@ -302,14 +301,11 @@ def get_service_history(service_id): template_history = TemplateHistory.query.filter_by(service_id=service_id).all() template_data, errors = template_history_schema.dump(template_history, many=True) - events = Event.query.all() - events_data = event_schema.dump(events, many=True).data - data = { 'service_history': service_data, 'api_key_history': api_keys_data, 'template_history': template_data, - 'events': events_data} + 'events': []} return jsonify(data=data) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 08847f2cb..ed9d67c7f 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -14,6 +14,7 @@ from app.models import ( NOTIFICATION_CREATED, NOTIFICATION_SENDING ) +from app.celery.letters_pdf_tasks import create_letters_pdf from app.celery.tasks import update_letter_notifications_to_sent_to_dvla from app.notifications.process_notifications import ( persist_notification, @@ -187,6 +188,12 @@ def process_letter_notification(*, letter_data, api_key, template): queue=QueueNames.RESEARCH_MODE ) + if api_key.service.has_permission('letters_as_pdf'): + create_letters_pdf.apply_async( + [str(notification.id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + return notification diff --git a/migrations/versions/0149_add_crown_to_services.py b/migrations/versions/0149_add_crown_to_services.py index bfbbf976c..0a6412e9d 100644 --- a/migrations/versions/0149_add_crown_to_services.py +++ b/migrations/versions/0149_add_crown_to_services.py @@ -1,6 +1,6 @@ """ -Revision ID: 0149_add_crown_column_to_services +Revision ID: 0149_add_crown_to_services Revises: 0148_add_letters_as_pdf_svc_perm Create Date: 2017-12-04 12:13:35.268712 diff --git a/migrations/versions/0151_refactor_letter_rates.py b/migrations/versions/0151_refactor_letter_rates.py new file mode 100644 index 000000000..7a969cc42 --- /dev/null +++ b/migrations/versions/0151_refactor_letter_rates.py @@ -0,0 +1,71 @@ +""" + +Revision ID: 0151_refactor_letter_rates +Revises: 0150_another_letter_org +Create Date: 2017-12-05 10:24:41.232128 + +""" +import uuid +from datetime import datetime + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0151_refactor_letter_rates' +down_revision = '0150_another_letter_org' + + +def upgrade(): + op.drop_table('letter_rate_details') + op.drop_table('letter_rates') + op.create_table('letter_rates', + sa.Column('id', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('start_date', sa.DateTime(), nullable=False), + sa.Column('end_date', sa.DateTime(), nullable=True), + sa.Column('sheet_count', sa.Integer(), nullable=False), + sa.Column('rate', sa.Numeric(), nullable=False), + sa.Column('crown', sa.Boolean(), nullable=False), + sa.Column('post_class', sa.String(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + + start_date = datetime(2016, 3, 31, 23, 00, 00) + op.execute("insert into letter_rates values('{}', '{}', null, 1, 0.30, True, 'second')".format( + str(uuid.uuid4()), start_date) + ) + op.execute("insert into letter_rates values('{}', '{}', null, 2, 0.33, True, 'second')".format( + str(uuid.uuid4()), start_date) + ) + op.execute("insert into letter_rates values('{}', '{}', null, 3, 0.36, True, 'second')".format( + str(uuid.uuid4()), start_date) + ) + + op.execute("insert into letter_rates values('{}', '{}', null, 1, 0.33, False, 'second')".format( + str(uuid.uuid4()), start_date) + ) + op.execute("insert into letter_rates values('{}', '{}', null, 2, 0.39, False, 'second')".format( + str(uuid.uuid4()), start_date) + ) + op.execute("insert into letter_rates values('{}', '{}', null, 3, 0.45, False, 'second')".format( + str(uuid.uuid4()), start_date) + ) + + +def downgrade(): + op.drop_table('letter_rates') + op.create_table('letter_rates', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('valid_from', postgresql.TIMESTAMP(), autoincrement=False, nullable=False), + sa.PrimaryKeyConstraint('id', name='letter_rates_pkey'), + postgresql_ignore_search_path=False + ) + op.create_table('letter_rate_details', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('letter_rate_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('page_total', sa.INTEGER(), autoincrement=False, nullable=False), + sa.Column('rate', sa.NUMERIC(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['letter_rate_id'], ['letter_rates.id'], + name='letter_rate_details_letter_rate_id_fkey'), + sa.PrimaryKeyConstraint('id', name='letter_rate_details_pkey') + ) diff --git a/requirements.txt b/requirements.txt index 2ae3d9a06..0c0b2432e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,7 +26,7 @@ notifications-python-client==4.6.0 awscli==1.14.4 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@23.2.1#egg=notifications-utils==23.2.1 +git+https://github.com/alphagov/notifications-utils.git@23.3.1#egg=notifications-utils==23.3.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py new file mode 100644 index 000000000..0af5c78ab --- /dev/null +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -0,0 +1,99 @@ +import pytest +import requests_mock + +from botocore.exceptions import ClientError +from celery.exceptions import MaxRetriesExceededError +from requests import RequestException +from sqlalchemy.orm.exc import NoResultFound + +from app.celery.letters_pdf_tasks import ( + create_letters_pdf, + get_letters_pdf, +) + +from tests.conftest import set_config_values + + +def test_should_have_decorated_tasks_functions(): + assert create_letters_pdf.__wrapped__.__name__ == 'create_letters_pdf' + + +@pytest.mark.parametrize('personalisation', [{'name': 'test'}, None]) +def test_get_letters_pdf_calls_notifications_template_preview_service_correctly( + notify_api, mocker, client, sample_letter_template, personalisation): + contact_block = 'Mr Foo,\n1 Test Street,\nLondon\nN1' + dvla_org_id = '002' + + with set_config_values(notify_api, { + 'TEMPLATE_PREVIEW_API_HOST': 'http://localhost/notifications-template-preview', + 'TEMPLATE_PREVIEW_API_KEY': 'test-key' + }): + with requests_mock.Mocker() as request_mock: + mock_post = request_mock.post( + 'http://localhost/notifications-template-preview/print.pdf', content=b'\x00\x01', status_code=200) + + get_letters_pdf( + sample_letter_template, contact_block=contact_block, org_id=dvla_org_id, values=personalisation) + + assert mock_post.last_request.json() == { + 'values': personalisation, + 'letter_contact_block': contact_block, + 'dvla_org_id': dvla_org_id, + 'template': { + 'subject': sample_letter_template.subject, + 'content': sample_letter_template.content + } + } + + +def test_create_letters_pdf_calls_upload_letters_pdf(mocker, sample_letter_notification): + mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=b'\x00\x01') + mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf') + + create_letters_pdf(sample_letter_notification.id) + + mock_s3.assert_called_with( + reference=sample_letter_notification.reference, + crown=sample_letter_notification.service.crown, + filedata=b'\x00\x01' + ) + + +def test_create_letters_pdf_non_existent_notification(notify_api, mocker, fake_uuid): + with pytest.raises(expected_exception=NoResultFound): + create_letters_pdf(fake_uuid) + + +def test_create_letters_pdf_handles_request_errors(mocker, sample_letter_notification): + mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', side_effect=RequestException) + mock_retry = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.retry') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_get_letters_pdf.called + assert mock_retry.called + + +def test_create_letters_pdf_handles_s3_errors(mocker, sample_letter_notification): + mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf') + mock_s3 = mocker.patch('app.celery.tasks.s3.upload_letters_pdf', side_effect=ClientError({}, 'operation_name')) + mock_retry = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.retry') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_s3.called + assert mock_retry.called + + +def test_create_letters_pdf_sets_technical_failure_max_retries(mocker, sample_letter_notification): + mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', side_effect=RequestException) + mock_retry = mocker.patch( + 'app.celery.letters_pdf_tasks.create_letters_pdf.retry', side_effect=MaxRetriesExceededError) + mock_update_noti = mocker.patch('app.celery.letters_pdf_tasks.update_notification_status_by_id') + + create_letters_pdf(sample_letter_notification.id) + + assert mock_get_letters_pdf.called + assert mock_retry.called + assert mock_update_noti.called + mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure') diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index f09333d2d..a7c9aaeb4 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -55,7 +55,7 @@ def test_send_delivery_status_to_service_post_https_request_to_service(notify_db "to": notification.to, "status": notification.status, "created_at": datestr.strftime(DATETIME_FORMAT), # the time GOV.UK email sent the request - "updated_at": datestr.strftime(DATETIME_FORMAT), # the last time the status was updated + "completed_at": datestr.strftime(DATETIME_FORMAT), # the last time the status was updated "sent_at": datestr.strftime(DATETIME_FORMAT), # the time the email was sent "notification_type": notification_type } diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index e33956a56..a5837b3a2 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -20,6 +20,7 @@ from app.celery.scheduled_tasks import check_job_status from app.celery.tasks import ( build_dvla_file, create_dvla_file_contents_for_job, + job_complete, process_job, process_row, save_sms, @@ -32,7 +33,7 @@ from app.celery.tasks import ( send_inbound_sms_to_service, ) from app.config import QueueNames -from app.dao import jobs_dao, services_dao +from app.dao import jobs_dao, services_dao, service_permissions_dao from app.models import ( Job, Notification, @@ -1039,6 +1040,85 @@ def test_save_letter_saves_letter_to_database(mocker, notify_db_session): assert notification_db.reply_to_text == "Address contact" +def test_save_letter_calls_update_noti_to_sent_task_with_letters_as_pdf_permission_in_research_mode( + mocker, notify_db_session, sample_letter_job): + sample_letter_job.service.research_mode = True + service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') + mock_update_letter_noti_sent = mocker.patch( + 'app.celery.tasks.update_letter_notifications_to_sent_to_dvla.apply_async') + mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life") + + personalisation = { + 'addressline1': 'Foo', + 'addressline2': 'Bar', + 'postcode': 'Flob', + } + notification_json = _notification_json( + template=sample_letter_job.template, + to='Foo', + personalisation=personalisation, + 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), + ) + + assert mock_update_letter_noti_sent.called + mock_update_letter_noti_sent.assert_called_once_with( + kwargs={'notification_references': ['this-is-random-in-real-life']}, + queue=QueueNames.RESEARCH_MODE + ) + + +def test_save_letter_calls_create_letters_pdf_task_with_letters_as_pdf_permission_and_not_in_research( + mocker, notify_db_session, sample_letter_job): + service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') + mock_create_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') + + personalisation = { + 'addressline1': 'Foo', + 'addressline2': 'Bar', + 'postcode': 'Flob', + } + notification_json = _notification_json( + template=sample_letter_job.template, + to='Foo', + personalisation=personalisation, + 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), + ) + + assert mock_create_letters_pdf.called + mock_create_letters_pdf.assert_called_once_with( + [str(notification_id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + + +def test_job_complete_does_not_call_build_dvla_file_with_letters_as_pdf_permission( + mocker, notify_db_session, sample_letter_job): + service_permissions_dao.dao_add_service_permission(sample_letter_job.service.id, 'letters_as_pdf') + mock_build_dvla_files = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') + + job_complete(sample_letter_job, sample_letter_job.service, sample_letter_job.template.template_type) + + assert not sample_letter_job.service.research_mode + assert not mock_build_dvla_files.called + assert sample_letter_job.job_status == JOB_STATUS_FINISHED + + def test_should_cancel_job_if_service_is_inactive(sample_service, sample_job, mocker): @@ -1436,9 +1516,9 @@ def test_process_incomplete_job_email(mocker, sample_email_template): def test_process_incomplete_job_letter(mocker, sample_letter_template): - mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_letter')) mock_letter_saver = mocker.patch('app.celery.tasks.save_letter.apply_async') + mock_build_dvla = mocker.patch('app.celery.tasks.build_dvla_file.apply_async') job = create_job(template=sample_letter_template, notification_count=10, created_at=datetime.utcnow() - timedelta(hours=2), @@ -1453,8 +1533,5 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): process_incomplete_job(str(job.id)) - completed_job = Job.query.filter(Job.id == job.id).one() - - assert completed_job.job_status == JOB_STATUS_FINISHED - + assert mock_build_dvla.called assert mock_letter_saver.call_count == 8 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 71b510767..e51aedd7d 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -161,7 +161,7 @@ def sample_service( 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, - 'created_by': user + 'created_by': user, } service = Service.query.filter_by(name=service_name).first() if not service: @@ -188,7 +188,7 @@ def sample_service_full_permissions(notify_db, notify_db_session): notify_db_session, # ensure name doesn't clash with regular sample service service_name="sample service full permissions", - permissions=SERVICE_PERMISSION_TYPES + permissions=set(SERVICE_PERMISSION_TYPES) - {'letters_as_pdf'} ) @@ -599,7 +599,7 @@ def sample_letter_notification(sample_letter_template): 'address_line_6': 'A6', 'postcode': 'A_POST' } - return create_notification(sample_letter_template, personalisation=address) + return create_notification(sample_letter_template, reference='foo', personalisation=address) @pytest.fixture(scope='function') diff --git a/tests/app/dao/notification_dao/test_letter_api_notification_dao.py b/tests/app/dao/notification_dao/test_letter_api_notification_dao.py index 4a624f3eb..c52de01e1 100644 --- a/tests/app/dao/notification_dao/test_letter_api_notification_dao.py +++ b/tests/app/dao/notification_dao/test_letter_api_notification_dao.py @@ -1,13 +1,15 @@ from app.models import ( + Notification, NOTIFICATION_CREATED, NOTIFICATION_PENDING, NOTIFICATION_SENDING, KEY_TYPE_TEST, - KEY_TYPE_NORMAL + KEY_TYPE_NORMAL, + LETTER_TYPE ) from app.dao.notifications_dao import dao_set_created_live_letter_api_notifications_to_pending -from tests.app.db import create_notification +from tests.app.db import create_notification, create_service, create_template def test_should_only_get_letter_notifications( @@ -23,6 +25,20 @@ def test_should_only_get_letter_notifications( assert ret == [sample_letter_notification] +def test_should_ignore_letters_as_pdf(sample_letter_notification): + service = create_service(service_permissions=[LETTER_TYPE, 'letters_as_pdf']) + template = create_template(service, template_type=LETTER_TYPE) + create_notification(template) + + all_noti = Notification.query.all() + assert len(all_noti) == 2 + + ret = dao_set_created_live_letter_api_notifications_to_pending() + + assert sample_letter_notification.status == NOTIFICATION_PENDING + assert ret == [sample_letter_notification] + + def test_should_only_get_created_letters(sample_letter_template): created_noti = create_notification(sample_letter_template, status=NOTIFICATION_CREATED) create_notification(sample_letter_template, status=NOTIFICATION_PENDING) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 0bb9b5322..8af596199 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1201,7 +1201,7 @@ def test_dao_timeout_notifications(sample_template): assert Notification.query.get(sending.id).status == 'sending' assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated = dao_timeout_notifications(1) + updated_ids = dao_timeout_notifications(1) assert Notification.query.get(created.id).status == 'technical-failure' assert Notification.query.get(sending.id).status == 'temporary-failure' assert Notification.query.get(pending.id).status == 'temporary-failure' @@ -1210,7 +1210,7 @@ def test_dao_timeout_notifications(sample_template): assert NotificationHistory.query.get(sending.id).status == 'temporary-failure' assert NotificationHistory.query.get(pending.id).status == 'temporary-failure' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert updated == 3 + assert len(updated_ids) == 3 def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_template): @@ -1224,12 +1224,12 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t assert Notification.query.get(sending.id).status == 'sending' assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated = dao_timeout_notifications(1) + updated_ids = dao_timeout_notifications(1) assert NotificationHistory.query.get(created.id).status == 'created' assert NotificationHistory.query.get(sending.id).status == 'sending' assert NotificationHistory.query.get(pending.id).status == 'pending' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert updated == 0 + assert len(updated_ids) == 0 def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template): @@ -1244,13 +1244,13 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated = dao_timeout_notifications(1) + updated_ids = dao_timeout_notifications(1) assert NotificationHistory.query.get(created.id).status == 'created' assert NotificationHistory.query.get(sending.id).status == 'sending' assert NotificationHistory.query.get(pending.id).status == 'pending' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert updated == 0 + assert len(updated_ids) == 0 def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key): diff --git a/tests/app/dao/test_letter_rate_dao.py b/tests/app/dao/test_letter_rate_dao.py new file mode 100644 index 000000000..f3fd0d139 --- /dev/null +++ b/tests/app/dao/test_letter_rate_dao.py @@ -0,0 +1,41 @@ +from datetime import datetime + +from decimal import Decimal + +from app.dao.letter_rate_dao import dao_create_letter_rate, get_letter_rates_for_daterange +from app.models import LetterRate + + +def test_dao_create_letter_rate(notify_db_session): + letter_rate = LetterRate(start_date=datetime(2017, 12, 1), + rate=0.33, + crown=True, + sheet_count=1, + post_class='second') + + dao_create_letter_rate(letter_rate) + + rates = LetterRate.query.all() + assert len(rates) == 1 + + +def test_get_letter_rates_for_daterange(notify_db_session): + letter_rate = LetterRate(start_date=datetime(2017, 12, 1), + rate=0.33, + crown=True, + sheet_count=1, + post_class='second') + + dao_create_letter_rate(letter_rate) + letter_rate = LetterRate(start_date=datetime(2016, 12, 1), + end_date=datetime(2017, 12, 1), + rate=0.30, + crown=True, + sheet_count=1, + post_class='second') + + dao_create_letter_rate(letter_rate) + + results = get_letter_rates_for_daterange(date=datetime(2017, 12, 2), crown=True, sheet_count=1, post_class='second') + assert len(results) == 1 + assert results[0].rate == Decimal('0.33') diff --git a/tests/app/test_config.py b/tests/app/test_config.py index 3a40b4db4..c023fa37a 100644 --- a/tests/app/test_config.py +++ b/tests/app/test_config.py @@ -63,7 +63,7 @@ def test_cloudfoundry_config_has_different_defaults(): def test_queue_names_all_queues_correct(): # Need to ensure that all_queues() only returns queue names used in API queues = QueueNames.all_queues() - assert len(queues) == 10 + assert len(queues) == 11 assert set([ QueueNames.PRIORITY, QueueNames.PERIODIC, @@ -74,5 +74,6 @@ def test_queue_names_all_queues_correct(): QueueNames.STATISTICS, QueueNames.JOBS, QueueNames.RETRY, - QueueNames.NOTIFY + QueueNames.NOTIFY, + QueueNames.CREATE_LETTERS_PDF ]) == set(queues) diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index d6eca3c8d..b433f83cd 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -4,6 +4,8 @@ from flask import json from flask import url_for import pytest +from app.config import QueueNames +from app.dao import service_permissions_dao from app.models import EMAIL_TYPE from app.models import Job from app.models import KEY_TYPE_NORMAL @@ -80,6 +82,29 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo assert not notification.reply_to_text +def test_post_letter_notification_for_letters_as_pdf_calls_celery_task(client, sample_letter_template, mocker): + service_permissions_dao.dao_add_service_permission(sample_letter_template.service.id, 'letters_as_pdf') + + 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' + }, + 'reference': 'foo' + } + fake_task = mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') + + letter_request(client, data, service_id=sample_letter_template.service_id) + + notification = Notification.query.one() + + fake_task.assert_called_once_with([str(notification.id)], queue=QueueNames.CREATE_LETTERS_PDF) + + def test_post_letter_notification_returns_400_and_missing_template( client, sample_service_full_permissions