Removing platform_default as a concept. No service actually wants to send letters with the default hm-government logo so we are going to remove it as a constraint.

However, until we can create a letter without a logo, we will still default to hm-government, because the dvla_organisation is set on the service.
This does simplify the code.
Also removed the inserts to letter_branding in the data migration file, because we can deploy this before the rest of the work is finished. But we will need to do it later.
This commit is contained in:
Rebecca Law
2019-01-25 15:03:01 +00:00
parent 7ee1d67df7
commit e030c2be88
11 changed files with 65 additions and 85 deletions

View File

@@ -203,7 +203,7 @@ def test_create_letters_gets_the_right_logo_when_service_has_dvla_logo(
def test_create_letters_gets_the_right_logo_when_service_has_letter_branding_logo(
notify_api, mocker, sample_letter_notification
):
letter_branding = create_letter_branding(name='test brand', filename='test-brand', platform_default=False)
letter_branding = create_letter_branding(name='test brand', filename='test-brand')
sample_letter_notification.service.letter_branding = letter_branding
mock_get_letters_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_letters_pdf', return_value=(b'\x00\x01', 1))
mocker.patch('app.letters.utils.s3upload')

View File

@@ -1,5 +1,5 @@
from app.dao.letter_branding_dao import (
dao_get_letter_branding_or_platform_default,
dao_get_letter_branding_by_domain,
dao_get_all_letter_branding,
dao_create_letter_branding,
dao_update_letter_branding
@@ -8,36 +8,29 @@ from app.models import LetterBranding
from tests.app.db import create_letter_branding
def test_dao_get_letter_branding_or_platform_default_returns_platform_default_if_domain_is_none(notify_db_session):
create_letter_branding()
result = dao_get_letter_branding_or_platform_default(domain=None)
assert result.filename == 'hm-government'
def test_dao_get_letter_branding_by_domain_returns_none_if_no_matching_domains(notify_db_session):
result = dao_get_letter_branding_by_domain(domain="test.domain")
assert not result
def test_dao_get_letter_branding_or_platform_default_if_domain_is_not_associated_with_a_brand(notify_db_session):
create_letter_branding()
result = dao_get_letter_branding_or_platform_default(domain="foo.bar")
assert result.filename == 'hm-government'
def test_dao_get_letter_branding_or_platform_default_returns_correct_brand_for_domain(notify_db_session):
create_letter_branding()
def test_dao_get_letter_branding_by_domain_returns_correct_brand_for_domain(notify_db_session):
create_letter_branding(domain='gov.uk')
test_domain_branding = create_letter_branding(
name='test domain', filename='test-domain', domain='test.domain', platform_default=False
name='test domain', filename='test-domain', domain='test.domain'
)
result = dao_get_letter_branding_or_platform_default(domain='test.domain')
result = dao_get_letter_branding_by_domain(domain='test.domain')
result == test_domain_branding
def test_dao_get_all_letter_branding(notify_db_session):
platform_default = create_letter_branding()
hm_gov = create_letter_branding()
test_domain = create_letter_branding(
name='test domain', filename='test-domain', domain='test.domain', platform_default=False
name='test domain', filename='test-domain', domain='test.domain'
)
results = dao_get_all_letter_branding()
assert platform_default in results
assert hm_gov in results
assert test_domain in results
assert len(results) == 2
@@ -61,7 +54,6 @@ def test_dao_create_letter_branding(notify_db_session):
assert new_letter_branding.name == data['name']
assert new_letter_branding.domain == data['domain']
assert new_letter_branding.filename == data['name']
assert not new_letter_branding.platform_default
def test_dao_update_letter_branding(notify_db_session):

View File

@@ -106,7 +106,7 @@ 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
name='test domain', filename='test-domain', domain='test.domain'
)
assert Service.query.count() == 0
service = Service(name="service_name",

View File

@@ -727,11 +727,10 @@ def create_template_folder(service, name='foo', parent=None):
return tf
def create_letter_branding(name='HM Government', filename='hm-government', domain=None, platform_default=True):
def create_letter_branding(name='HM Government', filename='hm-government', domain=None):
test_domain_branding = LetterBranding(name=name,
filename=filename,
domain=domain,
platform_default=platform_default
)
db.session.add(test_domain_branding)
db.session.commit()

View File

@@ -1,6 +1,5 @@
import json
import pytest
from app.models import LetterBranding
from tests import create_authorization_header
@@ -8,19 +7,21 @@ from tests.app.db import create_letter_branding
def test_get_all_letter_brands(client, notify_db_session):
platform_default = create_letter_branding()
hm_gov = create_letter_branding()
test_domain_branding = create_letter_branding(
name='test domain', filename='test-domain', domain='test.domain', platform_default=False
name='test domain', filename='test-domain', domain='test.domain'
)
response = client.get('/letter-branding', headers=[create_authorization_header()])
assert response.status_code == 200
json_response = json.loads(response.get_data(as_text=True))
assert len(json_response) == 2
for brand in json_response:
if brand['id'] == platform_default:
platform_default.serialize() == brand
if brand['id'] == str(hm_gov.id):
assert hm_gov.serialize() == brand
elif brand['id'] == str(test_domain_branding.id):
assert test_domain_branding.serialize() == brand
else:
test_domain_branding.serialize() == brand
assert False
def test_create_letter_branding(client, notify_db_session):
@@ -42,26 +43,6 @@ def test_create_letter_branding(client, notify_db_session):
assert letter_brand.name == form['name']
assert letter_brand.domain == form['domain']
assert letter_brand.filename == form['filename']
assert not letter_brand.platform_default
def test_create_letter_branding_returns_400_if_platform_default_is_passed_in_the_form(client, notify_db_session):
form = {
'name': 'super brand',
'domain': 'super.brand',
'filename': 'super-brand',
'platform_default': True
}
response = client.post(
'/letter-branding',
data=json.dumps(form),
headers=[('Content-Type', 'application/json'), create_authorization_header()],
)
assert response.status_code == 400
json_resp = json.loads(response.get_data(as_text=True))
assert json_resp['errors'][0]['message'] == \
"Additional properties are not allowed (platform_default was unexpected)"
def test_create_letter_branding_returns_400_if_name_already_exists(client, notify_db_session):
@@ -102,5 +83,4 @@ def test_update_letter_branding_returns_400_when_integrity_error_is_thrown(
assert response.status_code == 400
json_resp = json.loads(response.get_data(as_text=True))
# Why is this name and not domain? copied this pattern from email_branding
assert json_resp['message'] == {"name": ["Duplicate domain 'duplicate'"]}

View File

@@ -219,7 +219,6 @@ 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),
@@ -243,6 +242,7 @@ def test_create_service(client, sample_user):
assert not json_resp['data']['research_mode']
assert json_resp['data']['dvla_organisation'] == '001'
assert json_resp['data']['rate_limit'] == 3000
assert json_resp['data']['letter_branding'] is None
service_db = Service.query.get(json_resp['data']['id'])
assert service_db.name == 'created service'
@@ -261,12 +261,11 @@ 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
name='test domain', filename='test-domain', domain='test.domain'
)
data = {
'name': 'created service',
@@ -286,6 +285,7 @@ def test_create_service_with_domain_sets_letter_branding(client, sample_user):
headers=headers)
json_resp = resp.json
assert resp.status_code == 201
assert json_resp['data']['dvla_organisation'] == '001'
assert json_resp['data']['letter_branding'] == str(letter_branding.id)
assert json_resp['data']['letter_logo_filename'] == str(letter_branding.filename)
@@ -310,7 +310,7 @@ def test_create_service_when_letter_branding_is_empty(client, sample_user):
headers=headers)
json_resp = resp.json
assert resp.status_code == 201
assert not json_resp['data']['letter_branding']
assert json_resp['data']['letter_branding'] is None
assert json_resp['data']['letter_logo_filename'] == 'hm-government'
@@ -318,7 +318,7 @@ 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
name='test domain', filename='test-domain', domain='test.domain'
)
data = {
'letter_branding': str(letter_branding.id)
@@ -334,6 +334,7 @@ def test_get_service_by_id_returns_letter_branding_not_dvla_organisation(
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_branding'] == str(letter_branding.id)
assert json_resp['data']['letter_logo_filename'] == 'test-domain'
@@ -515,7 +516,7 @@ def test_update_service(client, notify_db, sample_service):
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)
letter_branding = create_letter_branding(name='test brand', filename='test-brand')
data = {
'letter_branding': str(letter_branding.id)
}
@@ -532,6 +533,35 @@ def test_update_service_letter_branding(client, notify_db, sample_service):
assert result['data']['letter_branding'] == str(letter_branding.id)
def test_update_service_remove_letter_branding(client, notify_db, sample_service):
letter_branding = create_letter_branding(name='test brand', filename='test-brand')
sample_service
data = {
'letter_branding': str(letter_branding.id)
}
auth_header = create_authorization_header()
client.post(
'/service/{}'.format(sample_service.id),
data=json.dumps(data),
headers=[('Content-Type', 'application/json'), auth_header]
)
data = {
'letter_branding': None
}
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'] is None
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