NRL-1798 Update result with filename instead of path

Author: sandyforresternhs

scripts/delete_pointers_by_id.py

@@ -83,7 +83,7 @@ class PointerDeletionContext:
     failed_deletes: list[str]
     start_time: datetime
     end_time: datetime
-    output_file_path: str
+    output_filename: str
 
 
 def _build_and_write_result(ctx: PointerDeletionContext) -> Dict[str, Any]:
@@ -109,13 +109,17 @@ def _build_and_write_result(ctx: PointerDeletionContext) -> Dict[str, Any]:
         },
         "failed_deletes": {"count": len(ctx.failed_deletes), "ids": ctx.failed_deletes},
         "deletes-took-secs": timedelta.total_seconds(ctx.end_time - ctx.start_time),
-        "output_file_path": ctx.output_file_path,
+        "output_filename": ctx.output_filename,
     }
+
+    script_dir = os.path.dirname(os.path.abspath(__file__)) or "."
+    output_file_path = os.path.join(script_dir, ctx.output_filename)
+
     try:
-        _write_result_file(result, ctx.output_file_path)
+        _write_result_file(result, output_file_path)
     except Exception as exc:
         result["_output_error"] = (
-            f"Failed to write result file {ctx.output_file_path}: {exc}"
+            f"Failed to write result file {ctx.output_filename}: {exc}"
         )
     _print_summary(result)
     return result
@@ -151,13 +155,13 @@ def count_from(field):
     print(f"  pointer_not_found:  {count_from('pointer_not_found')}")
     print(f"  deleted_pointers:   {count_from('deleted_pointers')}")
     print(f"  failed_deletes:     {count_from('failed_deletes')}")
-    if "_output_error" in result:
-        print(f"  output_error:       {result['_output_error']}")
     if "deletes-took-secs" in result:
         print(f"  deletes-took-secs:  {result.get('deletes-took-secs')}")
-    if "output_file_path" in result:
-        output_filename = os.path.basename(result.get("output_file_path"))
-        print(f"  See output file for full results:  {output_filename}")
+
+    if "_output_error" in result:
+        print(f"  output_error:       {result['_output_error']}")
+    elif "output_filename" in result:
+        print(f"  See output file for full results:  {result.get('output_filename')}")
     print("*******************************************************")
 
 
@@ -295,10 +299,7 @@ def _delete_pointers_by_id(
 
     start_time = datetime.now(tz=timezone.utc)
     timestamp = start_time.strftime("%Y%m%dT%H%M%SZ")
-    script_dir = os.path.dirname(os.path.abspath(__file__)) or "."
-    output_file_path = os.path.join(
-        script_dir, f"delete_results_{ods_code}_{timestamp}.json"
-    )
+    output_filename = f"delete_results_{ods_code}_{timestamp}.json"
 
     if not pointers_to_delete:
         end_time = datetime.now(tz=timezone.utc)
@@ -313,7 +314,7 @@ def _delete_pointers_by_id(
                 failed_deletes=[],
                 start_time=start_time,
                 end_time=end_time,
-                output_file_path=output_file_path,
+                output_filename=output_filename,
             )
         )
 
@@ -342,7 +343,7 @@ def _delete_pointers_by_id(
                 failed_deletes=[],
                 start_time=start_time,
                 end_time=end_time,
-                output_file_path=output_file_path,
+                output_filename=output_filename,
             )
         )
 
@@ -369,7 +370,7 @@ def _delete_pointers_by_id(
                 failed_deletes=[],
                 start_time=start_time,
                 end_time=end_time,
-                output_file_path=output_file_path,
+                output_filename=output_filename,
             )
         )
 
@@ -390,7 +391,7 @@ def _delete_pointers_by_id(
             failed_deletes=failed_deletes,
             start_time=start_time,
             end_time=end_time,
-            output_file_path=output_file_path,
+            output_filename=output_filename,
         )
     )
     print(" Done")

scripts/tests/test_delete_pointers_by_id.py

@@ -41,33 +41,6 @@ def test_load_empty_file(self, mock_open):
         assert result == []
 
 
