diff --git a/app/dao/fact_notification_status_dao.py b/app/dao/fact_notification_status_dao.py index 1390201e8..8925c81f0 100644 --- a/app/dao/fact_notification_status_dao.py +++ b/app/dao/fact_notification_status_dao.py @@ -186,3 +186,14 @@ def fetch_notification_status_totals_for_all_services(start_date, end_date): else: query = stats return query.all() + + +def fetch_notification_statuses_for_job(job_id): + return db.session.query( + FactNotificationStatus.notification_status.label('status'), + func.sum(FactNotificationStatus.notification_count).label('count'), + ).filter( + FactNotificationStatus.job_id == job_id, + ).group_by( + FactNotificationStatus.notification_status + ).all() diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 5eaf71457..e22c1c709 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -16,7 +16,7 @@ from app.models import ( JOB_STATUS_PENDING, JOB_STATUS_SCHEDULED, LETTER_TYPE, - NotificationHistory, + Notification, Template, ServiceDataRetention ) @@ -25,19 +25,14 @@ from app.variables import LETTER_TEST_API_FILENAME @statsd(namespace="dao") def dao_get_notification_outcomes_for_job(service_id, job_id): - query = db.session.query( - func.count(NotificationHistory.status).label('count'), - NotificationHistory.status - ) - - return query.filter( - NotificationHistory.service_id == service_id + return db.session.query( + func.count(Notification.status).label('count'), + Notification.status ).filter( - NotificationHistory.job_id == job_id + Notification.service_id == service_id, + Notification.job_id == job_id ).group_by( - NotificationHistory.status - ).order_by( - asc(NotificationHistory.status) + Notification.status ).all() diff --git a/app/job/rest.py b/app/job/rest.py index 6e64e045c..61665ec45 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -1,3 +1,4 @@ +import dateutil from flask import ( Blueprint, jsonify, @@ -13,6 +14,7 @@ from app.dao.jobs_dao import ( dao_get_jobs_by_service_id, dao_get_future_scheduled_job_by_id_and_service_id, dao_get_notification_outcomes_for_job) +from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job from app.dao.services_dao import dao_fetch_service_by_id from app.dao.templates_dao import dao_get_template_by_id from app.dao.notifications_dao import get_notifications_for_job @@ -24,7 +26,7 @@ from app.schemas import ( ) from app.celery.tasks import process_job from app.models import JOB_STATUS_SCHEDULED, JOB_STATUS_PENDING, JOB_STATUS_CANCELLED, LETTER_TYPE -from app.utils import pagination_links +from app.utils import pagination_links, midnight_n_days_ago from app.config import QueueNames from app.errors import ( register_errors, @@ -171,8 +173,14 @@ def get_paginated_jobs(service_id, limit_days, statuses, page): ) data = job_schema.dump(pagination.items, many=True).data for job_data in data: - statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id']) - job_data['statistics'] = [{'status': statistic[1], 'count': statistic[0]} for statistic in statistics] + created_at = dateutil.parser.parse(job_data['created_at']).replace(tzinfo=None) + if created_at < midnight_n_days_ago(3): + # ft_notification_status table + statistics = fetch_notification_statuses_for_job(job_data['id']) + else: + # notifications table + statistics = dao_get_notification_outcomes_for_job(service_id, job_data['id']) + job_data['statistics'] = [{'status': statistic.status, 'count': statistic.count} for statistic in statistics] return { 'data': data, diff --git a/tests/app/conftest.py b/tests/app/conftest.py index ef7317acb..15f76846f 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1101,7 +1101,9 @@ def restore_provider_details(notify_db, notify_db_session): @pytest.fixture def admin_request(client): + class AdminRequest: + app = client.application @staticmethod def get(endpoint, _expected_status=200, **endpoint_kwargs): diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index 9dcb975eb..db90c6d61 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -9,11 +9,12 @@ from app.dao.fact_notification_status_dao import ( fetch_notification_status_for_service_by_month, fetch_notification_status_for_service_for_day, fetch_notification_status_for_service_for_today_and_7_previous_days, - fetch_notification_status_totals_for_all_services + fetch_notification_status_totals_for_all_services, + fetch_notification_statuses_for_job, ) from app.models import FactNotificationStatus, KEY_TYPE_TEST, KEY_TYPE_TEAM, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE from freezegun import freeze_time -from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status +from tests.app.db import create_notification, create_service, create_template, create_ft_notification_status, create_job def test_update_fact_notification_status(notify_db_session): @@ -288,3 +289,18 @@ def set_up_data(): create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status='delivered') create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status='delivered') + + +def test_fetch_notification_statuses_for_job(sample_template): + j1 = create_job(sample_template) + j2 = create_job(sample_template) + + create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='created', count=1) + create_ft_notification_status(date(2018, 10, 1), job=j1, notification_status='delivered', count=2) + create_ft_notification_status(date(2018, 10, 2), job=j1, notification_status='created', count=4) + create_ft_notification_status(date(2018, 10, 1), job=j2, notification_status='created', count=8) + + assert {x.status: x.count for x in fetch_notification_statuses_for_job(j1.id)} == { + 'created': 5, + 'delivered': 2 + } diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 7d1d88346..524d17f8e 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -19,106 +19,55 @@ from app.models import ( Job, EMAIL_TYPE, SMS_TYPE, LETTER_TYPE ) -from tests.app.conftest import sample_job as create_job -from tests.app.conftest import sample_notification as create_notification -from tests.app.conftest import sample_service as create_service -from tests.app.conftest import sample_template as create_template -from tests.app.db import ( - create_user -) +from tests.app.db import create_job, create_service, create_template, create_notification def test_should_have_decorated_notifications_dao_functions(): assert dao_get_notification_outcomes_for_job.__wrapped__.__name__ == 'dao_get_notification_outcomes_for_job' # noqa -def test_should_get_all_statuses_for_notifications_associated_with_job( - notify_db, - notify_db_session, - sample_service, - sample_job -): - notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job) - notification(status='created') - notification(status='sending') - notification(status='delivered') - notification(status='pending') - notification(status='failed') - notification(status='technical-failure') - notification(status='temporary-failure') - notification(status='permanent-failure') - notification(status='sent') +def test_should_count_of_statuses_for_notifications_associated_with_job(sample_template, sample_job): + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='created') + create_notification(sample_template, job=sample_job, status='sending') + create_notification(sample_template, job=sample_job, status='delivered') - results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) - assert set([(row.count, row.status) for row in results]) == set([ - (1, 'created'), - (1, 'sending'), - (1, 'delivered'), - (1, 'pending'), - (1, 'failed'), - (1, 'technical-failure'), - (1, 'temporary-failure'), - (1, 'permanent-failure'), - (1, 'sent') - ]) - - -def test_should_count_of_statuses_for_notifications_associated_with_job( - notify_db, - notify_db_session, - sample_service, - sample_job -): - notification = partial(create_notification, notify_db, notify_db_session, service=sample_service, job=sample_job) - notification(status='created') - notification(status='created') - - notification(status='sending') - notification(status='sending') - notification(status='sending') - notification(status='sending') - notification(status='delivered') - notification(status='delivered') - - results = dao_get_notification_outcomes_for_job(sample_service.id, sample_job.id) - assert set([(row.count, row.status) for row in results]) == set([ - (2, 'created'), - (4, 'sending'), - (2, 'delivered') - ]) + results = dao_get_notification_outcomes_for_job(sample_template.service_id, sample_job.id) + assert {row.status: row.count for row in results} == { + 'created': 3, + 'sending': 1, + 'delivered': 1, + } def test_should_return_zero_length_array_if_no_notifications_for_job(sample_service, sample_job): assert len(dao_get_notification_outcomes_for_job(sample_job.id, sample_service.id)) == 0 -def test_should_return_notifications_only_for_this_job(notify_db, notify_db_session, sample_service): - job_1 = create_job(notify_db, notify_db_session, service=sample_service) - job_2 = create_job(notify_db, notify_db_session, service=sample_service) +def test_should_return_notifications_only_for_this_job(sample_template): + job_1 = create_job(sample_template) + job_2 = create_job(sample_template) - create_notification(notify_db, notify_db_session, service=sample_service, job=job_1, status='created') - create_notification(notify_db, notify_db_session, service=sample_service, job=job_2, status='created') + create_notification(sample_template, job=job_1, status='created') + create_notification(sample_template, job=job_2, status='sent') - results = dao_get_notification_outcomes_for_job(sample_service.id, job_1.id) - assert [(row.count, row.status) for row in results] == [ - (1, 'created') - ] + results = dao_get_notification_outcomes_for_job(sample_template.service_id, job_1.id) + assert {row.status: row.count for row in results} == {'created': 1} -def test_should_return_notifications_only_for_this_service(notify_db, notify_db_session): - service_1 = create_service(notify_db, notify_db_session, service_name="one", email_from="one") - service_2 = create_service(notify_db, notify_db_session, service_name="two", email_from="two") +def test_should_return_notifications_only_for_this_service(sample_notification_with_job): + other_service = create_service(service_name='one') + other_template = create_template(service=other_service) + other_job = create_job(other_template) - job_1 = create_job(notify_db, notify_db_session, service=service_1) - job_2 = create_job(notify_db, notify_db_session, service=service_2) + create_notification(other_template, job=other_job) - create_notification(notify_db, notify_db_session, service=service_1, job=job_1, status='created') - create_notification(notify_db, notify_db_session, service=service_2, job=job_2, status='created') - - assert len(dao_get_notification_outcomes_for_job(service_1.id, job_2.id)) == 0 + assert len(dao_get_notification_outcomes_for_job(sample_notification_with_job.service_id, other_job.id)) == 0 + assert len(dao_get_notification_outcomes_for_job(other_service.id, sample_notification_with_job.id)) == 0 -def test_create_job(sample_template): +def test_create_sample_job(sample_template): assert Job.query.count() == 0 job_id = uuid.uuid4() @@ -147,14 +96,12 @@ def test_get_job_by_id(sample_job): assert sample_job == job_from_db -def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) +def test_get_jobs_for_service(sample_template): + one_job = create_job(sample_template) - other_user = create_user(email="test@digital.cabinet-office.gov.uk") - other_service = create_service(notify_db, notify_db_session, user=other_user, service_name="other service", - email_from='other.service') - other_template = create_template(notify_db, notify_db_session, service=other_service) - other_job = create_job(notify_db, notify_db_session, service=other_service, template=other_template) + other_service = create_service(service_name="other service") + other_template = create_template(service=other_service) + other_job = create_job(other_template) one_job_from_db = dao_get_jobs_by_service_id(one_job.service_id).items other_job_from_db = dao_get_jobs_by_service_id(other_job.service_id).items @@ -168,10 +115,9 @@ def test_get_jobs_for_service(notify_db, notify_db_session, sample_template): assert one_job_from_db != other_job_from_db -def test_get_jobs_for_service_with_limit_days_param(notify_db, notify_db_session, sample_template): - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) - old_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template, - created_at=datetime.now() - timedelta(days=8)) +def test_get_jobs_for_service_with_limit_days_param(sample_template): + one_job = create_job(sample_template) + old_job = create_job(sample_template, created_at=datetime.now() - timedelta(days=8)) jobs = dao_get_jobs_by_service_id(one_job.service_id).items @@ -185,34 +131,27 @@ def test_get_jobs_for_service_with_limit_days_param(notify_db, notify_db_session assert old_job not in jobs_limit_days -def test_get_jobs_for_service_with_limit_days_edge_case(notify_db, notify_db_session, sample_template): - one_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template) - job_two = create_job(notify_db, notify_db_session, sample_template.service, sample_template, - created_at=(datetime.now() - timedelta(days=7)).date()) - one_second_after_midnight = datetime.combine((datetime.now() - timedelta(days=7)).date(), - datetime.strptime("000001", "%H%M%S").time()) - just_after_midnight_job = create_job(notify_db, notify_db_session, sample_template.service, sample_template, - created_at=one_second_after_midnight) - job_eight_days_old = create_job(notify_db, notify_db_session, sample_template.service, sample_template, - created_at=datetime.now() - timedelta(days=8)) +@freeze_time('2017-06-10') +def test_get_jobs_for_service_with_limit_days_edge_case(sample_template): + one_job = create_job(sample_template) + just_after_midnight_job = create_job(sample_template, created_at=datetime(2017, 6, 2, 23, 0, 1)) + just_before_midnight_job = create_job(sample_template, created_at=datetime(2017, 6, 2, 22, 59, 0)) jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items - assert len(jobs_limit_days) == 3 + assert len(jobs_limit_days) == 2 assert one_job in jobs_limit_days - assert job_two in jobs_limit_days assert just_after_midnight_job in jobs_limit_days - assert job_eight_days_old not in jobs_limit_days + assert just_before_midnight_job not in jobs_limit_days def test_get_jobs_for_service_in_processed_at_then_created_at_order(notify_db, notify_db_session, sample_template): - _create_job = partial(create_job, notify_db, notify_db_session, sample_template.service, sample_template) from_hour = partial(datetime, 2001, 1, 1) created_jobs = [ - _create_job(created_at=from_hour(2), processing_started=None), - _create_job(created_at=from_hour(1), processing_started=None), - _create_job(created_at=from_hour(1), processing_started=from_hour(4)), - _create_job(created_at=from_hour(2), processing_started=from_hour(3)), + create_job(sample_template, created_at=from_hour(2), processing_started=None), + create_job(sample_template, created_at=from_hour(1), processing_started=None), + create_job(sample_template, created_at=from_hour(1), processing_started=from_hour(4)), + create_job(sample_template, created_at=from_hour(2), processing_started=from_hour(3)), ] jobs = dao_get_jobs_by_service_id(sample_template.service.id).items @@ -235,21 +174,20 @@ def test_update_job(sample_job): assert job_from_db.job_status == 'in progress' -def test_set_scheduled_jobs_to_pending_gets_all_jobs_in_scheduled_state_before_now(notify_db, notify_db_session): +def test_set_scheduled_jobs_to_pending_gets_all_jobs_in_scheduled_state_before_now(sample_template): one_minute_ago = datetime.utcnow() - timedelta(minutes=1) one_hour_ago = datetime.utcnow() - timedelta(minutes=60) - job_new = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') - job_old = create_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled') + job_new = create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled') + job_old = create_job(sample_template, scheduled_for=one_hour_ago, job_status='scheduled') jobs = dao_set_scheduled_jobs_to_pending() assert len(jobs) == 2 assert jobs[0].id == job_old.id assert jobs[1].id == job_new.id -def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_not_scheduled(notify_db, notify_db_session): +def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_not_scheduled(sample_template, sample_job): one_minute_ago = datetime.utcnow() - timedelta(minutes=1) - create_job(notify_db, notify_db_session) - job_scheduled = create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') + job_scheduled = create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled') jobs = dao_set_scheduled_jobs_to_pending() assert len(jobs) == 1 assert jobs[0].id == job_scheduled.id @@ -260,11 +198,11 @@ def test_set_scheduled_jobs_to_pending_gets_ignores_jobs_scheduled_in_the_future assert len(jobs) == 0 -def test_set_scheduled_jobs_to_pending_updates_rows(notify_db, notify_db_session): +def test_set_scheduled_jobs_to_pending_updates_rows(sample_template): one_minute_ago = datetime.utcnow() - timedelta(minutes=1) one_hour_ago = datetime.utcnow() - timedelta(minutes=60) - create_job(notify_db, notify_db_session, scheduled_for=one_minute_ago, job_status='scheduled') - create_job(notify_db, notify_db_session, scheduled_for=one_hour_ago, job_status='scheduled') + create_job(sample_template, scheduled_for=one_minute_ago, job_status='scheduled') + create_job(sample_template, scheduled_for=one_hour_ago, job_status='scheduled') jobs = dao_set_scheduled_jobs_to_pending() assert len(jobs) == 2 assert jobs[0].job_status == 'pending' @@ -277,7 +215,7 @@ def test_get_future_scheduled_job_gets_a_job_yet_to_send(sample_scheduled_job): @freeze_time('2016-10-31 10:00:00') -def test_should_get_jobs_seven_days_old(notify_db, notify_db_session, sample_template): +def test_should_get_jobs_seven_days_old(sample_template): """ Jobs older than seven days are deleted, but only two day's worth (two-day window) """ @@ -289,12 +227,11 @@ def test_should_get_jobs_seven_days_old(notify_db, notify_db_session, sample_tem nine_days_ago = eight_days_ago - timedelta(days=2) nine_days_one_second_ago = nine_days_ago - timedelta(seconds=1) - job = partial(create_job, notify_db, notify_db_session) - job(created_at=seven_days_ago) - job(created_at=within_seven_days) - job_to_delete = job(created_at=eight_days_ago) - job(created_at=nine_days_ago, archived=True) - job(created_at=nine_days_one_second_ago, archived=True) + create_job(sample_template, created_at=seven_days_ago) + create_job(sample_template, created_at=within_seven_days) + job_to_delete = create_job(sample_template, created_at=eight_days_ago) + create_job(sample_template, created_at=nine_days_ago, archived=True) + create_job(sample_template, created_at=nine_days_one_second_ago, archived=True) jobs = dao_get_jobs_older_than_data_retention(notification_types=[sample_template.template_type]) @@ -306,7 +243,7 @@ def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_ with freeze_time('2015-01-01T00:00:00') as the_time: for _ in range(10): the_time.tick(timedelta(hours=1)) - create_job(notify_db, notify_db_session, sample_service, sample_template) + create_job(sample_template) res = dao_get_jobs_by_service_id(sample_service.id, page=1, page_size=2) @@ -328,19 +265,11 @@ def test_get_jobs_for_service_is_paginated(notify_db, notify_db_session, sample_ 'Report', ]) def test_get_jobs_for_service_doesnt_return_test_messages( - notify_db, - notify_db_session, sample_template, sample_job, file_name, ): - create_job( - notify_db, - notify_db_session, - sample_template.service, - sample_template, - original_file_name=file_name, - ) + create_job(sample_template, original_file_name=file_name,) jobs = dao_get_jobs_by_service_id(sample_job.service_id).items @@ -348,16 +277,15 @@ def test_get_jobs_for_service_doesnt_return_test_messages( @freeze_time('2016-10-31 10:00:00') -def test_should_get_jobs_seven_days_old_filters_type(notify_db, notify_db_session): +def test_should_get_jobs_seven_days_old_filters_type(sample_service): eight_days_ago = datetime.utcnow() - timedelta(days=8) - letter_template = create_template(notify_db, notify_db_session, template_type=LETTER_TYPE) - sms_template = create_template(notify_db, notify_db_session, template_type=SMS_TYPE) - email_template = create_template(notify_db, notify_db_session, template_type=EMAIL_TYPE) + letter_template = create_template(sample_service, template_type=LETTER_TYPE) + sms_template = create_template(sample_service, template_type=SMS_TYPE) + email_template = create_template(sample_service, template_type=EMAIL_TYPE) - job = partial(create_job, notify_db, notify_db_session, created_at=eight_days_ago) - job_to_remain = job(template=letter_template) - job(template=sms_template) - job(template=email_template) + job_to_remain = create_job(letter_template, created_at=eight_days_ago) + create_job(sms_template, created_at=eight_days_ago) + create_job(email_template, created_at=eight_days_ago) jobs = dao_get_jobs_older_than_data_retention( notification_types=[EMAIL_TYPE, SMS_TYPE] diff --git a/tests/app/db.py b/tests/app/db.py index 36e29953e..94d6ac287 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -164,7 +164,7 @@ def create_template( def create_notification( - template, + template=None, job=None, job_row_number=None, to_field=None, @@ -190,6 +190,10 @@ def create_notification( created_by_id=None, postage=None ): + assert job or template + if job: + template = job.template + if created_at is None: created_at = datetime.utcnow() @@ -557,6 +561,8 @@ def create_ft_notification_status( notification_status='delivered', count=1 ): + if job: + template = job.template if template: service = template.service notification_type = template.template_type diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 2dfe16854..9adb6e8ac 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -1,21 +1,18 @@ import json import uuid -from datetime import datetime, timedelta -from functools import partial +from datetime import datetime, timedelta, date from freezegun import freeze_time import pytest import pytz + import app.celery.tasks +from app.dao.templates_dao import dao_update_template +from app.models import JOB_STATUS_TYPES, JOB_STATUS_PENDING from tests import create_authorization_header from tests.conftest import set_config -from tests.app.conftest import ( - sample_job as create_job, - sample_notification as create_notification -) -from app.dao.templates_dao import dao_update_template -from app.models import NOTIFICATION_STATUS_TYPES, JOB_STATUS_TYPES, JOB_STATUS_PENDING +from tests.app.db import create_ft_notification_status, create_job, create_notification def test_get_job_with_invalid_service_id_returns404(client, sample_service): @@ -444,54 +441,26 @@ def test_create_job_returns_400_if_archived_template(client, sample_template, mo assert 'Template has been deleted' in resp_json['message']['template'] -def _setup_jobs(notify_db, notify_db_session, template, number_of_jobs=5): +def _setup_jobs(template, number_of_jobs=5): for i in range(number_of_jobs): - create_job( - notify_db, - notify_db_session, - service=template.service, - template=template) + create_job(template=template) -def test_get_all_notifications_for_job_in_order_of_job_number( - client, notify_db, notify_db_session, sample_service -): - main_job = create_job(notify_db, notify_db_session, service=sample_service) - another_job = create_job(notify_db, notify_db_session, service=sample_service) +def test_get_all_notifications_for_job_in_order_of_job_number(admin_request, sample_template): + main_job = create_job(sample_template) + another_job = create_job(sample_template) - notification_1 = create_notification( - notify_db, - notify_db_session, - job=main_job, - to_field="1", - created_at=datetime.utcnow(), - job_row_number=1 + notification_1 = create_notification(job=main_job, to_field="1", job_row_number=1) + notification_2 = create_notification(job=main_job, to_field="2", job_row_number=2) + notification_3 = create_notification(job=main_job, to_field="3", job_row_number=3) + create_notification(job=another_job) + + resp = admin_request.get( + 'job.get_all_notifications_for_service_job', + service_id=main_job.service_id, + job_id=main_job.id ) - notification_2 = create_notification( - notify_db, - notify_db_session, - job=main_job, - to_field="2", - created_at=datetime.utcnow(), - job_row_number=2 - ) - notification_3 = create_notification( - notify_db, - notify_db_session, - job=main_job, - to_field="3", - created_at=datetime.utcnow(), - job_row_number=3 - ) - create_notification(notify_db, notify_db_session, job=another_job) - auth_header = create_authorization_header() - - response = client.get( - path='/service/{}/job/{}/notifications'.format(sample_service.id, main_job.id), - headers=[auth_header]) - - resp = json.loads(response.get_data(as_text=True)) assert len(resp['notifications']) == 3 assert resp['notifications'][0]['to'] == notification_1.to assert resp['notifications'][0]['job_row_number'] == notification_1.job_row_number @@ -499,133 +468,76 @@ def test_get_all_notifications_for_job_in_order_of_job_number( assert resp['notifications'][1]['job_row_number'] == notification_2.job_row_number assert resp['notifications'][2]['to'] == notification_3.to assert resp['notifications'][2]['job_row_number'] == notification_3.job_row_number - assert response.status_code == 200 @pytest.mark.parametrize( "expected_notification_count, status_args", [ - (1, '?status={}'.format(NOTIFICATION_STATUS_TYPES[0])), - (0, '?status={}'.format(NOTIFICATION_STATUS_TYPES[1])), - (1, '?status={}&status={}&status={}'.format(*NOTIFICATION_STATUS_TYPES[0:3])), - (0, '?status={}&status={}&status={}'.format(*NOTIFICATION_STATUS_TYPES[3:6])), + (1, ['created']), + (0, ['sending']), + (1, ['created', 'sending']), + (0, ['sending', 'delivered']), ] ) def test_get_all_notifications_for_job_filtered_by_status( - client, - notify_db, - notify_db_session, - sample_service, + admin_request, + sample_job, expected_notification_count, status_args ): - job = create_job(notify_db, notify_db_session, service=sample_service) + create_notification(job=sample_job, to_field="1", status='created') - create_notification( - notify_db, - notify_db_session, - job=job, - to_field="1", - created_at=datetime.utcnow(), - status=NOTIFICATION_STATUS_TYPES[0], - job_row_number=1 + resp = admin_request.get( + 'job.get_all_notifications_for_service_job', + service_id=sample_job.service_id, + job_id=sample_job.id, + status=status_args ) - - response = client.get( - path='/service/{}/job/{}/notifications{}'.format(sample_service.id, job.id, status_args), - headers=[create_authorization_header()] - ) - resp = json.loads(response.get_data(as_text=True)) assert len(resp['notifications']) == expected_notification_count - assert response.status_code == 200 def test_get_all_notifications_for_job_returns_correct_format( - client, + admin_request, sample_notification_with_job ): service_id = sample_notification_with_job.service_id job_id = sample_notification_with_job.job_id - response = client.get( - path='/service/{}/job/{}/notifications'.format(service_id, job_id), - headers=[create_authorization_header()] - ) - assert response.status_code == 200 - resp = json.loads(response.get_data(as_text=True)) + + resp = admin_request.get('job.get_all_notifications_for_service_job', service_id=service_id, job_id=job_id) + assert len(resp['notifications']) == 1 assert resp['notifications'][0]['id'] == str(sample_notification_with_job.id) assert resp['notifications'][0]['status'] == sample_notification_with_job.status -def test_get_job_by_id(notify_api, sample_job): +def test_get_job_by_id(admin_request, sample_job): job_id = str(sample_job.id) service_id = sample_job.service.id - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/job/{}'.format(service_id, job_id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert resp_json['data']['id'] == job_id - assert resp_json['data']['statistics'] == [] - assert resp_json['data']['created_by']['name'] == 'Test User' + resp_json = admin_request.get('job.get_job_by_service_and_job_id', service_id=service_id, job_id=job_id) -def test_get_job_by_id_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_job): - job_id = str(sample_job.id) - service_id = sample_job.service.id - partial_notification = partial( - create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job - ) - partial_notification(status='created') - partial_notification(status='sending') - partial_notification(status='delivered') - partial_notification(status='pending') - partial_notification(status='failed') - partial_notification(status='technical-failure') # noqa - partial_notification(status='temporary-failure') # noqa - partial_notification(status='permanent-failure') # noqa - - path = '/service/{}/job/{}'.format(service_id, job_id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) assert resp_json['data']['id'] == job_id - assert {'status': 'created', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'delivered', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'pending', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'failed', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'technical-failure', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'temporary-failure', 'count': 1} in resp_json['data']['statistics'] - assert {'status': 'permanent-failure', 'count': 1} in resp_json['data']['statistics'] + assert resp_json['data']['statistics'] == [] assert resp_json['data']['created_by']['name'] == 'Test User' -def test_get_job_by_id_should_return_summed_statistics(client, notify_db, notify_db_session, notify_api, sample_job): +def test_get_job_by_id_should_return_summed_statistics(admin_request, sample_job): job_id = str(sample_job.id) service_id = sample_job.service.id - partial_notification = partial( - create_notification, notify_db, notify_db_session, service=sample_job.service, job=sample_job - ) - partial_notification(status='created') - partial_notification(status='created') - partial_notification(status='created') - partial_notification(status='sending') - partial_notification(status='failed') - partial_notification(status='failed') - partial_notification(status='failed') - partial_notification(status='technical-failure') - partial_notification(status='temporary-failure') - partial_notification(status='temporary-failure') - path = '/service/{}/job/{}'.format(service_id, job_id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) + create_notification(job=sample_job, status='created') + create_notification(job=sample_job, status='created') + create_notification(job=sample_job, status='created') + create_notification(job=sample_job, status='sending') + create_notification(job=sample_job, status='failed') + create_notification(job=sample_job, status='failed') + create_notification(job=sample_job, status='failed') + create_notification(job=sample_job, status='technical-failure') + create_notification(job=sample_job, status='temporary-failure') + create_notification(job=sample_job, status='temporary-failure') + + resp_json = admin_request.get('job.get_job_by_service_and_job_id', service_id=service_id, job_id=job_id) + assert resp_json['data']['id'] == job_id assert {'status': 'created', 'count': 3} in resp_json['data']['statistics'] assert {'status': 'sending', 'count': 1} in resp_json['data']['statistics'] @@ -635,32 +547,23 @@ def test_get_job_by_id_should_return_summed_statistics(client, notify_db, notify assert resp_json['data']['created_by']['name'] == 'Test User' -def test_get_jobs(client, notify_db, notify_db_session, sample_template): - _setup_jobs(notify_db, notify_db_session, sample_template) +def test_get_jobs(admin_request, sample_template): + _setup_jobs(sample_template) service_id = sample_template.service.id - path = '/service/{}/job'.format(service_id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) + resp_json = admin_request.get('job.get_jobs_by_service', service_id=service_id) assert len(resp_json['data']) == 5 -def test_get_jobs_with_limit_days(admin_request, notify_db, notify_db_session, sample_template): +def test_get_jobs_with_limit_days(admin_request, sample_template): for time in [ 'Sunday 1st July 2018 22:59', 'Sunday 2nd July 2018 23:00', # beginning of monday morning 'Monday 3rd July 2018 12:00' ]: with freeze_time(time): - create_job( - notify_db, - notify_db_session, - service=sample_template.service, - template=sample_template, - ) + create_job(template=sample_template) with freeze_time('Monday 9th July 2018 12:00'): resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id, limit_days=7) @@ -668,51 +571,35 @@ def test_get_jobs_with_limit_days(admin_request, notify_db, notify_db_session, s assert len(resp_json['data']) == 2 -def test_get_jobs_should_return_statistics(client, notify_db, notify_db_session, notify_api, sample_service): +def test_get_jobs_should_return_statistics(admin_request, sample_template): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) - job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) - job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now) - partial_notification = partial(create_notification, notify_db, notify_db_session, service=sample_service) - partial_notification(job=job_1, status='created') - partial_notification(job=job_1, status='created') - partial_notification(job=job_1, status='created') - partial_notification(job=job_2, status='sending') - partial_notification(job=job_2, status='sending') - partial_notification(job=job_2, status='sending') + job_1 = create_job(sample_template, created_at=earlier) + job_2 = create_job(sample_template, created_at=now) + create_notification(job=job_1, status='created') + create_notification(job=job_1, status='created') + create_notification(job=job_1, status='created') + create_notification(job=job_2, status='sending') + create_notification(job=job_2, status='sending') + create_notification(job=job_2, status='sending') - with notify_api.test_request_context(): - with notify_api.test_client() as client: - path = '/service/{}/job'.format(sample_service.id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 2 - assert resp_json['data'][0]['id'] == str(job_2.id) - assert {'status': 'sending', 'count': 3} in resp_json['data'][0]['statistics'] - assert resp_json['data'][1]['id'] == str(job_1.id) - assert {'status': 'created', 'count': 3} in resp_json['data'][1]['statistics'] + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id) + + assert len(resp_json['data']) == 2 + assert resp_json['data'][0]['id'] == str(job_2.id) + assert {'status': 'sending', 'count': 3} in resp_json['data'][0]['statistics'] + assert resp_json['data'][1]['id'] == str(job_1.id) + assert {'status': 'created', 'count': 3} in resp_json['data'][1]['statistics'] -def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications( - client, - notify_db, - notify_db_session, - notify_api, - sample_service, -): - +def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) - job_1 = create_job(notify_db, notify_db_session, service=sample_service, created_at=earlier) - job_2 = create_job(notify_db, notify_db_session, service=sample_service, created_at=now) + job_1 = create_job(sample_template, created_at=earlier) + job_2 = create_job(sample_template, created_at=now) + + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id) - path = '/service/{}/job'.format(sample_service.id) - auth_header = create_authorization_header() - response = client.get(path, headers=[auth_header]) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) assert len(resp_json['data']) == 2 assert resp_json['data'][0]['id'] == str(job_2.id) assert resp_json['data'][0]['statistics'] == [] @@ -720,23 +607,12 @@ def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications( assert resp_json['data'][1]['statistics'] == [] -def test_get_jobs_should_paginate( - notify_db, - notify_db_session, - client, - sample_template -): - create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template) +def test_get_jobs_should_paginate(admin_request, sample_template): + create_10_jobs(sample_template) - path = '/service/{}/job'.format(sample_template.service_id) - auth_header = create_authorization_header() + with set_config(admin_request.app, 'PAGE_SIZE', 2): + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id) - with set_config(client.application, 'PAGE_SIZE', 2): - response = client.get(path, headers=[auth_header]) - - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 2 assert resp_json['data'][0]['created_at'] == '2015-01-01T10:00:00+00:00' assert resp_json['data'][1]['created_at'] == '2015-01-01T09:00:00+00:00' assert resp_json['page_size'] == 2 @@ -745,23 +621,12 @@ def test_get_jobs_should_paginate( assert set(resp_json['links'].keys()) == {'next', 'last'} -def test_get_jobs_accepts_page_parameter( - notify_db, - notify_db_session, - client, - sample_template -): - create_10_jobs(notify_db, notify_db_session, sample_template.service, sample_template) +def test_get_jobs_accepts_page_parameter(admin_request, sample_template): + create_10_jobs(sample_template) - path = '/service/{}/job'.format(sample_template.service_id) - auth_header = create_authorization_header() + with set_config(admin_request.app, 'PAGE_SIZE', 2): + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id, page=2) - with set_config(client.application, 'PAGE_SIZE', 2): - response = client.get(path, headers=[auth_header], query_string={'page': 2}) - - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - assert len(resp_json['data']) == 2 assert resp_json['data'][0]['created_at'] == '2015-01-01T08:00:00+00:00' assert resp_json['data'][1]['created_at'] == '2015-01-01T07:00:00+00:00' assert resp_json['page_size'] == 2 @@ -778,71 +643,79 @@ def test_get_jobs_accepts_page_parameter( # bad statuses are accepted, just return no data ('foo', []) ]) -def test_get_jobs_can_filter_on_statuses( - notify_db, - notify_db_session, - client, - sample_service, - statuses_filter, - expected_statuses -): - create_job(notify_db, notify_db_session, job_status='pending') - create_job(notify_db, notify_db_session, job_status='in progress') - create_job(notify_db, notify_db_session, job_status='finished') - create_job(notify_db, notify_db_session, job_status='sending limits exceeded') - create_job(notify_db, notify_db_session, job_status='scheduled') - create_job(notify_db, notify_db_session, job_status='cancelled') - create_job(notify_db, notify_db_session, job_status='ready to send') - create_job(notify_db, notify_db_session, job_status='sent to dvla') - create_job(notify_db, notify_db_session, job_status='error') +def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses): + create_job(sample_template, job_status='pending') + create_job(sample_template, job_status='in progress') + create_job(sample_template, job_status='finished') + create_job(sample_template, job_status='sending limits exceeded') + create_job(sample_template, job_status='scheduled') + create_job(sample_template, job_status='cancelled') + create_job(sample_template, job_status='ready to send') + create_job(sample_template, job_status='sent to dvla') + create_job(sample_template, job_status='error') - path = '/service/{}/job'.format(sample_service.id) - response = client.get( - path, - headers=[create_authorization_header()], - query_string={'statuses': statuses_filter} + resp_json = admin_request.get( + 'job.get_jobs_by_service', + service_id=sample_template.service_id, + statuses=statuses_filter ) - assert response.status_code == 200 - resp_json = json.loads(response.get_data(as_text=True)) - from pprint import pprint - pprint(resp_json) assert {x['job_status'] for x in resp_json['data']} == set(expected_statuses) -def create_10_jobs(db, session, service, template): +def create_10_jobs(template): with freeze_time('2015-01-01T00:00:00') as the_time: for _ in range(10): the_time.tick(timedelta(hours=1)) - create_job(db, session, service, template) + create_job(template) -def test_get_all_notifications_for_job_returns_csv_format( - client, - notify_db, - notify_db_session, -): - job = create_job(notify_db, notify_db_session) - notification = create_notification( - notify_db, - notify_db_session, - job=job, - job_row_number=1, - created_at=datetime.utcnow(), +def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_notification_with_job): + resp = admin_request.get( + 'job.get_all_notifications_for_service_job', + service_id=sample_notification_with_job.service_id, + job_id=sample_notification_with_job.job_id, + format_for_csv=True ) - path = '/service/{}/job/{}/notifications'.format(notification.service.id, job.id) - - response = client.get( - path=path, - headers=[create_authorization_header()], - query_string={'format_for_csv': True} - ) - assert response.status_code == 200 - - resp = json.loads(response.get_data(as_text=True)) assert len(resp['notifications']) == 1 - notification = resp['notifications'][0] - assert set(notification.keys()) == \ - set(['created_at', 'created_by_name', 'created_by_email_address', 'template_type', - 'template_name', 'job_name', 'status', 'row_number', 'recipient']) + assert set(resp['notifications'][0].keys()) == { + 'created_at', + 'created_by_name', + 'created_by_email_address', + 'template_type', + 'template_name', + 'job_name', + 'status', + 'row_number', + 'recipient' + } + + +@freeze_time('2017-06-10 12:00') +def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template): + # it's the 10th today, so 3 days should include all of 7th, 8th, 9th, and some of 10th. + just_three_days_ago = datetime(2017, 6, 6, 22, 59, 59) + not_quite_three_days_ago = just_three_days_ago + timedelta(seconds=1) + + job_1 = create_job(sample_template, created_at=just_three_days_ago) + job_2 = create_job(sample_template, created_at=not_quite_three_days_ago) + + # some notifications created more than three days ago, some created after the midnight cutoff + create_ft_notification_status(date(2017, 6, 6), job=job_1, notification_status='delivered', count=2) + create_ft_notification_status(date(2017, 6, 7), job=job_1, notification_status='delivered', count=4) + # job2's new enough + create_notification(job=job_2, status='created', created_at=not_quite_three_days_ago) + + # this isn't picked up because the job is too new + create_ft_notification_status(date(2017, 6, 7), job=job_2, notification_status='delivered', count=8) + + # this isn't picked up because we're using the ft status table for job_1 as it's old + create_notification(job=job_1, status='created', created_at=not_quite_three_days_ago) + + resp_json = admin_request.get('job.get_jobs_by_service', service_id=sample_template.service_id) + + assert resp_json['data'][0]['id'] == str(job_2.id) + assert resp_json['data'][0]['statistics'] == [{'status': 'created', 'count': 1}] + assert resp_json['data'][1]['id'] == str(job_1.id) + assert resp_json['data'][1]['statistics'] == [{'status': 'delivered', 'count': 6}]