make sure POST /free-sms-fragment-limit always creates

refactored billing/rest.py and annual_billing_dao.py to remove logic
from the dao, and simplify the process around creating new rows. Make
sure that the POST always creates (it previously wouldn't create rows
for years that don't already exist). Clean up some tests that were
doing too much set-up/data verification via rest calls rather than
directly inserting test data in to the DB.

app/billing/rest.py

@@ -117,6 +117,7 @@ def get_free_sms_fragment_limit(service_id):
                 financial_year_start = get_current_financial_year_start_year()
 
             if int(financial_year_start) < get_current_financial_year_start_year():
+                # return the earliest historical entry
                 annual_billing = sms_list[0]   # The oldest entry
             else:
                 annual_billing = sms_list[-1]  # The newest entry
@@ -141,10 +142,21 @@ def create_or_update_free_sms_fragment_limit(service_id):
     return jsonify(form), 201
 
 
-def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, financial_year_start=None):
+def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, financial_year_start):
     current_year = get_current_financial_year_start_year()
-    if financial_year_start is None or financial_year_start >= current_year:
+    if not financial_year_start:
-        dao_update_annual_billing_for_current_and_future_years(service_id, free_sms_fragment_limit)
+        financial_year_start = current_year
-    else:
+
-        dao_create_or_update_annual_billing_for_year(service_id,
+    dao_create_or_update_annual_billing_for_year(
-                                                     free_sms_fragment_limit, financial_year_start)
+        service_id,
+        free_sms_fragment_limit,
+        financial_year_start
+    )
+    # if we're trying to update historical data, don't touch other rows.
+    # Otherwise, make sure that future years will get the new updated value.
+    if financial_year_start >= current_year:
+        dao_update_annual_billing_for_current_and_future_years(
+            service_id,
+            free_sms_fragment_limit,
+            financial_year_start
+        )

app/dao/annual_billing_dao.py

@@ -7,11 +7,7 @@ from app.dao.date_util import get_current_financial_year_start_year
 
 
 @transactional
-def dao_create_or_update_annual_billing_for_year(service_id, free_sms_fragment_limit, financial_year_start=None):
+def dao_create_or_update_annual_billing_for_year(service_id, free_sms_fragment_limit, financial_year_start):
-
-    if not financial_year_start:
-        financial_year_start = get_current_financial_year_start_year()
-
     result = dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start)
 
     if result:
@@ -30,11 +26,7 @@ def dao_get_annual_billing(service_id):
 
 
 @transactional
-def dao_update_annual_billing_for_current_and_future_years(service_id, free_sms_fragment_limit,
+def dao_update_annual_billing_for_current_and_future_years(service_id, free_sms_fragment_limit, financial_year_start):
-                                                           financial_year_start=None):
-    if not financial_year_start:
-        financial_year_start = get_current_financial_year_start_year()
-
     AnnualBilling.query.filter(
         AnnualBilling.service_id == service_id,
         AnnualBilling.financial_year_start >= financial_year_start

tests/app/billing/test_billing.py

@@ -1,25 +1,25 @@
 from datetime import datetime, timedelta
 import json
 
+import pytest
+
 from app.billing.rest import _transform_billing_for_month
 from app.dao.monthly_billing_dao import (
     create_or_update_monthly_billing,
     get_monthly_billing_by_notification_type,
 )
 from app.models import SMS_TYPE, EMAIL_TYPE
+from app.dao.date_util import get_current_financial_year_start_year
+from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year
 from tests.app.db import (
     create_notification,
     create_rate,
-    create_monthly_billing_entry
+    create_monthly_billing_entry,
+    create_annual_billing,
 )
-
-from tests import create_authorization_header
-
-from app.dao.date_util import get_current_financial_year_start_year
-from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year
-from tests.app.db import create_annual_billing
 from app.billing.rest import update_free_sms_fragment_limit_data
 
+from tests import create_authorization_header
 
 APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00)
 APR_2016_MONTH_END = datetime(2016, 4, 30, 22, 59, 59, 99999)
