Don't fallback to dvla_organisation if letter branding unset

The template preview app now accepts a null value for the `filename` 
parameter. If a service doesn't have a letter branding option set, 
previously we defaulted to their dvla_organisation (probably HM 
Government). Now, we pass through None, so that we generate letters 
without any logo or branding.

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
         )
 

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\
+        return service.letter_branding and service.letter_branding.filename
-            else service.dvla_organisation.filename
 
     def service_permissions(self, service):
         return [p.permission for p in service.permissions]

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 and service.letter_branding.filename
-        letter_logo_filename = service.letter_branding.filename if service.letter_branding \
-            else service.dvla_organisation.filename
         data = {
             'letter_contact_block': notification.reply_to_text,
             'template': template_for_letter_print,

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_no_logo(
-def test_create_letters_gets_the_right_logo_when_service_has_dvla_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
     )
 

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(

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