-class TestBuildAndWriteResult:
-    @patch("delete_pointers_by_id._write_result_file")
-    @patch("delete_pointers_by_id._print_summary")
-    def test_build_and_write_result(self, mock_print, mock_write):
-        start_time = datetime.now(tz=timezone.utc)
-        end_time = datetime.now(tz=timezone.utc)
-        ctx = PointerDeletionContext(
-            pointers_to_delete=["G3H9E-1", "G3H9E-2"],
-            ods_code="G3H9E",
-            matched_pointers=["G3H9E-1", "G3H9E-2"],
-            mismatched_pointers=[],
-            not_found_pointers=[],
-            pointers_deleted=["G3H9E-1", "G3H9E-2"],
-            failed_deletes=[],
-            start_time=start_time,
-            end_time=end_time,
-            output_file_path="/tmp/test.json",
-        )
-        result = _build_and_write_result(ctx)
-
-        assert result["pointers_to_delete"] == 2
-        assert result["deleted_pointers"]["count"] == 2
-        assert result["ods_code"] == "G3H9E"
-        mock_write.assert_called_once()
-        mock_print.assert_called_once()
-
-
 class TestParseJsonPointers:
     def test_invalid_json(self):
         content = '[{"id": "G3H9E-id1"'
@@ -131,6 +104,60 @@ def test_empty_plain_text(self):
         assert result == []
 
 
+class TestBuildAndWriteResult:
+    @patch("delete_pointers_by_id._write_result_file")
+    @patch("delete_pointers_by_id._print_summary")
+    def test_build_and_write_result_success(self, mock_print, mock_write):
+        start_time = datetime.now(tz=timezone.utc)
+        end_time = datetime.now(tz=timezone.utc)
+        ctx = PointerDeletionContext(
+            pointers_to_delete=["G3H9E-1", "G3H9E-2"],
+            ods_code="G3H9E",
+            matched_pointers=["G3H9E-1", "G3H9E-2"],
+            mismatched_pointers=[],
+            not_found_pointers=[],
+            pointers_deleted=["G3H9E-1", "G3H9E-2"],
+            failed_deletes=[],
+            start_time=start_time,
+            end_time=end_time,
+            output_filename="delete_results_G3H9E_20231125T120000Z.json",
+        )
+        result = _build_and_write_result(ctx)
+
+        assert result["pointers_to_delete"] == 2
+        assert result["deleted_pointers"]["count"] == 2
+        assert result["ods_code"] == "G3H9E"
+        assert result["output_filename"] == "delete_results_G3H9E_20231125T120000Z.json"
+        assert "_output_error" not in result
+        mock_write.assert_called_once()
+        mock_print.assert_called_once()
+
+    @patch("delete_pointers_by_id._write_result_file")
+    @patch("delete_pointers_by_id._print_summary")
+    def test_build_and_write_result_with_error(self, mock_print, mock_write):
+        mock_write.side_effect = Exception("Write failed")
+        start_time = datetime.now(tz=timezone.utc)
+        end_time = datetime.now(tz=timezone.utc)
+        ctx = PointerDeletionContext(
+            pointers_to_delete=["G3H9E-1"],
+            ods_code="G3H9E",
+            matched_pointers=["G3H9E-1"],
+            mismatched_pointers=[],
+            not_found_pointers=[],
+            pointers_deleted=["G3H9E-1"],
+            failed_deletes=[],
+            start_time=start_time,
+            end_time=end_time,
+            output_filename="delete_results_G3H9E_20231125T120000Z.json",
+        )
+        result = _build_and_write_result(ctx)
+
+        assert "_output_error" in result
+        assert "Write failed" in result["_output_error"]
+        mock_write.assert_called_once()
+        mock_print.assert_called_once()
+
+
 class TestCheckPointersMatchOdsCode:
     def test_all_match(self):
         ods = "G3H9E"
@@ -173,16 +200,6 @@ def test_none_exist(self, mock_dynamodb):
         existing, not_found = _batch_get_existing_pointers(table, ids)
         assert existing == [] and not_found == ids
 