@@ -270,70 +270,62 @@ def test_create_update_free_sms_fragment_limit_invalid_schema(client, sample_ser
     assert 'JSON' in json_resp['message']
 
 
-def test_create_free_sms_fragment_limit_current_year(client, sample_service):
+def test_create_free_sms_fragment_limit_current_year_updates_future_years(admin_request, sample_service):
     current_year = get_current_financial_year_start_year()
-    data = {'free_sms_fragment_limit': 9999}
+    future_billing = create_annual_billing(sample_service.id, 1, current_year + 1)
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
-                           data=json.dumps(data),
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
 
-    response_get = client.get(
+    admin_request.post(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'.format(sample_service.id, current_year),
+        'billing.create_or_update_free_sms_fragment_limit',
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
+        service_id=sample_service.id,
+        _data={'free_sms_fragment_limit': 9999},
+        _expected_status=201
+    )
 
-    json_resp = json.loads(response_get.get_data(as_text=True))
+    current_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
-    assert response.status_code == 201
+    assert future_billing.free_sms_fragment_limit == 9999
-    assert response_get.status_code == 200
+    assert current_billing.financial_year_start == current_year
-    assert json_resp['financial_year_start'] == current_year
+    assert current_billing.free_sms_fragment_limit == 9999
-    assert json_resp['free_sms_fragment_limit'] == 9999
 
 
-def test_create_free_sms_fragment_limit_past_year(client, sample_service):
+@pytest.mark.parametrize('update_existing', [True, False])
-
+def test_create_or_update_free_sms_fragment_limit_past_year_doenst_update_other_years(
-    data = {'financial_year_start': 2016, 'free_sms_fragment_limit': 9999}
+    admin_request,
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
+    sample_service,
-                           data=json.dumps(data),
+    update_existing
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
+):
-
-    response_get = client.get(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start=2016'.format(sample_service.id),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    json_resp = json.loads(response_get.get_data(as_text=True))
-    assert response.status_code == 201
-    assert response_get.status_code == 200
-    assert json_resp['financial_year_start'] == 2016
-    assert json_resp['free_sms_fragment_limit'] == 9999
-
-
-def test_update_free_sms_fragment_limit(client, sample_service):
     current_year = get_current_financial_year_start_year()
+    create_annual_billing(sample_service.id, 1, current_year)
+    if update_existing:
+        create_annual_billing(sample_service.id, 1, current_year - 1)
 
-    annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
+    data = {'financial_year_start': current_year - 1, 'free_sms_fragment_limit': 9999}
-    assert annual_billing.free_sms_fragment_limit == 250000
+    admin_request.post(
+        'billing.create_or_update_free_sms_fragment_limit',
+        service_id=sample_service.id,
+        _data=data,
+        _expected_status=201)
 
-    data_new = {'financial_year_start': current_year, 'free_sms_fragment_limit': 9999}
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1).free_sms_fragment_limit == 9999
-    response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year).free_sms_fragment_limit == 1
-                           data=json.dumps(data_new),
-                           headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    response_get = client.get(
-        'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'
-        .format(sample_service.id, current_year),
-        headers=[('Content-Type', 'application/json'), create_authorization_header()])
-
-    json_resp = json.loads(response_get.get_data(as_text=True))
-
-    assert response.status_code == 201
-    assert response_get.status_code == 200
-    assert json_resp['financial_year_start'] == current_year
-    assert json_resp['free_sms_fragment_limit'] == 9999
 
 
-def test_get_free_sms_fragment_limit_current_year(client, sample_service):
+def test_create_free_sms_fragment_limit_updates_existing_year(admin_request, sample_service):
+    current_year = get_current_financial_year_start_year()
+    annual_billing = create_annual_billing(sample_service.id, 1, current_year)
+
+    admin_request.post(
+        'billing.create_or_update_free_sms_fragment_limit',
+        service_id=sample_service.id,
+        _data={'financial_year_start': current_year, 'free_sms_fragment_limit': 2},
+        _expected_status=201)
+
+    assert annual_billing.free_sms_fragment_limit == 2
+
+
+def test_get_free_sms_fragment_limit_current_year_creates_new_row(client, sample_service):
 
     current_year = get_current_financial_year_start_year()
-    create_annual_billing(sample_service.id, free_sms_fragment_limit=9999, financial_year_start=current_year - 1)
+    create_annual_billing(sample_service.id, 9999, current_year - 1)
 
     response_get = client.get(
         'service/{}/billing/free-sms-fragment-limit'.format(sample_service.id),
@@ -342,7 +334,7 @@ def test_get_free_sms_fragment_limit_current_year(client, sample_service):
     json_resp = json.loads(response_get.get_data(as_text=True))
     assert response_get.status_code == 200
     assert json_resp['financial_year_start'] == get_current_financial_year_start_year()
-    assert json_resp['free_sms_fragment_limit'] == 250000
+    assert json_resp['free_sms_fragment_limit'] == 9999
 
 
 def test_get_free_sms_fragment_limit_past_year_not_exist(client, sample_service):
@@ -385,10 +377,9 @@ def test_get_free_sms_fragment_limit_future_year_not_exist(client, sample_servic
 
 def test_update_free_sms_fragment_limit_data(client, sample_service):
     current_year = get_current_financial_year_start_year()
-    annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
+    create_annual_billing(sample_service.id, free_sms_fragment_limit=250000, financial_year_start=current_year - 1)
-    assert annual_billing.free_sms_fragment_limit == 250000
 
-    update_free_sms_fragment_limit_data(sample_service.id, 9999)
+    update_free_sms_fragment_limit_data(sample_service.id, 9999, current_year)
 
     annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year)
     assert annual_billing.free_sms_fragment_limit == 9999

tests/app/dao/test_annual_billing_dao.py

@@ -8,16 +8,6 @@ from app.dao.annual_billing_dao import (
 from tests.app.db import create_annual_billing
 
 
-def test_get_sample_service_has_default_free_sms_fragment_limit(notify_db_session, sample_service):
-
-    # when sample_service was created, it automatically create an entry in the annual_billing table
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, get_current_financial_year_start_year())
-
-    assert free_limit.free_sms_fragment_limit == 250000
-    assert free_limit.financial_year_start == get_current_financial_year_start_year()
-    assert free_limit.service_id == sample_service.id
-
-
 def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service):
     new_limit = 9999
     year = get_current_financial_year_start_year()
@@ -27,16 +17,7 @@ def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service):
     assert new_free_limit.free_sms_fragment_limit == new_limit
 
 
