feat(delete-apis): update following code review

antarest/core/exceptions.py

@@ -4,6 +4,8 @@
 
 from fastapi.exceptions import HTTPException
 
+MAX_BINDING_CONSTRAINTS_TO_DISPLAY = 10
+
 
 class ShouldNotHappenException(Exception):
     pass
@@ -326,32 +328,38 @@ def __init__(self, is_variant: bool) -> None:
 
 
 class AreaDeletionNotAllowed(HTTPException):
-    def __init__(self, uuid: str, message: t.Optional[str] = None) -> None:
-        msg = f"Area {uuid} is not allowed to be deleted"
-        if message:
-            msg += f"\n{message}"
+    def __init__(self, uuid: str, binding_constraints_ids: t.List[str]) -> None:
+        first_bcs_ids = ""
+        for i, bc in enumerate(binding_constraints_ids[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]):
+            first_bcs_ids += f"{i+1}- {bc}\n"
+        message = "Area is referenced in the following binding constraints:\n" + first_bcs_ids
+        msg = f"Area {uuid} is not allowed to be deleted\n{message}"
         super().__init__(
             HTTPStatus.FORBIDDEN,
             msg,
         )
 
 
 class LinkDeletionNotAllowed(HTTPException):
-    def __init__(self, uuid: str, message: t.Optional[str] = None) -> None:
-        msg = f"Link {uuid} is not allowed to be deleted"
-        if message:
-            msg += f"\n{message}"
+    def __init__(self, uuid: str, binding_constraints_ids: t.List[str]) -> None:
+        first_bcs_ids = ""
+        for i, bc in enumerate(binding_constraints_ids[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]):
+            first_bcs_ids += f"{i+1}- {bc}\n"
+        message = "Link is referenced in the following binding constraints:\n" + first_bcs_ids
+        msg = f"Link {uuid} is not allowed to be deleted\n{message}"
         super().__init__(
             HTTPStatus.FORBIDDEN,
             msg,
         )
 
 
 class ClusterDeletionNotAllowed(HTTPException):
-    def __init__(self, uuid: str, message: t.Optional[str] = None) -> None:
-        msg = f"Cluster {uuid} is not allowed to be deleted"
-        if message:
-            msg += f"\n{message}"
+    def __init__(self, uuid: str, binding_constraints_ids: t.List[str]) -> None:
+        first_bcs_ids = ""
+        for i, bc in enumerate(binding_constraints_ids[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]):
+            first_bcs_ids += f"{i+1}- {bc}\n"
+        message = "Cluster is referenced in the following binding constraints:\n" + first_bcs_ids
+        msg = f"Cluster {uuid} is not allowed to be deleted\n{message}"
         super().__init__(
             HTTPStatus.FORBIDDEN,
             msg,

antarest/study/service.py

@@ -140,7 +140,6 @@
 logger = logging.getLogger(__name__)
 
 MAX_MISSING_STUDY_TIMEOUT = 2  # days
-MAX_BINDING_CONSTRAINTS_TO_DISPLAY = 10
 
 
 def get_disk_usage(path: t.Union[str, Path]) -> int:
@@ -1864,14 +1863,9 @@ def delete_area(self, uuid: str, area_id: str, params: RequestParameters) -> Non
         self._assert_study_unarchived(study)
         referencing_binding_constraints = self.binding_constraint_manager.get_binding_constraints(
             study, ConstraintFilters(area_name=area_id)
-        )[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]
+        )
         if referencing_binding_constraints:
