From 562facb49cd5ca940db5b427ed5f97dc32d13e56 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 24 Aug 2018 13:51:11 +0100 Subject: [PATCH] Now that EmailBranding includes the brand type we do not need two separate colours. This PR removes the new colour columns. --- app/delivery/send_to_providers.py | 16 ++------- app/email_branding/email_branding_schema.py | 4 --- app/models.py | 4 --- tests/app/delivery/test_send_to_providers.py | 36 +++----------------- tests/app/email_branding/test_rest.py | 14 +++----- 5 files changed, 10 insertions(+), 64 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 22439cf59..c285c00e2 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -30,8 +30,7 @@ from app.models import ( NOTIFICATION_CREATED, NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_SENT, - NOTIFICATION_SENDING, - BRANDING_BOTH + NOTIFICATION_SENDING ) @@ -199,10 +198,8 @@ def get_html_email_options(service): service.email_branding.logo ) if service.email_branding.logo else None - colour = _set_colour(service) - branding = { - 'brand_colour': colour, + 'brand_colour': service.email_branding.colour, 'brand_logo': logo_url, 'brand_name': service.email_branding.text, } @@ -212,15 +209,6 @@ def get_html_email_options(service): return dict(govuk_banner=govuk_banner, brand_banner=brand_banner, **branding) -def _set_colour(service): - if service.branding in [BRANDING_BOTH, BRANDING_ORG]: - return service.email_branding.single_id_colour or service.email_branding.colour - elif service.branding == BRANDING_ORG_BANNER: - return service.email_branding.banner_colour or service.email_branding.colour - elif service.branding == BRANDING_GOVUK: - return None - - def technical_failure(notification): notification.status = NOTIFICATION_TECHNICAL_FAILURE dao_update_notification(notification) diff --git a/app/email_branding/email_branding_schema.py b/app/email_branding/email_branding_schema.py index 6c0dcdaeb..6dbfaf761 100644 --- a/app/email_branding/email_branding_schema.py +++ b/app/email_branding/email_branding_schema.py @@ -6,8 +6,6 @@ post_create_email_branding_schema = { "type": "object", "properties": { "colour": {"type": ["string", "null"]}, - "banner_colour": {"type": ["string", "null"]}, - "single_id_colour": {"type": ["string", "null"]}, "name": {"type": ["string", "null"]}, "text": {"type": ["string", "null"]}, "logo": {"type": ["string", "null"]}, @@ -23,8 +21,6 @@ post_update_email_branding_schema = { "type": "object", "properties": { "colour": {"type": ["string", "null"]}, - "banner_colour": {"type": ["string", "null"]}, - "single_id_colour": {"type": ["string", "null"]}, "name": {"type": ["string", "null"]}, "text": {"type": ["string", "null"]}, "logo": {"type": ["string", "null"]}, diff --git a/app/models.py b/app/models.py index 2a9aa68e3..9a691a4d4 100644 --- a/app/models.py +++ b/app/models.py @@ -204,8 +204,6 @@ class EmailBranding(db.Model): __tablename__ = 'email_branding' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) colour = db.Column(db.String(7), nullable=True) - banner_colour = db.Column(db.String(7), nullable=True) - single_id_colour = db.Column(db.String(7), nullable=True) logo = db.Column(db.String(255), nullable=True) name = db.Column(db.String(255), nullable=True) text = db.Column(db.String(255), nullable=True) @@ -225,8 +223,6 @@ class EmailBranding(db.Model): "logo": self.logo, "name": self.name, "text": self.text, - "banner_colour": self.banner_colour, - "single_id_colour": self.single_id_colour, "domain": self.domain, "brand_type": self.brand_type } diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 3b95ba47e..48b5b6e04 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -13,7 +13,6 @@ from app import mmg_client, firetext_client from app.dao import (provider_details_dao, notifications_dao) from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier from app.delivery import send_to_providers -from app.delivery.send_to_providers import _set_colour from app.exceptions import NotificationTechnicalFailureException from app.models import ( Notification, @@ -456,13 +455,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', 'text', 'banner_colour', 'single_id_colour']) + EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo', 'text']) email_branding = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League', - text='League of Justice', - banner_colour='#ABEBC6', - single_id_colour='#DB6849') + text='League of Justice') service = Service(branding=BRANDING_ORG, email_branding=email_branding) renderer = send_to_providers.get_html_email_options(service) @@ -472,11 +469,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', 'text', 'banner_colour', 'single_id_colour']) + EmailBranding = namedtuple('EmailBranding', ['colour', 'name', 'logo', 'text']) - email_branding = EmailBranding(colour='#000000', logo=None, name='Justice League', text='League of Justice', - banner_colour='#ABEBC6', - single_id_colour='#DB6849') + email_branding = EmailBranding(colour='#000000', logo=None, name='Justice League', text='League of Justice') service = Service(branding=BRANDING_ORG_BANNER, email_branding=email_branding) renderer = send_to_providers.get_html_email_options(service) @@ -752,26 +747,3 @@ def test_send_email_to_provider_should_format_email_address(sample_email_notific html_body=ANY, reply_to_address=ANY, ) - - -@pytest.mark.parametrize('colour, banner_colour, single_id_colour, branding_type, expected_colour', [ - ('black', 'yellow', 'red', 'org', 'red'), - ('black', 'yellow', None, 'org', 'black'), - ('black', 'yellow', 'red', 'org_banner', 'yellow'), - ('black', None, 'red', 'org_banner', 'black'), - ('black', 'yellow', 'red', 'govuk', None), - ('black', 'yellow', 'red', 'both', 'red'), - ('black', 'yellow', None, 'both', 'black'), -]) -def test_set_colour(notify_db_session, colour, banner_colour, single_id_colour, branding_type, expected_colour): - Service = namedtuple('Service', ['branding', 'email_branding']) - email_branding = EmailBranding(colour=colour, - logo='justice-league.png', - name='Justice League', - text='League of Justice', - banner_colour=banner_colour, - single_id_colour=single_id_colour) - service = Service(branding=branding_type, email_branding=email_branding) - - colour = _set_colour(service) - assert colour == expected_colour diff --git a/tests/app/email_branding/test_rest.py b/tests/app/email_branding/test_rest.py index 9ee6505d0..6ef4edfd9 100644 --- a/tests/app/email_branding/test_rest.py +++ b/tests/app/email_branding/test_rest.py @@ -34,7 +34,7 @@ def test_get_email_branding_by_id(admin_request, notify_db, notify_db_session): ) assert set(response['email_branding'].keys()) == {'colour', 'logo', 'name', 'id', 'text', - 'banner_colour', 'single_id_colour', 'domain', 'brand_type'} + 'domain', 'brand_type'} assert response['email_branding']['colour'] == '#FFFFFF' assert response['email_branding']['logo'] == '/path/image.png' assert response['email_branding']['name'] == 'Some Org' @@ -47,8 +47,6 @@ def test_post_create_email_branding(admin_request, notify_db_session): data = { 'name': 'test email_branding', 'colour': '#0000ff', - 'banner_colour': '#808080', - 'single_id_colour': '#FF0000', 'logo': '/images/test_x2.png', 'domain': 'gov.uk', 'brand_type': BRANDING_ORG @@ -60,8 +58,6 @@ def test_post_create_email_branding(admin_request, notify_db_session): ) assert data['name'] == response['data']['name'] assert data['colour'] == response['data']['colour'] - assert data['banner_colour'] == response['data']['banner_colour'] - assert data['single_id_colour'] == response['data']['single_id_colour'] assert data['logo'] == response['data']['logo'] assert data['name'] == response['data']['text'] assert data['domain'] == response['data']['domain'] @@ -72,8 +68,6 @@ def test_post_create_email_branding_without_brand_type_defaults(admin_request, n data = { 'name': 'test email_branding', 'colour': '#0000ff', - 'banner_colour': '#808080', - 'single_id_colour': '#FF0000', 'logo': '/images/test_x2.png', 'domain': 'gov.uk', } @@ -170,9 +164,9 @@ def test_post_create_email_branding_with_text_as_none_and_name(admin_request, no @pytest.mark.parametrize('data_update', [ ({'name': 'test email_branding 1'}), ({'logo': 'images/text_x3.png', 'colour': '#ffffff'}), - ({'logo': 'images/text_x3.png', 'banner_colour': '#ffffff', 'single_id_colour': '#808080'}), - ({'logo': 'images/text_x3.png', 'banner_colour': '#ffffff', 'single_id_colour': '#808080', 'domain': 'gov.uk'}), - ({'logo': 'images/text_x3.png', 'banner_colour': '#ffffff', 'single_id_colour': '#808080', 'brand_type': 'org'}), + ({'logo': 'images/text_x3.png'}), + ({'logo': 'images/text_x3.png', 'domain': 'gov.uk'}), + ({'logo': 'images/text_x3.png', 'brand_type': 'org'}), ]) def test_post_update_email_branding_updates_field(admin_request, notify_db_session, data_update): data = {