From 01fdcb89986325c5723d4a263d434d3e5fd76b6e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 3 Sep 2018 16:07:56 +0100 Subject: [PATCH] Use NHS.UK branding is for all NHS services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Anyone choosing ‘NHS’ for their organisation type gets should get the NHS branding. We don’t want to hard-code an ID for NHS branding anywhere because it won’t be consistent between environments. So instead we can say that anyone who chooses ‘NHS’ as their organisation type should get whatever branding has `nhs.uk` as its domain. This allows us to easily manage the branding the same way we do other brands, but gives us the efficiency of having it auto applied. --- app/domains.yml | 3 ++- app/main/views/add_service.py | 1 + tests/app/main/views/test_add_service.py | 17 ++++++++++++----- tests/conftest.py | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/domains.yml b/app/domains.yml index ebaf0885c..ed72b8aea 100644 --- a/app/domains.yml +++ b/app/domains.yml @@ -65,7 +65,8 @@ innovateuk.gov.uk: owner: Innovate UK agreement_signed: true ncsc.gov.uk: gchq.gov.uk -nhs.net: +nhs.net: nhs.uk +nhs.uk: owner: NHS nhsdigital.nhs.uk: digital.nhs.uk digital.nhs.uk: diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 72ee367f4..e311a70e6 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -29,6 +29,7 @@ def _add_invited_user_to_service(invited_user): def _create_service(service_name, organisation_type, email_from, form): free_sms_fragment_limit = current_app.config['DEFAULT_FREE_SMS_FRAGMENT_LIMITS'].get(organisation_type) email_branding = email_branding_client.get_email_branding_id_for_domain( + 'nhs.uk' if organisation_type == 'nhs' else AgreementInfo.from_current_user().canonical_domain ) try: diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index ba0c606a3..56002884c 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -82,6 +82,7 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service( mock_create_service, mock_create_service_template, mock_get_services, + mock_update_service, api_user_active, organisation_type, free_allowance, @@ -111,10 +112,15 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service( assert response.location == url_for('main.service_dashboard', service_id=101, _external=True) -@pytest.mark.parametrize('email_address, expected_branding', [ - ('test@example.voa.gsi.gov.uk', '5'), - ('test@example.voa.gov.uk', '5'), - ('test@example.gov.uk', None), +@pytest.mark.parametrize('organisation_type, email_address, expected_branding', [ + ('central', 'test@example.voa.gsi.gov.uk', '5'), + ('central', 'test@example.voa.gov.uk', '5'), + ('central', 'test@example.gov.uk', None), + # Anyone choosing ‘NHS’ for organisation type gets NHS branding no + # matter what their email domain is (but we look it up based on the + # `nhs.uk` domain to avoid hard-coding a branding ID anywhere) + ('nhs', 'test@example.voa.gov.uk', '4'), + ('nhs', 'test@nhs.uk', '4'), ]) def test_should_lookup_branding_for_known_domain( app_, @@ -125,6 +131,7 @@ def test_should_lookup_branding_for_known_domain( mock_update_service, mock_create_or_update_free_sms_fragment_limit, mock_get_all_email_branding, + organisation_type, email_address, expected_branding, ): @@ -134,7 +141,7 @@ def test_should_lookup_branding_for_known_domain( 'main.add_service', _data={ 'name': 'testing the post', - 'organisation_type': 'central', + 'organisation_type': organisation_type, } ) mock_get_all_email_branding.assert_called_once_with() diff --git a/tests/conftest.py b/tests/conftest.py index 5781d9ebc..366ef556c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2463,7 +2463,7 @@ def mock_get_all_email_branding(mocker): non_standard_values = [ {'idx': 1, 'colour': 'red'}, {'idx': 2, 'colour': 'orange'}, - {'idx': 3, 'text': None}, + {'idx': 3, 'text': None, 'domain': 'nhs.uk'}, {'idx': 4, 'colour': 'blue', 'domain': 'voa.gov.uk'}, ] shuffle = sort_key is None