Adding new data models for letter branding.

app/dao/letter_branding_dao.py

@@ -0,0 +1,18 @@
+from app.models import LetterBranding
+
+
+def get_letter_branding_or_platform_default(domain=None):
+    letter_branding = None
+    if domain:
+        letter_branding = LetterBranding.query.filter(
+            LetterBranding.domain == domain
+        ).first()
+    if not letter_branding:
+        letter_branding = LetterBranding.query.filter(
+            LetterBranding.platform_default == True  # noqa
+        ).first()
+    return letter_branding
+
+
+def get_all_letter_branding():
+    return LetterBranding.query.order_by(LetterBranding.name).all()

app/dao/services_dao.py

@@ -150,7 +150,7 @@ def dao_fetch_service_by_id_and_user(service_id, user_id):
 
 @transactional
 @version_class(Service)
-def dao_create_service(service, user, service_id=None, service_permissions=None):
+def dao_create_service(service, user, service_id=None, service_permissions=None, letter_branding=None):
     # the default property does not appear to work when there is a difference between the sqlalchemy schema and the
     # db schema (ie: during a migration), so we have to set sms_sender manually here. After the GOVUK sms_sender
     # migration is completed, this code should be able to be removed.
@@ -172,6 +172,8 @@ def dao_create_service(service, user, service_id=None, service_permissions=None)
 
     # do we just add the default - or will we get a value from FE?
     insert_service_sms_sender(service, current_app.config['FROM_NUMBER'])
+    if letter_branding:
+        service.letter_branding = letter_branding
     db.session.add(service)
 
 

app/models.py

@@ -251,10 +251,11 @@ class DVLAOrganisation(db.Model):
 
 class LetterBranding(db.Model):
     __tablename__ = 'letter_branding'
-    id = db.Column(UUID(as_uuid=True), primary_key=True)
+    id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
     name = db.Column(db.String(255), unique=True, nullable=False)
     filename = db.Column(db.String(255), unique=True, nullable=False)
     domain = db.Column(db.Text, unique=True, nullable=True)
+    platform_default = db.Column(db.Boolean, nullable=False, default=False)
 
 
 service_letter_branding = db.Table(
@@ -389,6 +390,11 @@ class Service(db.Model, Versioned):
         secondary=service_email_branding,
         uselist=False,
         backref=db.backref('services', lazy='dynamic'))
+    letter_branding = db.relationship(
+        'LetterBranding',
+        secondary=service_letter_branding,
+        uselist=False,
+        backref=db.backref('services', lazy='dynamic'))
 
     @classmethod
     def from_json(cls, data):

app/schemas.py

@@ -213,7 +213,8 @@ class ServiceSchema(BaseSchema):
     letter_contact_block = fields.Method(serialize="get_letter_contact")
 
     def get_letter_logo_filename(self, service):
-        return service.dvla_organisation.filename
+        return service.letter_branding.filename if service.letter_branding\
+            else service.dvla_organisation.filename
 
     def service_permissions(self, service):
         return [p.permission for p in service.permissions]

app/service/rest.py

@@ -27,6 +27,7 @@ from app.dao.fact_notification_status_dao import (
     fetch_stats_for_all_services_by_date_range, fetch_monthly_template_usage_for_service
 )
 from app.dao.inbound_numbers_dao import dao_allocate_number_for_service
