From 4cdcc4e0355d760a4d787b2eea3a0273e2fb14e6 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 6 Dec 2017 11:01:18 +0000 Subject: [PATCH] no longer add an annual billing entry creating a service this is now handled by a separate call from the admin app --- app/dao/services_dao.py | 4 +-- app/service/rest.py | 14 ++++----- tests/app/conftest.py | 6 ++-- tests/app/dao/test_services_dao.py | 35 ++++++++++------------ tests/app/db.py | 7 +---- tests/app/service/test_rest.py | 47 ++++++------------------------ 6 files changed, 33 insertions(+), 80 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index ebf0153c3..6985a26e8 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -41,7 +41,6 @@ from app.models import ( ) from app.statsd_decorators import statsd from app.utils import get_london_month_from_utc_column, get_london_midnight_in_utc -from app.dao.annual_billing_dao import dao_insert_annual_billing_for_this_year DEFAULT_SERVICE_PERMISSIONS = [ SMS_TYPE, @@ -155,7 +154,7 @@ def dao_fetch_service_by_id_and_user(service_id, user_id): @transactional @version_class(Service) -def dao_create_service(service, user, free_sms_fragment_limit, service_id=None, service_permissions=None): +def dao_create_service(service, user, service_id=None, service_permissions=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. @@ -177,7 +176,6 @@ def dao_create_service(service, user, free_sms_fragment_limit, service_id=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']) - dao_insert_annual_billing_for_this_year(service, free_sms_fragment_limit) db.session.add(service) diff --git a/app/service/rest.py b/app/service/rest.py index a5cd8742a..d71cd2b90 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -153,24 +153,20 @@ def get_service_by_id(service_id): @service_blueprint.route('', methods=['POST']) def create_service(): data = request.get_json() - errors = { - required_field: ['Missing data for required field.'] - for required_field in ['user_id', 'free_sms_fragment_limit'] - if not data.get(required_field) - } - if errors: + + if not data.get('user_id'): + errors = {'user_id': ['Missing data for required field.']} raise InvalidRequest(errors, status_code=400) # validate json with marshmallow service_schema.load(data) - user = get_user_by_id(data.pop('user_id', None)) - free_sms_fragment_limit = data.pop('free_sms_fragment_limit') + user = get_user_by_id(data.pop('user_id')) # unpack valid json into service object valid_service = Service.from_json(data) - dao_create_service(valid_service, user, free_sms_fragment_limit) + dao_create_service(valid_service, user) return jsonify(data=service_schema.dump(valid_service).data), 201 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 6b678c16f..71b510767 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -150,7 +150,6 @@ def sample_service( email_from=None, permissions=None, research_mode=None, - free_sms_fragment_limit=250000 ): if user is None: user = create_user() @@ -167,7 +166,7 @@ def sample_service( service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) - dao_create_service(service, user, free_sms_fragment_limit, service_permissions=permissions) + dao_create_service(service, user, service_permissions=permissions) if research_mode: service.research_mode = research_mode @@ -1004,8 +1003,7 @@ def notify_service(notify_db, notify_db_session): dao_create_service( service=service, service_id=current_app.config['NOTIFY_SERVICE_ID'], - user=user, - free_sms_fragment_limit=250000 # live central gov service + user=user ) data = { diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index abeb06c32..32f16f27a 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -51,7 +51,6 @@ from app.models import ( Service, ServicePermission, ServicePermissionTypes, - AnnualBilling, BRANDING_GOVUK, DVLA_ORG_HM_GOVERNMENT, KEY_TYPE_NORMAL, @@ -90,7 +89,7 @@ def test_create_service(sample_user): restricted=False, organisation_type='central', created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) assert Service.query.count() == 1 service_db = Service.query.one() @@ -103,12 +102,8 @@ def test_create_service(sample_user): assert service_db.prefix_sms is True assert service.active is True assert sample_user in service_db.users - assert service_db.free_sms_fragment_limit == 250000 assert service_db.organisation_type == 'central' assert service_db.crown is True - annual_billing = AnnualBilling.query.one() - assert annual_billing.service == service_db - assert annual_billing.free_sms_fragment_limit == 12345 def test_cannot_create_two_services_with_same_name(sample_user): @@ -125,8 +120,8 @@ def test_cannot_create_two_services_with_same_name(sample_user): restricted=False, created_by=sample_user) with pytest.raises(IntegrityError) as excinfo: - dao_create_service(service1, sample_user, 12345) - dao_create_service(service2, sample_user, 12345) + dao_create_service(service1, sample_user) + dao_create_service(service2, sample_user) assert 'duplicate key value violates unique constraint "services_name_key"' in str(excinfo.value) @@ -143,8 +138,8 @@ def test_cannot_create_two_services_with_same_email_from(sample_user): restricted=False, created_by=sample_user) with pytest.raises(IntegrityError) as excinfo: - dao_create_service(service1, sample_user, 12345) - dao_create_service(service2, sample_user, 12345) + dao_create_service(service1, sample_user) + dao_create_service(service2, sample_user) assert 'duplicate key value violates unique constraint "services_email_from_key"' in str(excinfo.value) @@ -156,7 +151,7 @@ def test_cannot_create_service_with_no_user(notify_db_session, sample_user): restricted=False, created_by=sample_user) with pytest.raises(FlushError) as excinfo: - dao_create_service(service, None, 12345) + dao_create_service(service, None) assert "Can't flush None value found in collection Service.users" in str(excinfo.value) @@ -166,7 +161,7 @@ def test_should_add_user_to_service(sample_user): message_limit=1000, restricted=False, created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) assert sample_user in Service.query.first().users new_user = User( name='Test User', @@ -185,7 +180,7 @@ def test_should_remove_user_from_service(sample_user): message_limit=1000, restricted=False, created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) new_user = User( name='Test User', email_address='new_user@digital.cabinet-office.gov.uk', @@ -337,7 +332,7 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user): message_limit=1000, restricted=False, created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) assert Service.query.count() == 1 assert Service.get_history_model().query.count() == 1 @@ -364,7 +359,7 @@ def test_update_service_creates_a_history_record_with_current_data(sample_user): message_limit=1000, restricted=False, created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) assert Service.query.count() == 1 assert Service.query.first().version == 1 @@ -392,7 +387,7 @@ def test_update_service_permission_creates_a_history_record_with_current_data(sa message_limit=1000, restricted=False, created_by=sample_user) - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) service.permissions.append(ServicePermission(service_id=service.id, permission='letter')) dao_update_service(service) @@ -435,7 +430,7 @@ def test_create_service_and_history_is_transactional(sample_user): created_by=sample_user) with pytest.raises(IntegrityError) as excinfo: - dao_create_service(service, sample_user, 12345) + dao_create_service(service, sample_user) assert 'column "name" violates not-null constraint' in str(excinfo.value) assert Service.query.count() == 0 @@ -484,7 +479,7 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sam restricted=False, created_by=sample_user) - dao_create_service(service_one, sample_user, 12345) + dao_create_service(service_one, sample_user) assert sample_user.id == service_one.users[0].id test_user_permissions = Permission.query.filter_by(service=service_one, user=sample_user).all() assert len(test_user_permissions) == 8 @@ -501,7 +496,7 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sam message_limit=1000, restricted=False, created_by=other_user) - dao_create_service(service_two, other_user, 12345) + dao_create_service(service_two, other_user) assert other_user.id == service_two.users[0].id other_user_permissions = Permission.query.filter_by(service=service_two, user=other_user).all() @@ -530,7 +525,7 @@ def test_fetch_stats_filters_on_service(sample_notification): email_from="hello", restricted=False, message_limit=1000) - dao_create_service(service_two, sample_notification.service.created_by, 12345) + dao_create_service(service_two, sample_notification.service.created_by) stats = dao_fetch_stats_for_service(service_two.id) assert len(stats) == 0 diff --git a/tests/app/db.py b/tests/app/db.py index 16471d555..717b1aa2b 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -79,12 +79,7 @@ def create_service( prefix_sms=prefix_sms, ) - dao_create_service( - service, - service.created_by, - free_sms_fragment_limit=250000, - service_id=service_id, - service_permissions=service_permissions) + dao_create_service(service, service.created_by, service_id, service_permissions=service_permissions) service.active = active service.research_mode = research_mode diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 765e64e2f..707bffc40 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -132,8 +132,6 @@ def test_get_service_by_id(admin_request, sample_service): assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] assert json_resp['data']['prefix_sms'] is True - # deprecated field, no longer exists - assert 'free_sms_fragment_limit' not in json_resp['data'] @pytest.mark.parametrize('detailed', [True, False]) @@ -219,8 +217,7 @@ def test_create_service(client, sample_user): 'restricted': False, 'active': False, 'email_from': 'created.service', - 'created_by': str(sample_user.id), - 'free_sms_fragment_limit': 250000 + 'created_by': str(sample_user.id) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -265,8 +262,7 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u 'message_limit': 1000, 'restricted': False, 'active': False, - 'created_by': str(fake_uuid), - 'free_sms_fragment_limit': 250000 + 'created_by': str(fake_uuid) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -280,26 +276,6 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api, fake_u assert 'Missing data for required field.' in json_resp['message']['user_id'] -def test_create_service_free_sms_fragment_limit_is_not_optional(admin_request, sample_user): - data1 = { - 'name': 'service 1', - 'user_id': str(sample_user.id), - 'message_limit': 1000, - 'restricted': False, - 'active': False, - 'email_from': 'sample_user.email1', - 'created_by': str(sample_user.id) - } - - json_resp = admin_request.post( - 'service.create_service', - _data=data1, - _expected_status=400 - ) - assert json_resp['result'] == 'error' - assert 'Missing data for required field.' in json_resp['message']['free_sms_fragment_limit'] - - def test_should_error_if_created_by_missing(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: @@ -309,8 +285,7 @@ def test_should_error_if_created_by_missing(notify_api, sample_user): 'message_limit': 1000, 'restricted': False, 'active': False, - 'user_id': str(sample_user.id), - 'free_sms_fragment_limit': 250000 + 'user_id': str(sample_user.id) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -337,8 +312,7 @@ def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(no 'message_limit': 1000, 'restricted': False, 'active': False, - 'created_by': str(fake_uuid), - 'free_sms_fragment_limit': 250000 + 'created_by': str(fake_uuid) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -356,8 +330,7 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: data = { - 'user_id': str(sample_user.id), - 'free_sms_fragment_limit': 250000 + 'user_id': str(sample_user.id) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -385,8 +358,7 @@ def test_should_not_create_service_with_duplicate_name(notify_api, 'restricted': False, 'active': False, 'email_from': 'sample.service2', - 'created_by': str(sample_user.id), - 'free_sms_fragment_limit': 250000 + 'created_by': str(sample_user.id) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -413,8 +385,7 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email 'restricted': False, 'active': False, 'email_from': 'first.service', - 'created_by': str(sample_user.id), - 'free_sms_fragment_limit': 250000 + 'created_by': str(sample_user.id) } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] @@ -844,8 +815,8 @@ def test_default_permissions_are_added_for_user_service(notify_api, 'restricted': False, 'active': False, 'email_from': 'created.service', - 'created_by': str(sample_user.id), - 'free_sms_fragment_limit': 250000} + 'created_by': str(sample_user.id) + } auth_header = create_authorization_header() headers = [('Content-Type', 'application/json'), auth_header] resp = client.post(