no longer add an annual billing entry creating a service

this is now handled by a separate call from the admin app
This commit is contained in:
Leo Hemsted
2017-12-06 11:01:18 +00:00
parent 8cd422ebea
commit 4cdcc4e035
6 changed files with 33 additions and 80 deletions

View File

@@ -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)

View File

@@ -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

View File

@@ -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 = {

View File

@@ -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

View File

@@ -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

View File

@@ -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(