Merge pull request #2037 from alphagov/fix-email-branding

This PR fixes the check for the text key in the post data.

app/delivery/send_to_providers.py

@@ -201,7 +201,7 @@ def get_html_email_options(service):
         branding = {
             'brand_colour': service.email_branding.colour,
             'brand_logo': logo_url,
-            'brand_name': service.email_branding.name,
+            'brand_name': service.email_branding.text,
         }
     else:
         branding = {}

app/email_branding/rest.py

@@ -37,7 +37,7 @@ def create_email_branding():
     validate(data, post_create_email_branding_schema)
 
     email_branding = EmailBranding(**data)
-    if not data.get('text'):
+    if 'text' not in data.keys():
         email_branding.text = email_branding.name
 
     dao_create_email_branding(email_branding)
@@ -51,7 +51,7 @@ def update_email_branding(email_branding_id):
     validate(data, post_update_email_branding_schema)
 
     fetched_email_branding = dao_get_email_branding_by_id(email_branding_id)
-    if not data.get('text') and data.get('name'):
+    if 'text' not in data.keys() and 'name' in data.keys():
         data['text'] = data['name']
     dao_update_email_branding(fetched_email_branding, **data)
 

tests/app/delivery/test_send_to_providers.py

@@ -421,7 +421,8 @@ def test_get_html_email_renderer_should_return_for_normal_service(sample_service
 ])
 def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service):
     sample_service.branding = branding_type
-    email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League')
+    email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League',
+                                   text='League of Justice')
     sample_service.email_branding = email_branding
     notify_db.session.add_all([sample_service, email_branding])
     notify_db.session.commit()
@@ -430,7 +431,7 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann
 
     assert options['govuk_banner'] == govuk_banner
     assert options['brand_colour'] == '#000000'
-    assert options['brand_name'] == 'Justice League'
+    assert options['brand_name'] == 'League of Justice'
 
     if sample_service.branding == BRANDING_ORG_BANNER:
         assert options['brand_banner'] is True
@@ -440,7 +441,8 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann
 
 def test_get_html_email_renderer_with_branding_details_and_render_govuk_banner_only(notify_db, sample_service):
     sample_service.branding = BRANDING_GOVUK
-    email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League')
+    email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League',
+                                   text='League of Justice')
     sample_service.email_branding = email_branding
     notify_db.session.add_all([sample_service, email_branding])
     notify_db.session.commit()
@@ -452,9 +454,11 @@ def test_get_html_email_renderer_with_branding_details_and_render_govuk_banner_o
 
 def test_get_html_email_renderer_prepends_logo_path(notify_api):
     Service = namedtuple('Service', ['branding', 'email_branding'])
-    EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo'])
+    EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo', 'text'])
 
-    email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League')
+    email_branding = EmailBranding(colour='#000000', logo='justice-league.png',
+                                   name='Justice League',
+                                   text='League of Justice')
     service = Service(branding=BRANDING_ORG, email_branding=email_branding)
 
     renderer = send_to_providers.get_html_email_options(service)
@@ -464,9 +468,9 @@ def test_get_html_email_renderer_prepends_logo_path(notify_api):
 
 def test_get_html_email_renderer_handles_email_branding_without_logo(notify_api):
     Service = namedtuple('Service', ['branding', 'email_branding'])
-    EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo'])
+    EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo', 'text'])
 
-    email_branding = EmailBranding(colour='#000000', logo=None, name='Justice League')
+    email_branding = EmailBranding(colour='#000000', logo=None, name='Justice League', text='League of Justice')
     service = Service(branding=BRANDING_ORG, email_branding=email_branding)
 
     renderer = send_to_providers.get_html_email_options(service)

tests/app/email_branding/test_rest.py

@@ -120,6 +120,24 @@ def test_post_create_email_branding_with_text_and_name(admin_request, notify_db_
     assert response['data']['text'] == 'text for brand'
 
 
+def test_post_create_email_branding_with_text_as_none_and_name(admin_request, notify_db_session):
+    data = {
+        'name': 'name for brand',
+        'text': None,
+        'logo': 'images/text_x2.png'
+    }
+    response = admin_request.post(
+        'email_branding.create_email_branding',
+        _data=data,
+        _expected_status=201
+    )
+
+    assert response['data']['logo'] == data['logo']
+    assert response['data']['name'] == 'name for brand'
+    assert response['data']['colour'] is None
+    assert response['data']['text'] is None
+
+
 @pytest.mark.parametrize('data_update', [
     ({'name': 'test email_branding 1'}),
     ({'logo': 'images/text_x3.png', 'colour': '#ffffff'}),
@@ -155,6 +173,7 @@ def test_post_update_email_branding_updates_field(admin_request, notify_db_sessi
 @pytest.mark.parametrize('data_update', [
     ({'text': 'text email branding'}),
     ({'text': 'new text', 'name': 'new name'}),
+    ({'text': None, 'name': 'test name'}),
 ])
 def test_post_update_email_branding_updates_field_with_text(admin_request, notify_db_session, data_update):
     data = {