diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 8839307c01..613387dca8 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -113,9 +113,11 @@ def run_scheduled_jobs(): def archive_pending_files(): try: number_of_files_archived = dao_archive_pending_files() + base_params = {"number_of_files_archived": number_of_files_archived} current_app.logger.info( "Archived %(number_of_files_archived)s files created more than 24 hours ago that are still in pending", - extra={"number_of_files_archived": number_of_files_archived}, + base_params, + extra={**base_params}, ) except SQLAlchemyError: current_app.logger.exception("Failed to archive pending files") diff --git a/app/dao/template_email_files_dao.py b/app/dao/template_email_files_dao.py index 7dc2af53c7..a0b83bd433 100644 --- a/app/dao/template_email_files_dao.py +++ b/app/dao/template_email_files_dao.py @@ -137,22 +137,28 @@ def dao_make_pending_template_email_file_live(template_email_file: TemplateEmail @version_class( VersionOptions(TemplateEmailFile, history_class=TemplateEmailFileHistory), ) -def dao_archive_template_email_file(file_to_archive, archived_by_id, template_version=None): - if not template_version: - template_version = Template.query.get(file_to_archive.template_id).version - if not file_to_archive.archived_at: - file_to_archive.archived_at = datetime.datetime.utcnow() - file_to_archive.archived_by_id = archived_by_id - file_to_archive.template_version = template_version - db.session.add(file_to_archive) +def dao_archive_template_email_file(file_to_archive, archived_by_id, template_version): + _archive_template_email_file(file_to_archive, archived_by_id, template_version) +@autocommit def dao_archive_pending_files(): files_in_pending = TemplateEmailFile.query.filter( TemplateEmailFile.pending, + TemplateEmailFile.archived_at == None, # noqa: E711 datetime.datetime.utcnow() - TemplateEmailFile.created_at > datetime.timedelta(hours=current_app.config.get("TEMPLATE_EMAIL_FILE_ARCHIVE_PERIOD_IN_HOURS")), ).all() for file in files_in_pending: - dao_archive_template_email_file(file, archived_by_id=file.created_by_id) + _archive_template_email_file(file, file.created_by_id) return len(files_in_pending) + + +def _archive_template_email_file(file_to_archive, archived_by_id, template_version=None): + if not template_version: + template_version = Template.query.get(file_to_archive.template_id).version + if not file_to_archive.archived_at: + file_to_archive.archived_at = datetime.datetime.utcnow() + file_to_archive.archived_by_id = archived_by_id + file_to_archive.template_version = template_version + db.session.add(file_to_archive) diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 477d133946..aab8ada9f3 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -109,30 +109,45 @@ def test_should_update_scheduled_jobs_and_put_on_queue(mock_celery_task, sample_ ) -@pytest.mark.parametrize("pending_expirary_exceeded", (True, False)) -def test_archive_pending_files_only_scopes_stale_files(sample_email_template, pending_expirary_exceeded): - with freeze_time("2016-01-01 01:00:00.000000"): - pending_file = create_template_email_file( - template_id=sample_email_template.id, - created_by_id=sample_email_template.created_by_id, - pending=True, - created_at=datetime.utcnow(), - ) - live_file = create_template_email_file( - template_id=sample_email_template.id, - created_by_id=sample_email_template.created_by_id, - pending=False, - created_at=datetime.utcnow(), - ) - with freeze_time("2016-01-02 01:00:01.00000" if pending_expirary_exceeded else "2016-01-02 00:00:00.00000"): +@pytest.mark.parametrize("pending_expiry_exceeded", (True, False)) +def test_archive_pending_files_only_scopes_stale_files(sample_email_template, pending_expiry_exceeded, caplog): + pending_file = create_template_email_file( + template_id=sample_email_template.id, + created_by_id=sample_email_template.created_by_id, + pending=True, + created_at=datetime.fromisoformat("2016-01-01 01:00:00.000000"), + version=0, + ) + pending_file_already_archived = create_template_email_file( + template_id=sample_email_template.id, + created_by_id=sample_email_template.created_by_id, + pending=True, + created_at=datetime.fromisoformat("2016-01-01 01:00:00.000000"), + version=0, + archived_at=datetime.fromisoformat("2016-01-01 03:30:00.000000"), + ) + live_file = create_template_email_file( + template_id=sample_email_template.id, + created_by_id=sample_email_template.created_by_id, + pending=False, + created_at=datetime.fromisoformat("2016-01-01 01:00:00.000000"), + version=0, + ) + with freeze_time("2016-01-02 01:00:01.00000" if pending_expiry_exceeded else "2016-01-02 00:00:00.00000"): archive_pending_files() - expected_archived_file = dao_get_template_email_file_by_id(str(pending_file.id)) - expected_live_file = dao_get_template_email_file_by_id(str(live_file.id)) - if pending_expirary_exceeded: - assert expected_archived_file.archived_at == datetime.utcnow() - else: - assert not expected_archived_file.archived_at - assert not expected_live_file.archived_at + expected_archived_file = dao_get_template_email_file_by_id(str(pending_file.id)) + expected_pending_file_already_archived = dao_get_template_email_file_by_id(str(pending_file_already_archived.id)) + expected_live_file = dao_get_template_email_file_by_id(str(live_file.id)) + assert not expected_live_file.archived_at + assert expected_pending_file_already_archived.archived_at == datetime.fromisoformat("2016-01-01 03:30:00.000000") + assert len(caplog.messages) == 1 + if pending_expiry_exceeded: + assert expected_archived_file.archived_at == datetime.fromisoformat("2016-01-02 01:00:01.00000") + assert expected_archived_file.created_at == datetime.fromisoformat("2016-01-01 01:00:00.000000") + assert "Archived 1 files created more than 24 hours ago that are still in pending" in caplog.messages + else: + assert not expected_archived_file.archived_at + assert "Archived 0 files created more than 24 hours ago that are still in pending" in caplog.messages def test_should_update_all_scheduled_jobs_and_put_on_queue(sample_template, mock_celery_task): diff --git a/tests/app/db.py b/tests/app/db.py index f056dd262c..2da2015619 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -205,6 +205,8 @@ def create_template_email_file( retention_period=90, validate_users_email=True, pending=False, + version=0, + archived_at=None, ): data = { "filename": filename, @@ -215,6 +217,8 @@ def create_template_email_file( "created_by_id": created_by_id, "created_at": created_at if created_at is not None else datetime.utcnow(), "pending": pending, + "version": version, + "archived_at": archived_at, } template_email_file = TemplateEmailFile(**data) if pending: