diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 6cb126740..469fbfd6d 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -52,13 +52,10 @@ from app.models import ( def create_letters_pdf(self, notification_id): try: notification = get_notification_by_id(notification_id, _raise=True) - # We only need this while we are migrating to the new letter_branding model - letter_logo_file_name = notification.service.letter_branding.filename if notification.service.letter_branding\ - else notification.service.dvla_organisation.filename pdf_data, billable_units = get_letters_pdf( notification.template, contact_block=notification.reply_to_text, - filename=letter_logo_file_name, + filename=notification.service.letter_branding and notification.service.letter_branding.filename, values=notification.personalisation ) diff --git a/app/schemas.py b/app/schemas.py index 4353d98d0..00bc611eb 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -205,7 +205,7 @@ class ServiceSchema(BaseSchema): created_by = field_for(models.Service, 'created_by', required=True) organisation_type = field_for(models.Service, 'organisation_type') dvla_organisation = field_for(models.Service, 'dvla_organisation') - letter_logo_filename = fields.Method(serialize='get_letter_logo_filename') + letter_logo_filename = fields.Method(dump_only=True, serialize='get_letter_logo_filename') permissions = fields.Method("service_permissions") email_branding = field_for(models.Service, 'email_branding') organisation = field_for(models.Service, 'organisation') @@ -213,8 +213,7 @@ class ServiceSchema(BaseSchema): letter_contact_block = fields.Method(serialize="get_letter_contact") def get_letter_logo_filename(self, service): - return service.letter_branding.filename if service.letter_branding\ - else service.dvla_organisation.filename + return service.letter_branding and service.letter_branding.filename def service_permissions(self, service): return [p.permission for p in service.permissions] diff --git a/app/template/rest.py b/app/template/rest.py index d51111317..ac4bc9a64 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -265,9 +265,7 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file } service = dao_fetch_service_by_id(service_id) - # We only need this while we are migrating to the new letter_branding model - letter_logo_filename = service.letter_branding.filename if service.letter_branding \ - else service.dvla_organisation.filename + letter_logo_filename = service.letter_branding and service.letter_branding.filename data = { 'letter_contact_block': notification.reply_to_text, 'template': template_for_letter_print, diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 3a5089902..4a849a005 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -182,8 +182,7 @@ def test_create_letters_pdf_sets_technical_failure_max_retries(mocker, sample_le mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure') -# We only need this while we are migrating to the new letter_branding model -def test_create_letters_gets_the_right_logo_when_service_has_dvla_logo( +def test_create_letters_gets_the_right_logo_when_service_has_no_logo( notify_api, mocker, sample_letter_notification ): mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', 1)) @@ -194,7 +193,7 @@ def test_create_letters_gets_the_right_logo_when_service_has_dvla_logo( mock_get_letters_pdf.assert_called_once_with( sample_letter_notification.template, contact_block=sample_letter_notification.reply_to_text, - filename=sample_letter_notification.service.dvla_organisation.filename, + filename=None, values=sample_letter_notification.personalisation ) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index c99e9df64..31b250e76 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -141,7 +141,7 @@ def test_get_service_by_id(admin_request, sample_service): assert 'branding' not in json_resp['data'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['prefix_sms'] is True - assert json_resp['data']['letter_logo_filename'] == 'hm-government' + assert json_resp['data']['letter_logo_filename'] is None @pytest.mark.parametrize('detailed', [True, False]) @@ -294,10 +294,10 @@ def test_create_service_with_no_domain_doesnt_set_letter_branding(admin_request, json_resp = admin_request.post('service.create_service', _data=data, _expected_status=201) assert json_resp['data']['letter_branding'] is None - assert json_resp['data']['letter_logo_filename'] == 'hm-government' + assert json_resp['data']['letter_logo_filename'] is None -def test_get_service_by_id_returns_letter_branding_not_dvla_organisation( +def test_get_service_by_id_returns_letter_branding( client, sample_service ): letter_branding = create_letter_branding( diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index 017921c1a..2963bd3ec 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -982,6 +982,7 @@ def test_preview_letter_template_by_id_valid_file_type( 'postcode': 'A_POST', } assert post_json['date'] == '2012-12-12T00:00:00' + assert post_json['filename'] is None assert base64.b64decode(resp['content']) == content