NRL-2099 Just warn if attempting to add already added permissions or remove already removed permissions. Allow the rest to proceed

anjalitrace2-nhs · anjalitrace2-nhs · commit d544f88b3984 · 2026-04-10T16:57:55.000+01:00
diff --git a/scripts/manage_permissions.py b/scripts/manage_permissions.py
@@ -172,7 +172,8 @@ def _save_updated_perms(
     print()
     show_perms(supplier_type, app_id, org_ods)
 
-    print("💡 Remember to update the lambda layer for these changes to take effect")
+    print()
+    print("💡 Remember to update the lambda layer for these changes to take effect 💡")
     print()
 
 
@@ -415,12 +416,16 @@ def add_perm(
         item for item in items_to_add if item in current_permission_items
     ]
     if already_added_items:
-        print(
-            f"❌ Error: Unable to add {permission_name}. These {permission_name} are already assigned to {lookup_path}:"
-        )
+        if len(already_added_items) == len(items_to_add):
+            print(
+                f"⏭️  Skipping: All requested {permission_name} are already assigned to {lookup_path}"
+            )
+            print()
+            return
+
+        print(f"👬 Skipping {permission_name} already assigned to {lookup_path}:")
         _print_perm_with_lookup("", already_added_items, permission_lookup)
         print()
-        return
 
     proposed_permission_items = current_permission_items + list(items_to_add)
     _print_perm_with_lookup(
@@ -497,17 +502,20 @@ def remove_perm(
     current_perms = json.loads(perms_ugly)
     current_permission_items: list = current_perms.get(permission_key, [])
 
-    # Cannot remove permission items that aren't already assigned
     items_not_assigned = [
         item for item in items_to_remove if item not in current_permission_items
     ]
     if items_not_assigned:
-        print(
-            f"❌ Error: Unable to remove {permission_name}. These {permission_name} aren't assigned to {lookup_path}:"
-        )
+        if len(items_not_assigned) == len(items_to_remove):
+            print(
+                f"⏭️  Skipping: None of the requested {permission_name} are assigned to {lookup_path}"
+            )
+            print()
+            return
+
+        print(f"👬 Skipping {permission_name} not already assigned to {lookup_path}:")
         _print_perm("", items_not_assigned)
         print()
-        return
 
     proposed_permission_items = [
         item for item in current_permission_items if item not in items_to_remove
@@ -547,7 +555,7 @@ def clear_perms(supplier_type: SupplierType, app_id: str, org_ods=None) -> None:
     current_perms = _get_perms_from_s3(lookup_path)
     if not current_perms or current_perms == "{}":
         print(
-            f"⏭️ No need to clear permissions for {lookup_path} as it currently has no permissions set."
+            f"⏭️  No need to clear permissions for {lookup_path} as it currently has no permissions set."
         )
         return
 
diff --git a/scripts/tests/test_manage_permissions.py b/scripts/tests/test_manage_permissions.py
@@ -234,13 +234,43 @@ def test_remove_perm_removes_access_control(mock_get_s3):
 
 
 @patch(f"{MODULE}._get_s3_client")
-def test_remove_perm_rejects_items_not_currently_assigned(mock_get_s3, capsys):
+def test_remove_perm_skips_update_if_all_items_not_currently_assigned(
+    mock_get_s3, capsys
+):
     existing_perms = {"types": [SAMPLE_POINTER_TYPES[0]]}
     mock_get_s3.return_value = _make_s3_mock(body=json.dumps(existing_perms).encode())
 
     remove_perm("types", "consumer", APP_ID, None, SAMPLE_POINTER_TYPES[1])
 
-    assert "aren't assigned" in capsys.readouterr().out
+    assert (
+        "Skipping: None of the requested pointer types are assigned"
+        in capsys.readouterr().out
+    )
+    mock_get_s3.put_object.assert_not_called()
+
+
+@patch(f"{MODULE}._get_s3_client")
+def test_remove_perm_skips_items_not_currently_assigned(mock_get_s3, capsys):
+    existing_perms = {"access_controls": [SAMPLE_ACCESS_CONTROLS[0]]}
+    s3 = _make_s3_mock(body=json.dumps(existing_perms).encode())
+    mock_get_s3.return_value = s3
+
+    remove_perm(
+        "access_controls",
+        "producer",
+        APP_ID,
+        ORG_ODS,
+        SAMPLE_ACCESS_CONTROLS[0],
+        SAMPLE_ACCESS_CONTROLS[1],
+    )
+
+    assert "Skipping access controls not already assigned" in capsys.readouterr().out
+    s3.put_object.assert_called_once_with(
+        Bucket=BUCKET,
+        Key=f"producer/{APP_ID}/{ORG_ODS}.json",
+        Body=json.dumps({"access_controls": []}, indent=4),
+        ContentType="application/json",
+    )
 
 
 @patch(f"{MODULE}._get_s3_client")
