diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 6595711209..f61d601351 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -2,13 +2,14 @@ from datetime import datetime, timedelta from flask import current_app +from notifications_utils.clients.redis import RequestCache from notifications_utils.letter_timings import ( CANCELLABLE_JOB_LETTER_STATUSES, letter_can_be_cancelled, ) from sqlalchemy import and_, asc, desc, func -from app import db +from app import db, redis_store from app.constants import ( JOB_STATUS_CANCELLED, JOB_STATUS_FINISHED, @@ -17,8 +18,10 @@ LETTER_TYPE, NOTIFICATION_CANCELLED, NOTIFICATION_CREATED, + NOTIFICATION_STATUS_TYPES_COMPLETED, ) from app.dao.dao_utils import autocommit +from app.dao.fact_notification_status_dao import fetch_notification_statuses_for_job from app.dao.templates_dao import dao_get_template_by_id from app.models import ( FactNotificationStatus, @@ -258,3 +261,30 @@ def find_missing_row_for_job(job_id, job_size): .filter(Notification.job_row_number == None) # noqa ) return query.all() + + +redis_cache = RequestCache(redis_store) + + +@redis_cache.set("job-{job_id}-notification-outcomes", ttl_in_seconds=timedelta(days=1).total_seconds()) +def get_possibly_cached_notification_outcomes_for_job( + job_id: uuid.UUID | str, notification_count: int | None, processing_started: datetime | None +): + if processing_started is None: + statuses = [] + elif processing_started.replace(tzinfo=None) < midnight_n_days_ago(3): + # ft_notification_status table + statuses = fetch_notification_statuses_for_job(job_id) + else: + # notifications table + statuses = dao_get_notification_outcomes_for_job(job_id) + + return RequestCache.CacheResultWrapper( + value=[{"status": status.status, "count": status.count} for status in statuses], + # cache if all rows of the job are accounted for and no + # notifications are in a state still likely to change + cache_decision=bool( + sum(status.count for status in statuses) == notification_count + and all(status.status in NOTIFICATION_STATUS_TYPES_COMPLETED for status in statuses) + ), + ) diff --git a/app/job/rest.py b/app/job/rest.py index e2a509cb77..0c62b3a517 100644 --- a/app/job/rest.py +++ b/app/job/rest.py @@ -11,9 +11,6 @@ JOB_STATUS_SCHEDULED, LETTER_TYPE, ) -from app.dao.fact_notification_status_dao import ( - fetch_notification_statuses_for_job, -) from app.dao.jobs_dao import ( can_letter_job_be_cancelled, dao_cancel_letter_job, @@ -24,6 +21,7 @@ dao_get_scheduled_job_by_id_and_service_id, dao_get_scheduled_job_stats, dao_update_job, + get_possibly_cached_notification_outcomes_for_job, ) from app.dao.notifications_dao import ( dao_get_notification_count_for_job_id, @@ -39,7 +37,7 @@ notifications_filter_schema, unarchived_template_schema, ) -from app.utils import midnight_n_days_ago, pagination_links +from app.utils import pagination_links job_blueprint = Blueprint("job", __name__, url_prefix="/service//job") @@ -220,15 +218,9 @@ def get_paginated_jobs( start = job_data["processing_started"] start = dateutil.parser.parse(start).replace(tzinfo=None) if start else None - if start is None: - statistics = [] - elif start.replace(tzinfo=None) < 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(job_data["id"]) - job_data["statistics"] = [{"status": statistic.status, "count": statistic.count} for statistic in statistics] + job_data["statistics"] = get_possibly_cached_notification_outcomes_for_job( + job_data["id"], job_data["notification_count"], start + ) return { "data": data, diff --git a/app/upload/rest.py b/app/upload/rest.py index 65103c1640..91995ccb78 100644 --- a/app/upload/rest.py +++ b/app/upload/rest.py @@ -2,17 +2,14 @@ from flask import Blueprint, abort, current_app, jsonify, request -from app.dao.fact_notification_status_dao import ( - fetch_notification_statuses_for_job, -) -from app.dao.jobs_dao import dao_get_notification_outcomes_for_job +from app.dao.jobs_dao import get_possibly_cached_notification_outcomes_for_job from app.dao.uploads_dao import ( dao_get_uploaded_letters_by_print_date, dao_get_uploads_by_service_id, ) from app.errors import register_errors from app.schemas import notification_with_template_schema -from app.utils import midnight_n_days_ago, pagination_links +from app.utils import pagination_links upload_blueprint = Blueprint("upload", __name__, url_prefix="/service//upload") @@ -49,19 +46,9 @@ def get_paginated_uploads(service_id, limit_days, page): "recipient": upload.recipient, } if upload.upload_type == "job": - start = upload.processing_started - - if start is None: - statistics = [] - elif start.replace(tzinfo=None) < midnight_n_days_ago(3): - # ft_notification_status table - statistics = fetch_notification_statuses_for_job(upload.id) - else: - # notifications table - statistics = dao_get_notification_outcomes_for_job(upload.id) - upload_dict["statistics"] = [ - {"status": statistic.status, "count": statistic.count} for statistic in statistics - ] + upload_dict["statistics"] = get_possibly_cached_notification_outcomes_for_job( + upload.id, upload.notification_count, upload.processing_started + ) else: upload_dict["statistics"] = [] data.append(upload_dict) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 72e27a453f..de23452f45 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -1,4 +1,6 @@ +import json import uuid +from collections import Counter from datetime import datetime, timedelta from functools import partial @@ -20,9 +22,11 @@ dao_update_job, find_jobs_with_missing_rows, find_missing_row_for_job, + get_possibly_cached_notification_outcomes_for_job, ) from app.models import Job from tests.app.db import ( + create_ft_notification_status, create_job, create_notification, create_service, @@ -566,3 +570,195 @@ def test_unique_key_on_job_id_and_job_row_number_no_error_if_row_number_for_diff job_2 = create_job(template=sample_email_template) create_notification(job=job_1, job_row_number=0) create_notification(job=job_2, job_row_number=0) + + +@pytest.mark.parametrize( + [ + "notification_statuses", + "notification_count", + "processing_started", + "create_notifications", + "expect_redis_set", + "expected_retval", + ], + ( + ( + ( + "permanent-failure", + "delivered", + "permanent-failure", + "technical-failure", + "technical-failure", + "permanent-failure", + ), + 6, + datetime.fromisoformat("2020-02-10T09:00:00"), + True, + True, + [ + {"status": "delivered", "count": 1}, + {"status": "permanent-failure", "count": 3}, + {"status": "technical-failure", "count": 2}, + ], + ), + ( + ( + "sent", + "delivered", + "delivered", + "delivered", + ), + 4, + datetime.fromisoformat("2020-02-09T09:00:00"), + True, + True, + [ + {"status": "delivered", "count": 3}, + {"status": "sent", "count": 1}, + ], + ), + ( + ( + "technical-failure", + "delivered", + "technical-failure", + ), + 3, + datetime.fromisoformat("2020-02-04T09:00:00"), # so from ft_notification_status + False, + True, + [ + {"status": "technical-failure", "count": 2}, + {"status": "delivered", "count": 1}, + ], + ), + ( + ( + "technical-failure", + "delivered", + "created", + "technical-failure", + ), + 4, + datetime.fromisoformat("2020-02-09T23:59:58"), + True, + False, # because non-complete status + [ + {"status": "technical-failure", "count": 2}, + {"status": "created", "count": 1}, + {"status": "delivered", "count": 1}, + ], + ), + ( + ( + "sent", + "delivered", + "delivered", + ), + 4, + datetime.fromisoformat("2020-02-10T09:00:00"), + True, + False, # because missing rows + [ + {"status": "delivered", "count": 2}, + {"status": "sent", "count": 1}, + ], + ), + ( + ( + "delivered", + "created", + "delivered", + ), + 3, + None, + True, + False, # because non-complete status + [], + ), + ), +) +def test_get_possibly_cached_notification_outcomes_for_job_empty_cache( + sample_email_template, + notification_statuses, + notification_count, + processing_started, + create_notifications, + expect_redis_set, + expected_retval, + mocker, +): + call_datetime = datetime.fromisoformat("2020-02-10T10:00:00") + ft_status_bins = Counter() + + with freeze_time(processing_started) as frozen_time: + job = create_job(template=sample_email_template, processing_started=processing_started) + for i, status in enumerate(notification_statuses): + frozen_time.tick() + + if create_notifications: + create_notification(job=job, status=status, job_row_number=i) + + d = datetime.now().date() + if d != call_datetime.date(): + ft_status_bins[d, status] += 1 + + for (d, status), count in ft_status_bins.items(): + create_ft_notification_status( + bst_date=d, + notification_type="email", + service=job.service, + job=job, + template=job.template, + key_type="normal", + notification_status=status, + count=count, + ) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mock_redis_set = mocker.patch("app.redis_store.set", return_value=None) + + with freeze_time(call_datetime): + retval = get_possibly_cached_notification_outcomes_for_job(job.id, notification_count, processing_started) + + assert sorted(retval, key=lambda x: x["status"]) == sorted(expected_retval, key=lambda x: x["status"]) + + if expect_redis_set: + assert mock_redis_set.mock_calls == [ + mocker.call( + f"job-{job.id}-notification-outcomes", json.dumps(retval), ex=timedelta(days=1).total_seconds() + ), + ] + else: + assert not mock_redis_set.mock_calls + + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job.id}-notification-outcomes"), + ] + + +def test_get_possibly_cached_notification_outcomes_for_job_present_cache( + fake_uuid, + mocker, +): + mocker.patch( + "app.dao.jobs_dao.fetch_notification_statuses_for_job", + side_effect=AssertionError("fetch_notification_statuses_for_job call not expected"), + ) + mocker.patch( + "app.dao.jobs_dao.dao_get_notification_outcomes_for_job", + side_effect=AssertionError("dao_get_notification_outcomes_for_job call not expected"), + ) + + mock_redis_get = mocker.patch( + "app.redis_store.get", return_value=b'[{"status": "delivered", "count": 12}, {"status": "sent", "count": 34}]' + ) + mocker.patch("app.redis_store.set", return_value=AssertionError("redis set call not expected")) + + retval = get_possibly_cached_notification_outcomes_for_job(fake_uuid, 46, datetime.now()) + + assert retval == [{"status": "delivered", "count": 12}, {"status": "sent", "count": 34}] + + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{fake_uuid}-notification-outcomes"), + ] diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 46886df5af..d6b5b08362 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -700,9 +700,14 @@ def __create_ft_status(job, status, count): @freeze_time("2017-07-17 07:17") -def test_get_jobs(admin_request, sample_template): +def test_get_jobs(admin_request, sample_template, mocker): _setup_jobs(sample_template) + mock_redis_get = mocker.patch( + "app.redis_store.get", + return_value=None, + ) + service_id = sample_template.service.id resp_json = admin_request.get("job.get_jobs_by_service", service_id=service_id) @@ -731,27 +736,39 @@ def test_get_jobs(admin_request, sample_template): "updated_at": None, "contact_list_id": None, } + assert len(mock_redis_get.mock_calls) == 5 -def test_get_jobs_with_limit_days(admin_request, sample_template): +def test_get_jobs_with_limit_days(admin_request, sample_template, mocker): + jobs = [] 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(template=sample_template) + jobs.append(create_job(template=sample_template)) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) 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) assert len(resp_json["data"]) == 2 + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{jobs[2].id}-notification-outcomes"), + mocker.call(f"job-{jobs[1].id}-notification-outcomes"), + ] -def test_get_jobs_by_contact_list(admin_request, sample_template): +def test_get_jobs_by_contact_list(admin_request, sample_template, mocker): contact_list = create_service_contact_list() - create_job(template=sample_template) - create_job(template=sample_template, contact_list_id=contact_list.id) + job_1 = create_job(template=sample_template) # noqa + job_2 = create_job(template=sample_template, contact_list_id=contact_list.id) + + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) resp_json = admin_request.get( "job.get_jobs_by_service", @@ -760,9 +777,12 @@ def test_get_jobs_by_contact_list(admin_request, sample_template): ) assert len(resp_json["data"]) == 1 + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + ] -def test_get_jobs_should_return_statistics(admin_request, sample_template): +def test_get_jobs_should_return_statistics(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(sample_template, processing_started=earlier) @@ -774,6 +794,9 @@ def test_get_jobs_should_return_statistics(admin_request, sample_template): create_notification(job=job_2, status="sending") create_notification(job=job_2, status="sending") + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert len(resp_json["data"]) == 2 @@ -781,14 +804,21 @@ def test_get_jobs_should_return_statistics(admin_request, sample_template): 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"] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + mocker.call(f"job-{job_1.id}-notification-outcomes"), + ] -def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template): +def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(sample_template, created_at=earlier) job_2 = create_job(sample_template, created_at=now) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert len(resp_json["data"]) == 2 @@ -796,11 +826,18 @@ def test_get_jobs_should_return_no_stats_if_no_rows_in_notifications(admin_reque assert resp_json["data"][0]["statistics"] == [] assert resp_json["data"][1]["id"] == str(job_1.id) assert resp_json["data"][1]["statistics"] == [] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_2.id}-notification-outcomes"), + mocker.call(f"job-{job_1.id}-notification-outcomes"), + ] -def test_get_jobs_should_paginate(admin_request, sample_template): +def test_get_jobs_should_paginate(admin_request, sample_template, mocker): create_10_jobs(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + 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) @@ -810,11 +847,15 @@ def test_get_jobs_should_paginate(admin_request, sample_template): assert resp_json["total"] == 10 assert "links" in resp_json assert set(resp_json["links"].keys()) == {"next", "last"} + assert len(mock_redis_get.mock_calls) == 2 -def test_get_jobs_accepts_page_parameter(admin_request, sample_template): +def test_get_jobs_accepts_page_parameter(admin_request, sample_template, mocker): create_10_jobs(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + 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) @@ -824,6 +865,10 @@ def test_get_jobs_accepts_page_parameter(admin_request, sample_template): assert resp_json["total"] == 10 assert "links" in resp_json assert set(resp_json["links"].keys()) == {"prev", "next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] @pytest.mark.parametrize( @@ -839,7 +884,7 @@ def test_get_jobs_accepts_page_parameter(admin_request, sample_template): ("foo", []), ], ) -def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses): +def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuses_filter, expected_statuses, mocker): create_job(sample_template, job_status="pending") create_job(sample_template, job_status="in progress") create_job(sample_template, job_status="finished") @@ -850,11 +895,15 @@ def test_get_jobs_can_filter_on_statuses(admin_request, sample_template, statuse create_job(sample_template, job_status="sent to dvla") create_job(sample_template, job_status="error") + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get( "job.get_jobs_by_service", service_id=sample_template.service_id, statuses=statuses_filter ) assert {x["job_status"] for x in resp_json["data"]} == set(expected_statuses) + assert len(mock_redis_get.mock_calls) == len(expected_statuses) def create_10_jobs(template): @@ -890,7 +939,7 @@ def test_get_all_notifications_for_job_returns_csv_format(admin_request, sample_ @freeze_time("2017-06-10 12:00") -def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template): +def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template, mocker): # 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) @@ -914,6 +963,11 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin # 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) + mock_redis_get = mocker.patch( + "app.redis_store.get", + return_value=None, + ) + resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) assert resp_json["data"][0]["id"] == str(job_3.id) @@ -922,6 +976,11 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin assert resp_json["data"][1]["statistics"] == [{"status": "created", "count": 1}] assert resp_json["data"][2]["id"] == str(job_1.id) assert resp_json["data"][2]["statistics"] == [{"status": "delivered", "count": 6}] + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][2]["id"]}-notification-outcomes'), + ] @freeze_time("2017-07-17 07:17") diff --git a/tests/app/upload/test_rest.py b/tests/app/upload/test_rest.py index f2bcc53373..79e2d20bcf 100644 --- a/tests/app/upload/test_rest.py +++ b/tests/app/upload/test_rest.py @@ -38,7 +38,7 @@ def create_precompiled_template(service): @freeze_time("2020-02-02 14:00") -def test_get_uploads(admin_request, sample_template): +def test_get_uploads(admin_request, sample_template, mocker): letter_template = create_precompiled_template(sample_template.service) create_uploaded_letter( @@ -69,6 +69,9 @@ def test_get_uploads(admin_request, sample_template): service_id = sample_template.service.id + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=service_id) data = resp_json["data"] assert len(data) == 4 @@ -103,8 +106,14 @@ def test_get_uploads(admin_request, sample_template): "upload_type": "job", } + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{upload_5.id}-notification-outcomes"), + mocker.call(f"job-{upload_4.id}-notification-outcomes"), + mocker.call(f"job-{upload_2.id}-notification-outcomes"), + ] + -def test_get_uploads_should_return_statistics(admin_request, sample_template): +def test_get_uploads_should_return_statistics(admin_request, sample_template, mocker): now = datetime.utcnow() earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(template=sample_template, job_status="pending") @@ -121,6 +130,9 @@ def test_get_uploads_should_return_statistics(admin_request, sample_template): letter_template, sample_template.service, status="delivered", created_at=datetime.utcnow() - timedelta(days=3) ) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id)["data"] assert len(resp_json) == 4 assert resp_json[0]["id"] == str(job_1.id) @@ -132,11 +144,20 @@ def test_get_uploads_should_return_statistics(admin_request, sample_template): assert resp_json[3]["id"] is None assert resp_json[3]["statistics"] == [] + assert mock_redis_get.mock_calls == [ + mocker.call(f"job-{job_1.id}-notification-outcomes"), + mocker.call(f"job-{job_3.id}-notification-outcomes"), + mocker.call(f"job-{job_2.id}-notification-outcomes"), + ] -def test_get_uploads_should_paginate(admin_request, sample_template): + +def test_get_uploads_should_paginate(admin_request, sample_template, mocker): for _ in range(10): create_job(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id) @@ -146,11 +167,19 @@ def test_get_uploads_should_paginate(admin_request, sample_template): assert "links" in resp_json assert set(resp_json["links"].keys()) == {"next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] -def test_get_uploads_accepts_page_parameter(admin_request, sample_template): + +def test_get_uploads_accepts_page_parameter(admin_request, sample_template, mocker): for _ in range(10): create_job(sample_template) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + with set_config(admin_request.app, "PAGE_SIZE", 2): resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id, page=2) @@ -160,9 +189,14 @@ def test_get_uploads_accepts_page_parameter(admin_request, sample_template): assert "links" in resp_json assert set(resp_json["links"].keys()) == {"prev", "next", "last"} + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json["data"][0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json["data"][1]["id"]}-notification-outcomes'), + ] + @freeze_time("2017-06-10 12:00") -def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template): +def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(admin_request, sample_template, mocker): # 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) @@ -186,6 +220,9 @@ def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(ad # 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) + mock_redis_get = mocker.patch("app.redis_store.get", return_value=None) + mocker.patch("app.redis_store.set", side_effect=AssertionError("Set call not expected")) + resp_json = admin_request.get("upload.get_uploads_by_service", service_id=sample_template.service_id)["data"] assert resp_json[0]["id"] == str(job_3.id) @@ -195,6 +232,12 @@ def test_get_uploads_should_retrieve_from_ft_notification_status_for_old_jobs(ad assert resp_json[2]["id"] == str(job_1.id) assert resp_json[2]["statistics"] == [{"status": "delivered", "count": 6}] + assert mock_redis_get.mock_calls == [ + mocker.call(f'job-{resp_json[0]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json[1]["id"]}-notification-outcomes'), + mocker.call(f'job-{resp_json[2]["id"]}-notification-outcomes'), + ] + @freeze_time("2020-02-02 14:00") def test_get_uploaded_letters_by_print_date(admin_request, sample_template):