-    @patch("delete_pointers_by_id.dynamodb")
-    def test_some_exist(self, mock_dynamodb):
-        table = "t"
-        ids = ["G3H9E-1", "G3H9E-3"]
-        mock_dynamodb.batch_get_item.return_value = {
-            "Responses": {table: [{"pk": {"S": "D#G3H9E-1"}}]}
-        }
-        existing, not_found = _batch_get_existing_pointers(table, ids)
-        assert existing == ["G3H9E-1"] and not_found == ["G3H9E-3"]
-
 
 class TestBatchDeletePointers:
     @patch("delete_pointers_by_id.dynamodb")
@@ -224,6 +241,23 @@ def test_both_params_provided(self):
                 "t", "G3H9E", pointers_to_delete=["a"], pointers_file="./f"
             )
 
+    @patch("delete_pointers_by_id._build_and_write_result")
+    @patch("delete_pointers_by_id._check_pointers_match_ods_code")
+    @patch("delete_pointers_by_id._batch_get_existing_pointers")
+    @patch("delete_pointers_by_id._batch_delete_pointers")
+    def test_empty_pointers_list(self, mock_delete, mock_get, mock_check, mock_build):
+        mock_build.return_value = {
+            "pointers_to_delete": 0,
+            "ods_code_matched": {"count": 0},
+            "output_filename": "delete_results_G3H9E_20231125T120000Z.json",
+        }
+        result = _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=[])
+        assert result["pointers_to_delete"] == 0
+        mock_build.assert_called_once()
+        mock_check.assert_not_called()
+        mock_get.assert_not_called()
+        mock_delete.assert_not_called()
+
     @patch("delete_pointers_by_id._build_and_write_result")
     @patch("delete_pointers_by_id._check_pointers_match_ods_code")
     @patch("delete_pointers_by_id._batch_get_existing_pointers")
@@ -233,13 +267,16 @@ def test_no_matched_ods_codes(self, mock_delete, mock_get, mock_check, mock_buil
         mock_build.return_value = {
             "ods_code_matched": {"count": 0},
             "ods_code_mismatched": {"count": 2},
+            "output_filename": "delete_results_G3H9E_20231125T120000Z.json",
         }
         result = _delete_pointers_by_id(
             "t", "G3H9E", pointers_to_delete=["RAT-1", "RAT-2"]
         )
         assert result["ods_code_matched"]["count"] == 0
         assert result["ods_code_mismatched"]["count"] == 2
         mock_build.assert_called_once()
+        mock_get.assert_not_called()
+        mock_delete.assert_not_called()
 
     @patch("delete_pointers_by_id._build_and_write_result")
     @patch("delete_pointers_by_id._check_pointers_match_ods_code")
@@ -254,6 +291,7 @@ def test_successful_flow(self, mock_delete, mock_get, mock_check, mock_build):
             "pointers_to_delete": 2,
             "deleted_pointers": {"count": 2},
             "pointer_not_found": {"count": 0},
+            "output_filename": "delete_results_G3H9E_20231125T120000Z.json",
         }
         result = _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=ids)
         assert result["pointers_to_delete"] == 2
@@ -274,6 +312,7 @@ def test_partial_with_failures(self, mock_delete, mock_get, mock_check, mock_bui
             "ods_code_mismatched": {"count": 1},
             "deleted_pointers": {"count": 2},
             "failed_deletes": {"count": 1, "ids": ["G3H9E-3"]},
+            "output_filename": "delete_results_G3H9E_20231125T120000Z.json",
         }
         result = _delete_pointers_by_id(
             "t", "G3H9E", pointers_to_delete=matched + ["RAT-1"]
@@ -298,6 +337,7 @@ def test_some_pointers_not_found(
         mock_build.return_value = {
             "pointer_not_found": {"count": 1, "ids": ["G3H9E-3"]},
             "deleted_pointers": {"count": 2},
+            "output_filename": "delete_results_G3H9E_20231125T120000Z.json",
         }
         result = _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=matched)
         assert result["pointer_not_found"]["count"] == 1