diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 59263cd9f..c28b2a686 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -14,7 +14,7 @@ from notifications_utils.recipients import ( InvalidEmailError, ) from werkzeug.datastructures import MultiDict -from sqlalchemy import (desc, func, or_, and_, asc) +from sqlalchemy import (desc, func, or_, asc) from sqlalchemy.orm import joinedload from sqlalchemy.sql.expression import case from sqlalchemy.sql import functions @@ -22,13 +22,10 @@ from notifications_utils.international_billing_rates import INTERNATIONAL_BILLIN from app import db, create_uuid from app.dao import days_ago -from app.dao.date_util import get_financial_year from app.models import ( - Service, Notification, NotificationEmailReplyTo, NotificationHistory, - NotificationStatistics, ScheduledNotification, ServiceEmailReplyTo, Template, @@ -53,62 +50,6 @@ from app.dao.dao_utils import transactional from app.statsd_decorators import statsd -def dao_get_notification_statistics_for_service_and_day(service_id, day): - # only used by stat-updating code in tasks.py - return NotificationStatistics.query.filter_by( - service_id=service_id, - day=day - ).order_by(desc(NotificationStatistics.day)).first() - - -@statsd(namespace="dao") -def dao_get_potential_notification_statistics_for_day(day): - all_services = db.session.query( - Service.id, - NotificationStatistics - ).outerjoin( - NotificationStatistics, - and_( - Service.id == NotificationStatistics.service_id, - or_( - NotificationStatistics.day == day, - NotificationStatistics.day == None # noqa - ) - ) - ).order_by( - asc(Service.created_at) - ) - - notification_statistics = [] - for service_notification_stats_pair in all_services: - if service_notification_stats_pair.NotificationStatistics: - notification_statistics.append( - service_notification_stats_pair.NotificationStatistics - ) - else: - notification_statistics.append( - create_notification_statistics_dict( - service_notification_stats_pair, - day - ) - ) - return notification_statistics - - -def create_notification_statistics_dict(service_id, day): - return { - 'id': None, - 'emails_requested': 0, - 'emails_delivered': 0, - 'emails_failed': 0, - 'sms_requested': 0, - 'sms_delivered': 0, - 'sms_failed': 0, - 'day': day.isoformat(), - 'service': service_id - } - - @statsd(namespace="dao") def dao_get_template_usage(service_id, limit_days=None): query_filter = [] diff --git a/app/dao/provider_statistics_dao.py b/app/dao/provider_statistics_dao.py index 761e79de6..705d08fec 100644 --- a/app/dao/provider_statistics_dao.py +++ b/app/dao/provider_statistics_dao.py @@ -1,6 +1,7 @@ from sqlalchemy import func from app import db +from app.dao.date_util import get_financial_year from app.models import ( NotificationHistory, SMS_TYPE, @@ -8,7 +9,6 @@ from app.models import ( NOTIFICATION_STATUS_TYPES_BILLABLE, KEY_TYPE_TEST ) -from app.dao.notifications_dao import get_financial_year def get_fragment_count(service_id, year=None): diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index b22c07e58..93905e7e6 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -13,7 +13,6 @@ from app.dao.dao_utils import ( from app.dao.date_util import get_financial_year from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.models import ( - NotificationStatistics, ProviderStatistics, VerifyCode, ApiKey, @@ -232,7 +231,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(NotificationStatistics.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)) diff --git a/app/models.py b/app/models.py index ff8cb1acf..76bf0edfd 100644 --- a/app/models.py +++ b/app/models.py @@ -516,25 +516,6 @@ class KeyTypes(db.Model): name = db.Column(db.String(255), primary_key=True) -class NotificationStatistics(db.Model): - __tablename__ = 'notification_statistics' - - id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) - day = db.Column(db.Date, index=True, nullable=False, unique=False, default=datetime.date.today) - service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref=db.backref('service_notification_stats', lazy='dynamic')) - emails_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - emails_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - emails_failed = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - sms_requested = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - sms_delivered = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - sms_failed = db.Column(db.BigInteger, index=False, unique=False, nullable=False, default=0) - - __table_args__ = ( - UniqueConstraint('service_id', 'day', name='uix_service_to_day'), - ) - - class TemplateProcessTypes(db.Model): __tablename__ = 'template_process_type' name = db.Column(db.String(255), primary_key=True) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 85ee022a7..5a640b0e7 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -34,9 +34,7 @@ from app.schemas import ( email_notification_schema, sms_template_notification_schema, notification_with_personalisation_schema, - notifications_filter_schema, - notifications_statistics_schema, - day_schema + notifications_filter_schema ) from app.service.utils import service_allowed_to_send_to from app.utils import pagination_links, get_template_instance, get_public_notify_type_text @@ -86,16 +84,6 @@ def get_all_notifications(): ), 200 -@notifications.route('/notifications/statistics') -def get_notification_statistics_for_day(): - data = day_schema.load(request.args).data - statistics = notifications_dao.dao_get_potential_notification_statistics_for_day( - day=data['day'] - ) - data, errors = notifications_statistics_schema.dump(statistics, many=True) - return jsonify(data=data), 200 - - @notifications.route('/notifications/', methods=['POST']) def send_notification(notification_type): diff --git a/app/schemas.py b/app/schemas.py index b6ffe738c..a143e3bbb 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -325,18 +325,6 @@ class TemplateHistorySchema(BaseSchema): model = models.TemplateHistory -class NotificationsStatisticsSchema(BaseSchema): - class Meta: - model = models.NotificationStatistics - strict = True - - @pre_dump - def handle_date_str(self, in_data): - if isinstance(in_data, dict) and 'day' in in_data: - in_data['day'] = datetime.strptime(in_data['day'], '%Y-%m-%d').date() - return in_data - - class ApiKeySchema(BaseSchema): created_by = field_for(models.ApiKey, 'created_by', required=True) @@ -673,7 +661,6 @@ notification_with_personalisation_schema = NotificationWithPersonalisationSchema invited_user_schema = InvitedUserSchema() permission_schema = PermissionSchema() email_data_request_schema = EmailDataSchema() -notifications_statistics_schema = NotificationsStatisticsSchema() notifications_filter_schema = NotificationsFilterSchema() service_history_schema = ServiceHistorySchema() api_key_history_schema = ApiKeyHistorySchema() diff --git a/app/service/utils.py b/app/service/utils.py index 475d420e2..687f8939e 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -1,12 +1,13 @@ import itertools +from app.dao.date_util import get_financial_year from app.models import ( ServiceWhitelist, MOBILE_TYPE, EMAIL_TYPE, 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 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 7f3bd3008..e03b2b7dd 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -24,7 +24,6 @@ from app.models import ( ProviderDetails, ProviderDetailsHistory, ProviderRates, - NotificationStatistics, ScheduledNotification, ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, @@ -840,35 +839,6 @@ def sample_provider_statistics(notify_db, return stats -@pytest.fixture(scope='function') -def sample_notification_statistics(notify_db, - notify_db_session, - service=None, - day=None, - emails_requested=2, - emails_delivered=1, - emails_failed=1, - sms_requested=2, - sms_delivered=1, - sms_failed=1): - if service is None: - service = sample_service(notify_db, notify_db_session) - if day is None: - day = date.today() - stats = NotificationStatistics( - service=service, - day=day, - emails_requested=emails_requested, - emails_delivered=emails_delivered, - emails_failed=emails_failed, - sms_requested=sms_requested, - sms_delivered=sms_delivered, - sms_failed=sms_failed) - 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/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index eb8bac3a5..cec15552d 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -13,7 +13,6 @@ from app.models import ( NotificationEmailReplyTo, NotificationHistory, NotificationSmsSender, - NotificationStatistics, ScheduledNotification, EMAIL_TYPE, SMS_TYPE, @@ -38,8 +37,6 @@ from app.dao.notifications_dao import ( dao_get_notification_email_reply_for_notification, dao_get_notifications_by_to_field, dao_get_notification_sms_sender_mapping, - dao_get_notification_statistics_for_service_and_day, - dao_get_potential_notification_statistics_for_day, dao_get_scheduled_notifications, dao_get_template_usage, dao_timeout_notifications, @@ -79,7 +76,6 @@ from tests.app.conftest import ( def test_should_have_decorated_notifications_dao_functions(): assert dao_get_last_template_usage.__wrapped__.__name__ == 'dao_get_last_template_usage' # noqa assert dao_get_template_usage.__wrapped__.__name__ == 'dao_get_template_usage' # noqa - assert dao_get_potential_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_potential_notification_statistics_for_day' # noqa assert dao_create_notification.__wrapped__.__name__ == 'dao_create_notification' # noqa assert update_notification_status_by_id.__wrapped__.__name__ == 'update_notification_status_by_id' # noqa assert dao_update_notification.__wrapped__.__name__ == 'dao_update_notification' # noqa @@ -581,31 +577,10 @@ def test_should_return_zero_count_if_no_notification_with_reference(): assert not update_notification_status_by_reference('something', 'delivered') -def test_should_return_none_if_no_statistics_for_a_service_for_a_day(sample_template, mmg_provider): - data = _notification_json(sample_template) - - notification = Notification(**data) - dao_create_notification(notification) - assert not dao_get_notification_statistics_for_service_and_day( - sample_template.service.id, (datetime.utcnow() - timedelta(days=1)).date()) - - -def test_should_be_able_to_get_all_statistics_for_a_service(sample_template, mmg_provider): - data = _notification_json(sample_template) - - notification_1 = Notification(**data) - notification_2 = Notification(**data) - notification_3 = Notification(**data) - dao_create_notification(notification_1) - dao_create_notification(notification_2) - dao_create_notification(notification_3) - - def test_create_notification_creates_notification_with_personalisation(notify_db, notify_db_session, sample_template_with_placeholders, sample_job, mmg_provider): assert Notification.query.count() == 0 - assert NotificationStatistics.query.count() == 0 data = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, template=sample_template_with_placeholders, @@ -628,7 +603,6 @@ def test_create_notification_creates_notification_with_personalisation(notify_db def test_save_notification_creates_sms(sample_template, sample_job, mmg_provider): assert Notification.query.count() == 0 - assert NotificationStatistics.query.count() == 0 data = _notification_json(sample_template, job_id=sample_job.id) @@ -649,7 +623,6 @@ def test_save_notification_creates_sms(sample_template, sample_job, mmg_provider def test_save_notification_and_create_email(sample_email_template, sample_job): assert Notification.query.count() == 0 - assert NotificationStatistics.query.count() == 0 data = _notification_json(sample_email_template, job_id=sample_job.id) @@ -773,7 +746,6 @@ def test_not_save_notification_and_not_create_stats_on_commit_error(sample_templ assert Notification.query.count() == 0 assert Job.query.get(sample_job.id).notifications_sent == 0 - assert NotificationStatistics.query.count() == 0 def test_save_notification_and_increment_job(sample_template, sample_job, mmg_provider): diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 70bc79eef..76c06d088 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -35,7 +35,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 ( - NotificationStatistics, ProviderStatistics, VerifyCode, ApiKey, @@ -447,7 +446,6 @@ def test_delete_service_and_associated_objects(notify_db, assert ServicePermission.query.count() == 3 delete_service_and_all_associated_db_objects(sample_service) - assert NotificationStatistics.query.count() == 0 assert ProviderStatistics.query.count() == 0 assert VerifyCode.query.count() == 0 assert ApiKey.query.count() == 0 diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 03fede0fa..ff2cb417e 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -9,7 +9,6 @@ import app.celery.tasks from app.dao.notifications_dao import ( get_notification_by_id ) -from app.models import NotificationStatistics from tests.app.notifications.test_notifications_ses_callback import ses_confirmation_callback from tests.app.conftest import sample_notification as create_sample_notification @@ -431,10 +430,6 @@ def test_firetext_callback_should_record_statsd(client, notify_db, notify_db_ses app.statsd_client.incr.assert_any_call("callback.firetext.delivered") -def get_notification_stats(service_id): - return NotificationStatistics.query.filter_by(service_id=service_id).one() - - def _sample_sns_s3_callback(): return json.dumps({ "SigningCertURL": "foo.pem", diff --git a/tests/app/notifications/rest/test_notification_statistics.py b/tests/app/notifications/rest/test_notification_statistics.py deleted file mode 100644 index 6e9de437a..000000000 --- a/tests/app/notifications/rest/test_notification_statistics.py +++ /dev/null @@ -1,225 +0,0 @@ -from datetime import date, timedelta - -from flask import json -from freezegun import freeze_time -from datetime import datetime - -from tests import create_authorization_header -from tests.app.conftest import ( - sample_notification_statistics as create_sample_notification_statistics, - sample_service as create_sample_service -) - - -def test_get_notification_statistics(notify_api, sample_notification_statistics): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header( - service_id=sample_notification_statistics.service_id - ) - - response = client.get( - '/notifications/statistics?day={}'.format(date.today().isoformat()), - headers=[auth_header] - ) - - notifications = json.loads(response.get_data(as_text=True)) - - assert len(notifications['data']) == 1 - stats = notifications['data'][0] - assert stats['emails_requested'] == 2 - assert stats['emails_delivered'] == 1 - assert stats['emails_failed'] == 1 - assert stats['sms_requested'] == 2 - assert stats['sms_delivered'] == 1 - assert stats['sms_failed'] == 1 - assert stats['service'] == str(sample_notification_statistics.service_id) - assert response.status_code == 200 - - -@freeze_time('1955-11-05T12:00:00') -def test_get_notification_statistics_only_returns_today(notify_api, notify_db, notify_db_session, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - yesterdays_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - service=sample_service, - day=date.today() - timedelta(days=1) - ) - todays_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - service=sample_service, - day=date.today() - ) - tomorrows_notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - service=sample_service, - day=date.today() + timedelta(days=1) - ) - - auth_header = create_authorization_header( - service_id=sample_service.id - ) - - response = client.get( - '/notifications/statistics?day={}'.format(date.today().isoformat()), - headers=[auth_header] - ) - - notifications = json.loads(response.get_data(as_text=True)) - - assert len(notifications['data']) == 1 - assert notifications['data'][0]['day'] == date.today().isoformat() - assert response.status_code == 200 - - -def test_get_notification_statistics_fails_if_no_date(notify_api, sample_notification_statistics): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header( - service_id=sample_notification_statistics.service_id - ) - - response = client.get( - '/notifications/statistics', - headers=[auth_header] - ) - - resp = json.loads(response.get_data(as_text=True)) - assert resp['result'] == 'error' - assert resp['message'] == {'day': ['Missing data for required field.']} - assert response.status_code == 400 - - -def test_get_notification_statistics_fails_if_invalid_date(notify_api, sample_notification_statistics): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header( - service_id=sample_notification_statistics.service_id - ) - - response = client.get( - '/notifications/statistics?day=2016-99-99', - headers=[auth_header] - ) - - resp = json.loads(response.get_data(as_text=True)) - assert resp['result'] == 'error' - assert resp['message'] == {'day': ['Not a valid date.']} - assert response.status_code == 400 - - -def test_get_notification_statistics_returns_zeros_if_not_in_db(notify_api, sample_service): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - auth_header = create_authorization_header( - service_id=sample_service.id - ) - - response = client.get( - '/notifications/statistics?day={}'.format(date.today().isoformat()), - headers=[auth_header] - ) - - notifications = json.loads(response.get_data(as_text=True)) - - assert len(notifications['data']) == 1 - stats = notifications['data'][0] - assert stats['emails_requested'] == 0 - assert stats['emails_delivered'] == 0 - assert stats['emails_failed'] == 0 - assert stats['sms_requested'] == 0 - assert stats['sms_delivered'] == 0 - assert stats['sms_failed'] == 0 - assert stats['service'] == str(sample_service.id) - assert response.status_code == 200 - - -def test_get_notification_statistics_returns_both_existing_stats_and_generated_zeros( - notify_api, - notify_db, - notify_db_session -): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - service_with_stats = create_sample_service( - notify_db, - notify_db_session, - service_name='service_with_stats', - email_from='service_with_stats' - ) - service_without_stats = create_sample_service( - notify_db, - notify_db_session, - service_name='service_without_stats', - email_from='service_without_stats' - ) - notification_statistics = create_sample_notification_statistics( - notify_db, - notify_db_session, - service=service_with_stats, - day=date.today() - ) - auth_header = create_authorization_header( - service_id=service_with_stats.id - ) - - response = client.get( - '/notifications/statistics?day={}'.format(date.today().isoformat()), - headers=[auth_header] - ) - - notifications = json.loads(response.get_data(as_text=True)) - - assert len(notifications['data']) == 2 - retrieved_stats = notifications['data'][0] - generated_stats = notifications['data'][1] - - assert retrieved_stats['emails_requested'] == 2 - assert retrieved_stats['emails_delivered'] == 1 - assert retrieved_stats['emails_failed'] == 1 - assert retrieved_stats['sms_requested'] == 2 - assert retrieved_stats['sms_delivered'] == 1 - assert retrieved_stats['sms_failed'] == 1 - assert retrieved_stats['service'] == str(service_with_stats.id) - - assert generated_stats['emails_requested'] == 0 - assert generated_stats['emails_delivered'] == 0 - assert generated_stats['emails_failed'] == 0 - assert generated_stats['sms_requested'] == 0 - assert generated_stats['sms_delivered'] == 0 - assert generated_stats['sms_failed'] == 0 - assert generated_stats['service'] == str(service_without_stats.id) - - assert response.status_code == 200 - - -def test_get_notification_statistics_returns_zeros_when_only_stats_for_different_date( - notify_api, - sample_notification_statistics -): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - with freeze_time('1985-10-26T00:06:00'): - auth_header = create_authorization_header( - service_id=sample_notification_statistics.service_id - ) - response = client.get( - '/notifications/statistics?day={}'.format(datetime.utcnow().isoformat()), - headers=[auth_header] - ) - - notifications = json.loads(response.get_data(as_text=True)) - - assert response.status_code == 200 - assert len(notifications['data']) == 1 - assert notifications['data'][0]['emails_requested'] == 0 - assert notifications['data'][0]['emails_delivered'] == 0 - assert notifications['data'][0]['emails_failed'] == 0 - assert notifications['data'][0]['sms_requested'] == 0 - assert notifications['data'][0]['sms_delivered'] == 0 - assert notifications['data'][0]['sms_failed'] == 0 - assert notifications['data'][0]['service'] == str(sample_notification_statistics.service_id)