Deprecate service branding column

We want to drop this column. First we have to stop using it anywhere.

Needs to be made nullable so we can stop writing to it.

app/models.py

@@ -346,13 +346,6 @@ class Service(db.Model, Versioned):
         default=DVLA_ORG_HM_GOVERNMENT
     )
     dvla_organisation = db.relationship('DVLAOrganisation')
-    branding = db.Column(
-        db.String(255),
-        db.ForeignKey('branding_type.name'),
-        index=True,
-        nullable=False,
-        default=BRANDING_GOVUK
-    )
     organisation_type = db.Column(
         db.String(255),
         nullable=True,

app/schemas.py

@@ -204,7 +204,6 @@ class ServiceSchema(BaseSchema):
 
     created_by = field_for(models.Service, 'created_by', required=True)
     organisation_type = field_for(models.Service, 'organisation_type')
-    branding = field_for(models.Service, 'branding')
     dvla_organisation = field_for(models.Service, 'dvla_organisation')
     permissions = fields.Method("service_permissions")
     email_branding = field_for(models.Service, 'email_branding')

migrations/versions/0221_nullable_service_branding.py

@@ -0,0 +1,32 @@
+"""
+ Revision ID: 0221_nullable_service_branding
+Revises: 0220_email_brand_type_non_null
+Create Date: 2018-08-24 13:36:49.346156
+ """
+from alembic import op
+
+
+revision = '0221_nullable_service_branding'
+down_revision = '0220_email_brand_type_non_null'
+
+
+def upgrade():
+
+    op.drop_constraint('services_branding_fkey', 'services', type_='foreignkey')
+
+    op.drop_index('ix_services_history_branding', table_name='services_history')
+    op.drop_index('ix_services_branding', table_name='services')
+
+    op.alter_column('services_history', 'branding', nullable=True)
+    op.alter_column('services', 'branding', nullable=True)
+
+
+def downgrade():
+
+    op.create_index(op.f('ix_services_branding'), 'services', ['branding'], unique=False)
+    op.create_index(op.f('ix_services_history_branding'), 'services_history', ['branding'], unique=False)
+
+    op.create_foreign_key(None, 'services', 'branding_type', ['branding'], ['name'])
+
+    op.alter_column('services', 'branding', nullable=False)
+    op.alter_column('services_history', 'branding', nullable=False)

tests/app/dao/test_services_dao.py

@@ -49,7 +49,6 @@ from app.models import (
     Service,
     ServicePermission,
     ServicePermissionTypes,
-    BRANDING_GOVUK,
     DVLA_ORG_HM_GOVERNMENT,
     KEY_TYPE_NORMAL,
     KEY_TYPE_TEAM,
@@ -97,7 +96,6 @@ def test_create_service(sample_user):
     service_db = Service.query.one()
     assert service_db.name == "service_name"
     assert service_db.id == service.id
-    assert service_db.branding == BRANDING_GOVUK
     assert service_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT
     assert service_db.email_from == 'email_from'
     assert service_db.research_mode is False
@@ -349,9 +347,7 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user):
     assert service_from_db.version == service_history.version
     assert sample_user.id == service_history.created_by_id
     assert service_from_db.created_by.id == service_history.created_by_id
-    assert service_from_db.branding == BRANDING_GOVUK
     assert service_from_db.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT
-    assert service_history.branding == BRANDING_GOVUK
     assert service_history.dvla_organisation_id == DVLA_ORG_HM_GOVERNMENT
 
 

tests/app/db.py

@@ -78,7 +78,6 @@ def create_service(
         prefix_sms=True,
         message_limit=1000,
         organisation_type='central',
-        branding=None
 ):
     service = Service(
         name=service_name,
@@ -88,7 +87,6 @@ def create_service(
         created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())),
         prefix_sms=prefix_sms,
         organisation_type=organisation_type,
-        branding=branding
     )
 
     dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions)

tests/app/delivery/test_send_to_providers.py

@@ -422,8 +422,6 @@ 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_GOVUK  # Expected to be ignored
-
     email_branding = EmailBranding(
         brand_type=branding_type,
         colour='#000000',
@@ -448,7 +446,6 @@ 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(
         brand_type=BRANDING_GOVUK,
         colour='#000000',
@@ -466,7 +463,7 @@ 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'])
+    Service = namedtuple('Service', ['email_branding'])
     EmailBranding = namedtuple('EmailBranding', ['brand_type', 'colour', 'name', 'logo', 'text'])
 
     email_branding = EmailBranding(
@@ -477,7 +474,6 @@ def test_get_html_email_renderer_prepends_logo_path(notify_api):
         text='League of Justice',
     )
     service = Service(
-        branding=BRANDING_GOVUK,  # expected to be ignored
         email_branding=email_branding,
     )
 
@@ -487,7 +483,7 @@ 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'])
+    Service = namedtuple('Service', ['email_branding'])
     EmailBranding = namedtuple('EmailBranding', ['brand_type', 'colour', 'name', 'logo', 'text'])
 
     email_branding = EmailBranding(
@@ -498,7 +494,6 @@ def test_get_html_email_renderer_handles_email_branding_without_logo(notify_api)
         text='League of Justice',
     )
     service = Service(
-        branding=BRANDING_GOVUK,  # Expected to be ignored
         email_branding=email_branding,
     )
 

tests/app/service/test_rest.py

@@ -135,7 +135,7 @@ def test_get_service_by_id(admin_request, sample_service):
     assert json_resp['data']['id'] == str(sample_service.id)
     assert not json_resp['data']['research_mode']
     assert json_resp['data']['email_branding'] is None
-    assert json_resp['data']['branding'] == 'govuk'
+    assert 'branding' not in json_resp['data']
     assert json_resp['data']['dvla_organisation'] == '001'
     assert json_resp['data']['prefix_sms'] is True