fix bug where created_at is overwritten for pending files at version 0

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")

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)

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):

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: