From 98249158cb687b7c7e732a055971894545ffe483 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Apr 2019 11:15:42 +0100 Subject: [PATCH] Stop trying to infer branding when adding services The API handles this now. --- app/main/views/add_service.py | 10 +---- tests/app/main/views/test_add_service.py | 55 ++---------------------- tests/conftest.py | 2 - 3 files changed, 6 insertions(+), 61 deletions(-) diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py index 8e6d942f1..8fce160ae 100644 --- a/app/main/views/add_service.py +++ b/app/main/views/add_service.py @@ -2,17 +2,15 @@ from flask import current_app, redirect, render_template, session, url_for from flask_login import login_required from notifications_python_client.errors import HTTPError -from app import billing_api_client, email_branding_client, service_api_client +from app import billing_api_client, service_api_client from app.main import main from app.main.forms import CreateServiceForm -from app.utils import AgreementInfo, email_safe, user_is_gov_user +from app.utils import email_safe, user_is_gov_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) - domain = 'nhs.uk' if organisation_type == 'nhs' else AgreementInfo.from_current_user().canonical_domain - email_branding = email_branding_client.get_email_branding_id_for_domain(domain) try: service_id = service_api_client.create_service( service_name=service_name, @@ -21,13 +19,9 @@ def _create_service(service_name, organisation_type, email_from, form): restricted=True, user_id=session['user_id'], email_from=email_from, - service_domain=domain ) session['service_id'] = service_id - if email_branding: - service_api_client.update_service(service_id, email_branding=email_branding) - billing_api_client.create_or_update_free_sms_fragment_limit(service_id, free_sms_fragment_limit) return service_id, None diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 5395366e7..18d00b199 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -56,7 +56,6 @@ def test_should_add_service_and_redirect_to_tour_when_no_services( restricted=True, user_id=api_user_active.id, email_from='testing.the.post', - service_domain=None ) mock_create_service_template.assert_called_once_with( 'Example text message template', @@ -71,10 +70,10 @@ def test_should_add_service_and_redirect_to_tour_when_no_services( mock_create_or_update_free_sms_fragment_limit.assert_called_once_with(101, 25000) -@pytest.mark.parametrize('organisation_type, free_allowance, service_domain', [ - ('central', 250 * 1000, None), - ('local', 25 * 1000, None), - ('nhs', 25 * 1000, 'nhs.uk'), +@pytest.mark.parametrize('organisation_type, free_allowance', [ + ('central', 250 * 1000), + ('local', 25 * 1000), + ('nhs', 25 * 1000), ]) def test_should_add_service_and_redirect_to_dashboard_when_existing_service( app_, @@ -82,11 +81,9 @@ 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, - service_domain, mock_create_or_update_free_sms_fragment_limit, mock_get_all_email_branding, ): @@ -111,56 +108,12 @@ def test_should_add_service_and_redirect_to_dashboard_when_existing_service( restricted=True, user_id=api_user_active.id, email_from='testing.the.post', - service_domain=service_domain ) mock_create_or_update_free_sms_fragment_limit.assert_called_once_with(101, free_allowance) assert len(mock_create_service_template.call_args_list) == 0 assert session['service_id'] == 101 -@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_, - client_request, - active_user_with_permissions, - mock_create_service, - mock_get_services, - mock_update_service, - mock_create_or_update_free_sms_fragment_limit, - mock_get_all_email_branding, - organisation_type, - email_address, - expected_branding, -): - active_user_with_permissions.email_address = email_address - client_request.login(active_user_with_permissions) - client_request.post( - 'main.add_service', - _data={ - 'name': 'testing the post', - 'organisation_type': organisation_type, - } - ) - mock_get_all_email_branding.assert_called_once_with() - assert mock_create_service.called is True - if expected_branding: - mock_update_service.assert_called_once_with( - 101, - email_branding=expected_branding, - ) - else: - assert mock_update_service.called is False - - def test_should_return_form_errors_when_service_name_is_empty( client_request ): diff --git a/tests/conftest.py b/tests/conftest.py index 0ba53ab17..f29ffb2d2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -625,7 +625,6 @@ def mock_create_service(mocker): restricted, user_id, email_from, - service_domain, ): service = service_json( 101, service_name, [user_id], message_limit=message_limit, restricted=restricted, email_from=email_from) @@ -644,7 +643,6 @@ def mock_create_duplicate_service(mocker): restricted, user_id, email_from, - service_domain, ): json_mock = Mock(return_value={'message': {'name': ["Duplicate service name '{}'".format(service_name)]}}) resp_mock = Mock(status_code=400, json=json_mock)