-            first_bcs_ids = ""
-            for i, bc in enumerate(referencing_binding_constraints):
-                first_bcs_ids += f"{i+1}- {bc.id}\n"
-            raise AreaDeletionNotAllowed(
-                area_id, "Area is referenced in the following binding constraints:\n" + first_bcs_ids
-            )
+            raise AreaDeletionNotAllowed(area_id, [bc.id for bc in referencing_binding_constraints])
         self.areas.delete_area(study, area_id)
         self.event_bus.push(
             Event(
@@ -1894,14 +1888,9 @@ def delete_link(
         link_id = LinkTerm(area1=area_from, area2=area_to).generate_id()
         referencing_binding_constraints = self.binding_constraint_manager.get_binding_constraints(
             study, ConstraintFilters(link_id=link_id)
-        )[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]
+        )
         if referencing_binding_constraints:
-            first_bcs_ids = ""
-            for i, bc in enumerate(referencing_binding_constraints):
-                first_bcs_ids += f"{i+1}- {bc.id}\n"
-            raise LinkDeletionNotAllowed(
-                link_id, "Link is referenced in the following binding constraints:\n" + first_bcs_ids
-            )
+            raise LinkDeletionNotAllowed(link_id, [bc.id for bc in referencing_binding_constraints])
         self.links.delete_link(study, area_from, area_to)
         self.event_bus.push(
             Event(
@@ -2544,27 +2533,26 @@ def get_matrix_with_index_and_header(
 
         return df_matrix
 
-    def assert_no_cluster_referenced_in_bcs(self, study: Study, area_id: str, cluster_ids: t.Sequence[str]) -> None:
+    def asserts_no_thermal_in_binding_constraints(
+        self, study: Study, area_id: str, cluster_ids: t.Sequence[str]
+    ) -> None:
         """
-        Check that no cluster is referenced in a binding constraint otherwise raise an ClusterDeletionNotAllowed Exception
+        Check that no cluster is referenced in a binding constraint otherwise raise an ClusterDeletionNotAllowed
+        Exception
 
         Args:
-            study: input study
+            study: input study for which an update is to be committed
             area_id: area ID to be checked
-            cluster_ids: cluster IDs to be checked
+            cluster_ids: IDs of the thermal clusters to be checked
 
-        Returns:
+        Returns: None
 
+        Raises: ClusterDeletionNotAllowed if a cluster is referenced in a binding constraint
         """
 
         for cluster_id in cluster_ids:
             referencing_binding_constraints = self.binding_constraint_manager.get_binding_constraints(
                 study, ConstraintFilters(cluster_id=f"{area_id}.{cluster_id}")
-            )[:MAX_BINDING_CONSTRAINTS_TO_DISPLAY]
+            )
             if referencing_binding_constraints:
-                first_bcs_ids = ""
-                for i, bc in enumerate(referencing_binding_constraints):
-                    first_bcs_ids += f"{i+1}- {bc.id}\n"
-                raise ClusterDeletionNotAllowed(
-                    cluster_id, "Cluster is referenced in the following binding constraints:\n" + first_bcs_ids
-                )
+                raise ClusterDeletionNotAllowed(cluster_id, [bc.id for bc in referencing_binding_constraints])

antarest/study/web/study_data_blueprint.py

@@ -2185,7 +2185,7 @@ def delete_thermal_clusters(
         )
         request_params = RequestParameters(user=current_user)
         study = study_service.check_study_access(uuid, StudyPermissionType.WRITE, request_params)
-        study_service.assert_no_cluster_referenced_in_bcs(study, area_id, cluster_ids)
+        study_service.asserts_no_thermal_in_binding_constraints(study, area_id, cluster_ids)
         study_service.thermal_manager.delete_clusters(study, area_id, cluster_ids)
 
     @bp.get(

tests/integration/study_data_blueprint/test_thermal.py

@@ -654,12 +654,9 @@ def test_lifecycle(
             json=[fr_gas_conventional_id],
         )
         assert res.status_code == 403, res.json()
-        assert res.json() == {
-            "description": "Cluster FR_Gas conventional is not allowed to be deleted\n"
-            "Cluster is referenced in the following binding constraints:"
-            "\n1- binding constraint\n",
-            "exception": "ClusterDeletionNotAllowed",
-        }
+        description = res.json()["description"]
+        assert all([elm in description for elm in [fr_gas_conventional, "binding constraint"]])
+        assert res.json()["exception"] == "ClusterDeletionNotAllowed"
 
         # delete the binding constraint
         res = client.delete(
@@ -1051,21 +1048,13 @@ def test_variant_lifecycle(self, client: TestClient, user_access_token: str, var
             "remove_cluster",
         ]
 
-    def test_thermal_cluster_deletion(self, client: TestClient, user_access_token: str) -> None:
+    def test_thermal_cluster_deletion(self, client: TestClient, user_access_token: str, study_id: str) -> None:
         """
         Test that creating a thermal cluster with invalid properties raises a validation error.
         """
 
         client.headers = {"Authorization": f"Bearer {user_access_token}"}
 
-        # Create a new study
-        res = client.post(
-            f"/v1/studies",
-            params={"name": "My Study"},
-        )
-        assert res.status_code in {200, 201}, res.json()
-        study_id = res.json()
-
         # Create an area "area_1" in the study
         res = client.post(
             f"/v1/studies/{study_id}/areas",

tests/integration/test_integration.py

@@ -1431,10 +1431,8 @@ def test_area_management(client: TestClient, admin_access_token: str) -> None:
     result = client.delete(f"/v1/studies/{study_id}/areas/area%201")
     assert result.status_code == 403, res.json()
     # verify the error message
-    assert (
-        result.json()["description"] == "Area area 1 is not allowed to be deleted\nArea is referenced "
-        "in the following binding constraints:\n1- binding constraint 1\n"
-    )
+    description = result.json()["description"]
+    assert all([elm in description for elm in ["area 1", "binding constraint 1"]])
     # check the exception
     assert result.json()["exception"] == "AreaDeletionNotAllowed"
 
@@ -1717,17 +1715,14 @@ def test_copy(client: TestClient, admin_access_token: str, study_id: str) -> Non
     assert res["public_mode"] == "READ"
 
 
-def test_links_deletion(client: TestClient, user_access_token: str) -> None:
+def test_links_deletion(client: TestClient, user_access_token: str, study_id: str) -> None:
     """
     Test the deletion of links between areas.
     """
 
     # set client headers to user access token
     client.headers = {"Authorization": f"Bearer {user_access_token}"}
 
-    # create a study
-    study_id = client.post("/v1/studies?name=test").json()
-
     # Create an area "area_1" in the study
     res = client.post(
         f"/v1/studies/{study_id}/areas",
@@ -1783,12 +1778,9 @@ def test_links_deletion(client: TestClient, user_access_token: str) -> None:
     # try to delete the link before deleting the binding constraint
     res = client.delete(f"/v1/studies/{study_id}/links/area_1/area_2")
     assert res.status_code == 403, res.json()
-    assert res.json() == {
-        "description": "Link area_1%area_2 is not allowed to be deleted\n"
-        "Link is referenced in the following binding constraints:\n"
-        "1- bc_1\n",
-        "exception": "LinkDeletionNotAllowed",
-    }
+    description = res.json()["description"]
+    assert all([elm in description for elm in ["area_1%area_2", "bc_1"]])
+    assert res.json()["exception"] == "LinkDeletionNotAllowed"
 
     # delete the binding constraint
     res = client.delete(f"/v1/studies/{study_id}/bindingconstraints/bc_1")