diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py index 36d12e540..5caa46011 100644 --- a/app/dao/annual_billing_dao.py +++ b/app/dao/annual_billing_dao.py @@ -53,7 +53,7 @@ def dao_get_all_free_sms_fragment_limit(service_id): ).order_by(AnnualBilling.financial_year_start).all() -def set_default_free_allowance_for_service(service, year_start=None): +def set_default_free_allowance_for_service(service, year_start=None, commit=True): default_free_sms_fragment_limits = { 'central': { 2020: 250_000, @@ -90,6 +90,7 @@ def set_default_free_allowance_for_service(service, year_start=None): } if not year_start: year_start = get_current_financial_year_start_year() + # handle cases where the year is less than 2020 or greater than 2021 if year_start < 2020: year_start = 2020 if year_start > 2021: diff --git a/app/organisation/rest.py b/app/organisation/rest.py index 77761fcf6..d745a7adb 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -1,8 +1,9 @@ from flask import Blueprint, abort, current_app, jsonify, request -from sqlalchemy.exc import IntegrityError +from sqlalchemy.exc import IntegrityError, SQLAlchemyError from app.config import QueueNames +from app.dao.annual_billing_dao import set_default_free_allowance_for_service from app.dao.fact_billing_dao import fetch_usage_year_for_organisation from app.dao.organisation_dao import ( dao_add_service_to_organisation, @@ -119,6 +120,16 @@ def link_service_to_organisation(organisation_id): service.organisation = None dao_add_service_to_organisation(service, organisation_id) + # 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(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 update annual billing when the organisation " + f"changed for service: {service.id} to organisation: {organisation_id}") return '', 204 diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py index 37e8e4fb2..4fa0954c6 100644 --- a/tests/app/dao/test_annual_billing_dao.py +++ b/tests/app/dao/test_annual_billing_dao.py @@ -91,3 +91,21 @@ def test_set_default_free_allowance_for_service_using_correct_year(sample_servic 25000, 2020 ) + + +@freeze_time('2021-04-01 14:02:00') +def test_set_default_free_allowance_for_service_updates_existing_year(sample_service): + set_default_free_allowance_for_service(service=sample_service, year_start=None) + annual_billing = AnnualBilling.query.all() + assert not sample_service.organisation_type + assert len(annual_billing) == 1 + assert annual_billing[0].service_id == sample_service.id + assert annual_billing[0].free_sms_fragment_limit == 10000 + + sample_service.organisation_type = 'central' + + set_default_free_allowance_for_service(service=sample_service, year_start=None) + annual_billing = AnnualBilling.query.all() + assert len(annual_billing) == 1 + assert annual_billing[0].service_id == sample_service.id + assert annual_billing[0].free_sms_fragment_limit == 150000 diff --git a/tests/app/organisation/test_rest.py b/tests/app/organisation/test_rest.py index a3734083a..02889e574 100644 --- a/tests/app/organisation/test_rest.py +++ b/tests/app/organisation/test_rest.py @@ -3,13 +3,14 @@ from datetime import datetime import pytest from freezegun import freeze_time +from sqlalchemy.exc import SQLAlchemyError from app.dao.organisation_dao import ( dao_add_service_to_organisation, dao_add_user_to_organisation, ) from app.dao.services_dao import dao_archive_service -from app.models import Organisation +from app.models import AnnualBilling, Organisation from tests.app.db import ( create_annual_billing, create_domain, @@ -491,19 +492,25 @@ def test_post_update_organisation_set_mou_emails_signed_by( } -def test_post_link_service_to_organisation(admin_request, sample_service, sample_organisation): +def test_post_link_service_to_organisation(admin_request, sample_service): data = { 'service_id': str(sample_service.id) } - + organisation = create_organisation(organisation_type='central') + assert len(organisation.services) == 0 + assert len(AnnualBilling.query.all()) == 0 admin_request.post( 'organisation.link_service_to_organisation', _data=data, - organisation_id=sample_organisation.id, + organisation_id=organisation.id, _expected_status=204 ) - assert len(sample_organisation.services) == 1 + assert len(organisation.services) == 1 + assert sample_service.organisation_type == 'central' + annual_billing = AnnualBilling.query.all() + assert len(annual_billing) == 1 + assert annual_billing[0].free_sms_fragment_limit == 150000 def test_post_link_service_to_another_org( @@ -511,7 +518,8 @@ def test_post_link_service_to_another_org( data = { 'service_id': str(sample_service.id) } - + assert len(sample_organisation.services) == 0 + assert not sample_service.organisation_type admin_request.post( 'organisation.link_service_to_organisation', _data=data, @@ -520,8 +528,9 @@ def test_post_link_service_to_another_org( ) assert len(sample_organisation.services) == 1 + assert not sample_service.organisation_type - new_org = create_organisation() + new_org = create_organisation(organisation_type='central') admin_request.post( 'organisation.link_service_to_organisation', _data=data, @@ -530,6 +539,10 @@ def test_post_link_service_to_another_org( ) assert not sample_organisation.services assert len(new_org.services) == 1 + assert sample_service.organisation_type == 'central' + annual_billing = AnnualBilling.query.all() + assert len(annual_billing) == 1 + assert annual_billing[0].free_sms_fragment_limit == 150000 def test_post_link_service_to_organisation_nonexistent_organisation( @@ -569,6 +582,22 @@ 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) + data = { + 'service_id': str(sample_service.id) + } + admin_request.post( + 'organisation.link_service_to_organisation', + organisation_id=str(sample_organisation.id), + _data=data, + _expected_status=204 + ) + assert sample_service.organisation_id == sample_organisation.id + + def test_rest_get_organisation_services( admin_request, sample_organisation, sample_service): dao_add_service_to_organisation(sample_service, sample_organisation.id)