-def test_create_annual_billing_not_specify_year(notify_db_session, sample_service):
+def test_create_annual_billing(sample_service):
-
-    dao_create_or_update_annual_billing_for_year(sample_service.id, 9999)
-
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id)
-
-    assert free_limit.free_sms_fragment_limit == 9999
-
-
-def test_create_annual_billing_specify_year(notify_db_session, sample_service):
 
     dao_create_or_update_annual_billing_for_year(sample_service.id, 9999, 2016)
 
@@ -47,16 +28,15 @@ def test_create_annual_billing_specify_year(notify_db_session, sample_service):
 
 def test_dao_update_annual_billing_for_current_and_future_years(notify_db_session, sample_service):
     current_year = get_current_financial_year_start_year()
-    limits = [240000, 250000, 260000, 270000]
+    limits = [1, 2, 3, 4]
     create_annual_billing(sample_service.id, limits[0], current_year - 1)
     create_annual_billing(sample_service.id, limits[2], current_year + 1)
     create_annual_billing(sample_service.id, limits[3], current_year + 2)
 
     dao_update_annual_billing_for_current_and_future_years(sample_service.id, 9999, current_year)
 
-    free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1)
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year - 1).free_sms_fragment_limit == 1
-    assert free_limit.free_sms_fragment_limit == 240000
+    # current year is not created
-
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year) is None
-    for year in range(current_year, current_year + 3):
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year + 1).free_sms_fragment_limit == 9999
-        free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year)
+    assert dao_get_free_sms_fragment_limit_for_year(sample_service.id, current_year + 2).free_sms_fragment_limit == 9999
-        assert free_limit.free_sms_fragment_limit == 9999