Merge branch 'master' into pyup-update-pytest-3.7.3-to-3.7.4

app/delivery/send_to_providers.py

@@ -25,7 +25,6 @@ from app.models import (
     KEY_TYPE_TEST,
     BRANDING_BOTH,
     BRANDING_ORG_BANNER,
-    BRANDING_GOVUK,
     EMAIL_TYPE,
     NOTIFICATION_CREATED,
     NOTIFICATION_TECHNICAL_FAILURE,
@@ -190,10 +189,7 @@ def get_logo_url(base_url, logo_file):
 
 def get_html_email_options(service):
 
-    if (
+    if service.email_branding is None:
-        service.email_branding is None or
-        service.email_branding.brand_type == BRANDING_GOVUK
-    ):
         return {
             'govuk_banner': True,
             'brand_banner': False,

app/models.py

@@ -188,11 +188,11 @@ user_to_organisation = db.Table(
 )
 
 
-BRANDING_GOVUK = 'govuk'
+BRANDING_GOVUK = 'govuk'  # Deprecated outside migrations
 BRANDING_ORG = 'org'
 BRANDING_BOTH = 'both'
 BRANDING_ORG_BANNER = 'org_banner'
-BRANDING_TYPES = [BRANDING_GOVUK, BRANDING_ORG, BRANDING_BOTH, BRANDING_ORG_BANNER]
+BRANDING_TYPES = [BRANDING_ORG, BRANDING_BOTH, BRANDING_ORG_BANNER]
 
 
 class BrandingTypes(db.Model):
@@ -207,13 +207,13 @@ class EmailBranding(db.Model):
     logo = db.Column(db.String(255), nullable=True)
     name = db.Column(db.String(255), nullable=True)
     text = db.Column(db.String(255), nullable=True)
-    domain = db.Column(db.Text, nullable=True)
+    domain = db.Column(db.Text, unique=True, nullable=True)
     brand_type = db.Column(
         db.String(255),
         db.ForeignKey('branding_type.name'),
         index=True,
         nullable=True,
-        default=BRANDING_GOVUK
+        default=BRANDING_ORG
     )
 
     def serialize(self):
@@ -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,57 @@
+"""
+ 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
+from app.models import BRANDING_ORG, BRANDING_GOVUK
+
+
+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)
+
+    op.execute("""
+        update
+            email_branding
+        set
+            brand_type = '{}'
+        where
+            brand_type = '{}'
+    """.format(BRANDING_ORG, BRANDING_GOVUK))
+
+    op.execute("""
+        delete from
+            branding_type
+        where
+            name = '{}'
+    """.format(BRANDING_GOVUK))
+
+
+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)
+
+    op.execute("""
+        insert into
+            branding_type
+                (name)
+            values
+                ('{}')
+    """.format(BRANDING_GOVUK))

migrations/versions/0222_drop_service_branding.py

@@ -0,0 +1,23 @@
+"""
+ Revision ID: 0222_drop_service_branding
+Revises: 0221_nullable_service_branding
+Create Date: 2018-08-24 13:36:49.346156
+ """
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = '0222_drop_service_branding'
+down_revision = '0221_nullable_service_branding'
+
+
+def upgrade():
+
+    op.drop_column('services_history', 'branding')
+    op.drop_column('services', 'branding')
+
+
+def downgrade():
+
+    op.add_column('services', sa.Column('branding', sa.String(length=255)))
+    op.add_column('services_history', sa.Column('branding', sa.String(length=255)))

migrations/versions/0223_add_domain_constraint.py

@@ -0,0 +1,20 @@
+"""
+Revision ID: 0223_add_domain_constraint
+Revises: 0222_drop_service_branding
+Create Date: 2018-08-24 13:36:49.346156
+ """
+from alembic import op
+
+
+revision = '0223_add_domain_constraint'
+down_revision = '0222_drop_service_branding'
+
+
+def upgrade():
+
+    op.create_unique_constraint('uq_email_branding_domain', 'email_branding', ['domain'])
+
+
+def downgrade():
+
+    op.drop_constraint('uq_email_branding_domain', 'email_branding')

requirements_for_test.txt

@@ -1,12 +1,12 @@
 -r requirements.txt
 flake8==3.5.0
-moto==1.3.4
 pytest==3.7.4
+moto==1.3.5
 pytest-env==0.6.2
 pytest-mock==1.10.0
 pytest-cov==2.5.1
 pytest-xdist==1.23.0
-coveralls==1.4.0
+coveralls==1.5.0
 freezegun==0.3.10
 requests-mock==1.5.2
 # optional requirements for jsonschema

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

@@ -21,7 +21,6 @@ from app.models import (
     KEY_TYPE_TEST,
     KEY_TYPE_TEAM,
     BRANDING_ORG,
-    BRANDING_GOVUK,
     BRANDING_BOTH,
     BRANDING_ORG_BANNER
 )
@@ -202,7 +201,6 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
 def test_should_call_send_sms_response_task_if_research_mode(
         notify_db, sample_service, sample_notification, mocker, research_mode, key_type
 ):
-    sample_service.branding = BRANDING_GOVUK
     mocker.patch('app.mmg_client.send_sms')
     mocker.patch('app.delivery.send_to_providers.send_sms_response')
 
@@ -422,8 +420,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,16 +444,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
+    sample_service.email_branding = None
-    email_branding = EmailBranding(
+    notify_db.session.add_all([sample_service])
-        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()
 
     options = send_to_providers.get_html_email_options(sample_service)
@@ -466,7 +454,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 +465,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 +474,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 +485,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/email_branding/test_rest.py

@@ -1,6 +1,6 @@
 import pytest
 
-from app.models import EmailBranding, BRANDING_GOVUK, BRANDING_ORG
+from app.models import EmailBranding, BRANDING_ORG
 from tests.app.db import create_email_branding
 
 
@@ -76,7 +76,7 @@ def test_post_create_email_branding_without_brand_type_defaults(admin_request, n
         _data=data,
         _expected_status=201
     )
-    assert BRANDING_GOVUK == response['data']['brand_type']
+    assert BRANDING_ORG == response['data']['brand_type']
 
 
 def test_post_create_email_branding_without_logo_is_ok(admin_request, notify_db_session):
@@ -240,7 +240,7 @@ def test_create_email_branding_reject_invalid_brand_type(admin_request):
         _expected_status=400
     )
 
-    assert response['errors'][0]['message'] == 'brand_type NOT A TYPE is not one of [govuk, org, both, org_banner]'
+    assert response['errors'][0]['message'] == 'brand_type NOT A TYPE is not one of [org, both, org_banner]'
 
 
 def test_update_email_branding_reject_invalid_brand_type(admin_request, notify_db_session):
@@ -256,4 +256,4 @@ def test_update_email_branding_reject_invalid_brand_type(admin_request, notify_d
         email_branding_id=email_branding.id
     )
 
-    assert response['errors'][0]['message'] == 'brand_type NOT A TYPE is not one of [govuk, org, both, org_banner]'
+    assert response['errors'][0]['message'] == 'brand_type NOT A TYPE is not one of [org, both, org_banner]'

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