From b3edae35095dfd083b14862e6b37374e9ac00ef5 Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Wed, 25 Sep 2024 14:03:50 +0000 Subject: [PATCH 1/5] fix(dao_get_jobs_by_service_id): use util function to calculate query time frame --- app/dao/jobs_dao.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index 28a8b1f15d..f9105d9978 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -12,6 +12,7 @@ from app import db from app.dao.dao_utils import transactional +from app.dao.date_util import get_query_date_based_on_retention_period from app.dao.templates_dao import dao_get_template_by_id from app.models import ( JOB_STATUS_CANCELLED, @@ -58,7 +59,8 @@ def dao_get_jobs_by_service_id(service_id, limit_days=None, page=1, page_size=50 Job.original_file_name != current_app.config["ONE_OFF_MESSAGE_FILENAME"], ] if limit_days is not None: - query_filter.append(Job.created_at >= midnight_n_days_ago(limit_days)) + query_filter.append(Job.created_at > get_query_date_based_on_retention_period(limit_days)) + if statuses is not None and statuses != [""]: query_filter.append(Job.job_status.in_(statuses)) return ( From f7ae724164b2770653071806d0193822dfc4542e Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Wed, 25 Sep 2024 14:04:18 +0000 Subject: [PATCH 2/5] chore: add tests for `dao_get_jobs_by_service_id` --- tests/app/dao/test_jobs_dao.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 58e3007739..f92753a3d8 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -160,18 +160,22 @@ def test_get_jobs_for_service_with_limit_days_param(sample_template): assert old_job not in jobs_limit_days -@freeze_time("2017-06-10") -# This test assumes the local timezone is EST +@freeze_time("2024-09-25 12:25:00") 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, 3, 4, 0, 1)) - just_before_midnight_job = create_job(sample_template, created_at=datetime(2017, 6, 3, 3, 59, 0)) + # create 1 job for each day of the last 10 days + for i in range(1, 10): + create_job(sample_template, created_at=datetime(2024, 9, 25 - i, i, 0, 0)) + + too_old_job = create_job(sample_template, created_at=datetime(2024, 9, 18, 0, 1, 0)) + just_right_job = create_job(sample_template, created_at=datetime(2024, 9, 19, 0, 0, 0)) jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items - assert len(jobs_limit_days) == 2 + + assert len(jobs_limit_days) == 8 # one for each day: today (1) + the last 6 days (6) + one for just_right_job (1) assert one_job in jobs_limit_days - assert just_after_midnight_job in jobs_limit_days - assert just_before_midnight_job not in jobs_limit_days + assert just_right_job in jobs_limit_days + assert too_old_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): From 35e09bb6b23568e7c33e58db3fdab47d353cbcf4 Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Wed, 25 Sep 2024 14:05:01 +0000 Subject: [PATCH 3/5] chore: formatting --- app/dao/jobs_dao.py | 1 - 1 file changed, 1 deletion(-) diff --git a/app/dao/jobs_dao.py b/app/dao/jobs_dao.py index f9105d9978..969aef8ef0 100644 --- a/app/dao/jobs_dao.py +++ b/app/dao/jobs_dao.py @@ -29,7 +29,6 @@ ServiceDataRetention, Template, ) -from app.utils import midnight_n_days_ago @statsd(namespace="dao") From 3ce0b2985112ef8105bf336fa436bac6d6707e80 Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Thu, 26 Sep 2024 11:35:04 +0000 Subject: [PATCH 4/5] fix: update tests --- tests/app/job/test_rest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 338d3a5d84..6a49c8ce63 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -751,7 +751,9 @@ def test_get_jobs_with_limit_days(admin_request, sample_template): limit_days=7, ) - assert len(resp_json["data"]) == 2 + # get_jobs_by_service should return data from the current day (Monday 9th) and the previous 6 days (Tuesday 3rd) + # so only 1 job should be returned + assert len(resp_json["data"]) == 1 def test_get_jobs_should_return_statistics(admin_request, sample_template): From 45beec5bb502f18bdc483e11d166e568eef31009 Mon Sep 17 00:00:00 2001 From: Andrew Leith Date: Thu, 26 Sep 2024 13:08:53 +0000 Subject: [PATCH 5/5] fix(tests): add some 00:00 and 23:59 timestamps to the test to ensure no rogue jobs are slipping in --- tests/app/dao/test_jobs_dao.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index f92753a3d8..d599360a9f 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -163,16 +163,17 @@ def test_get_jobs_for_service_with_limit_days_param(sample_template): @freeze_time("2024-09-25 12:25:00") def test_get_jobs_for_service_with_limit_days_edge_case(sample_template): one_job = create_job(sample_template) - # create 1 job for each day of the last 10 days - for i in range(1, 10): - create_job(sample_template, created_at=datetime(2024, 9, 25 - i, i, 0, 0)) + # create 2 jobs for each day of the last 10 days + for i in range(1, 11): + create_job(sample_template, created_at=datetime(2024, 9, 25 - i, 0, 0, 0)) + create_job(sample_template, created_at=datetime(2024, 9, 25 - i, 23, 59, 59)) too_old_job = create_job(sample_template, created_at=datetime(2024, 9, 18, 0, 1, 0)) just_right_job = create_job(sample_template, created_at=datetime(2024, 9, 19, 0, 0, 0)) jobs_limit_days = dao_get_jobs_by_service_id(one_job.service_id, limit_days=7).items - assert len(jobs_limit_days) == 8 # one for each day: today (1) + the last 6 days (6) + one for just_right_job (1) + assert len(jobs_limit_days) == 14 # 2 for one for each day: today (1) + 12 the last 6 days (12) + one for just_right_job (1) assert one_job in jobs_limit_days assert just_right_job in jobs_limit_days assert too_old_job not in jobs_limit_days