From 7c0c2a9a0f69e1bf289992c6e243783330504ee6 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 14 Nov 2017 09:43:28 +0000 Subject: [PATCH 1/9] migrate data from service table to annual billing --- .../0136_migrate_sms_allowance_data.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 migrations/versions/0136_migrate_sms_allowance_data.py diff --git a/migrations/versions/0136_migrate_sms_allowance_data.py b/migrations/versions/0136_migrate_sms_allowance_data.py new file mode 100644 index 000000000..ba0ef3562 --- /dev/null +++ b/migrations/versions/0136_migrate_sms_allowance_data.py @@ -0,0 +1,49 @@ +""" + +Revision ID: 013_migrate_sms_allowance_data.py +Revises: 0135_stats_template_usage.py +Create Date: 2017-11-10 21:42:59.715203 + +""" +from datetime import datetime +from alembic import op +import uuid +from app.dao.date_util import get_current_financial_year_start_year + + +revision = '0136_migrate_sms_allowance_data' +down_revision = '0135_stats_template_usage' + + +def upgrade(): + current_year = get_current_financial_year_start_year() + default_limit = 250000 + + # Step 1: update the column free_sms_fragment_limit in service table if it is empty + update_service_table = """ + UPDATE services SET free_sms_fragment_limit = {} where free_sms_fragment_limit is null + """.format(default_limit) + + op.execute(update_service_table) + + # Step 2: insert at least one row for every service in current year if none exist for that service + insert_row_if_not_exist = """ + INSERT INTO annual_billing + (id, service_id, financial_year_start, free_sms_fragment_limit, created_at, updated_at) + SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, {}, {}, '{}', '{}' + FROM services WHERE id NOT IN + (select service_id from annual_billing) + """.format(current_year, default_limit, datetime.utcnow(), datetime.utcnow()) + op.execute(insert_row_if_not_exist) + + # Step 3: copy the free_sms_fragment_limit data from the services table across to annual_billing table. + update_sms_allowance = """ + UPDATE annual_billing SET free_sms_fragment_limit = services.free_sms_fragment_limit + FROM services + WHERE annual_billing.service_id = services.id + """ + op.execute(update_sms_allowance) + + +def downgrade(): + print('There is no action for downgrading to the previous version.') \ No newline at end of file From 4e487e708be1a1210cdc83edc3b1a16cdbdbac21 Mon Sep 17 00:00:00 2001 From: venusbb Date: Tue, 14 Nov 2017 16:11:59 +0000 Subject: [PATCH 2/9] Bridging API to update annual_billing table --- app/billing/rest.py | 10 ++++++---- app/service/rest.py | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/billing/rest.py b/app/billing/rest.py index 614842d3f..1bdeaf6a9 100644 --- a/app/billing/rest.py +++ b/app/billing/rest.py @@ -135,14 +135,16 @@ def create_or_update_free_sms_fragment_limit(service_id): form = validate(req_args, 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') + update_free_sms_fragment_limit_data(service_id, + free_sms_fragment_limit=form.get('free_sms_fragment_limit'), + financial_year_start=form.get('financial_year_start')) + return jsonify(form), 201 + +def update_free_sms_fragment_limit_data(service_id, free_sms_fragment_limit, financial_year_start=None): current_year = get_current_financial_year_start_year() if financial_year_start is None or financial_year_start >= current_year: dao_update_annual_billing_for_current_and_future_years(service_id, free_sms_fragment_limit) else: dao_create_or_update_annual_billing_for_year(service_id, free_sms_fragment_limit, financial_year_start) - - return jsonify(form), 201 diff --git a/app/service/rest.py b/app/service/rest.py index c6e8c4201..a17920d36 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -98,6 +98,7 @@ from app.schemas import ( detailed_service_schema ) from app.utils import pagination_links +from app.billing.rest import update_free_sms_fragment_limit_data service_blueprint = Blueprint('service', __name__) @@ -202,6 +203,10 @@ def update_service(service_id): if 'letter_contact_block' in req_json: create_or_update_letter_contact(fetched_service.id, req_json['letter_contact_block']) + # bridging code between frontend is deployed and data has not been migrated yet. Can only update current year + if 'free_sms_fragment_limit' in req_json: + update_free_sms_fragment_limit_data(fetched_service.id, req_json['free_sms_fragment_limit']) + if service_going_live: send_notification_to_service_users( service_id=service_id, From e903257156f93d8dea364cd189407259436d1f9d Mon Sep 17 00:00:00 2001 From: venusbb Date: Wed, 15 Nov 2017 11:25:27 +0000 Subject: [PATCH 3/9] version change because of other commits --- ...nce_data.py => 0138_migrate_sms_allowance_data.py} | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) rename migrations/versions/{0136_migrate_sms_allowance_data.py => 0138_migrate_sms_allowance_data.py} (81%) diff --git a/migrations/versions/0136_migrate_sms_allowance_data.py b/migrations/versions/0138_migrate_sms_allowance_data.py similarity index 81% rename from migrations/versions/0136_migrate_sms_allowance_data.py rename to migrations/versions/0138_migrate_sms_allowance_data.py index ba0ef3562..95bb8f0a7 100644 --- a/migrations/versions/0136_migrate_sms_allowance_data.py +++ b/migrations/versions/0138_migrate_sms_allowance_data.py @@ -1,7 +1,7 @@ """ -Revision ID: 013_migrate_sms_allowance_data.py -Revises: 0135_stats_template_usage.py +Revision ID: 0138_migrate_sms_allowance_data.py +Revises: 0137_stats_template_usage.py Create Date: 2017-11-10 21:42:59.715203 """ @@ -11,8 +11,8 @@ import uuid from app.dao.date_util import get_current_financial_year_start_year -revision = '0136_migrate_sms_allowance_data' -down_revision = '0135_stats_template_usage' +revision = '0138_migrate_sms_allowance_data' +down_revision = '0137_notification_template_hist' def upgrade(): @@ -30,7 +30,7 @@ def upgrade(): insert_row_if_not_exist = """ INSERT INTO annual_billing (id, service_id, financial_year_start, free_sms_fragment_limit, created_at, updated_at) - SELECT uuid_in(md5(random()::text || now()::text)::cstring), id, {}, {}, '{}', '{}' + SELECT uuid_in(md5(random()::text)::cstring), id, {}, {}, '{}', '{}' FROM services WHERE id NOT IN (select service_id from annual_billing) """.format(current_year, default_limit, datetime.utcnow(), datetime.utcnow()) @@ -46,4 +46,5 @@ def upgrade(): def downgrade(): + # There is no schema change. Only data migration and filling in gaps. print('There is no action for downgrading to the previous version.') \ No newline at end of file From d9050999628f0e57bc44748a75150b21af105322 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 15 Nov 2017 11:43:52 +0000 Subject: [PATCH 4/9] If the post to a url for the inbound sms message to the service throwns an unknown exception - or RequestException, the base exception for request, the error will be logged and retried. --- app/celery/tasks.py | 38 +++++++++++++++++----------------- tests/app/celery/test_tasks.py | 21 +++++++++++++++++++ 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 5b92db0a5..9935ad609 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -14,7 +14,8 @@ from notifications_utils.template import ( ) from requests import ( HTTPError, - request + request, + RequestException ) from sqlalchemy.exc import SQLAlchemyError from app import ( @@ -492,25 +493,24 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id): "date_received": inbound_sms.provider_date.strftime(DATETIME_FORMAT) } - response = request( - method="POST", - url=inbound_api.url, - data=json.dumps(data), - headers={ - 'Content-Type': 'application/json', - 'Authorization': 'Bearer {}'.format(inbound_api.bearer_token) - }, - timeout=60 - ) - current_app.logger.info('send_inbound_sms_to_service sending {} to {}, response {}'.format( - inbound_sms_id, - inbound_api.url, - response.status_code - )) - try: + response = request( + method="POST", + url=inbound_api.url, + data=json.dumps(data), + headers={ + 'Content-Type': 'application/json', + 'Authorization': 'Bearer {}'.format(inbound_api.bearer_token) + }, + timeout=60 + ) + current_app.logger.info('send_inbound_sms_to_service sending {} to {}, response {}'.format( + inbound_sms_id, + inbound_api.url, + response.status_code + )) response.raise_for_status() - except HTTPError as e: + except RequestException as e: current_app.logger.warning( "send_inbound_sms_to_service request failed for service_id: {} and url: {}. exc: {}".format( service_id, @@ -518,7 +518,7 @@ def send_inbound_sms_to_service(self, inbound_sms_id, service_id): e ) ) - if e.response.status_code >= 500: + if not isinstance(e, HTTPError) or e.response.status_code >= 500: try: self.retry(queue=QueueNames.RETRY, exc='Unable to send_inbound_sms_to_service for service_id: {} and url: {}. \n{}'.format( diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 69f32c421..5267dd436 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -7,6 +7,7 @@ import pytest import requests_mock from flask import current_app from freezegun import freeze_time +from requests import RequestException from sqlalchemy.exc import SQLAlchemyError from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate, LetterDVLATemplate from celery.exceptions import Retry @@ -1160,6 +1161,26 @@ def test_send_inbound_sms_to_service_retries_if_request_returns_500(notify_api, assert exc_msg in mocked.call_args[1]['exc'] +def test_send_inbound_sms_to_service_retries_if_request_throws_unknown(notify_api, sample_service, mocker): + inbound_api = create_service_inbound_api(service=sample_service, url="https://some.service.gov.uk/", + bearer_token="something_unique") + inbound_sms = create_inbound_sms(service=sample_service, notify_number="0751421", user_number="447700900111", + provider_date=datetime(2017, 6, 20), content="Here is some content") + + mocked = mocker.patch('app.celery.tasks.send_inbound_sms_to_service.retry') + mocker.patch("app.celery.tasks.request", side_effect=RequestException()) + + send_inbound_sms_to_service(inbound_sms.id, inbound_sms.service_id) + + exc_msg = 'Unable to send_inbound_sms_to_service for service_id: {} and url: {url}'.format( + sample_service.id, + url=inbound_api.url + ) + assert mocked.call_count == 1 + assert mocked.call_args[1]['queue'] == 'retry-tasks' + assert exc_msg in mocked.call_args[1]['exc'] + + def test_send_inbound_sms_to_service_does_not_retries_if_request_returns_404(notify_api, sample_service, mocker): inbound_api = create_service_inbound_api(service=sample_service, url="https://some.service.gov.uk/", bearer_token="something_unique") From 7ac03469614183433b7a18548ef8e97f6c7aaaae Mon Sep 17 00:00:00 2001 From: venusbb Date: Thu, 16 Nov 2017 13:44:55 +0000 Subject: [PATCH 5/9] change migration version --- ...lowance_data.py => 0139_migrate_sms_allowance_data.py} | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) rename migrations/versions/{0138_migrate_sms_allowance_data.py => 0139_migrate_sms_allowance_data.py} (90%) diff --git a/migrations/versions/0138_migrate_sms_allowance_data.py b/migrations/versions/0139_migrate_sms_allowance_data.py similarity index 90% rename from migrations/versions/0138_migrate_sms_allowance_data.py rename to migrations/versions/0139_migrate_sms_allowance_data.py index 95bb8f0a7..8fc0d0afa 100644 --- a/migrations/versions/0138_migrate_sms_allowance_data.py +++ b/migrations/versions/0139_migrate_sms_allowance_data.py @@ -1,7 +1,7 @@ """ -Revision ID: 0138_migrate_sms_allowance_data.py -Revises: 0137_stats_template_usage.py +Revision ID: 0139_migrate_sms_allowance_data.py +Revises: 0138_sms_sender_nullable.py Create Date: 2017-11-10 21:42:59.715203 """ @@ -11,8 +11,8 @@ import uuid from app.dao.date_util import get_current_financial_year_start_year -revision = '0138_migrate_sms_allowance_data' -down_revision = '0137_notification_template_hist' +revision = '0139_migrate_sms_allowance_data' +down_revision = '0138_sms_sender_nullable' def upgrade(): From 5ba58031b18b9cb801ce45bcd9f55505abaf3ad1 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 16 Nov 2017 13:51:42 +0000 Subject: [PATCH 6/9] Fixed Bug in stats aggregation The check for aggregation was too broad and hence was adding together totals based on template_id and not the unqiue combination of template id, month and year. - Added test to test for the failure - Added check and a test to for template_id, mon and year matches - Celery process name did not match the task --- app/dao/services_dao.py | 6 ++- tests/app/dao/test_services_dao.py | 62 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 0fe8be7d0..396f2a762 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -571,7 +571,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) extract('year', year).label('year'), func.count().label('count') ).join( - Template, Notification.template_id == Template.id + Template, Notification.template_id == Template.id, ).filter( Notification.created_at >= start_date ).group_by( @@ -586,7 +586,9 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) for today_result in today_results: add_to_stats = True for stat in stats: - if today_result.template_id == stat.template_id: + if today_result.template_id == stat.template_id and \ + today_result.month == stat.month and \ + today_result.year == stat.year: stat.count = stat.count + today_result.count add_to_stats = False diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f880ba4e3..0539ca1cb 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1292,3 +1292,65 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_get_this_yea ) assert len(result) == 2 + + +@freeze_time("2017-11-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_combined_historical_current( + notify_db, + notify_db_session, + sample_service +): + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered' + ) + + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + + date = datetime.now() + day = date.day + month = date.month + year = date.year + + n = notification_history(created_at=datetime(year, month, day) - timedelta(days=30), sample_template=template_one) + + daily_stats_template_usage_by_month() + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id, 2017), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 1 + + assert result[0].template_id == template_one.id + assert result[0].month == 10 + assert result[0].year == 2017 + assert result[0].count == 1 + + create_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template_one, + created_at=datetime.utcnow() + ) + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(n.service_id, 2017), + key=lambda x: (x.month, x.year) + ) + + assert len(result) == 2 + + assert result[0].template_id == template_one.id + assert result[0].month == 10 + assert result[0].year == 2017 + assert result[0].count == 1 + + assert result[1].template_id == template_one.id + assert result[1].month == 11 + assert result[1].year == 2017 + assert result[1].count == 1 From e7de0c0900ccf2ab20c11ddc6e025c008bf8a422 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 16 Nov 2017 14:41:07 +0000 Subject: [PATCH 7/9] Added a template_type to the rest call Added a template type which is required for the admin UI and updated the tests. The rest tests needed updated because of the bug fix for aggregation. --- app/config.py | 2 +- app/dao/services_dao.py | 4 +++ app/dao/stats_template_usage_by_month_dao.py | 1 + app/service/rest.py | 1 + tests/app/dao/test_services_dao.py | 26 ++++++++++++++++ .../test_stats_template_usage_by_month_dao.py | 1 + tests/app/service/test_rest.py | 31 ++++++++++++++++--- 7 files changed, 60 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index 86a635985..3ce5bca68 100644 --- a/app/config.py +++ b/app/config.py @@ -243,7 +243,7 @@ class Config(object): 'options': {'queue': QueueNames.PERIODIC} }, 'daily-stats-template-usage-by-month': { - 'task': 'daily-stats-template_usage_by_month', + 'task': 'daily-stats-template-usage-by-month', 'schedule': crontab(hour=0, minute=50), 'options': {'queue': QueueNames.PERIODIC} } diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 396f2a762..302cd8952 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -554,6 +554,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) for result in results: stat = type("", (), {})() stat.template_id = result.template_id + stat.template_type = result.template_type stat.name = str(result.name) stat.month = result.month stat.year = result.year @@ -567,6 +568,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) today_results = db.session.query( Notification.template_id, Template.name, + Template.template_type, extract('month', month).label('month'), extract('year', year).label('year'), func.count().label('count') @@ -577,6 +579,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) ).group_by( Notification.template_id, Template.name, + Template.template_type, month, year ).order_by( @@ -595,6 +598,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) if add_to_stats: new_stat = type("", (), {})() new_stat.template_id = today_result.template_id + new_stat.template_type = today_result.template_type new_stat.name = today_result.name new_stat.month = today_result.month new_stat.year = today_result.year diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index bacc7a900..84d2550a6 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -34,6 +34,7 @@ def dao_get_template_usage_stats_by_service(service_id, year): return db.session.query( StatsTemplateUsageByMonth.template_id, Template.name, + Template.template_type, StatsTemplateUsageByMonth.month, StatsTemplateUsageByMonth.year, StatsTemplateUsageByMonth.count diff --git a/app/service/rest.py b/app/service/rest.py index 250430d7f..b4f25287f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -539,6 +539,7 @@ def get_monthly_template_usage(service_id): { 'template_id': str(i.template_id), 'name': i.name, + 'type': i.template_type, 'month': i.month, 'year': i.year, 'count': i.count diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 0539ca1cb..3f9418a78 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1069,11 +1069,15 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_no_stats_tod assert len(result) == 2 assert result[0].template_id == template_two.id + assert result[0].name == template_two.name + assert result[0].template_type == template_two.template_type assert result[0].month == 4 assert result[0].year == 2017 assert result[0].count == 2 assert result[1].template_id == template_one.id + assert result[1].name == template_one.name + assert result[1].template_type == template_two.template_type assert result[1].month == 10 assert result[1].year == 2017 assert result[1].count == 1 @@ -1115,11 +1119,15 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_add_to_histo assert len(result) == 2 assert result[0].template_id == template_one.id + assert result[0].name == template_one.name + assert result[0].template_type == template_one.template_type assert result[0].month == 9 assert result[0].year == 2017 assert result[0].count == 1 assert result[1].template_id == template_two.id + assert result[1].name == template_two.name + assert result[1].template_type == template_two.template_type assert result[1].month == 11 assert result[1].year == 2017 assert result[1].count == 2 @@ -1147,16 +1155,22 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_add_to_histo assert len(result) == 3 assert result[0].template_id == template_one.id + assert result[0].name == template_one.name + assert result[0].template_type == template_one.template_type assert result[0].month == 9 assert result[0].year == 2017 assert result[0].count == 1 assert result[1].template_id == template_two.id + assert result[1].name == template_two.name + assert result[1].template_type == template_two.template_type assert result[1].month == month assert result[1].year == year assert result[1].count == 3 assert result[2].template_id == template_three.id + assert result[2].name == template_three.name + assert result[2].template_type == template_three.template_type assert result[2].month == 11 assert result[2].year == 2017 assert result[2].count == 1 @@ -1198,11 +1212,15 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_does_add_old assert len(result) == 2 assert result[0].template_id == template_one.id + assert result[0].name == template_one.name + assert result[0].template_type == template_one.template_type assert result[0].month == 9 assert result[0].year == 2017 assert result[0].count == 1 assert result[1].template_id == template_two.id + assert result[1].name == template_two.name + assert result[1].template_type == template_two.template_type assert result[1].month == 11 assert result[1].year == 2017 assert result[1].count == 2 @@ -1259,6 +1277,8 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_get_this_yea assert len(result) == 1 assert result[0].template_id == template_two.id + assert result[0].name == template_two.name + assert result[0].template_type == template_two.template_type assert result[0].month == 11 assert result[0].year == 2017 assert result[0].count == 2 @@ -1326,6 +1346,8 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_combined_his assert len(result) == 1 assert result[0].template_id == template_one.id + assert result[0].name == template_one.name + assert result[0].template_type == template_one.template_type assert result[0].month == 10 assert result[0].year == 2017 assert result[0].count == 1 @@ -1346,11 +1368,15 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_combined_his assert len(result) == 2 assert result[0].template_id == template_one.id + assert result[0].name == template_one.name + assert result[0].template_type == template_one.template_type assert result[0].month == 10 assert result[0].year == 2017 assert result[0].count == 1 assert result[1].template_id == template_one.id + assert result[1].name == template_one.name + assert result[1].template_type == template_one.template_type assert result[1].month == 11 assert result[1].year == 2017 assert result[1].count == 1 diff --git a/tests/app/dao/test_stats_template_usage_by_month_dao.py b/tests/app/dao/test_stats_template_usage_by_month_dao.py index cd21e3af6..99112c6ca 100644 --- a/tests/app/dao/test_stats_template_usage_by_month_dao.py +++ b/tests/app/dao/test_stats_template_usage_by_month_dao.py @@ -99,6 +99,7 @@ def test_dao_get_template_usage_stats_by_service_specific_year(sample_service): assert len(result) == 1 assert result[0].template_id == email_template.id assert result[0].name == email_template.name + assert result[0].template_type == email_template.template_type assert result[0].month == 2 assert result[0].year == 2017 assert result[0].count == 10 diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index f7655be72..fad550593 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1865,6 +1865,7 @@ def test_get_template_stats_by_month_returns_correct_data(notify_db, notify_db_s assert resp_json["2016-05"][str(sample_template.id)]["counts"]["permanent-failure"] == 1 +@freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_correct_data( notify_db, notify_db_session, @@ -1918,15 +1919,24 @@ def test_get_template_usage_by_month_returns_correct_data( resp_json = json.loads(resp.get_data(as_text=True)).get('stats') assert resp.status_code == 200 - assert len(resp_json) == 1 + assert len(resp_json) == 2 assert resp_json[0]["template_id"] == str(sample_template.id) assert resp_json[0]["name"] == sample_template.name + assert resp_json[0]["type"] == sample_template.template_type assert resp_json[0]["month"] == 4 assert resp_json[0]["year"] == 2016 - assert resp_json[0]["count"] == 5 + assert resp_json[0]["count"] == 4 + + assert resp_json[1]["template_id"] == str(sample_template.id) + assert resp_json[1]["name"] == sample_template.name + assert resp_json[1]["type"] == sample_template.template_type + assert resp_json[1]["month"] == 11 + assert resp_json[1]["year"] == 2017 + assert resp_json[1]["count"] == 1 +@freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_two_templates( notify_db, notify_db_session, @@ -1983,19 +1993,30 @@ def test_get_template_usage_by_month_returns_two_templates( resp_json = json.loads(resp.get_data(as_text=True)).get('stats') assert resp.status_code == 200 - assert len(resp_json) == 2 + assert len(resp_json) == 3 - resp_json = sorted(resp_json, key=lambda k: k.get('count', 0)) + resp_json = sorted(resp_json, key=lambda k: (k.get('year', 0), k.get('month', 0), k.get('count', 0))) assert resp_json[0]["template_id"] == str(template_one.id) + assert resp_json[0]["name"] == template_one.name + assert resp_json[0]["type"] == template_one.template_type assert resp_json[0]["month"] == 4 assert resp_json[0]["year"] == 2016 assert resp_json[0]["count"] == 1 assert resp_json[1]["template_id"] == str(sample_template.id) + assert resp_json[1]["name"] == sample_template.name + assert resp_json[1]["type"] == sample_template.template_type assert resp_json[1]["month"] == 4 assert resp_json[1]["year"] == 2016 - assert resp_json[1]["count"] == 4 + assert resp_json[1]["count"] == 3 + + assert resp_json[2]["template_id"] == str(sample_template.id) + assert resp_json[2]["name"] == sample_template.name + assert resp_json[2]["type"] == sample_template.template_type + assert resp_json[2]["month"] == 11 + assert resp_json[2]["year"] == 2017 + assert resp_json[2]["count"] == 1 @pytest.mark.parametrize('query_string, expected_status, expected_json', [ From fd89a252e0897577464d6848706dd97261aad3da Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 16 Nov 2017 15:16:31 +0000 Subject: [PATCH 8/9] Code Style Updates Updated the to conform with the line ends expected by the code style in tests rather than PyCharm. --- app/dao/services_dao.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 302cd8952..0b91e2267 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -589,9 +589,8 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) for today_result in today_results: add_to_stats = True for stat in stats: - if today_result.template_id == stat.template_id and \ - today_result.month == stat.month and \ - today_result.year == stat.year: + if today_result.template_id == stat.template_id and today_result.month == stat.month \ + and today_result.year == stat.year: stat.count = stat.count + today_result.count add_to_stats = False From b56825a680080c00f1eb6925883abfe14c710e0a Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Thu, 16 Nov 2017 15:43:21 +0000 Subject: [PATCH 9/9] Updated Tests to use a variety of templates All of the tests defaulted to sms templates, updated a few to use a variety of templates types so all types are tested. --- tests/app/dao/test_services_dao.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 3f9418a78..05c564c02 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1189,9 +1189,9 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_does_add_old status='delivered' ) - template_one = create_sample_template(notify_db, notify_db_session, template_name='1') - template_two = create_sample_template(notify_db, notify_db_session, template_name='2') - template_three = create_sample_template(notify_db, notify_db_session, template_name='3') + template_one = create_sample_template(notify_db, notify_db_session, template_name='1', template_type='email') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2', template_type='sms') + template_three = create_sample_template(notify_db, notify_db_session, template_name='3', template_type='letter') date = datetime.now() day = date.day @@ -1254,9 +1254,9 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_get_this_yea status='delivered' ) - template_one = create_sample_template(notify_db, notify_db_session, template_name='1') - template_two = create_sample_template(notify_db, notify_db_session, template_name='2') - template_three = create_sample_template(notify_db, notify_db_session, template_name='3') + template_one = create_sample_template(notify_db, notify_db_session, template_name='1', template_type='email') + template_two = create_sample_template(notify_db, notify_db_session, template_name='2', template_type='sms') + template_three = create_sample_template(notify_db, notify_db_session, template_name='3', template_type='letter') date = datetime.now() day = date.day