From 947e31efea8c533f9a1b2ff208826b48ab4c9d86 Mon Sep 17 00:00:00 2001
From: jbannister <Jon.Bannister@man.com>
Date: Fri, 13 Dec 2024 13:12:44 +0000
Subject: [PATCH] Fix an issue where deleting a report was not deleting the
 associated GridFS objects as expected.

---
 notebooker/serialization/mongo.py | 11 +++++------
 pyproject.toml                    |  5 ++++-
 tests/integration/test_mongo.py   |  9 ++++-----
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/notebooker/serialization/mongo.py b/notebooker/serialization/mongo.py
index 4ddf97b2..7f8e1df5 100644
--- a/notebooker/serialization/mongo.py
+++ b/notebooker/serialization/mongo.py
@@ -524,15 +524,14 @@ def n_all_results_for_report_name(self, report_name: str) -> int:
     def delete_result(self, job_id: AnyStr) -> Dict[str, Any]:
         result = self._get_raw_check_result(job_id)
         status = JobStatus.from_string(result["status"])
-        gridfs_filenames = []
-        if status == JobStatus.DONE:
-            gridfs_filenames = load_files_from_gridfs(self.result_data_store, result, do_read=False)
-        elif status in (JobStatus.ERROR, JobStatus.TIMEOUT, JobStatus.CANCELLED):
-            gridfs_filenames = [_error_info_filename(job_id)]
+        gridfs_filenames = load_files_from_gridfs(self.result_data_store, result, do_read=False)
+        if status in (JobStatus.ERROR, JobStatus.TIMEOUT, JobStatus.CANCELLED):
+            gridfs_filenames.append(_error_info_filename(job_id))
         self.update_check_status(job_id, JobStatus.DELETED)
         for filename in gridfs_filenames:
             logger.info(f"Deleting {filename}")
-            self.result_data_store.delete(filename)
+            for grid_out in self.result_data_store.find({"filename": filename}):
+                self.result_data_store.delete(grid_out._id)
         return {"deleted_result_document": result, "gridfs_filenames": gridfs_filenames}
 
 
diff --git a/pyproject.toml b/pyproject.toml
index 66c2d539..1c19393a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,3 +1,6 @@
 [tool.black]
 line-length = 120
-target-version = ['py36', 'py37', 'py38']
\ No newline at end of file
+target-version = ['py36', 'py37', 'py38']
+
+[tool.ruff]
+line-length = 120
\ No newline at end of file
diff --git a/tests/integration/test_mongo.py b/tests/integration/test_mongo.py
index b3a6ccf6..57347d21 100644
--- a/tests/integration/test_mongo.py
+++ b/tests/integration/test_mongo.py
@@ -2,7 +2,6 @@
 import uuid
 
 import pytest
-from gridfs.errors import NoFile
 
 from notebooker.constants import JobStatus, NotebookResultComplete, NotebookResultError
 from notebooker.serialization.serialization import initialize_serializer_from_config
@@ -83,9 +82,9 @@ def test_delete(bson_library, webapp_config):
     assert result is not None
     assert result.raw_html == raw_html
     deleted_stuff = serializer.delete_result(job_id)
+    assert [r for r in serializer.result_data_store.list()] == []
     for filename in deleted_stuff["gridfs_filenames"]:
-        with pytest.raises(NoFile):
-            serializer.result_data_store.get(filename)
+        assert serializer.result_data_store.exists(filename=filename) is False
     assert serializer.get_check_result(job_id) is None
 
 
@@ -111,7 +110,7 @@ def test_delete_error(bson_library, webapp_config, job_status):
     result = serializer.get_check_result(job_id)
     assert result is not None
     deleted_stuff = serializer.delete_result(job_id)
+    assert [r for r in serializer.result_data_store.list()] == []
     for filename in deleted_stuff["gridfs_filenames"]:
-        with pytest.raises(NoFile):
-            serializer.result_data_store.get(filename)
+        assert serializer.result_data_store.exists(filename=filename) is False
     assert serializer.get_check_result(job_id) is None