Merge pull request #2064 from alphagov/look-in-branding-brand_type

Look in email_branding for brand_type, not service

app/delivery/send_to_providers.py

@@ -23,7 +23,7 @@ from app.exceptions import NotificationTechnicalFailureException
 from app.models import (
     SMS_TYPE,
     KEY_TYPE_TEST,
-    BRANDING_ORG,
+    BRANDING_BOTH,
     BRANDING_ORG_BANNER,
     BRANDING_GOVUK,
     EMAIL_TYPE,
@@ -189,24 +189,28 @@ def get_logo_url(base_url, logo_file):
 
 
 def get_html_email_options(service):
-    govuk_banner = service.branding not in (BRANDING_ORG, BRANDING_ORG_BANNER)
-    brand_banner = service.branding == BRANDING_ORG_BANNER
-    if service.branding != BRANDING_GOVUK and service.email_branding:
 
-        logo_url = get_logo_url(
-            current_app.config['ADMIN_BASE_URL'],
-            service.email_branding.logo
-        ) if service.email_branding.logo else None
-
-        branding = {
-            'brand_colour': service.email_branding.colour,
-            'brand_logo': logo_url,
-            'brand_name': service.email_branding.text,
+    if (
+        service.email_branding is None or
+        service.email_branding.brand_type == BRANDING_GOVUK
+    ):
+        return {
+            'govuk_banner': True,
+            'brand_banner': False,
         }
-    else:
-        branding = {}
 
-    return dict(govuk_banner=govuk_banner, brand_banner=brand_banner, **branding)
+    logo_url = get_logo_url(
+        current_app.config['ADMIN_BASE_URL'],
+        service.email_branding.logo
+    ) if service.email_branding.logo else None
+
+    return {
+        'govuk_banner': service.email_branding.brand_type == BRANDING_BOTH,
+        'brand_banner': service.email_branding.brand_type == BRANDING_ORG_BANNER,
+        'brand_colour': service.email_branding.colour,
+        'brand_logo': logo_url,
+        'brand_name': service.email_branding.text,
+    }
 
 
 def technical_failure(notification):

migrations/versions/0217_default_email_branding.py

@@ -0,0 +1,25 @@
+"""
+ Revision ID: 0217_default_email_branding
+Revises: 0216_remove_colours
+Create Date: 2018-08-24 13:36:49.346156
+ """
+from alembic import op
+from app.models import BRANDING_ORG
+
+revision = '0217_default_email_branding'
+down_revision = '0216_remove_colours'
+
+
+def upgrade():
+    op.execute("""
+        update
+            email_branding
+        set
+            brand_type = '{}'
+        where
+            brand_type = null
+    """.format(BRANDING_ORG))
+
+
+def downgrade():
+    pass

tests/app/delivery/test_send_to_providers.py

@@ -409,7 +409,7 @@ def test_send_email_should_use_service_reply_to_email(
 
 def test_get_html_email_renderer_should_return_for_normal_service(sample_service):
     options = send_to_providers.get_html_email_options(sample_service)
-    assert options['govuk_banner']
+    assert options['govuk_banner'] is True
     assert 'brand_colour' not in options.keys()
     assert 'brand_logo' not in options.keys()
     assert 'brand_name' not in options.keys()
@@ -421,9 +421,16 @@ def test_get_html_email_renderer_should_return_for_normal_service(sample_service
     (BRANDING_ORG_BANNER, False)
 ])
 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',
-                                   text='League of Justice')
+
+    sample_service.branding = BRANDING_GOVUK  # Expected to be ignored
+
+    email_branding = EmailBranding(
+        brand_type=branding_type,
+        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()
@@ -434,7 +441,7 @@ def test_get_html_email_renderer_with_branding_details(branding_type, govuk_bann
     assert options['brand_colour'] == '#000000'
     assert options['brand_name'] == 'League of Justice'
 
-    if sample_service.branding == BRANDING_ORG_BANNER:
+    if branding_type == BRANDING_ORG_BANNER:
         assert options['brand_banner'] is True
     else:
         assert options['brand_banner'] is False
@@ -442,8 +449,13 @@ 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',
-                                   text='League of Justice')
+    email_branding = EmailBranding(
+        brand_type=BRANDING_GOVUK,
+        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()
@@ -455,12 +467,19 @@ 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', 'text'])
+    EmailBranding = namedtuple('EmailBranding', ['brand_type', 'colour', 'name', 'logo', 'text'])
 
-    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)
+    email_branding = EmailBranding(
+        brand_type=BRANDING_ORG,
+        colour='#000000',
+        logo='justice-league.png',
+        name='Justice League',
+        text='League of Justice',
+    )
+    service = Service(
+        branding=BRANDING_GOVUK,  # expected to be ignored
+        email_branding=email_branding,
+    )
 
     renderer = send_to_providers.get_html_email_options(service)
 
@@ -469,14 +488,27 @@ 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', 'text'])
+    EmailBranding = namedtuple('EmailBranding', ['brand_type', 'colour', 'name', 'logo', 'text'])
 
-    email_branding = EmailBranding(colour='#000000', logo=None, name='Justice League', text='League of Justice')
-    service = Service(branding=BRANDING_ORG_BANNER, email_branding=email_branding)
+    email_branding = EmailBranding(
+        brand_type=BRANDING_ORG_BANNER,
+        colour='#000000',
+        logo=None,
+        name='Justice League',
+        text='League of Justice',
+    )
+    service = Service(
+        branding=BRANDING_GOVUK,  # Expected to be ignored
+        email_branding=email_branding,
+    )
 
     renderer = send_to_providers.get_html_email_options(service)
 
+    assert renderer['govuk_banner'] is False
+    assert renderer['brand_banner'] is True
     assert renderer['brand_logo'] is None
+    assert renderer['brand_name'] == 'League of Justice'
+    assert renderer['brand_colour'] == '#000000'
 
 
 @pytest.mark.parametrize('base_url, expected_url', [