From 71796311fa93406431681d181d256256205ba95c Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 21 May 2018 11:23:54 +0100 Subject: [PATCH 1/2] Delete unused aggregate_statistics endpoint `@service_blueprint.route('//fragment/aggregate_statistics')` is not being used anywhere, so has been removed. The `provider_statistics_dao` can also be removed, since the function this contained was only used for the endpoint. --- app/dao/provider_statistics_dao.py | 42 --------- app/service/rest.py | 12 --- tests/app/dao/test_provider_statistics_dao.py | 90 ------------------- tests/app/service/test_rest.py | 20 ----- 4 files changed, 164 deletions(-) delete mode 100644 app/dao/provider_statistics_dao.py delete mode 100644 tests/app/dao/test_provider_statistics_dao.py diff --git a/app/dao/provider_statistics_dao.py b/app/dao/provider_statistics_dao.py deleted file mode 100644 index 705d08fec..000000000 --- a/app/dao/provider_statistics_dao.py +++ /dev/null @@ -1,42 +0,0 @@ -from sqlalchemy import func - -from app import db -from app.dao.date_util import get_financial_year -from app.models import ( - NotificationHistory, - SMS_TYPE, - EMAIL_TYPE, - NOTIFICATION_STATUS_TYPES_BILLABLE, - KEY_TYPE_TEST -) - - -def get_fragment_count(service_id, year=None): - shared_filters = [ - NotificationHistory.service_id == service_id, - NotificationHistory.status.in_(NOTIFICATION_STATUS_TYPES_BILLABLE), - NotificationHistory.key_type != KEY_TYPE_TEST - ] - - if year: - shared_filters.append(NotificationHistory.created_at.between( - *get_financial_year(year) - )) - - sms_count = db.session.query( - func.sum(NotificationHistory.billable_units) - ).filter( - NotificationHistory.notification_type == SMS_TYPE, - *shared_filters - ) - - email_count = db.session.query( - func.count(NotificationHistory.id) - ).filter( - NotificationHistory.notification_type == EMAIL_TYPE, - *shared_filters - ) - return { - 'sms_count': int(sms_count.scalar() or 0), - 'email_count': email_count.scalar() or 0 - } diff --git a/app/service/rest.py b/app/service/rest.py index 136ddc99e..2f0d516f4 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -65,7 +65,6 @@ from app.dao.service_letter_contact_dao import ( add_letter_contact_for_service, update_letter_contact ) -from app.dao.provider_statistics_dao import get_fragment_count from app.dao.users_dao import get_user_by_id from app.errors import ( InvalidRequest, @@ -299,17 +298,6 @@ def remove_user_from_service(service_id, user_id): return jsonify({}), 204 -@service_blueprint.route('//fragment/aggregate_statistics') -def get_service_provider_aggregate_statistics(service_id): - year = request.args.get('year') - if year is not None: - try: - year = int(year) - except ValueError: - raise InvalidRequest('Year must be a number', status_code=400) - return jsonify(data=get_fragment_count(service_id, year=year)) - - # This is placeholder get method until more thought # goes into how we want to fetch and view various items in history # tables. This is so product owner can pass stories as done diff --git a/tests/app/dao/test_provider_statistics_dao.py b/tests/app/dao/test_provider_statistics_dao.py deleted file mode 100644 index c34395c76..000000000 --- a/tests/app/dao/test_provider_statistics_dao.py +++ /dev/null @@ -1,90 +0,0 @@ -from datetime import datetime -import uuid - -import pytest -from freezegun import freeze_time - -from app.models import ( - NotificationHistory, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, - NOTIFICATION_STATUS_TYPES_BILLABLE -) -from app.dao.provider_statistics_dao import get_fragment_count - - -def test_get_fragment_count_with_no_data(sample_template): - assert get_fragment_count(sample_template.service_id)['sms_count'] == 0 - assert get_fragment_count(sample_template.service_id)['email_count'] == 0 - - -def test_get_fragment_count_separates_sms_and_email(notify_db, sample_template, sample_email_template): - noti_hist(notify_db, sample_template) - noti_hist(notify_db, sample_template) - noti_hist(notify_db, sample_email_template) - assert get_fragment_count(sample_template.service_id) == { - 'sms_count': 2, - 'email_count': 1 - } - - -def test_get_fragment_count_filters_on_status(notify_db, sample_template): - for status in NOTIFICATION_STATUS_TYPES_BILLABLE: - noti_hist(notify_db, sample_template, status=status) - # sending, sent, delivered, failed, temporary-failure, permanent-failure - assert get_fragment_count(sample_template.service_id)['sms_count'] == 6 - - -def test_get_fragment_count_filters_on_service_id(notify_db, sample_template, service_factory): - service_2 = service_factory.get('service 2', email_from='service.2') - noti_hist(notify_db, sample_template) - assert get_fragment_count(service_2.id)['sms_count'] == 0 - - -@pytest.mark.parametrize('creation_time, expected_count', [ - ('2000-03-31 22:59:59', 0), # before the start of the year - ('2000-04-01 00:00:00', 1), # after the start of the year - ('2001-03-31 22:59:59', 1), # before the end of the year - ('2001-04-01 00:00:00', 0), # after the end of the year -]) -def test_get_fragment_count_filters_on_year( - notify_db, sample_template, creation_time, expected_count -): - with freeze_time(creation_time): - noti_hist(notify_db, sample_template) - assert get_fragment_count(sample_template.service_id, year=2000)['sms_count'] == expected_count - - -def test_get_fragment_count_sums_billable_units_for_sms(notify_db, sample_template): - noti_hist(notify_db, sample_template, billable_units=1) - noti_hist(notify_db, sample_template, billable_units=2) - assert get_fragment_count(sample_template.service_id)['sms_count'] == 3 - - -@pytest.mark.parametrize('key_type,sms_count', [ - (KEY_TYPE_NORMAL, 1), - (KEY_TYPE_TEAM, 1), - (KEY_TYPE_TEST, 0), -]) -def test_get_fragment_count_ignores_test_api_keys(notify_db, sample_template, key_type, sms_count): - noti_hist(notify_db, sample_template, key_type=key_type) - assert get_fragment_count(sample_template.service_id)['sms_count'] == sms_count - - -def noti_hist(notify_db, template, status='delivered', billable_units=None, key_type=KEY_TYPE_NORMAL): - if not billable_units and template.template_type == 'sms': - billable_units = 1 - - notification_history = NotificationHistory( - id=uuid.uuid4(), - service=template.service, - template_id=template.id, - template_version=template.version, - status=status, - created_at=datetime.utcnow(), - billable_units=billable_units, - notification_type=template.template_type, - key_type=key_type - ) - notify_db.session.add(notification_history) - notify_db.session.commit() - - return notification_history diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 1f19922ad..23c273727 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1678,26 +1678,6 @@ def test_get_detailed_services_for_date_range(notify_db, notify_db_session, set_ } -@pytest.mark.parametrize('query_string, expected_status, expected_json', [ - ('', 200, {'data': {'email_count': 0, 'sms_count': 0}}), - ('?year=2000', 200, {'data': {'email_count': 0, 'sms_count': 0}}), - ('?year=abcd', 400, {'message': 'Year must be a number', 'result': 'error'}), -]) -def test_get_service_provider_aggregate_statistics( - client, - sample_service, - query_string, - expected_status, - expected_json, -): - response = client.get( - '/service/{}/fragment/aggregate_statistics{}'.format(sample_service.id, query_string), - headers=[create_authorization_header()] - ) - assert response.status_code == expected_status - assert json.loads(response.get_data(as_text=True)) == expected_json - - @freeze_time('2017-11-11 02:00') def test_get_template_usage_by_month_returns_correct_data( notify_db, From 0d4fb81235881909d59a14fd40a64bbc2b9ae6b3 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 21 May 2018 12:12:53 +0100 Subject: [PATCH 2/2] Delete ProviderStatistics model This is not being used anywhere, so can be deleted safely. --- app/dao/services_dao.py | 2 -- app/models.py | 14 -------------- tests/app/conftest.py | 25 +------------------------ tests/app/dao/test_services_dao.py | 5 +---- 4 files changed, 2 insertions(+), 44 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5f326b372..68f0ea781 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -23,7 +23,6 @@ from app.models import ( Notification, NotificationHistory, Permission, - ProviderStatistics, Service, ServicePermission, ServiceSmsSender, @@ -223,7 +222,6 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(TemplateRedacted.query.filter(TemplateRedacted.template_id.in_(subq))) _delete_commit(ServiceSmsSender.query.filter_by(service=service)) - _delete_commit(ProviderStatistics.query.filter_by(service=service)) _delete_commit(InvitedUser.query.filter_by(service=service)) _delete_commit(Permission.query.filter_by(service=service)) _delete_commit(NotificationHistory.query.filter_by(service=service)) diff --git a/app/models.py b/app/models.py index 2984edeab..d55fff3ca 100644 --- a/app/models.py +++ b/app/models.py @@ -864,20 +864,6 @@ NOTIFICATION_TYPE = [EMAIL_TYPE, SMS_TYPE, LETTER_TYPE] notification_types = db.Enum(*NOTIFICATION_TYPE, name='notification_type') -class ProviderStatistics(db.Model): - __tablename__ = 'provider_statistics' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - day = db.Column(db.Date, nullable=False) - provider_id = db.Column(UUID(as_uuid=True), db.ForeignKey('provider_details.id'), index=True, nullable=False) - provider = db.relationship( - 'ProviderDetails', backref=db.backref('provider_stats', lazy='dynamic') - ) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref=db.backref('service_provider_stats', lazy='dynamic')) - unit_count = db.Column(db.BigInteger, nullable=False) - - class ProviderRates(db.Model): __tablename__ = 'provider_rates' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 9dad50c18..ed6d432c8 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,4 +1,4 @@ -from datetime import (datetime, date, timedelta) +from datetime import datetime, timedelta import json import uuid @@ -22,7 +22,6 @@ from app.models import ( NotificationHistory, InvitedUser, Permission, - ProviderStatistics, ProviderDetails, ProviderDetailsHistory, ProviderRates, @@ -839,28 +838,6 @@ def mmg_provider(): return ProviderDetails.query.filter_by(identifier='mmg').one() -@pytest.fixture(scope='function') -def sample_provider_statistics(notify_db, - notify_db_session, - sample_service, - provider=None, - day=None, - unit_count=1): - - if provider is None: - provider = ProviderDetails.query.filter_by(identifier='mmg').first() - if day is None: - day = date.today() - stats = ProviderStatistics( - service=sample_service, - provider_id=provider.id, - day=day, - unit_count=unit_count) - notify_db.session.add(stats) - notify_db.session.commit() - return stats - - @pytest.fixture(scope='function') def mock_firetext_client(mocker, statsd_client=None): client = FiretextClient() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index b0c5a0ccb..997920fd5 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -37,7 +37,6 @@ from app.dao.services_dao import ( from app.dao.service_permissions_dao import dao_add_service_permission, dao_remove_service_permission from app.dao.users_dao import save_model_user from app.models import ( - ProviderStatistics, VerifyCode, ApiKey, Template, @@ -460,14 +459,12 @@ def test_delete_service_and_associated_objects(notify_db, sample_job, sample_notification, sample_invited_user, - sample_permission, - sample_provider_statistics): + sample_permission): assert ServicePermission.query.count() == len(( SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, INTERNATIONAL_SMS_TYPE )) delete_service_and_all_associated_db_objects(sample_service) - assert ProviderStatistics.query.count() == 0 assert VerifyCode.query.count() == 0 assert ApiKey.query.count() == 0 assert ApiKey.get_history_model().query.count() == 0