From d21d9cabd134abda85668755839d8c21fb10e8ef Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 26 Sep 2017 15:06:09 +0100 Subject: [PATCH 1/4] Use new default letter contact in the DVLA celery task --- app/celery/tasks.py | 2 +- tests/app/celery/test_tasks.py | 35 +++++++++++++++++++++++----------- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index a34cae506..a64bb9cae 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -370,7 +370,7 @@ def create_dvla_file_contents_for_notifications(notifications): notification.template.__dict__, notification.personalisation, notification_reference=notification.reference, - contact_block=notification.service.letter_contact_block, + contact_block=notification.service.get_default_letter_contact(), org_id=notification.service.dvla_organisation.id, )) for notification in notifications diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index e16f819ea..574ed2eb7 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -2,7 +2,6 @@ import json import uuid from datetime import datetime from unittest.mock import Mock - import pytest import requests_mock from flask import current_app @@ -29,14 +28,17 @@ from app.celery.tasks import ( from app.config import QueueNames from app.dao import jobs_dao, services_dao from app.models import ( - Notification, + EMAIL_TYPE, + JOB_STATUS_ERROR, + KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - KEY_TYPE_NORMAL, - SMS_TYPE, - EMAIL_TYPE, LETTER_TYPE, - Job) + SERVICE_PERMISSION_TYPES, + SMS_TYPE, + Job, + Notification +) from tests.app import load_example_csv from tests.app.conftest import ( @@ -46,7 +48,16 @@ from tests.app.conftest import ( sample_email_template as create_sample_email_template, sample_notification as create_sample_notification ) -from tests.app.db import create_user, create_notification, create_job, create_service_inbound_api, create_inbound_sms +from tests.app.db import ( + create_inbound_sms, + create_job, + create_letter_contact, + create_notification, + create_service_inbound_api, + create_service, + create_template, + create_user +) class AnyStringWith(str): @@ -1076,8 +1087,11 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let mocked_send_task.assert_not_called() -def test_create_dvla_file_contents_for_job(sample_letter_template, mocker): - job = create_job(template=sample_letter_template, notification_count=2) +def test_create_dvla_file_contents(notify_db_session, mocker): + service = create_service(service_permissions=SERVICE_PERMISSION_TYPES) + create_letter_contact(service=service, contact_block='London,\nNW1A 1AA') + letter_template = create_template(service=service, template_type=LETTER_TYPE) + job = create_job(template=letter_template, notification_count=2) create_notification(template=job.template, job=job, reference=1) create_notification(template=job.template, job=job, reference=2) mocked_letter_template = mocker.patch("app.celery.tasks.LetterDVLATemplate") @@ -1093,9 +1107,8 @@ def test_create_dvla_file_contents_for_job(sample_letter_template, mocker): # Personalisation assert not calls[0][0][1] assert not calls[1][0][1] - # Named arguments - assert calls[1][1]['contact_block'] == 'London,\nSW1A 1AA' + assert calls[1][1]['contact_block'] == 'London,\nNW1A 1AA' assert calls[0][1]['notification_reference'] == '1' assert calls[1][1]['notification_reference'] == '2' assert calls[1][1]['org_id'] == '001' From 4eebd9a83c36c4284da02eec26cf8c35e7a0a285 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 26 Sep 2017 15:08:08 +0100 Subject: [PATCH 2/4] Remove the letter_contact_block from the Service model --- app/models.py | 6 ++---- tests/app/test_model.py | 5 ----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/app/models.py b/app/models.py index 930fe0744..a7ecfdddd 100644 --- a/app/models.py +++ b/app/models.py @@ -208,8 +208,7 @@ class Service(db.Model, Versioned): created_by = db.relationship('User') created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) _reply_to_email_address = db.Column("reply_to_email_address", db.Text, index=False, unique=False, nullable=True) - letter_contact_block = db.Column(db.Text, index=False, unique=False, nullable=True) - # This column is now deprecated + _letter_contact_block = db.Column('letter_contact_block', db.Text, index=False, unique=False, nullable=True) sms_sender = db.Column(db.String(11), nullable=False, default=lambda: current_app.config['FROM_NUMBER']) organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) organisation = db.relationship('Organisation') @@ -268,8 +267,7 @@ class Service(db.Model, Versioned): if len(default_letter_contact) > 1: raise Exception("There should only ever be one default") else: - return default_letter_contact[0].contact_block if default_letter_contact else \ - self.letter_contact_block # need to update this to None after dropping the letter_contact_block column + return default_letter_contact[0].contact_block if default_letter_contact else None class InboundNumber(db.Model): diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 0303782ff..03f3fb146 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -274,8 +274,3 @@ def test_service_get_default_contact_letter(sample_service): create_letter_contact(service=sample_service, contact_block='London,\nNW1A 1AA') assert sample_service.get_default_letter_contact() == 'London,\nNW1A 1AA' - - -# this test will need to be removed after letter_contact_block is dropped -def test_service_get_default_letter_contact_block_from_service(sample_service): - assert sample_service.get_default_letter_contact() == sample_service.letter_contact_block From cb37ba5f783c1a6dd65171526235a40ef8725b52 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Tue, 26 Sep 2017 15:09:02 +0100 Subject: [PATCH 3/4] Update schema to return default letter contact for a service --- app/schemas.py | 6 +++++- app/service/rest.py | 1 - tests/app/conftest.py | 3 +-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/schemas.py b/app/schemas.py index e880a1589..38799baa2 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -183,6 +183,7 @@ class ServiceSchema(BaseSchema): permissions = fields.Method("service_permissions") override_flag = False reply_to_email_address = fields.Method(method_name="get_reply_to_email_address") + letter_contact_block = fields.Method(method_name="get_letter_contact") def get_free_sms_fragment_limit(selfs, service): return service.free_sms_fragment_limit() @@ -193,9 +194,12 @@ class ServiceSchema(BaseSchema): def get_reply_to_email_address(self, service): return service.get_default_reply_to_email_address() + def get_letter_contact(self, service): + return service.get_default_letter_contact() + class Meta: model = models.Service - dump_only = ['free_sms_fragment_limit', 'reply_to_email_address'] + dump_only = ['free_sms_fragment_limit', 'reply_to_email_address', 'letter_contact_block'] exclude = ( 'updated_at', 'created_at', diff --git a/app/service/rest.py b/app/service/rest.py index df1457556..c4a2d9e59 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -147,7 +147,6 @@ def update_service(service_id): fetched_service = dao_fetch_service_by_id(service_id) # Capture the status change here as Marshmallow changes this later service_going_live = fetched_service.restricted and not req_json.get('restricted', True) - current_data = dict(service_schema.dump(fetched_service).data.items()) current_data.update(request.get_json()) update_dict = service_schema.load(current_data).data diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 042a20905..86c748df3 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -150,8 +150,7 @@ def sample_service( 'message_limit': limit, 'restricted': restricted, 'email_from': email_from, - 'created_by': user, - 'letter_contact_block': letter_contact_block, + 'created_by': user } service = Service.query.filter_by(name=service_name).first() if not service: From dede336b3bcbfee9083cb2c05f2b97c2e47f11a4 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 3 Oct 2017 13:35:09 +0100 Subject: [PATCH 4/4] Update tests to not use letter contact block from services table A few test updates were needed after rebasing onto master. --- app/models.py | 2 +- tests/app/conftest.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/models.py b/app/models.py index a7ecfdddd..edf0745a4 100644 --- a/app/models.py +++ b/app/models.py @@ -541,7 +541,7 @@ class Template(db.Model): return LetterDVLATemplate( {'content': self.content, 'subject': self.subject}, notification_reference=1, - contact_block=self.service.letter_contact_block, + contact_block=self.service.get_default_letter_contact(), ) def serialize(self): diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 86c748df3..a9dfa0a1b 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -46,7 +46,8 @@ from tests.app.db import ( create_notification, create_service, create_api_key, - create_inbound_number + create_inbound_number, + create_letter_contact, ) @@ -138,7 +139,6 @@ def sample_service( email_from=None, permissions=[SMS_TYPE, EMAIL_TYPE], research_mode=None, - letter_contact_block='London,\nSW1A 1AA', ): if user is None: user = create_user() @@ -183,7 +183,9 @@ def sample_service_full_permissions(notify_db, notify_db_session): @pytest.fixture(scope='function') def sample_service_custom_letter_contact_block(notify_db, notify_db_session): - return sample_service(notify_db, notify_db_session, letter_contact_block='((contact block))') + service = sample_service(notify_db, notify_db_session) + create_letter_contact(service, contact_block='((contact block))') + return service @pytest.fixture(scope='function')