From 59dd343254cb1b2bbe847692e5ce1287b16a11d3 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 24 Oct 2017 13:23:24 +0100 Subject: [PATCH 1/8] Added free_sms_fragment_limit model, schema, dao and Rest --- app/billing/billing_schemas.py | 14 +++ app/billing/rest.py | 60 +++++++++++ app/dao/annual_billing_dao.py | 39 ++++++++ app/models.py | 9 +- app/service/rest.py | 4 +- tests/app/billing/test_billing.py | 122 +++++++++++++++++++++++ tests/app/dao/test_annual_billing_dao.py | 46 +++++++++ 7 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 app/billing/billing_schemas.py create mode 100644 app/dao/annual_billing_dao.py create mode 100644 tests/app/dao/test_annual_billing_dao.py diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py new file mode 100644 index 000000000..0cd35208f --- /dev/null +++ b/app/billing/billing_schemas.py @@ -0,0 +1,14 @@ +from app.schema_validation.definitions import uuid, https_url + + +create_or_update_free_sms_fragment_limit_schema = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "POST annual billing schema", + "type": "object", + "title": "Create", + "properties": { + "free_sms_fragment_limit": {"type": "integer", "minimum": 1}, + "financial_year_start": {"type": "integer", "minimum": 1} + }, + "required": ["free_sms_fragment_limit", "financial_year_start"] +} diff --git a/app/billing/rest.py b/app/billing/rest.py index 197adadf2..f25d282dd 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -11,6 +11,14 @@ from app.dao.date_util import get_financial_year, get_months_for_financial_year from app.errors import register_errors from app.models import SMS_TYPE, EMAIL_TYPE from app.utils import convert_utc_to_bst +from app.dao.annual_billing_dao import (dao_get_free_sms_fragment_limit_for_year, + dao_get_all_free_sms_fragment_limit, + dao_create_new_annual_billing_for_year, + dao_update_new_free_sms_fragment_limit_for_year) +from app.billing.billing_schemas import create_or_update_free_sms_fragment_limit_schema +from app.errors import InvalidRequest +from app.schema_validation import validate +from app.models import AnnualBilling billing_blueprint = Blueprint( 'billing', @@ -86,3 +94,55 @@ def _transform_billing_for_month(billing_for_month): "notification_type": billing_for_month.notification_type, "rate": rate } + + +# @billing_blueprint.route('/annual-billing', methods=["GET"]) +# def get_annual_billing(service_id): +# +# results = dao_get_annual_billing(service_id) +# +# if len(results)==0: +# raise InvalidRequest('no annual billing information for this service', status_code=404) +# +# return jsonify(data=[row.serialize() for row in results]), 200 + + +@billing_blueprint.route('/free-sms-fragment-limit', methods=["GET"]) +def get_free_sms_fragment_limit(service_id): + + financial_year_start = request.args.get('financial_year_start') + + if financial_year_start is None: + results = dao_get_all_free_sms_fragment_limit(service_id) + + if len(results) == 0: + raise InvalidRequest('no annual billing information for this service', status_code=404) + return jsonify(data=[row.serialize() for row in results]), 200 + else: + result = dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start) + if result is None: + raise InvalidRequest('no free-sms-fragment-limit-info for this service and year', status_code=404) + + return jsonify(data=result.serialize()), 200 + + +@billing_blueprint.route('/free-sms-fragment-limit', methods=["POST"]) +def create_or_update_free_sms_fragment_limit(service_id): + + form = validate(request.get_json(), create_or_update_free_sms_fragment_limit_schema) + + financial_year_start = form.get('financial_year_start') + free_sms_fragment_limit = form.get('free_sms_fragment_limit') + + result = dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start) + + annual_billing = AnnualBilling(service_id=service_id, financial_year_start=financial_year_start, + free_sms_fragment_limit=free_sms_fragment_limit) + + if result is None: + dao_create_new_annual_billing_for_year(annual_billing) + + else: + dao_update_new_free_sms_fragment_limit_for_year(annual_billing) + + return jsonify(data=form), 201 diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py new file mode 100644 index 000000000..9d28cca44 --- /dev/null +++ b/app/dao/annual_billing_dao.py @@ -0,0 +1,39 @@ +from app import db, create_uuid +from app.dao.dao_utils import ( + transactional, + version_class +) +from app.models import AnnualBilling +from datetime import datetime + + +def dao_get_annual_billing(service_id): + return AnnualBilling.query.filter_by( + service_id=service_id, + ).all() + + +def dao_create_new_annual_billing_for_year(annual_billing): + annual_billing.id = create_uuid() + db.session.add(annual_billing) + db.session.commit() + + +def dao_get_free_sms_fragment_limit_for_year(service_id, year): + + return AnnualBilling.query.filter_by( + service_id=service_id, + financial_year_start=year + ).first() + + +def dao_get_all_free_sms_fragment_limit(service_id): + + return AnnualBilling.query.filter_by( + service_id=service_id, + ).all() + + +def dao_update_new_free_sms_fragment_limit_for_year(annual_billing): + db.session.add(annual_billing) + db.session.commit() diff --git a/app/models.py b/app/models.py index 0637293fa..19f89dafe 100644 --- a/app/models.py +++ b/app/models.py @@ -191,12 +191,8 @@ class AnnualBilling(db.Model): def serialize(self): return { - 'id': str(self.id), - 'service_id': str(self.service_id), - 'free_sms_fragment_limit': str(self.free_sms_fragment_limit), - 'financial_year_start': str(self.financial_year_start), - 'created_at': self.created_at.strftime(DATETIME_FORMAT), - 'updated_at': self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None + 'free_sms_fragment_limit': self.free_sms_fragment_limit, + 'financial_year_start': self.financial_year_start, } @@ -234,7 +230,6 @@ class Service(db.Model, Versioned): organisation_id = db.Column(UUID(as_uuid=True), db.ForeignKey('organisation.id'), index=True, nullable=True) free_sms_fragment_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=True) organisation = db.relationship('Organisation') - annual_billing = db.relationship('AnnualBilling') dvla_organisation_id = db.Column( db.String, db.ForeignKey('dvla_organisation.id'), diff --git a/app/service/rest.py b/app/service/rest.py index 66cd90317..f36046d7c 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -66,7 +66,8 @@ from app.errors import ( InvalidRequest, register_errors ) -from app.models import Service, ServiceInboundApi + +from app.models import Service, ServiceInboundApi, AnnualBilling from app.schema_validation import validate from app.service import statistics from app.service.service_inbound_api_schema import service_inbound_api, update_service_inbound_api_schema @@ -87,6 +88,7 @@ from app.schemas import ( detailed_service_schema ) from app.utils import pagination_links +from app.dao.notifications_dao import get_financial_year service_blueprint = Blueprint('service', __name__) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 8c1a6be8a..edf04016b 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -14,6 +14,8 @@ from tests.app.db import ( ) from tests import create_authorization_header +from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year, dao_create_new_annual_billing_for_year +from app.models import AnnualBilling APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00) APR_2016_MONTH_END = datetime(2016, 4, 30, 22, 59, 59, 99999) @@ -251,3 +253,123 @@ def test_transform_billing_calculates_with_different_rate_multipliers(sample_ser 'month': 'April', 'rate': 0.12, }) + + +# def test_get_annual_billing(client, sample_service): +# +# years = [2016, 2017, 2018] +# for y in years: +# data = AnnualBilling( +# free_sms_fragment_limit=250000, +# financial_year_start=y, +# service_id=sample_service.id, +# ) +# dao_create_new_free_sms_fragment_limit_for_year(data) +# +# response = client.get('service/{}/annual-billing'.format(sample_service.id), +# headers=[('Content-Type', 'application/json'), create_authorization_header()]) +# +# json_resp = json.loads(response.get_data(as_text=True)) +# +# assert len(json_resp['data']) == 3 +# assert response.status_code == 200 +# +# +# def test_get_annual_billing_no_data_throws_error(client, sample_service): +# +# response = client.get('service/{}/annual-billing'.format(sample_service.id), +# headers=[('Content-Type', 'application/json'), create_authorization_header()]) +# +# json_resp = json.loads(response.get_data(as_text=True)) +# +# assert response.status_code == 404 + + +def test_get_free_sms_fragment_limit(client, sample_service): + years = [2016, 2017, 2018] + sms_allowance = [1000, 2000, 3000] + for i in range(0, len(years)): + y = years[i] + sms_l = sms_allowance[i] + data = AnnualBilling( + free_sms_fragment_limit=sms_l, + financial_year_start=y, + service_id=sample_service.id, + ) + dao_create_new_annual_billing_for_year(data) + + response = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start=2017' + .format(sample_service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + + assert json_resp['data']['free_sms_fragment_limit'] == 2000 + assert json_resp['data']['financial_year_start'] == 2017 + + response = client.get( + 'service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + json_resp = json.loads(response.get_data(as_text=True)) + + assert len(json_resp['data']) == 3 + assert response.status_code == 200 + for i in range(0, len(years)): + assert json_resp['data'][i]['free_sms_fragment_limit'] == sms_allowance[i] + assert json_resp['data'][i]['financial_year_start'] == years[i] + + +def test_create_update_free_sms_fragment_limit_invalid_schema(client, sample_service): + + response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + data={}, + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(response.get_data(as_text=True)) + + assert response.status_code == 400 + assert 'JSON' in json_resp['message'] + + +def test_create_free_sms_fragment_limit(client, sample_service): + + data = {'financial_year_start': 2017, 'free_sms_fragment_limit': 250} + 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( + 'service/{}/billing/free-sms-fragment-limit?financial_year_start=2017'.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['data']['financial_year_start'] == 2017 + assert json_resp['data']['free_sms_fragment_limit'] == 250 + + +def test_update_free_sms_fragment_limit(client, sample_service): + + data_old = {'financial_year_start': 2015, 'free_sms_fragment_limit': 1000} + response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + data=json.dumps(data_old), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + data_new = {'financial_year_start': 2015, 'free_sms_fragment_limit': 9999} + response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + 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=2015'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + + json_resp = json.loads(response_get.get_data(as_text=True)) + new_free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, 2015) + + assert response.status_code == 201 + assert response_get.status_code == 200 + assert json_resp['data']['financial_year_start'] == 2015 + assert json_resp['data']['free_sms_fragment_limit'] == 9999 diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py new file mode 100644 index 000000000..9046583d6 --- /dev/null +++ b/tests/app/dao/test_annual_billing_dao.py @@ -0,0 +1,46 @@ +# from datetime import datetime, timedelta +# import uuid +# import functools +# import pytest + +from app.models import AnnualBilling +from app.dao.annual_billing_dao import ( + dao_create_new_annual_billing_for_year, + dao_get_free_sms_fragment_limit_for_year, + dao_update_new_free_sms_fragment_limit_for_year +) + + +def test_dao_create_get_free_sms_fragment_limit(notify_db_session, sample_service): + year = 2016 + data = AnnualBilling( + free_sms_fragment_limit=250000, + financial_year_start=year, + service_id=sample_service.id, + ) + dao_create_new_annual_billing_for_year(data) + + free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year) + + assert free_limit.free_sms_fragment_limit == 250000 + assert free_limit.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): + year = 2016 + old_limit = 1000 + new_limit = 9999 + + data = AnnualBilling( + free_sms_fragment_limit=old_limit, + financial_year_start=year, + service_id=sample_service.id, + ) + + dao_create_new_annual_billing_for_year(data) + data.free_sms_fragment_limit = new_limit + dao_update_new_free_sms_fragment_limit_for_year(data) + new_free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year) + + assert new_free_limit.free_sms_fragment_limit == new_limit From 15e3b4171bf40e65955fc6656dab5515d0c5a597 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 24 Oct 2017 14:46:10 +0100 Subject: [PATCH 2/8] fixed object persistence problem --- app/billing/rest.py | 26 +++----- app/dao/annual_billing_dao.py | 10 +-- tests/app/billing/test_billing.py | 79 ++++++++++++++---------- tests/app/dao/test_annual_billing_dao.py | 34 +++++----- 4 files changed, 74 insertions(+), 75 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index f25d282dd..7d254c751 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -13,8 +13,7 @@ from app.models import SMS_TYPE, EMAIL_TYPE from app.utils import convert_utc_to_bst from app.dao.annual_billing_dao import (dao_get_free_sms_fragment_limit_for_year, dao_get_all_free_sms_fragment_limit, - dao_create_new_annual_billing_for_year, - dao_update_new_free_sms_fragment_limit_for_year) + dao_create_or_update_annual_billing_for_year) from app.billing.billing_schemas import create_or_update_free_sms_fragment_limit_schema from app.errors import InvalidRequest from app.schema_validation import validate @@ -96,17 +95,6 @@ def _transform_billing_for_month(billing_for_month): } -# @billing_blueprint.route('/annual-billing', methods=["GET"]) -# def get_annual_billing(service_id): -# -# results = dao_get_annual_billing(service_id) -# -# if len(results)==0: -# raise InvalidRequest('no annual billing information for this service', status_code=404) -# -# return jsonify(data=[row.serialize() for row in results]), 200 - - @billing_blueprint.route('/free-sms-fragment-limit', methods=["GET"]) def get_free_sms_fragment_limit(service_id): @@ -136,13 +124,13 @@ def create_or_update_free_sms_fragment_limit(service_id): result = dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start) - annual_billing = AnnualBilling(service_id=service_id, financial_year_start=financial_year_start, - free_sms_fragment_limit=free_sms_fragment_limit) - - if result is None: - dao_create_new_annual_billing_for_year(annual_billing) + if result: + result.free_sms_fragment_limit = free_sms_fragment_limit + dao_create_or_update_annual_billing_for_year(result) else: - dao_update_new_free_sms_fragment_limit_for_year(annual_billing) + annual_billing = AnnualBilling(service_id=service_id, financial_year_start=financial_year_start, + free_sms_fragment_limit=free_sms_fragment_limit) + dao_create_or_update_annual_billing_for_year(annual_billing) return jsonify(data=form), 201 diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py index 9d28cca44..2343104b3 100644 --- a/app/dao/annual_billing_dao.py +++ b/app/dao/annual_billing_dao.py @@ -13,8 +13,9 @@ def dao_get_annual_billing(service_id): ).all() -def dao_create_new_annual_billing_for_year(annual_billing): - annual_billing.id = create_uuid() +def dao_create_or_update_annual_billing_for_year(annual_billing): + if annual_billing.id is None: + annual_billing.id = create_uuid() db.session.add(annual_billing) db.session.commit() @@ -32,8 +33,3 @@ def dao_get_all_free_sms_fragment_limit(service_id): return AnnualBilling.query.filter_by( service_id=service_id, ).all() - - -def dao_update_new_free_sms_fragment_limit_for_year(annual_billing): - db.session.add(annual_billing) - db.session.commit() diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index edf04016b..e49cea604 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -14,7 +14,8 @@ from tests.app.db import ( ) from tests import create_authorization_header -from app.dao.annual_billing_dao import dao_get_free_sms_fragment_limit_for_year, dao_create_new_annual_billing_for_year +from app.dao.annual_billing_dao import (dao_get_free_sms_fragment_limit_for_year, + dao_create_or_update_annual_billing_for_year) from app.models import AnnualBilling APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00) @@ -255,36 +256,6 @@ def test_transform_billing_calculates_with_different_rate_multipliers(sample_ser }) -# def test_get_annual_billing(client, sample_service): -# -# years = [2016, 2017, 2018] -# for y in years: -# data = AnnualBilling( -# free_sms_fragment_limit=250000, -# financial_year_start=y, -# service_id=sample_service.id, -# ) -# dao_create_new_free_sms_fragment_limit_for_year(data) -# -# response = client.get('service/{}/annual-billing'.format(sample_service.id), -# headers=[('Content-Type', 'application/json'), create_authorization_header()]) -# -# json_resp = json.loads(response.get_data(as_text=True)) -# -# assert len(json_resp['data']) == 3 -# assert response.status_code == 200 -# -# -# def test_get_annual_billing_no_data_throws_error(client, sample_service): -# -# response = client.get('service/{}/annual-billing'.format(sample_service.id), -# headers=[('Content-Type', 'application/json'), create_authorization_header()]) -# -# json_resp = json.loads(response.get_data(as_text=True)) -# -# assert response.status_code == 404 - - def test_get_free_sms_fragment_limit(client, sample_service): years = [2016, 2017, 2018] sms_allowance = [1000, 2000, 3000] @@ -296,7 +267,7 @@ def test_get_free_sms_fragment_limit(client, sample_service): financial_year_start=y, service_id=sample_service.id, ) - dao_create_new_annual_billing_for_year(data) + dao_create_or_update_annual_billing_for_year(data) response = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start=2017' .format(sample_service.id), @@ -367,9 +338,51 @@ def test_update_free_sms_fragment_limit(client, sample_service): headers=[('Content-Type', 'application/json'), create_authorization_header()]) json_resp = json.loads(response_get.get_data(as_text=True)) - new_free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, 2015) assert response.status_code == 201 assert response_get.status_code == 200 assert json_resp['data']['financial_year_start'] == 2015 assert json_resp['data']['free_sms_fragment_limit'] == 9999 + + +def test_get_free_sms_fragment_limit_year_return_correct_data(client, sample_service): + years = [2015, 2016, 2017] + limits = [1000, 2000, 3000] + for i in range(0, len(years)): + data = AnnualBilling( + free_sms_fragment_limit=limits[i], + financial_year_start=years[i], + service_id=sample_service.id, + ) + dao_create_or_update_annual_billing_for_year(data) + + for i in range(0, len(years)): + response_get = client.get( + 'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'.format(sample_service.id, years[i]), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(response_get.get_data(as_text=True)) + assert response_get.status_code == 200 + assert json_resp['data']['free_sms_fragment_limit'] == limits[i] + + +def test_get_free_sms_fragment_limit_for_all_year(client, sample_service): + years = [2015, 2016, 2017] + limits = [1000, 2000, 3000] + for i in range(0, len(years)): + data = AnnualBilling( + free_sms_fragment_limit=limits[i], + financial_year_start=years[i], + service_id=sample_service.id, + ) + dao_create_or_update_annual_billing_for_year(data) + + response_get = client.get( + # Not specify a particular year to return all data for that service + 'service/{}/billing/free-sms-fragment-limit'.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_get.status_code == 200 + assert len(json_resp['data']) == 3 + for i in [0, 1, 2]: + assert json_resp['data'][i]['free_sms_fragment_limit'] == limits[i] + assert json_resp['data'][i]['financial_year_start'] == years[i] diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py index 9046583d6..73b230431 100644 --- a/tests/app/dao/test_annual_billing_dao.py +++ b/tests/app/dao/test_annual_billing_dao.py @@ -5,26 +5,28 @@ from app.models import AnnualBilling from app.dao.annual_billing_dao import ( - dao_create_new_annual_billing_for_year, - dao_get_free_sms_fragment_limit_for_year, - dao_update_new_free_sms_fragment_limit_for_year + dao_create_or_update_annual_billing_for_year, + dao_get_free_sms_fragment_limit_for_year ) def test_dao_create_get_free_sms_fragment_limit(notify_db_session, sample_service): - year = 2016 - data = AnnualBilling( - free_sms_fragment_limit=250000, - financial_year_start=year, - service_id=sample_service.id, - ) - dao_create_new_annual_billing_for_year(data) + years = [2015, 2016, 2017] + free_limit_data = [1000, 2000, 3000] + for i in range(0, len(years)): + data = AnnualBilling( + free_sms_fragment_limit=free_limit_data[i], + financial_year_start=years[i], + service_id=sample_service.id, + ) + dao_create_or_update_annual_billing_for_year(data) - free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year) + for i in range(0, len(years)): + free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, years[i]) - assert free_limit.free_sms_fragment_limit == 250000 - assert free_limit.financial_year_start == year - assert free_limit.service_id == sample_service.id + assert free_limit.free_sms_fragment_limit == free_limit_data[i] + assert free_limit.financial_year_start == years[i] + assert free_limit.service_id == sample_service.id def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service): @@ -38,9 +40,9 @@ def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service): service_id=sample_service.id, ) - dao_create_new_annual_billing_for_year(data) + dao_create_or_update_annual_billing_for_year(data) data.free_sms_fragment_limit = new_limit - dao_update_new_free_sms_fragment_limit_for_year(data) + dao_create_or_update_annual_billing_for_year(data) new_free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year) assert new_free_limit.free_sms_fragment_limit == new_limit From 8ad98f2806376d297b78d9803c685c561eb68741 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 25 Oct 2017 11:35:13 +0100 Subject: [PATCH 3/8] create entry when creating a new service --- app/billing/rest.py | 9 +++--- app/dao/annual_billing_dao.py | 16 ++++++++-- app/dao/services_dao.py | 7 +++++ app/models.py | 35 +++++++++++---------- app/service/rest.py | 4 +-- app/service/utils.py | 11 +++++++ tests/app/billing/test_billing.py | 40 +++++++++++++----------- tests/app/dao/test_annual_billing_dao.py | 26 ++++----------- tests/app/dao/test_services_dao.py | 5 +-- tests/app/db.py | 1 + tests/app/service/test_rest.py | 20 ++++++++++++ 11 files changed, 107 insertions(+), 67 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index 7d254c751..7d73bc098 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -126,11 +126,10 @@ def create_or_update_free_sms_fragment_limit(service_id): if result: result.free_sms_fragment_limit = free_sms_fragment_limit - dao_create_or_update_annual_billing_for_year(result) - else: - annual_billing = AnnualBilling(service_id=service_id, financial_year_start=financial_year_start, - free_sms_fragment_limit=free_sms_fragment_limit) - dao_create_or_update_annual_billing_for_year(annual_billing) + result = AnnualBilling(service_id=service_id, financial_year_start=financial_year_start, + free_sms_fragment_limit=free_sms_fragment_limit) + + dao_create_or_update_annual_billing_for_year(result) return jsonify(data=form), 201 diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py index 2343104b3..8e8f7c886 100644 --- a/app/dao/annual_billing_dao.py +++ b/app/dao/annual_billing_dao.py @@ -5,6 +5,7 @@ from app.dao.dao_utils import ( ) from app.models import AnnualBilling from datetime import datetime +from app.service.utils import get_current_financial_year_start_year def dao_get_annual_billing(service_id): @@ -14,8 +15,6 @@ def dao_get_annual_billing(service_id): def dao_create_or_update_annual_billing_for_year(annual_billing): - if annual_billing.id is None: - annual_billing.id = create_uuid() db.session.add(annual_billing) db.session.commit() @@ -33,3 +32,16 @@ def dao_get_all_free_sms_fragment_limit(service_id): return AnnualBilling.query.filter_by( service_id=service_id, ).all() + + +def insert_annual_billing(service): + """ + This method is called from create_service which is wrapped in a transaction. + """ + annual_billing = AnnualBilling( + free_sms_fragment_limit=service.free_sms_fragment_limit, + financial_year_start=get_current_financial_year_start_year(), + service=service, + ) + + db.session.add(annual_billing) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index e6e677adc..3f9e1b9c5 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -37,10 +37,12 @@ from app.models import ( EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, ServiceSmsSender, + AnnualBilling ) from app.service.statistics import format_monthly_template_notification_stats 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 insert_annual_billing DEFAULT_SERVICE_PERMISSIONS = [ SMS_TYPE, @@ -164,6 +166,9 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) if service_permissions is None: service_permissions = DEFAULT_SERVICE_PERMISSIONS + if service.free_sms_fragment_limit is None: + service.free_sms_fragment_limit = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) @@ -176,6 +181,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.permissions.append(service_permission) insert_service_sms_sender(service, service.sms_sender) + insert_annual_billing(service) db.session.add(service) @@ -238,6 +244,7 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(ServicePermission.query.filter_by(service_id=service.id)) _delete_commit(ApiKey.query.filter_by(service=service)) _delete_commit(ApiKey.get_history_model().query.filter_by(service_id=service.id)) + _delete_commit(AnnualBilling.query.filter_by(service_id=service.id)) verify_codes = VerifyCode.query.join(User).filter(User.id.in_([x.id for x in service.users])) list(map(db.session.delete, verify_codes)) diff --git a/app/models.py b/app/models.py index 31cbd0a3d..a757485bc 100644 --- a/app/models.py +++ b/app/models.py @@ -179,23 +179,6 @@ class ServicePermissionTypes(db.Model): name = db.Column(db.String(255), primary_key=True) -class AnnualBilling(db.Model): - __tablename__ = "annual_billing" - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4, unique=False) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=False, index=True, nullable=False) - financial_year_start = db.Column(db.Integer, nullable=False, default=True, unique=False) - free_sms_fragment_limit = db.Column(db.Integer, nullable=False, index=False, unique=False) - updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) - UniqueConstraint('financial_year_start', 'service_id', name='ix_annual_billing_service_id') - - def serialize(self): - return { - 'free_sms_fragment_limit': self.free_sms_fragment_limit, - 'financial_year_start': self.financial_year_start, - } - - class Service(db.Model, Versioned): __tablename__ = 'services' @@ -286,6 +269,24 @@ class Service(db.Model, Versioned): return default_letter_contact[0].contact_block if default_letter_contact else None +class AnnualBilling(db.Model): + __tablename__ = "annual_billing" + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4, unique=False) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), unique=False, index=True, nullable=False) + financial_year_start = db.Column(db.Integer, nullable=False, default=True, unique=False) + free_sms_fragment_limit = db.Column(db.Integer, nullable=False, index=False, unique=False) + updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) + created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) + UniqueConstraint('financial_year_start', 'service_id', name='ix_annual_billing_service_id') + service = db.relationship(Service, backref=db.backref("annual_billing", uselist=True)) + + def serialize(self): + return { + 'free_sms_fragment_limit': self.free_sms_fragment_limit, + 'financial_year_start': self.financial_year_start, + } + + class InboundNumber(db.Model): __tablename__ = "inbound_numbers" diff --git a/app/service/rest.py b/app/service/rest.py index eab409bc1..be08e5028 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -170,8 +170,8 @@ def create_service(): raise InvalidRequest(errors, status_code=400) # TODO: to be removed when front-end is updated - if 'free_sms_fragment_limit' not in data: - data['free_sms_fragment_limit'] = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + # if 'free_sms_fragment_limit' not in data: + # data['free_sms_fragment_limit'] = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] # validate json with marshmallow service_schema.load(request.get_json()) diff --git a/app/service/utils.py b/app/service/utils.py index 2c4ccbf04..475d420e2 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -6,6 +6,8 @@ from app.models import ( KEY_TYPE_TEST, KEY_TYPE_TEAM, KEY_TYPE_NORMAL) from notifications_utils.recipients import allowed_to_send_to +from app.dao.notifications_dao import get_financial_year +from datetime import datetime def get_recipients_from_request(request_json, key, type): @@ -51,3 +53,12 @@ def service_allowed_to_send_to(recipient, service, key_type): whitelist_members ) ) + + +def get_current_financial_year_start_year(): + now = datetime.now() + financial_year_start = now.year + start_date, end_date = get_financial_year(now.year) + if now < start_date: + financial_year_start = financial_year_start - 1 + return financial_year_start diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index e49cea604..86bfcc6d6 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -259,15 +259,19 @@ def test_transform_billing_calculates_with_different_rate_multipliers(sample_ser def test_get_free_sms_fragment_limit(client, sample_service): years = [2016, 2017, 2018] sms_allowance = [1000, 2000, 3000] + for i in range(0, len(years)): y = years[i] sms_l = sms_allowance[i] - data = AnnualBilling( - free_sms_fragment_limit=sms_l, - financial_year_start=y, - service_id=sample_service.id, - ) - dao_create_or_update_annual_billing_for_year(data) + annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, years[i]) + if annual_billing: + annual_billing.free_sms_fragment_limit = sms_allowance[i] + else: + annual_billing = AnnualBilling(service_id=sample_service.id, + financial_year_start=years[i], + free_sms_fragment_limit=sms_allowance[i]) + + dao_create_or_update_annual_billing_for_year(annual_billing) response = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start=2017' .format(sample_service.id), @@ -348,13 +352,12 @@ def test_update_free_sms_fragment_limit(client, sample_service): def test_get_free_sms_fragment_limit_year_return_correct_data(client, sample_service): years = [2015, 2016, 2017] limits = [1000, 2000, 3000] + for i in range(0, len(years)): - data = AnnualBilling( - free_sms_fragment_limit=limits[i], - financial_year_start=years[i], - service_id=sample_service.id, - ) - dao_create_or_update_annual_billing_for_year(data) + annual_billing = {'financial_year_start': years[i], 'free_sms_fragment_limit': limits[i]} + response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + data=json.dumps(annual_billing), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) for i in range(0, len(years)): response_get = client.get( @@ -365,16 +368,15 @@ def test_get_free_sms_fragment_limit_year_return_correct_data(client, sample_ser assert json_resp['data']['free_sms_fragment_limit'] == limits[i] -def test_get_free_sms_fragment_limit_for_all_year(client, sample_service): +def test_get_free_sms_fragment_limit_for_all_years(client, sample_service): years = [2015, 2016, 2017] limits = [1000, 2000, 3000] + for i in range(0, len(years)): - data = AnnualBilling( - free_sms_fragment_limit=limits[i], - financial_year_start=years[i], - service_id=sample_service.id, - ) - dao_create_or_update_annual_billing_for_year(data) + annual_billing = {'financial_year_start': years[i], 'free_sms_fragment_limit': limits[i]} + response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), + data=json.dumps(annual_billing), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) response_get = client.get( # Not specify a particular year to return all data for that service diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py index 73b230431..e971c6460 100644 --- a/tests/app/dao/test_annual_billing_dao.py +++ b/tests/app/dao/test_annual_billing_dao.py @@ -1,8 +1,4 @@ -# from datetime import datetime, timedelta -# import uuid -# import functools -# import pytest - +from app.service.utils import get_current_financial_year_start_year from app.models import AnnualBilling from app.dao.annual_billing_dao import ( dao_create_or_update_annual_billing_for_year, @@ -10,23 +6,13 @@ from app.dao.annual_billing_dao import ( ) -def test_dao_create_get_free_sms_fragment_limit(notify_db_session, sample_service): - years = [2015, 2016, 2017] - free_limit_data = [1000, 2000, 3000] - for i in range(0, len(years)): - data = AnnualBilling( - free_sms_fragment_limit=free_limit_data[i], - financial_year_start=years[i], - service_id=sample_service.id, - ) - dao_create_or_update_annual_billing_for_year(data) +def test_sample_service_has_free_sms_fragment_limit(notify_db_session, sample_service): - for i in range(0, len(years)): - free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, years[i]) + 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 == free_limit_data[i] - assert free_limit.financial_year_start == years[i] - assert free_limit.service_id == sample_service.id + 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): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a1538eebf..f672c7e42 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -104,7 +104,7 @@ def test_cannot_create_two_services_with_same_name(sample_user): email_from="email_from1", message_limit=1000, restricted=False, - created_by=sample_user) + created_by=sample_user,) service2 = Service(name="service_name", email_from="email_from2", @@ -171,7 +171,8 @@ def test_should_remove_user_from_service(sample_user): email_from="email_from", message_limit=1000, restricted=False, - created_by=sample_user) + created_by=sample_user, + free_sms_fragment_limit=9999) dao_create_service(service, sample_user) new_user = User( name='Test User', diff --git a/tests/app/db.py b/tests/app/db.py index 0cdf7cfee..74d2033e2 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -68,6 +68,7 @@ def create_service( service = Service( name=service_name, message_limit=1000, + free_sms_fragment_limit=7777, restricted=restricted, email_from=email_from if email_from else service_name.lower().replace(' ', '.'), created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())), diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3e1483d1a..1a5ec4e99 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -37,6 +37,7 @@ from tests.app.db import ( create_service_sms_sender ) from tests.app.db import create_user +from app.service.utils import get_current_financial_year_start_year def test_get_service_list(client, service_factory): @@ -300,6 +301,7 @@ def test_create_service(client, sample_user): assert not json_resp['data']['research_mode'] assert json_resp['data']['dvla_organisation'] == '001' assert json_resp['data']['sms_sender'] == current_app.config['FROM_NUMBER'] + # TODO: Remove this after the new data is used assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] service_db = Service.query.get(json_resp['data']['id']) @@ -365,6 +367,15 @@ def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user) headers=headers) json_resp = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 201 + + # Test data from the new annual billing table + service_id = json_resp['data']['id'] + annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}' + .format(service_id, get_current_financial_year_start_year()), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(annual_billing.get_data(as_text=True)) + assert json_resp['data']['free_sms_fragment_limit'] == 9999 + # TODO: Remove this after the new data is used assert json_resp['data']['free_sms_fragment_limit'] == 9999 data2 = { @@ -385,6 +396,14 @@ def test_create_service_free_sms_fragment_limit_is_optional(client, sample_user) headers=headers) json_resp = json.loads(resp.get_data(as_text=True)) assert resp.status_code == 201 + # Test data from the new annual billing table + service_id = json_resp['data']['id'] + annual_billing = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start={}' + .format(service_id, get_current_financial_year_start_year()), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(annual_billing.get_data(as_text=True)) + assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] + # TODO: Remove this after the new data is used assert json_resp['data']['free_sms_fragment_limit'] == current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] @@ -623,6 +642,7 @@ def test_update_service_flags_will_remove_service_permissions(client, notify_db, assert set([p.permission for p in permissions]) == set([SMS_TYPE, EMAIL_TYPE]) +# TODO: Remove after new table is created and verified def test_update_service_free_sms_fragment_limit(client, notify_db, sample_service): org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') notify_db.session.add(org) From e8f659837aac0c51c192d4148ae1ee9a72e59139 Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 25 Oct 2017 12:50:13 +0100 Subject: [PATCH 4/8] remove unused codes --- app/dao/annual_billing_dao.py | 4 +- app/service/rest.py | 3 -- tests/app/billing/test_billing.py | 51 +++++------------------- tests/app/dao/test_annual_billing_dao.py | 3 +- tests/app/dao/test_services_dao.py | 3 +- tests/app/db.py | 1 - 6 files changed, 16 insertions(+), 49 deletions(-) diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py index 8e8f7c886..6e76f6ce5 100644 --- a/app/dao/annual_billing_dao.py +++ b/app/dao/annual_billing_dao.py @@ -11,7 +11,7 @@ from app.service.utils import get_current_financial_year_start_year def dao_get_annual_billing(service_id): return AnnualBilling.query.filter_by( service_id=service_id, - ).all() + ).order_by(AnnualBilling.financial_year_start).all() def dao_create_or_update_annual_billing_for_year(annual_billing): @@ -31,7 +31,7 @@ def dao_get_all_free_sms_fragment_limit(service_id): return AnnualBilling.query.filter_by( service_id=service_id, - ).all() + ).order_by(AnnualBilling.financial_year_start).all() def insert_annual_billing(service): diff --git a/app/service/rest.py b/app/service/rest.py index be08e5028..3c19fb4ed 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -98,7 +98,6 @@ from app.schemas import ( detailed_service_schema ) from app.utils import pagination_links -from app.dao.notifications_dao import get_financial_year service_blueprint = Blueprint('service', __name__) @@ -170,8 +169,6 @@ def create_service(): raise InvalidRequest(errors, status_code=400) # TODO: to be removed when front-end is updated - # if 'free_sms_fragment_limit' not in data: - # data['free_sms_fragment_limit'] = current_app.config['FREE_SMS_TIER_FRAGMENT_COUNT'] # validate json with marshmallow service_schema.load(request.get_json()) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 86bfcc6d6..fdd797242 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -256,46 +256,6 @@ def test_transform_billing_calculates_with_different_rate_multipliers(sample_ser }) -def test_get_free_sms_fragment_limit(client, sample_service): - years = [2016, 2017, 2018] - sms_allowance = [1000, 2000, 3000] - - for i in range(0, len(years)): - y = years[i] - sms_l = sms_allowance[i] - annual_billing = dao_get_free_sms_fragment_limit_for_year(sample_service.id, years[i]) - if annual_billing: - annual_billing.free_sms_fragment_limit = sms_allowance[i] - else: - annual_billing = AnnualBilling(service_id=sample_service.id, - financial_year_start=years[i], - free_sms_fragment_limit=sms_allowance[i]) - - dao_create_or_update_annual_billing_for_year(annual_billing) - - response = client.get('service/{}/billing/free-sms-fragment-limit?financial_year_start=2017' - .format(sample_service.id), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - - assert json_resp['data']['free_sms_fragment_limit'] == 2000 - assert json_resp['data']['financial_year_start'] == 2017 - - response = client.get( - 'service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), - headers=[('Content-Type', 'application/json'), create_authorization_header()]) - - json_resp = json.loads(response.get_data(as_text=True)) - - assert len(json_resp['data']) == 3 - assert response.status_code == 200 - for i in range(0, len(years)): - assert json_resp['data'][i]['free_sms_fragment_limit'] == sms_allowance[i] - assert json_resp['data'][i]['financial_year_start'] == years[i] - - def test_create_update_free_sms_fragment_limit_invalid_schema(client, sample_service): response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), @@ -385,6 +345,17 @@ def test_get_free_sms_fragment_limit_for_all_years(client, sample_service): json_resp = json.loads(response_get.get_data(as_text=True)) assert response_get.status_code == 200 assert len(json_resp['data']) == 3 + print(json_resp) for i in [0, 1, 2]: assert json_resp['data'][i]['free_sms_fragment_limit'] == limits[i] assert json_resp['data'][i]['financial_year_start'] == years[i] + + +def test_get_free_sms_fragment_limit_no_year_data_return_404(client, sample_service): + + response_get = client.get( + 'service/{}/billing/free-sms-fragment-limit?financial_year_start={}'.format(sample_service.id, 1999), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(response_get.get_data(as_text=True)) + + assert response_get.status_code == 404 diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py index e971c6460..b356687c7 100644 --- a/tests/app/dao/test_annual_billing_dao.py +++ b/tests/app/dao/test_annual_billing_dao.py @@ -8,6 +8,7 @@ from app.dao.annual_billing_dao import ( def test_sample_service_has_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 @@ -16,7 +17,7 @@ def test_sample_service_has_free_sms_fragment_limit(notify_db_session, sample_se def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service): - year = 2016 + year = 1999 old_limit = 1000 new_limit = 9999 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a38d81b24..9edec2feb 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -171,8 +171,7 @@ def test_should_remove_user_from_service(sample_user): email_from="email_from", message_limit=1000, restricted=False, - created_by=sample_user, - free_sms_fragment_limit=9999) + created_by=sample_user) dao_create_service(service, sample_user) new_user = User( name='Test User', diff --git a/tests/app/db.py b/tests/app/db.py index 74d2033e2..0cdf7cfee 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -68,7 +68,6 @@ def create_service( service = Service( name=service_name, message_limit=1000, - free_sms_fragment_limit=7777, restricted=restricted, email_from=email_from if email_from else service_name.lower().replace(' ', '.'), created_by=user or create_user(email='{}@digital.cabinet-office.gov.uk'.format(uuid.uuid4())), From 9aa74896554540609cfb6a34eadb7945ffb2b445 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 26 Oct 2017 11:49:56 +0100 Subject: [PATCH 5/8] incorporate reviewers comments --- app/dao/annual_billing_dao.py | 2 +- app/dao/services_dao.py | 4 ++-- tests/app/billing/test_billing.py | 10 +++++++++ tests/app/dao/test_annual_billing_dao.py | 26 ++++++++++++++++++++++-- tests/app/service/test_utils.py | 15 ++++++++++++++ 5 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 tests/app/service/test_utils.py diff --git a/app/dao/annual_billing_dao.py b/app/dao/annual_billing_dao.py index 6e76f6ce5..2df3e1497 100644 --- a/app/dao/annual_billing_dao.py +++ b/app/dao/annual_billing_dao.py @@ -34,7 +34,7 @@ def dao_get_all_free_sms_fragment_limit(service_id): ).order_by(AnnualBilling.financial_year_start).all() -def insert_annual_billing(service): +def dao_insert_annual_billing(service): """ This method is called from create_service which is wrapped in a transaction. """ diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 3f9e1b9c5..870998c2d 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -42,7 +42,7 @@ from app.models import ( from app.service.statistics import format_monthly_template_notification_stats 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 insert_annual_billing +from app.dao.annual_billing_dao import dao_insert_annual_billing DEFAULT_SERVICE_PERMISSIONS = [ SMS_TYPE, @@ -181,7 +181,7 @@ def dao_create_service(service, user, service_id=None, service_permissions=None) service.permissions.append(service_permission) insert_service_sms_sender(service, service.sms_sender) - insert_annual_billing(service) + dao_insert_annual_billing(service) db.session.add(service) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index fdd797242..737ccab11 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -17,6 +17,7 @@ from tests import create_authorization_header from app.dao.annual_billing_dao import (dao_get_free_sms_fragment_limit_for_year, dao_create_or_update_annual_billing_for_year) from app.models import AnnualBilling +import uuid APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00) APR_2016_MONTH_END = datetime(2016, 4, 30, 22, 59, 59, 99999) @@ -359,3 +360,12 @@ def test_get_free_sms_fragment_limit_no_year_data_return_404(client, sample_serv json_resp = json.loads(response_get.get_data(as_text=True)) assert response_get.status_code == 404 + + +def test_get_free_sms_fragment_limit_unknown_service_id_return_404(client): + + response_get = client.get( + 'service/{}/billing/free-sms-fragment-limit'.format(uuid.uuid4()), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(response_get.get_data(as_text=True)) + assert response_get.status_code == 404 diff --git a/tests/app/dao/test_annual_billing_dao.py b/tests/app/dao/test_annual_billing_dao.py index b356687c7..10c6cd201 100644 --- a/tests/app/dao/test_annual_billing_dao.py +++ b/tests/app/dao/test_annual_billing_dao.py @@ -2,11 +2,12 @@ from app.service.utils import get_current_financial_year_start_year from app.models import AnnualBilling from app.dao.annual_billing_dao import ( dao_create_or_update_annual_billing_for_year, - dao_get_free_sms_fragment_limit_for_year + dao_get_free_sms_fragment_limit_for_year, + dao_get_annual_billing ) -def test_sample_service_has_free_sms_fragment_limit(notify_db_session, sample_service): +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()) @@ -33,3 +34,24 @@ def test_dao_update_free_sms_fragment_limit(notify_db_session, sample_service): new_free_limit = dao_get_free_sms_fragment_limit_for_year(sample_service.id, year) assert new_free_limit.free_sms_fragment_limit == new_limit + + +def test_create_then_get_annual_billing(notify_db_session, sample_service): + years = [1999, 2001] + limits = [1000, 2000] + + for i in [0, 1]: + data = AnnualBilling( + free_sms_fragment_limit=limits[i], + financial_year_start=years[i], + service_id=sample_service.id, + ) + dao_create_or_update_annual_billing_for_year(data) + + free_limit = dao_get_annual_billing(sample_service.id) + assert len(free_limit) == 3 # sample service already has one entry + assert free_limit[0].free_sms_fragment_limit == 1000 + assert free_limit[0].financial_year_start == 1999 + assert free_limit[0].service_id == sample_service.id + assert free_limit[1].free_sms_fragment_limit == 2000 + assert free_limit[1].financial_year_start == 2001 diff --git a/tests/app/service/test_utils.py b/tests/app/service/test_utils.py new file mode 100644 index 000000000..09ce33f7d --- /dev/null +++ b/tests/app/service/test_utils.py @@ -0,0 +1,15 @@ +from app.service.utils import get_current_financial_year_start_year +from freezegun import freeze_time + + +# see get_financial_year for conversion of financial years. +@freeze_time("2001-03-31 22:59:59.999999") +def test_get_current_financial_year_start_year_before_march(): + current_fy = get_current_financial_year_start_year() + assert current_fy == 2000 + + +@freeze_time("2001-03-31 23:00:00.000000") +def test_get_current_financial_year_start_year_after_april(): + current_fy = get_current_financial_year_start_year() + assert current_fy == 2001 From c10cde6b224b698e936ad76bc13f97ea84d470af Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 26 Oct 2017 13:25:11 +0100 Subject: [PATCH 6/8] modified serialized method and schema --- app/billing/billing_schemas.py | 2 +- app/billing/rest.py | 4 ++-- app/models.py | 19 ++++++++++++++++++- tests/app/billing/test_billing.py | 12 ++++++------ tests/app/service/test_utils.py | 8 ++++---- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/app/billing/billing_schemas.py b/app/billing/billing_schemas.py index 0cd35208f..f9f592e7d 100644 --- a/app/billing/billing_schemas.py +++ b/app/billing/billing_schemas.py @@ -8,7 +8,7 @@ create_or_update_free_sms_fragment_limit_schema = { "title": "Create", "properties": { "free_sms_fragment_limit": {"type": "integer", "minimum": 1}, - "financial_year_start": {"type": "integer", "minimum": 1} + "financial_year_start": {"type": "integer", "minimum": 2016} }, "required": ["free_sms_fragment_limit", "financial_year_start"] } diff --git a/app/billing/rest.py b/app/billing/rest.py index 7d73bc098..f4c337903 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -105,13 +105,13 @@ def get_free_sms_fragment_limit(service_id): if len(results) == 0: raise InvalidRequest('no annual billing information for this service', status_code=404) - return jsonify(data=[row.serialize() for row in results]), 200 + return jsonify(data=[row.serialize_free_sms_items() for row in results]), 200 else: result = dao_get_free_sms_fragment_limit_for_year(service_id, financial_year_start) if result is None: raise InvalidRequest('no free-sms-fragment-limit-info for this service and year', status_code=404) - return jsonify(data=result.serialize()), 200 + return jsonify(data=result.serialize_free_sms_items()), 200 @billing_blueprint.route('/free-sms-fragment-limit', methods=["POST"]) diff --git a/app/models.py b/app/models.py index d59164896..485edf944 100644 --- a/app/models.py +++ b/app/models.py @@ -278,12 +278,29 @@ class AnnualBilling(db.Model): UniqueConstraint('financial_year_start', 'service_id', name='ix_annual_billing_service_id') service = db.relationship(Service, backref=db.backref("annual_billing", uselist=True)) - def serialize(self): + def serialize_free_sms_items(self): return { 'free_sms_fragment_limit': self.free_sms_fragment_limit, 'financial_year_start': self.financial_year_start, } + def serialize(self): + def serialize_service(): + return { + "id": str(self.service_id), + "name": self.service.name + } + + return{ + "id": str(self.id), + 'free_sms_fragment_limit': self.free_sms_fragment_limit, + 'service_id': self.service_id, + 'financial_year_start': self.financial_year_start, + "created_at": self.created_at.strftime(DATETIME_FORMAT), + "updated_at": self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None, + "service": serialize_service() if self.service else None, + } + class InboundNumber(db.Model): __tablename__ = "inbound_numbers" diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index 737ccab11..e6655f293 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -288,30 +288,30 @@ def test_create_free_sms_fragment_limit(client, sample_service): def test_update_free_sms_fragment_limit(client, sample_service): - data_old = {'financial_year_start': 2015, 'free_sms_fragment_limit': 1000} + data_old = {'financial_year_start': 2016, 'free_sms_fragment_limit': 1000} response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), data=json.dumps(data_old), headers=[('Content-Type', 'application/json'), create_authorization_header()]) - data_new = {'financial_year_start': 2015, 'free_sms_fragment_limit': 9999} + data_new = {'financial_year_start': 2016, 'free_sms_fragment_limit': 9999} response = client.post('service/{}/billing/free-sms-fragment-limit'.format(sample_service.id), 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=2015'.format(sample_service.id), + '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['data']['financial_year_start'] == 2015 + assert json_resp['data']['financial_year_start'] == 2016 assert json_resp['data']['free_sms_fragment_limit'] == 9999 def test_get_free_sms_fragment_limit_year_return_correct_data(client, sample_service): - years = [2015, 2016, 2017] + years = [2016, 2017, 2018] limits = [1000, 2000, 3000] for i in range(0, len(years)): @@ -330,7 +330,7 @@ def test_get_free_sms_fragment_limit_year_return_correct_data(client, sample_ser def test_get_free_sms_fragment_limit_for_all_years(client, sample_service): - years = [2015, 2016, 2017] + years = [2016, 2017, 2018] limits = [1000, 2000, 3000] for i in range(0, len(years)): diff --git a/tests/app/service/test_utils.py b/tests/app/service/test_utils.py index 09ce33f7d..4d59af22b 100644 --- a/tests/app/service/test_utils.py +++ b/tests/app/service/test_utils.py @@ -3,13 +3,13 @@ from freezegun import freeze_time # see get_financial_year for conversion of financial years. -@freeze_time("2001-03-31 22:59:59.999999") +@freeze_time("2017-03-31 22:59:59.999999") def test_get_current_financial_year_start_year_before_march(): current_fy = get_current_financial_year_start_year() - assert current_fy == 2000 + assert current_fy == 2016 -@freeze_time("2001-03-31 23:00:00.000000") +@freeze_time("2017-03-31 23:00:00.000000") def test_get_current_financial_year_start_year_after_april(): current_fy = get_current_financial_year_start_year() - assert current_fy == 2001 + assert current_fy == 2017 From eca93a5a2458a109b40598202a82a95e63c28025 Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 26 Oct 2017 17:21:35 +0100 Subject: [PATCH 7/8] added a new end point current-year and tests --- app/billing/rest.py | 5 +++++ tests/app/billing/test_billing.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/app/billing/rest.py b/app/billing/rest.py index f4c337903..45b3b5baa 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -18,6 +18,7 @@ from app.billing.billing_schemas import create_or_update_free_sms_fragment_limit from app.errors import InvalidRequest from app.schema_validation import validate from app.models import AnnualBilling +from app.service.utils import get_current_financial_year_start_year billing_blueprint = Blueprint( 'billing', @@ -96,10 +97,14 @@ def _transform_billing_for_month(billing_for_month): @billing_blueprint.route('/free-sms-fragment-limit', methods=["GET"]) +@billing_blueprint.route('/free-sms-fragment-limit/current-year', methods=["GET"]) def get_free_sms_fragment_limit(service_id): financial_year_start = request.args.get('financial_year_start') + if request.path.split('/')[-1] == 'current-year': + financial_year_start = get_current_financial_year_start_year() + if financial_year_start is None: results = dao_get_all_free_sms_fragment_limit(service_id) diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index e6655f293..fb946e657 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -369,3 +369,12 @@ def test_get_free_sms_fragment_limit_unknown_service_id_return_404(client): headers=[('Content-Type', 'application/json'), create_authorization_header()]) json_resp = json.loads(response_get.get_data(as_text=True)) assert response_get.status_code == 404 + + +def test_get_free_sms_fragment_limit_current_year(client, sample_service): + response = client.get( + 'service/{}/billing/free-sms-fragment-limit/current-year'.format(sample_service.id, True), + headers=[('Content-Type', 'application/json'), create_authorization_header()]) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert json_resp['data']['free_sms_fragment_limit'] == 250000 From d85a71758ccd8b0dfb742dd7d77fc4374bdb3f61 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Mon, 30 Oct 2017 13:43:23 +0000 Subject: [PATCH 8/8] Retry in only certain scenarios Instead of retrying if there are genuine errors, only retry if there are errors which are unexpected as otherwise the retries will happen and fail for the same reason e.g. that the message has changed format and will require a code update. - Updated process_ses_results to only retry if there in an unknown exception - Update test and assert that there is a retry there is a unknown exception --- app/celery/tasks.py | 9 ++++++--- tests/app/celery/test_tasks.py | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index abafa6b78..b5e732df5 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -555,7 +555,10 @@ def process_incomplete_job(job_id): @notify_celery.task(bind=True, name="process-ses-result", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") def process_ses_results(self, response): - errors = process_ses_response(response) - if errors: - current_app.logger.error(errors) + try: + errors = process_ses_response(response) + if errors: + current_app.logger.error(errors) + except Exception: + current_app.logger.exception('Error processing SES results') self.retry(queue=QueueNames.RETRY, exc="SES responses processed with error") diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 39bce8e69..69f32c421 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1398,7 +1398,15 @@ def test_process_ses_results(notify_db, notify_db_session, sample_email_template assert process_ses_results(response=response) is None +def test_process_ses_results_does_not_retry_if_errors(notify_db, mocker): + mocked = mocker.patch('app.celery.tasks.process_ses_results.retry') + response = json.loads(ses_notification_callback()) + process_ses_results(response=response) + assert mocked.call_count == 0 + + def test_process_ses_results_retry_called(notify_db, mocker): + mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference", side_effect=Exception("EXPECTED")) mocked = mocker.patch('app.celery.tasks.process_ses_results.retry') response = json.loads(ses_notification_callback()) process_ses_results(response=response)