+from app.dao.letter_branding_dao import get_letter_branding_or_platform_default
 from app.dao.organisation_dao import dao_get_organisation_by_service_id
 from app.dao.service_data_retention_dao import (
     fetch_service_data_retention,
@@ -82,7 +83,7 @@ from app.errors import (
     register_errors
 )
 from app.letters.utils import letter_print_day
-from app.models import LETTER_TYPE, NOTIFICATION_CANCELLED, Service, EmailBranding
+from app.models import LETTER_TYPE, NOTIFICATION_CANCELLED, Service, EmailBranding, LetterBranding
 from app.schema_validation import validate
 from app.service import statistics
 from app.service.service_data_retention_schema import (
@@ -182,7 +183,7 @@ def create_service():
     if not data.get('user_id'):
         errors = {'user_id': ['Missing data for required field.']}
         raise InvalidRequest(errors, status_code=400)
-
+    domain = data.pop('service_domain', None)
     # validate json with marshmallow
     service_schema.load(data)
 
@@ -191,7 +192,9 @@ def create_service():
     # unpack valid json into service object
     valid_service = Service.from_json(data)
 
-    dao_create_service(valid_service, user)
+    letter_branding = get_letter_branding_or_platform_default(domain)
+    dao_create_service(valid_service, user, letter_branding=letter_branding)
+
     return jsonify(data=service_schema.dump(valid_service).data), 201
 
 
@@ -212,7 +215,9 @@ def update_service(service_id):
     if 'email_branding' in req_json:
         email_branding_id = req_json['email_branding']
         service.email_branding = None if not email_branding_id else EmailBranding.query.get(email_branding_id)
-
+    if 'letter_branding' in req_json:
+        letter_branding_id = req_json['letter_branding']
+        service.letter_branding = None if not letter_branding_id else LetterBranding.query.get(letter_branding_id)
     dao_update_service(service)
 
     if service_going_live:

migrations/versions/0252_letter_branding_table.py

@@ -1,7 +1,7 @@
 """
 
-Revision ID: 0251_letter_branding_table
-Revises: 0250_drop_stats_template_table
+Revision ID: 0252_letter_branding_table
+Revises: 0251_another_letter_org
 Create Date: 2019-01-17 15:45:33.242955
 
 """
@@ -10,8 +10,8 @@ from alembic import op
 import sqlalchemy as sa
 from sqlalchemy.dialects import postgresql
 
-revision = '0251_letter_branding_table'
-down_revision = '0250_drop_stats_template_table'
+revision = '0252_letter_branding_table'
+down_revision = '0251_another_letter_org'
 
 
 def upgrade():
@@ -20,6 +20,7 @@ def upgrade():
                     sa.Column('name', sa.String(length=255), nullable=False),
                     sa.Column('filename', sa.String(length=255), nullable=False),
                     sa.Column('domain', sa.Text(), nullable=True),
+                    sa.Column('platform_default', sa.Boolean(), nullable=False),
                     sa.PrimaryKeyConstraint('id'),
                     sa.UniqueConstraint('domain'),
                     sa.UniqueConstraint('filename'),
@@ -35,16 +36,13 @@ def upgrade():
 
     op.get_bind()
 
-    op.execute("""INSERT INTO letter_branding (id, name, filename, domain)
-                SELECT uuid_in(md5(random()::text)::cstring), name, filename, null 
+    op.execute("""INSERT INTO letter_branding (id, name, filename, domain, platform_default)
+                SELECT uuid_in(md5(random()::text)::cstring), name, filename, null, false 
                 from dvla_organisation""")
 
-    op.execute("""INSERT INTO service_letter_branding (service_id, letter_branding_id)
-               SELECT S.id, LB.id
-                 FROM services s
-                 JOIN dvla_organisation d on (s.dvla_organisation_id = d.id)
-                 JOIN letter_branding lb on (lb.filename = d.filename)
-                 """)
+    op.execute("""UPDATE letter_branding set platform_default = True 
+                   WHERE filename='hm-government'
+               """)
 
 
 def downgrade():

tests/app/dao/test_letter_branding_dao.py

@@ -0,0 +1,23 @@
+from app.dao.letter_branding_dao import get_letter_branding_or_platform_default
+from tests.app.db import create_letter_branding
+
+
+def test_get_letter_branding_or_platform_default_returns_platform_default_if_domain_is_none(notify_db_session):
+    create_letter_branding()
+    result = get_letter_branding_or_platform_default(domain=None)
+    assert result.filename == 'hm-government'
+
+
+def test_get_letter_branding_or_platform_default_if_domain_is_not_associated_with_a_brand(notify_db_session):
+    create_letter_branding()
+    result = get_letter_branding_or_platform_default(domain="foo.bar")
+    assert result.filename == 'hm-government'
+
+
+def test_get_letter_branding_or_platform_default_returns_correct_brand_for_domain(notify_db_session):
+    create_letter_branding()
+    test_domain_branding = create_letter_branding(
+        name='test domain', filename='test-domain', domain='test.domain', platform_default=False
+    )
+    result = get_letter_branding_or_platform_default(domain='test.domain')
+    result == test_domain_branding

tests/app/dao/test_services_dao.py

@@ -65,7 +65,8 @@ from tests.app.db import (
     create_template,
     create_notification,
     create_api_key,
-    create_invited_user
+    create_invited_user,
+    create_letter_branding,
 )
 
 
@@ -76,6 +77,7 @@ def test_should_have_decorated_services_dao_functions():
 
 def test_create_service(notify_db_session):
     user = create_user()
+    create_letter_branding()
     assert Service.query.count() == 0
     service = Service(name="service_name",
                       email_from="email_from",
@@ -97,6 +99,27 @@ def test_create_service(notify_db_session):
     assert user in service_db.users
     assert service_db.organisation_type == 'central'
     assert service_db.crown is True
+    assert not service.letter_branding
+
+
+def test_create_service_with_letter_branding(notify_db_session):
+    user = create_user()
+    create_letter_branding()
+    letter_branding = create_letter_branding(
+        name='test domain', filename='test-domain', domain='test.domain', platform_default=False
+    )
+    assert Service.query.count() == 0
+    service = Service(name="service_name",
+                      email_from="email_from",
+                      message_limit=1000,
+                      restricted=False,
+                      organisation_type='central',
+                      created_by=user)
+    dao_create_service(service, user, letter_branding=letter_branding)
+    assert Service.query.count() == 1
+    service_db = Service.query.one()
+    assert service_db.id == service.id
+    assert service.letter_branding == letter_branding
 
 
 def test_cannot_create_two_services_with_same_name(notify_db_session):
@@ -332,6 +355,7 @@ def test_create_service_by_id_adding_and_removing_letter_returns_service_without
 
 def test_create_service_creates_a_history_record_with_current_data(notify_db_session):
     user = create_user()
+    create_letter_branding()
     assert Service.query.count() == 0
     assert Service.get_history_model().query.count() == 0
     service = Service(name="service_name",

tests/app/db.py

@@ -50,6 +50,7 @@ from app.models import (
     Complaint,
     InvitedUser,
     TemplateFolder,
+    LetterBranding
 )
 
 
@@ -724,3 +725,14 @@ def create_template_folder(service, name='foo', parent=None):
     db.session.add(tf)
     db.session.commit()
     return tf
+
+
+def create_letter_branding(name='HM Government', filename='hm-government', domain=None, platform_default=True):
+    test_domain_branding = LetterBranding(name=name,
+                                          filename=filename,
+                                          domain=domain,
+                                          platform_default=platform_default
+                                          )
+    db.session.add(test_domain_branding)
+    db.session.commit()
+    return test_domain_branding

tests/app/service/test_rest.py

@@ -43,7 +43,8 @@ from tests.app.db import (
     create_letter_contact,
     create_inbound_number,
     create_service_sms_sender,
-    create_service_with_defined_sms_sender
+    create_service_with_defined_sms_sender,
+    create_letter_branding
 )
 from tests.app.db import create_user
 
@@ -218,6 +219,7 @@ def test_get_service_by_id_should_404_if_no_service_for_user(notify_api, sample_
 
 
 def test_create_service(client, sample_user):
+    platform_default = create_letter_branding()
     data = {
         'name': 'created service',
         'user_id': str(sample_user.id),
@@ -259,6 +261,80 @@ def test_create_service(client, sample_user):
     service_sms_senders = ServiceSmsSender.query.filter_by(service_id=service_db.id).all()
     assert len(service_sms_senders) == 1
     assert service_sms_senders[0].sms_sender == current_app.config['FROM_NUMBER']
+    assert json_resp['data']['letter_branding'] == str(platform_default.id)
+
+
+def test_create_service_with_domain_sets_letter_branding(client, sample_user):
+    letter_branding = create_letter_branding(
+        name='test domain', filename='test-domain', domain='test.domain', platform_default=False
+    )
+    data = {
+        'name': 'created service',
+        'user_id': str(sample_user.id),
+        'message_limit': 1000,
+        'restricted': False,
+        'active': False,
+        'email_from': 'created.service',
+        'created_by': str(sample_user.id),
+        'service_domain': letter_branding.domain
+    }
+    auth_header = create_authorization_header()
+    headers = [('Content-Type', 'application/json'), auth_header]
+    resp = client.post(
+        '/service',
+        data=json.dumps(data),
+        headers=headers)
+    json_resp = resp.json
+    assert resp.status_code == 201
+    assert json_resp['data']['letter_branding'] == str(letter_branding.id)
+    assert json_resp['data']['letter_logo_filename'] == str(letter_branding.filename)
+
+
+def test_create_service_when_letter_branding_is_empty(client, sample_user):
+    # test create service before the data migration
+    data = {
+        'name': 'created service',
+        'user_id': str(sample_user.id),
+        'message_limit': 1000,
+        'restricted': False,
+        'active': False,
+        'email_from': 'created.service',
+        'created_by': str(sample_user.id),
+        'service_domain': 'test.domain'
+    }
+    auth_header = create_authorization_header()
+    headers = [('Content-Type', 'application/json'), auth_header]
+    resp = client.post(
+        '/service',
+        data=json.dumps(data),
+        headers=headers)
+    json_resp = resp.json
+    assert resp.status_code == 201
+    assert not json_resp['data']['letter_branding']
+    assert json_resp['data']['letter_logo_filename'] == 'hm-government'
+
+
+def test_get_service_by_id_returns_letter_branding_not_dvla_organisation(
+        client, sample_service
+):
+    letter_branding = create_letter_branding(
+        name='test domain', filename='test-domain', domain='test.domain', platform_default=False
+    )
+    data = {
+        'letter_branding': str(letter_branding.id)
+    }
+    client.post(
+        '/service/{}'.format(sample_service.id),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), create_authorization_header()]
+    )
+    resp = client.get('/service/{}'.format(sample_service.id),
+                      headers=[('Content-Type', 'application/json'), create_authorization_header()])
+    json_resp = resp.json
+    assert json_resp['data']['name'] == sample_service.name
+    assert json_resp['data']['id'] == str(sample_service.id)
+    assert json_resp['data']['dvla_organisation'] == '001'
+    assert json_resp['data']['letter_logo_filename'] == 'test-domain'
 
 
 def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_uuid):
@@ -438,6 +514,24 @@ def test_update_service(client, notify_db, sample_service):
     assert result['data']['organisation_type'] == 'foo'
 
 
+def test_update_service_letter_branding(client, notify_db, sample_service):
+    letter_branding = create_letter_branding(name='test brand', filename='test-brand', platform_default=False)
+    data = {
+        'letter_branding': str(letter_branding.id)
+    }
+
+    auth_header = create_authorization_header()
+
+    resp = client.post(
+        '/service/{}'.format(sample_service.id),
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), auth_header]
+    )
+    result = resp.json
+    assert resp.status_code == 200
+    assert result['data']['letter_branding'] == str(letter_branding.id)
+
+
 def test_update_service_remove_email_branding(admin_request, notify_db, sample_service):
     brand = EmailBranding(colour='#000000', logo='justice-league.png', name='Justice League')
     sample_service.email_branding = brand