When a service is associated with a organisation set the free allowance to

the default free allowance for the organisation type.

The update/insert for the default free allowance is done in a separate
transaction. Updates to services need to happen in a transaction to
trigger the insert into the ServicesHistory table. For that reason the
call to set_default_free_allowance_for_service is done after the service
is updated.
I've added a try/except around the set_default_free_allowance_for_service call to ensure we still get the update to the service but get an exception log if the update to annual_billing fails. I believe it's important to preserve the update to the service in the unlikely event that the annual_billing upsert fails.

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:

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
 

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

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)