NRL-1798 Remove return

Author: sandyforresternhs

scripts/delete_pointers_by_id.py

@@ -265,7 +265,7 @@ def _delete_pointers_by_id(
     ods_code: str,
     pointers_to_delete: List[str] | None = None,
     pointers_file: str | None = None,
-) -> Dict[str, Any]:
+) -> None:
     """
     Delete pointers from DynamoDB table.
 
@@ -303,7 +303,7 @@ def _delete_pointers_by_id(
 
     if not pointers_to_delete:
         end_time = datetime.now(tz=timezone.utc)
-        return _build_and_write_result(
+        _build_and_write_result(
             PointerDeletionContext(
                 pointers_to_delete=pointers_to_delete,
                 ods_code=ods_code,
@@ -317,6 +317,7 @@ def _delete_pointers_by_id(
                 output_filename=output_filename,
             )
         )
+        return
 
     print(
         f"Validating {len(pointers_to_delete)} pointers against ODS code {ods_code}..."
@@ -332,7 +333,7 @@ def _delete_pointers_by_id(
     if not matched_pointers:
         print(f"None of the pointer IDs are a match for ODS code {ods_code}. Exiting.")
         end_time = datetime.now(tz=timezone.utc)
-        return _build_and_write_result(
+        _build_and_write_result(
             PointerDeletionContext(
                 pointers_to_delete=pointers_to_delete,
                 ods_code=ods_code,
@@ -346,6 +347,7 @@ def _delete_pointers_by_id(
                 output_filename=output_filename,
             )
         )
+        return
 
     print(f"Checking existence of {len(matched_pointers)} pointers in {table_name}...")
     existing_pointers, not_found_pointers = _batch_get_existing_pointers(
@@ -359,7 +361,7 @@ def _delete_pointers_by_id(
     if not existing_pointers:
         print("No pointers found to delete. Exiting.")
         end_time = datetime.now(tz=timezone.utc)
-        return _build_and_write_result(
+        _build_and_write_result(
             PointerDeletionContext(
                 pointers_to_delete=pointers_to_delete,
                 ods_code=ods_code,
@@ -373,14 +375,15 @@ def _delete_pointers_by_id(
                 output_filename=output_filename,
             )
         )
+        return
 
     # Proceed with deletion using BatchWriteItem
     pointers_deleted, failed_deletes = _batch_delete_pointers(
         table_name, existing_pointers
     )
 
     end_time = datetime.now(tz=timezone.utc)
-    result = _build_and_write_result(
+    _build_and_write_result(
         PointerDeletionContext(
             pointers_to_delete=pointers_to_delete,
             ods_code=ods_code,
@@ -395,7 +398,6 @@ def _delete_pointers_by_id(
         )
     )
     print(" Done")
-    return result
 
 
 if __name__ == "__main__":

scripts/tests/test_delete_pointers_by_id.py

@@ -1,6 +1,6 @@
 import json
 from datetime import datetime, timezone
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, call, patch
 
 import pytest
 from delete_pointers_by_id import (
@@ -232,11 +232,16 @@ def test_some_unprocessed(self, mock_dynamodb):
 
 class TestDeletePointersById:
     def test_missing_params(self):
-        with pytest.raises(ValueError):
+        with pytest.raises(
+            ValueError, match="Provide either pointers_to_delete or pointers_file"
+        ):
             _delete_pointers_by_id("t", "G3H9E")
 
     def test_both_params_provided(self):
-        with pytest.raises(ValueError):
+        with pytest.raises(
+            ValueError,
+            match="Provide either pointers_to_delete or pointers_file, not both",
+        ):
             _delete_pointers_by_id(
                 "t", "G3H9E", pointers_to_delete=["a"], pointers_file="./f"
             )
@@ -246,38 +251,36 @@ def test_both_params_provided(self):
     @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
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=[])
+
         mock_build.assert_called_once()
+
         mock_check.assert_not_called()
         mock_get.assert_not_called()
         mock_delete.assert_not_called()
 
+        call_args = mock_build.call_args[0][0]
+        assert call_args.pointers_to_delete == []
+        assert call_args.matched_pointers == []
+
     @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_no_matched_ods_codes(self, mock_delete, mock_get, mock_check, mock_build):
         mock_check.return_value = ([], ["RAT-1", "RAT-2"])
-        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
+
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=["RAT-1", "RAT-2"])
+
         mock_build.assert_called_once()
+        mock_check.assert_called_once()
         mock_get.assert_not_called()
         mock_delete.assert_not_called()
 
+        call_args = mock_build.call_args[0][0]
+        assert call_args.matched_pointers == []
+        assert call_args.mismatched_pointers == ["RAT-1", "RAT-2"]
+
     @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")
@@ -287,17 +290,17 @@ def test_successful_flow(self, mock_delete, mock_get, mock_check, mock_build):
         mock_check.return_value = (ids, [])
         mock_get.return_value = (ids, [])
         mock_delete.return_value = (ids, [])
-        mock_build.return_value = {
-            "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
-        assert result["deleted_pointers"]["count"] == 2
-        assert result["pointer_not_found"]["count"] == 0
+
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=ids)
+
         mock_build.assert_called_once()
+        mock_check.assert_called_once()
+        mock_get.assert_called_once()
+        mock_delete.assert_called_once()
+
+        call_args = mock_build.call_args[0][0]
+        assert call_args.pointers_deleted == ids
+        assert call_args.failed_deletes == []
 
     @patch("delete_pointers_by_id._build_and_write_result")
     @patch("delete_pointers_by_id._check_pointers_match_ods_code")
@@ -308,21 +311,16 @@ def test_partial_with_failures(self, mock_delete, mock_get, mock_check, mock_bui
         mock_check.return_value = (matched, ["RAT-1"])
         mock_get.return_value = (matched, [])
         mock_delete.return_value = (["G3H9E-1", "G3H9E-2"], ["G3H9E-3"])
-        mock_build.return_value = {
-            "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"]
-        )
-        assert result["ods_code_mismatched"]["count"] == 1
-        assert result["deleted_pointers"]["count"] == 2
-        assert result["failed_deletes"]["count"] == 1
-        assert "G3H9E-3" in result["failed_deletes"]["ids"]
+
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=matched + ["RAT-1"])
+
         mock_build.assert_called_once()
 
+        call_args = mock_build.call_args[0][0]
+        assert call_args.mismatched_pointers == ["RAT-1"]
+        assert call_args.pointers_deleted == ["G3H9E-1", "G3H9E-2"]
+        assert call_args.failed_deletes == ["G3H9E-3"]
+
     @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")
@@ -334,13 +332,30 @@ def test_some_pointers_not_found(
         mock_check.return_value = (matched, [])
         mock_get.return_value = (["G3H9E-1", "G3H9E-2"], ["G3H9E-3"])
         mock_delete.return_value = (["G3H9E-1", "G3H9E-2"], [])
-        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
-        assert "G3H9E-3" in result["pointer_not_found"]["ids"]
-        assert result["deleted_pointers"]["count"] == 2
+
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=matched)
+
+        mock_build.assert_called_once()
+
+        call_args = mock_build.call_args[0][0]
+        assert call_args.not_found_pointers == ["G3H9E-3"]
+        assert call_args.pointers_deleted == ["G3H9E-1", "G3H9E-2"]
+
+    @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_no_existing_pointers(self, mock_delete, mock_get, mock_check, mock_build):
+        matched = ["G3H9E-1", "G3H9E-2"]
+        mock_check.return_value = (matched, [])
+        mock_get.return_value = ([], matched)
+
+        _delete_pointers_by_id("t", "G3H9E", pointers_to_delete=matched)
+
         mock_build.assert_called_once()
+        mock_delete.assert_not_called()
+
+        # Verify the context
+        call_args = mock_build.call_args[0][0]
+        assert call_args.not_found_pointers == matched
+        assert call_args.pointers_deleted == []