diff --git a/app/service/rest.py b/app/service/rest.py index 09ef7f27e..03d111be9 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -4,12 +4,13 @@ from datetime import datetime from flask import Blueprint, current_app, jsonify, request from notifications_utils.letter_timings import letter_can_be_cancelled from notifications_utils.timezones import convert_utc_to_bst -from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import IntegrityError, SQLAlchemyError from sqlalchemy.orm.exc import NoResultFound from app.aws import s3 from app.config import QueueNames from app.dao import fact_notification_status_dao, notifications_dao +from app.dao.annual_billing_dao import set_default_free_allowance_for_service from app.dao.api_key_dao import ( expire_api_key, get_model_api_keys, @@ -255,6 +256,17 @@ def create_service(): dao_create_service(valid_service, user) + # Need to do the annual billing update in a separate transaction because the both the + # dao_add_service_to_organisation and set_default_free_allowance_for_service are wrapped in a transaction. + # Catch and report an error if the annual billing doesn't happen - but don't rollback the service update. + try: + set_default_free_allowance_for_service(valid_service, year_start=None) + except SQLAlchemyError: + # No need to worry about key errors because service.organisation_type has a foreign key to organisation_types + current_app.logger.exception( + f"Exception caught when trying to insert annual billing creating a service {valid_service.id} " + f"for organisation_type {valid_service.organisation_type}") + return jsonify(data=service_schema.dump(valid_service).data), 201 diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index 02889e574..5b2def4e4 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -585,7 +585,7 @@ def test_post_link_service_to_organisation_missing_payload( def test_link_service_to_organisation_updates_service_if_annual_billing_update_fails( mocker, admin_request, sample_service, sample_organisation ): - mocker.patch('app.dao.annual_billing_dao.set_default_free_allowance_for_service', raises=SQLAlchemyError) + mocker.patch('app.organisation.rest.set_default_free_allowance_for_service', raises=SQLAlchemyError) data = { 'service_id': str(sample_service.id) } @@ -596,6 +596,7 @@ def test_link_service_to_organisation_updates_service_if_annual_billing_update_f _expected_status=204 ) assert sample_service.organisation_id == sample_organisation.id + assert len(AnnualBilling.query.all()) == 0 def test_rest_get_organisation_services( diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1878befb0..6e2599a45 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -6,6 +6,7 @@ from unittest.mock import ANY import pytest from flask import current_app, url_for from freezegun import freeze_time +from sqlalchemy.exc import SQLAlchemyError from app.dao.organisation_dao import dao_add_service_to_organisation from app.dao.service_sms_sender_dao import dao_get_sms_senders_by_service_id @@ -31,6 +32,7 @@ from app.models import ( SERVICE_PERMISSION_TYPES, SMS_TYPE, UPLOAD_LETTERS, + AnnualBilling, EmailBranding, InboundNumber, Notification, @@ -482,6 +484,46 @@ def test_create_service_with_domain_sets_organisation( assert json_resp['data']['organisation'] is None +def test_create_service_should_create_annual_billing_for_service( + admin_request, sample_user +): + data = { + 'name': 'created service', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'created.service', + 'created_by': str(sample_user.id) + } + assert len(AnnualBilling.query.all()) == 0 + admin_request.post('service.create_service', _data=data, _expected_status=201) + + annual_billing = AnnualBilling.query.all() + assert len(annual_billing) == 1 + + +def test_create_service_should_create_service_if_annual_billing_query_fails( + admin_request, sample_user, mocker +): + mocker.patch('app.service.rest.set_default_free_allowance_for_service', raises=SQLAlchemyError) + data = { + 'name': 'created service', + 'user_id': str(sample_user.id), + 'message_limit': 1000, + 'restricted': False, + 'active': False, + 'email_from': 'created.service', + 'created_by': str(sample_user.id) + } + assert len(AnnualBilling.query.all()) == 0 + admin_request.post('service.create_service', _data=data, _expected_status=201) + + annual_billing = AnnualBilling.query.all() + assert len(annual_billing) == 0 + assert len(Service.query.filter(Service.name == 'created service').all()) == 1 + + def test_create_service_inherits_branding_from_organisation( admin_request, sample_user,