From e638653f11fe310c3104efad8bf1e0231a9ae319 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jun 2018 10:30:50 +0100 Subject: [PATCH 1/4] When we get complaints we'd like to know how many we get in a day or other date range, so if there is a spike in complaints we can act on it. Add new endpoint to return the number of complaints in a date range. Unit tests to follow in the next commit. --- app/complaint/complaint_rest.py | 26 +++++++++++++++++++++++++- app/complaint/complaint_schema.py | 11 +++++++++++ app/dao/complaint_dao.py | 4 ++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 app/complaint/complaint_schema.py diff --git a/app/complaint/complaint_rest.py b/app/complaint/complaint_rest.py index f59b12457..a01e29ef2 100644 --- a/app/complaint/complaint_rest.py +++ b/app/complaint/complaint_rest.py @@ -1,8 +1,13 @@ -from flask import Blueprint, jsonify +from datetime import datetime + +from flask import Blueprint, jsonify, request from sqlalchemy import desc +from app.complaint.complaint_schema import complaint_count_request +from app.dao.complaint_dao import fetch_count_of_complaints from app.errors import register_errors from app.models import Complaint +from app.schema_validation import validate complaint_blueprint = Blueprint('complaint', __name__, url_prefix='/complaint') @@ -14,3 +19,22 @@ def get_all_complaints(): complaints = Complaint.query.order_by(desc(Complaint.created_at)).all() return jsonify([x.serialize() for x in complaints]), 200 + + +@complaint_blueprint.route('/total-per-day', methods=['GET']) +def get_complaint_count(): + request_json = request.args.to_dict + start_date = None + end_date = None + if request_json: + validate(request_json, complaint_count_request) + start_date = request_json.get('start_date', None) + end_date = request_json.get('end_date', None) + if not start_date: + start_date = datetime.utcnow().date() + if not end_date: + end_date = datetime.utcnow().date() + + count_of_complaints = fetch_count_of_complaints(start_date=start_date, end_date=end_date) + + return jsonify(count_of_complaints), 200 diff --git a/app/complaint/complaint_schema.py b/app/complaint/complaint_schema.py new file mode 100644 index 000000000..6cd88d22c --- /dev/null +++ b/app/complaint/complaint_schema.py @@ -0,0 +1,11 @@ + +complaint_count_request = { + "$schema": "http://json-schema.org/draft-04/schema#", + "description": "complaint count request schema", + "type": "object", + "title": "Complaint count request", + "properties": { + "start_date": {"type": ["string", "null"], "format": "datetime"}, + "end_date": {"type": ["string", "null"], "format": "datetime"}, + } +} diff --git a/app/dao/complaint_dao.py b/app/dao/complaint_dao.py index 85ebe3b0b..77bd5e15f 100644 --- a/app/dao/complaint_dao.py +++ b/app/dao/complaint_dao.py @@ -10,3 +10,7 @@ def save_complaint(complaint): def fetch_complaints_by_service(service_id): return Complaint.query.filter_by(service_id=service_id).all() + + +def fetch_count_of_complaints(start_date, end_date): + return Complaint.count.filter(Complaint.created_at >= start_date, Complaint.created_at < end_date) From 28beeebbf446edbf6f3dd3edcccc05e067bd3b59 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jun 2018 16:01:41 +0100 Subject: [PATCH 2/4] Added a query and endpoint to return the count of complaints for a date-range. Unit tests for complaints_rest.get_complaint_count have yet to be written. Maybe there is a better name for the new endpoint. --- app/complaint/complaint_rest.py | 3 ++- app/dao/complaint_dao.py | 12 +++++++-- tests/app/dao/test_complaint_dao.py | 41 ++++++++++++++++++++++++----- tests/app/db.py | 6 +++-- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/app/complaint/complaint_rest.py b/app/complaint/complaint_rest.py index a01e29ef2..07574e06e 100644 --- a/app/complaint/complaint_rest.py +++ b/app/complaint/complaint_rest.py @@ -21,11 +21,12 @@ def get_all_complaints(): return jsonify([x.serialize() for x in complaints]), 200 -@complaint_blueprint.route('/total-per-day', methods=['GET']) +@complaint_blueprint.route('/count-by-date-range', methods=['GET']) def get_complaint_count(): request_json = request.args.to_dict start_date = None end_date = None + # TODO: unit tests have yet to be written, need to test setting start and end date if request_json: validate(request_json, complaint_count_request) start_date = request_json.get('start_date', None) diff --git a/app/dao/complaint_dao.py b/app/dao/complaint_dao.py index 77bd5e15f..e23a28955 100644 --- a/app/dao/complaint_dao.py +++ b/app/dao/complaint_dao.py @@ -1,6 +1,11 @@ +from datetime import timedelta + +from sqlalchemy import desc + from app import db from app.dao.dao_utils import transactional from app.models import Complaint +from app.utils import get_london_midnight_in_utc @transactional @@ -9,8 +14,11 @@ def save_complaint(complaint): def fetch_complaints_by_service(service_id): - return Complaint.query.filter_by(service_id=service_id).all() + return Complaint.query.filter_by(service_id=service_id).order_by(desc(Complaint.created_at)).all() def fetch_count_of_complaints(start_date, end_date): - return Complaint.count.filter(Complaint.created_at >= start_date, Complaint.created_at < end_date) + start_date = get_london_midnight_in_utc(start_date) + end_date = get_london_midnight_in_utc(end_date + timedelta(days=1)) + + return Complaint.query.filter(Complaint.created_at >= start_date, Complaint.created_at < end_date).count() diff --git a/tests/app/dao/test_complaint_dao.py b/tests/app/dao/test_complaint_dao.py index d1acc93f7..5533f43f7 100644 --- a/tests/app/dao/test_complaint_dao.py +++ b/tests/app/dao/test_complaint_dao.py @@ -1,10 +1,10 @@ import uuid -from datetime import datetime +from datetime import datetime, timedelta -from app.dao.complaint_dao import save_complaint, fetch_complaints_by_service +from app.dao.complaint_dao import save_complaint, fetch_complaints_by_service, fetch_count_of_complaints from app.models import Complaint -from tests.app.db import create_service, create_template, create_notification +from tests.app.db import create_service, create_template, create_notification, create_complaint def test_fetch_complaint_by_service_returns_one(sample_service, sample_email_notification): @@ -51,7 +51,8 @@ def test_fetch_complaint_by_service_return_many(notify_db_session): service_id=service_2.id, ses_feedback_id=str(uuid.uuid4()), complaint_type='abuse', - complaint_date=datetime.utcnow() + complaint_date=datetime.utcnow(), + created_at=datetime.utcnow() + timedelta(minutes=1) ) save_complaint(complaint_1) @@ -60,5 +61,33 @@ def test_fetch_complaint_by_service_return_many(notify_db_session): complaints = fetch_complaints_by_service(service_id=service_2.id) assert len(complaints) == 2 - assert complaints[0] == complaint_2 - assert complaints[1] == complaint_3 + assert complaints[0] == complaint_3 + assert complaints[1] == complaint_2 + + +def test_fetch_count_of_complaints(sample_email_notification): + create_complaint(service=sample_email_notification.service, + notification=sample_email_notification, + created_at=datetime(2018, 6, 6, 22, 00, 00)) + create_complaint(service=sample_email_notification.service, + notification=sample_email_notification, + created_at=datetime(2018, 6, 6, 23, 00, 00)) + create_complaint(service=sample_email_notification.service, + notification=sample_email_notification, + created_at=datetime(2018, 6, 7, 00, 00, 00)) + create_complaint(service=sample_email_notification.service, + notification=sample_email_notification, + created_at=datetime(2018, 6, 7, 13, 00, 00)) + create_complaint(service=sample_email_notification.service, + notification=sample_email_notification, + created_at=datetime(2018, 6, 7, 23)) + + count_of_complaints = fetch_count_of_complaints(start_date=datetime(2018, 6, 7), + end_date=datetime(2018, 6, 7)) + assert count_of_complaints == 3 + + +def test_fetch_count_of_complaints_returns_zero(notify_db): + count_of_complaints = fetch_count_of_complaints(start_date=datetime(2018, 6, 7), + end_date=datetime(2018, 6, 7)) + assert count_of_complaints == 0 diff --git a/tests/app/db.py b/tests/app/db.py index 5970165b1..c2535e47d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -566,7 +566,8 @@ def create_ft_billing(bst_date, def create_complaint(service=None, - notification=None): + notification=None, + created_at=None): if not service: service = create_service() if not notification: @@ -577,7 +578,8 @@ def create_complaint(service=None, service_id=service.id, ses_feedback_id=str(uuid.uuid4()), complaint_type='abuse', - complaint_date=datetime.utcnow() + complaint_date=datetime.utcnow(), + created_at=created_at if created_at else datetime.now() ) db.session.add(complaint) db.session.commit() From 7f4b828affc9981c28774aa6e65911939ad46077 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 12 Jun 2018 12:10:58 +0100 Subject: [PATCH 3/4] Add tests for get_complaint_count endpoint * Added unit tests for the get_complaint_count endpoint * Updated the schema to use 'date' format instead of 'datetime' * Updated the complaint endpoint to convert start_date and end_date to be dates instead of strings --- app/complaint/complaint_rest.py | 19 ++++------ app/complaint/complaint_schema.py | 4 +-- tests/app/complaint/test_complaint_rest.py | 40 ++++++++++++++++++++++ 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/app/complaint/complaint_rest.py b/app/complaint/complaint_rest.py index 07574e06e..88123d796 100644 --- a/app/complaint/complaint_rest.py +++ b/app/complaint/complaint_rest.py @@ -23,19 +23,14 @@ def get_all_complaints(): @complaint_blueprint.route('/count-by-date-range', methods=['GET']) def get_complaint_count(): - request_json = request.args.to_dict - start_date = None - end_date = None - # TODO: unit tests have yet to be written, need to test setting start and end date - if request_json: - validate(request_json, complaint_count_request) - start_date = request_json.get('start_date', None) - end_date = request_json.get('end_date', None) - if not start_date: - start_date = datetime.utcnow().date() - if not end_date: - end_date = datetime.utcnow().date() + if request.args: + validate(request.args, complaint_count_request) + # If start and end date are not set, we are expecting today's stats. + today = str(datetime.utcnow().date()) + + start_date = datetime.strptime(request.args.get('start_date', today), '%Y-%m-%d').date() + end_date = datetime.strptime(request.args.get('end_date', today), '%Y-%m-%d').date() count_of_complaints = fetch_count_of_complaints(start_date=start_date, end_date=end_date) return jsonify(count_of_complaints), 200 diff --git a/app/complaint/complaint_schema.py b/app/complaint/complaint_schema.py index 6cd88d22c..d6c782405 100644 --- a/app/complaint/complaint_schema.py +++ b/app/complaint/complaint_schema.py @@ -5,7 +5,7 @@ complaint_count_request = { "type": "object", "title": "Complaint count request", "properties": { - "start_date": {"type": ["string", "null"], "format": "datetime"}, - "end_date": {"type": ["string", "null"], "format": "datetime"}, + "start_date": {"type": ["string", "null"], "format": "date"}, + "end_date": {"type": ["string", "null"], "format": "date"}, } } diff --git a/tests/app/complaint/test_complaint_rest.py b/tests/app/complaint/test_complaint_rest.py index 274c071d4..17de1406d 100644 --- a/tests/app/complaint/test_complaint_rest.py +++ b/tests/app/complaint/test_complaint_rest.py @@ -1,5 +1,9 @@ +from datetime import date import json +from flask import url_for +from freezegun import freeze_time + from tests import create_authorization_header from tests.app.db import create_complaint, create_service, create_template, create_notification @@ -22,3 +26,39 @@ def test_get_all_complaints_returns_empty_list(client): assert response.status_code == 200 assert json.loads(response.get_data(as_text=True)) == [] + + +def test_get_complaint_with_start_and_end_date_passes_these_to_dao_function(mocker, client): + start_date = date(2018, 6, 11) + end_date = date(2018, 6, 11) + dao_mock = mocker.patch('app.complaint.complaint_rest.fetch_count_of_complaints', return_value=3) + response = client.get( + url_for('complaint.get_complaint_count', start_date=start_date, end_date=end_date), + headers=[create_authorization_header()] + ) + + dao_mock.assert_called_once_with(start_date=start_date, end_date=end_date) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == 3 + + +@freeze_time("2018-06-01 11:00:00") +def test_get_complaint_sets_start_and_end_date_to_today_if_not_specified(mocker, client): + dao_mock = mocker.patch('app.complaint.complaint_rest.fetch_count_of_complaints', return_value=5) + response = client.get(url_for('complaint.get_complaint_count'), headers=[create_authorization_header()]) + + dao_mock.assert_called_once_with(start_date=date.today(), end_date=date.today()) + assert response.status_code == 200 + assert json.loads(response.get_data(as_text=True)) == 5 + + +def test_get_complaint_with_invalid_data_returns_400_status_code(client): + start_date = '1234-56-78' + response = client.get( + url_for('complaint.get_complaint_count', start_date=start_date), + headers=[create_authorization_header()] + ) + + assert response.status_code == 400 + assert response.json['errors'][0]['message'] == 'start_date time data {} does not match format %Y-%m-%d'.format( + start_date) From 8c22a6afdad87a6318934691bbea680650938928 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 12 Jun 2018 12:18:33 +0100 Subject: [PATCH 4/4] Change schema format name of datetime format Renamed the 'datetime' format to 'datetime_within_next_day'. This format is used to validate the date and time of scheduled notifications, not to check the format of a datetime. --- app/schema_validation/__init__.py | 2 +- app/v2/notifications/notification_schemas.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index b17efec6d..44cb0f9ce 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -29,7 +29,7 @@ def validate(json_to_validate, schema): validate_email_address(instance) return True - @format_checker.checks('datetime', raises=ValidationError) + @format_checker.checks('datetime_within_next_day', raises=ValidationError) def validate_schema_date_with_hour(instance): if isinstance(instance, str): try: diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 1e66ab7f6..39c78d727 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -136,7 +136,7 @@ post_sms_request = { "phone_number": {"type": "string", "format": "phone_number"}, "template_id": uuid, "personalisation": personalisation, - "scheduled_for": {"type": ["string", "null"], "format": "datetime"}, + "scheduled_for": {"type": ["string", "null"], "format": "datetime_within_next_day"}, "sms_sender_id": uuid }, "required": ["phone_number", "template_id"], @@ -182,7 +182,7 @@ post_email_request = { "email_address": {"type": "string", "format": "email_address"}, "template_id": uuid, "personalisation": personalisation, - "scheduled_for": {"type": ["string", "null"], "format": "datetime"}, + "scheduled_for": {"type": ["string", "null"], "format": "datetime_within_next_day"}, "email_reply_to_id": uuid }, "required": ["email_address", "template_id"],