From 58b0658a132a8ffd4ecac8d2228794846ed599a7 Mon Sep 17 00:00:00 2001 From: Richard Chapman Date: Mon, 20 Nov 2017 10:01:45 +0000 Subject: [PATCH 1/3] Return financial year not calendar and ensure double precision values are not returned from queries - Updated stats_template_usage_by_month_dao.py to return the results for financial year not calendar, as the report os for FY only and hence only the FY data is required - Updated services_dao.py to ensure double precision values are converted to an int as the 'exact' function returns double precision from the database query, as the admin code requires the value for month to be an int --- app/dao/services_dao.py | 8 +- app/dao/stats_template_usage_by_month_dao.py | 15 +- tests/app/dao/test_services_dao.py | 148 ++++++++++++++++++ .../test_stats_template_usage_by_month_dao.py | 70 ++++++--- 4 files changed, 210 insertions(+), 31 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 0b91e2267..cafca04fd 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -550,7 +550,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) results = dao_get_template_usage_stats_by_service(service_id, year) - stats = list() + stats = [] for result in results: stat = type("", (), {})() stat.template_id = result.template_id @@ -575,7 +575,7 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) ).join( Template, Notification.template_id == Template.id, ).filter( - Notification.created_at >= start_date + Notification.created_at >= start_date, ).group_by( Notification.template_id, Template.name, @@ -599,8 +599,8 @@ def dao_fetch_monthly_historical_usage_by_template_for_service(service_id, year) 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 + new_stat.month = int(today_result.month) + new_stat.year = int(today_result.year) new_stat.count = today_result.count stats.append(new_stat) diff --git a/app/dao/stats_template_usage_by_month_dao.py b/app/dao/stats_template_usage_by_month_dao.py index 84d2550a6..aa88924f9 100644 --- a/app/dao/stats_template_usage_by_month_dao.py +++ b/app/dao/stats_template_usage_by_month_dao.py @@ -1,3 +1,5 @@ +from sqlalchemy import or_, and_ + from app import db from app.statsd_decorators import statsd from app.dao.dao_utils import transactional @@ -41,6 +43,15 @@ def dao_get_template_usage_stats_by_service(service_id, year): ).join( Template, StatsTemplateUsageByMonth.template_id == Template.id ).filter( - Template.service_id == service_id, - StatsTemplateUsageByMonth.year == year + Template.service_id == service_id + ).filter( + or_( + and_( + StatsTemplateUsageByMonth.month.in_([4, 5, 6, 7, 8, 9, 10, 11, 12]), + StatsTemplateUsageByMonth.year == year + ), and_( + StatsTemplateUsageByMonth.month.in_([1, 2, 3]), + StatsTemplateUsageByMonth.year == year + 1 + ) + ) ).all() diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 05c564c02..926e5ea57 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -1380,3 +1380,151 @@ def test_dao_fetch_monthly_historical_usage_by_template_for_service_combined_his assert result[1].month == 11 assert result[1].year == 2017 assert result[1].count == 1 + + +@freeze_time("2017-11-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_does_not_return_double_precision_values( + notify_db, + notify_db_session, + sample_service +): + + template_one = create_sample_template(notify_db, notify_db_session, template_name='1') + + n = 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) == 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 == 11 + assert len(str(result[0].month)) == 2 + assert result[0].year == 2017 + assert len(str(result[0].year)) == 4 + assert result[0].count == 1 + + +@freeze_time("2018-03-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_returns_financial_year( + notify_db, + notify_db_session, + sample_service +): + template_one = create_sample_template(notify_db, notify_db_session, template_name='1', template_type='email') + + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered', + sample_template=template_one + ) + + date = datetime.now() + day = date.day + year = date.year + + notification_history(created_at=datetime(year - 1, 1, day)) + notification_history(created_at=datetime(year - 1, 3, day)) + notification_history(created_at=datetime(year - 1, 4, day)) + notification_history(created_at=datetime(year - 1, 5, day)) + notification_history(created_at=datetime(year, 1, day)) + notification_history(created_at=datetime(year, 2, day)) + + daily_stats_template_usage_by_month() + + n = 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.year, x.month) + ) + + assert len(result) == 5 + + assert result[0].month == 4 + assert result[0].year == 2017 + assert result[1].month == 5 + assert result[1].year == 2017 + assert result[2].month == 1 + assert result[2].year == 2018 + assert result[3].month == 2 + assert result[3].year == 2018 + assert result[4].month == 3 + assert result[4].year == 2018 + + +@freeze_time("2018-03-10 11:09:00.000000") +def test_dao_fetch_monthly_historical_usage_by_template_for_service_only_returns_for_service( + notify_db, + notify_db_session, + sample_service +): + template_one = create_sample_template(notify_db, notify_db_session, template_name='1', template_type='email') + + notification_history = functools.partial( + create_notification_history, + notify_db, + notify_db_session, + status='delivered', + sample_template=template_one + ) + + date = datetime.now() + day = date.day + year = date.year + + notification_history(created_at=datetime(year, 1, day)) + notification_history(created_at=datetime(year, 2, day)) + + service_two = create_service(service_name='other_service') + + daily_stats_template_usage_by_month() + + n = create_notification( + notify_db, + notify_db_session, + service=sample_service, + template=template_one, + created_at=datetime.utcnow() + ) + + create_notification( + notify_db, + notify_db_session, + service=service_two, + 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.year, x.month) + ) + + assert len(result) == 3 + + result = sorted( + dao_fetch_monthly_historical_usage_by_template_for_service(service_two.id, 2017), + key=lambda x: (x.year, x.month) + ) + + assert len(result) == 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 99112c6ca..7d21696b5 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 @@ -51,55 +51,75 @@ def test_dao_get_template_usage_stats_by_service(sample_service): email_template = create_template(service=sample_service, template_type="email") - stats1 = StatsTemplateUsageByMonth( + new_service = create_service(service_name="service_one") + + template_new_service = create_template(service=new_service) + + db.session.add(StatsTemplateUsageByMonth( template_id=email_template.id, - month=1, + month=4, year=2017, count=10 - ) + )) - stats2 = StatsTemplateUsageByMonth( - template_id=email_template.id, - month=2, + db.session.add(StatsTemplateUsageByMonth( + template_id=template_new_service.id, + month=4, year=2017, count=10 - ) - - db.session.add(stats1) - db.session.add(stats2) + )) result = dao_get_template_usage_stats_by_service(sample_service.id, 2017) - assert len(result) == 2 + assert len(result) == 1 def test_dao_get_template_usage_stats_by_service_specific_year(sample_service): email_template = create_template(service=sample_service, template_type="email") - stats1 = StatsTemplateUsageByMonth( + db.session.add(StatsTemplateUsageByMonth( template_id=email_template.id, - month=1, - year=2016, - count=10 - ) - - stats2 = StatsTemplateUsageByMonth( - template_id=email_template.id, - month=2, + month=3, year=2017, count=10 - ) + )) - db.session.add(stats1) - db.session.add(stats2) + db.session.add(StatsTemplateUsageByMonth( + template_id=email_template.id, + month=4, + year=2017, + count=10 + )) + + db.session.add(StatsTemplateUsageByMonth( + template_id=email_template.id, + month=3, + year=2018, + count=10 + )) + + db.session.add(StatsTemplateUsageByMonth( + template_id=email_template.id, + month=4, + year=2018, + count=10 + )) result = dao_get_template_usage_stats_by_service(sample_service.id, 2017) - assert len(result) == 1 + assert len(result) == 2 + 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].month == 4 assert result[0].year == 2017 assert result[0].count == 10 + + assert result[1].template_id == email_template.id + assert result[1].name == email_template.name + assert result[1].template_type == email_template.template_type + assert result[1].month == 3 + assert result[1].year == 2018 + assert result[1].count == 10 From 93f19956b8f09488c52e2da8cced3130b5e5c2a8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 20 Nov 2017 13:13:13 +0000 Subject: [PATCH 2/3] Remove notification_statistics and Service.sms_sender --- migrations/versions/0141_remove_unused.py | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 migrations/versions/0141_remove_unused.py diff --git a/migrations/versions/0141_remove_unused.py b/migrations/versions/0141_remove_unused.py new file mode 100644 index 000000000..edb8584f5 --- /dev/null +++ b/migrations/versions/0141_remove_unused.py @@ -0,0 +1,39 @@ +""" + +Revision ID: 0141_remove_unused +Revises: 0140_sms_prefix_non_nullable +Create Date: 2017-11-20 11:35:24.402021 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0141_remove_unused' +down_revision = '0140_sms_prefix_non_nullable' + + +def upgrade(): + op.drop_table('notification_statistics') + op.drop_column('services', 'sms_sender') + op.drop_column('services_history', 'sms_sender') + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('services_history', sa.Column('sms_sender', sa.VARCHAR(length=11), autoincrement=False, nullable=True)) + op.add_column('services', sa.Column('sms_sender', sa.VARCHAR(length=11), autoincrement=False, nullable=True)) + op.create_table('notification_statistics', + sa.Column('id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('service_id', postgresql.UUID(), autoincrement=False, nullable=False), + sa.Column('emails_requested', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('emails_delivered', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('emails_failed', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_requested', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_delivered', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('sms_failed', sa.BIGINT(), autoincrement=False, nullable=False), + sa.Column('day', sa.DATE(), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['service_id'], ['services.id'], name='notification_statistics_service_id_fkey'), + sa.PrimaryKeyConstraint('id', name='notification_statistics_pkey'), + sa.UniqueConstraint('service_id', 'day', name='uix_service_to_day') + ) From 0a50993fc23c14cccf21cf5383a694f09c665b22 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Wed, 15 Nov 2017 14:48:50 +0000 Subject: [PATCH 3/3] Validate notifcations.template_history foreign key constraints Removes "NOT VALID" mark from the notifications -> template_history foreign key constraint by validating existing records. From postgres docs: > This form validates a foreign key or check constraint that was > previously created as NOT VALID, by scanning the table to ensure there > are no rows for which the constraint is not satisfied. Nothing happens > if the constraint is already marked valid. > Validation can be a long process on larger tables. The value of > separating validation from initial creation is that you can defer > validation to less busy times, or can be used to give additional time > to correct pre-existing errors while preventing new errors. Note also > that validation on its own does not prevent normal write commands > against the table while it runs. > Validation acquires only a SHARE UPDATE EXCLUSIVE lock on the table > being altered. If the constraint is a foreign key then a ROW SHARE > lock is also required on the table referenced by the constraint. --- .../versions/0142_validate_constraints.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 migrations/versions/0142_validate_constraints.py diff --git a/migrations/versions/0142_validate_constraints.py b/migrations/versions/0142_validate_constraints.py new file mode 100644 index 000000000..dea88b707 --- /dev/null +++ b/migrations/versions/0142_validate_constraints.py @@ -0,0 +1,21 @@ +""" + +Revision ID: 0142_validate_constraint +Revises: 0141_remove_unused +Create Date: 2017-11-15 14:39:13.657666 + +""" +from alembic import op +from sqlalchemy.dialects import postgresql + +revision = '0142_validate_constraint' +down_revision = '0141_remove_unused' + + +def upgrade(): + op.execute('ALTER TABLE notifications VALIDATE CONSTRAINT "notifications_templates_history_fkey"') + op.execute('ALTER TABLE notification_history VALIDATE CONSTRAINT "notification_history_templates_history_fkey"') + + +def downgrade(): + pass