Merge pull request #415 from ral-facilities/update-catalogue-category…

…-property-e2e-tests-#377

Update catalogue category property e2e tests #377

test/e2e/conftest.py

@@ -90,22 +90,36 @@ class E2ETestHelpers:
     """
 
     @staticmethod
-    def check_created_and_modified_times_updated_correctly(post_response: Response, patch_response: Response):
-        """Checks that an updated entity has a created_time that is the same as its original, but an updated_time
+    def check_created_and_modified_times_updated_correctly(post_response: Response, new_response: Response):
+        """Checks that an updated entity has a `created_time` that is the same as its original, but an `updated_time`
         that is newer
 
         :param post_response: Original response for the entity post request
-        :param patch_response: Updated response for the entity patch request
+        :param new_response: Updated response for the entity patch/get request
         """
 
         original_data = post_response.json()
-        updated_data = patch_response.json()
+        updated_data = new_response.json()
 
         assert original_data["created_time"] == updated_data["created_time"]
         assert datetime.fromisoformat(updated_data["modified_time"]) > datetime.fromisoformat(
             original_data["modified_time"]
         )
 
+    @staticmethod
+    def check_created_and_modified_times_not_updated(post_response: Response, new_response: Response):
+        """Checks that an entity still has the same `created_time` and `updated_time` as its original
+
+        :param post_response: Original response for the entity post request
+        :param new_response: Updated response for the entity patch/get request
+        """
+
+        original_data = post_response.json()
+        updated_data = new_response.json()
+
+        assert original_data["created_time"] == updated_data["created_time"]
+        assert original_data["modified_time"] == updated_data["modified_time"]
+
     @staticmethod
     def replace_unit_values_with_ids_in_properties(data: dict, unit_value_id_dict: dict[str, str]) -> dict:
         """Inserts unit IDs into some data that may have a 'properties' list within it while removing the unit value.

test/e2e/test_catalogue_category.py

@@ -57,7 +57,7 @@ def setup(self, test_client):
         self.test_client = test_client
         self.unit_value_id_dict = {}
 
-    def add_unit_value_and_id(self, unit_value: str, unit_id: str) -> None:
+    def set_unit_value_and_id(self, unit_value: str, unit_id: str) -> None:
         """
         Stores a unit value and ID inside the `unit_value_id_dict` for tests that need to have a
         non-existent or invalid unit ID.
@@ -75,7 +75,7 @@ def post_unit(self, unit_post_data: dict) -> None:
         """
 
         post_response = self.test_client.post("/v1/units", json=unit_post_data)
-        self.add_unit_value_and_id(unit_post_data["value"], post_response.json()["id"])
+        self.set_unit_value_and_id(unit_post_data["value"], post_response.json()["id"])
 
     def post_catalogue_category(self, catalogue_category_data: dict) -> Optional[str]:
         """
@@ -263,16 +263,16 @@ def test_create_leaf_with_properties(self):
         self.check_post_catalogue_category_success(CATALOGUE_CATEGORY_GET_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM)
 
     def test_create_leaf_with_properties_with_non_existent_unit_id(self):
-        """Test creating a leaf catalogue category with a property with a non-existent unit ID provided."""
+        """Test creating a leaf catalogue category with a property with a non-existent unit ID."""
 
-        self.add_unit_value_and_id("mm", str(ObjectId()))
+        self.set_unit_value_and_id("mm", str(ObjectId()))
         self.post_catalogue_category(CATALOGUE_CATEGORY_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM)
         self.check_post_catalogue_category_failed_with_detail(422, "The specified unit does not exist")
 
     def test_create_leaf_with_properties_with_invalid_unit_id(self):
-        """Test creating a leaf catalogue category with a property with an invalid unit ID provided."""
+        """Test creating a leaf catalogue category with a property with an invalid unit ID."""
 
-        self.add_unit_value_and_id("mm", "invalid-id")
+        self.set_unit_value_and_id("mm", "invalid-id")
         self.post_catalogue_category(CATALOGUE_CATEGORY_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM)
         self.check_post_catalogue_category_failed_with_detail(422, "The specified unit does not exist")
 
@@ -327,7 +327,7 @@ def test_create_leaf_property_with_invalid_allowed_values_type(self):
         )
 
     def test_create_leaf_property_with_empty_allowed_values_list(self):
-        """Test creating a leaf catalogue category with a property with an allowed values list that is empty."""
+        """Test creating a leaf catalogue category with a property with an empty allowed values list."""
 
         self.post_leaf_catalogue_category_with_allowed_values("string", {"type": "list", "values": []})
         self.check_post_catalogue_category_failed_with_validation_message(
@@ -336,8 +336,8 @@ def test_create_leaf_property_with_empty_allowed_values_list(self):
         )
 
     def test_create_leaf_with_string_property_with_allowed_values_list_invalid_value(self):
-        """Test creating a leaf catalogue category with a string property with an allowed values list with an invalid
-        number value in it."""
+        """Test creating a leaf catalogue category with a string property with an allowed values list containing an
+        invalid number."""
 
         self.post_leaf_catalogue_category_with_allowed_values("string", {"type": "list", "values": ["1", "2", 3, "4"]})
         self.check_post_catalogue_category_failed_with_validation_message(
@@ -348,7 +348,7 @@ def test_create_leaf_with_string_property_with_allowed_values_list_invalid_value
 
     def test_create_leaf_with_string_property_with_allowed_values_list_duplicate_value(self):
         """Test creating a leaf catalogue category with a string property with an allowed values list with a duplicate
-        string value in it."""
+        string value."""
 
         # Capitalisation is different as it shouldn't matter for this test
         self.post_leaf_catalogue_category_with_allowed_values(
@@ -360,8 +360,8 @@ def test_create_leaf_with_string_property_with_allowed_values_list_duplicate_val
         )
 
     def test_create_leaf_with_number_property_with_allowed_values_list_invalid_value(self):
-        """Test creating a leaf catalogue category with a number property with an allowed values list with an invalid
-        number value in it."""
+        """Test creating a leaf catalogue category with a number property with an allowed values list containing an
+        invalid number."""
 
         self.post_leaf_catalogue_category_with_allowed_values("number", {"type": "list", "values": [1, 2, "3", 4]})
         self.check_post_catalogue_category_failed_with_validation_message(
@@ -372,7 +372,7 @@ def test_create_leaf_with_number_property_with_allowed_values_list_invalid_value
 
     def test_create_leaf_with_number_property_with_allowed_values_list_duplicate_value(self):
         """Test creating a leaf catalogue category with a number property with an allowed values list with a duplicate
-        number value in it."""
+        number value."""
 
         self.post_leaf_catalogue_category_with_allowed_values("number", {"type": "list", "values": [1, 2, 1, 3]})
         self.check_post_catalogue_category_failed_with_validation_message(
@@ -411,9 +411,9 @@ def check_get_catalogue_category_success(self, expected_catalogue_category_get_d
         Checks that a prior call to `get_catalogue_category` gave a successful response with the expected data returned.
 
         :param expected_catalogue_category_get_data: Dictionary containing the expected catalogue category data returned
-                                                     as would be required for a `CatalogueCategorySchema`. Does not need
-                                                     unit IDs as they will be added automatically to check they are as
-                                                     expected.
+                                        as would be required for a `CatalogueCategorySchema` but with any `unit_id`'s
+                                        replaced by the `unit` value in its properties as the IDs will be added
+                                        automatically.
         """
 
         assert self._get_response_catalogue_category.status_code == 200
@@ -777,7 +777,7 @@ def patch_properties_with_property_with_allowed_values(
             },
         )
 
-    def check_patch_catalogue_category_response_success(self, expected_catalogue_category_get_data: dict) -> None:
+    def check_patch_catalogue_category_success(self, expected_catalogue_category_get_data: dict) -> None:
         """
         Checks that a prior call to `patch_catalogue_category` gave a successful response with the expected data
         returned.
@@ -829,7 +829,7 @@ def test_partial_update_name(self):
 
         catalogue_category_id = self.post_catalogue_category(CATALOGUE_CATEGORY_POST_DATA_NON_LEAF_REQUIRED_VALUES_ONLY)
         self.patch_catalogue_category(catalogue_category_id, {"name": "New Name"})
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_REQUIRED_VALUES_ONLY, "name": "New Name", "code": "new-name"}
         )
 
@@ -842,7 +842,7 @@ def test_partial_update_parent_id(self):
         )
 
         self.patch_catalogue_category(catalogue_category_id, {"parent_id": parent_id})
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_B, "parent_id": parent_id}
         )
 
@@ -929,7 +929,7 @@ def test_partial_update_name_capitalisation(self):
             {**CATALOGUE_CATEGORY_POST_DATA_NON_LEAF_REQUIRED_VALUES_ONLY, "name": "Test catalogue category"}
         )
         self.patch_catalogue_category(catalogue_category_id, {"name": "Test Catalogue Category"})
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {
                 **CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_REQUIRED_VALUES_ONLY,
                 "name": "Test Catalogue Category",
@@ -950,7 +950,7 @@ def test_partial_update_non_leaf_all_valid_values_when_no_children(self):
             {**CATALOGUE_CATEGORY_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM, "parent_id": new_parent_id},
         )
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM, "parent_id": new_parent_id}
         )
 
@@ -962,7 +962,7 @@ def test_partial_update_non_leaf_to_leaf_without_properties(self):
 
         self.patch_catalogue_category(catalogue_category_id, {"is_leaf": True})
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_REQUIRED_VALUES_ONLY, "is_leaf": True}
         )
 
@@ -978,7 +978,7 @@ def test_partial_update_non_leaf_all_valid_values_when_has_child_catalogue_categ
 
         self.patch_catalogue_category(catalogue_category_id, update_data)
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_REQUIRED_VALUES_ONLY, **update_data, "code": "new-name"}
         )
 
@@ -1009,7 +1009,7 @@ def test_partial_update_leaf_all_valid_values_when_no_children(self):
             {**CATALOGUE_CATEGORY_POST_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_A, "parent_id": new_parent_id},
         )
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_A, "parent_id": new_parent_id}
         )
 
@@ -1025,7 +1025,7 @@ def test_partial_update_leaf_all_valid_values_when_has_child_catalogue_item(self
 
         self.patch_catalogue_category(catalogue_category_id, update_data)
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_LEAF_NO_PARENT_NO_PROPERTIES, **update_data, "code": "new-name"}
         )
 
@@ -1074,7 +1074,7 @@ def test_partial_update_leaf_to_non_leaf_with_properties(self):
             },
         )
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_A, "parent_id": new_parent_id}
         )
 
@@ -1083,7 +1083,7 @@ def test_partial_update_is_leaf_no_children(self):
 
         catalogue_category_id = self.post_catalogue_category(CATALOGUE_CATEGORY_POST_DATA_NON_LEAF_REQUIRED_VALUES_ONLY)
         self.patch_catalogue_category(catalogue_category_id, {"is_leaf": True})
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {**CATALOGUE_CATEGORY_GET_DATA_NON_LEAF_REQUIRED_VALUES_ONLY, "is_leaf": True}
         )
 
@@ -1112,7 +1112,7 @@ def test_partial_update_leaf_properties(self):
             {"properties": CATALOGUE_CATEGORY_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM["properties"]},
         )
 
-        self.check_patch_catalogue_category_response_success(
+        self.check_patch_catalogue_category_success(
             {
                 **CATALOGUE_CATEGORY_GET_DATA_LEAF_NO_PARENT_NO_PROPERTIES,
                 "properties": CATALOGUE_CATEGORY_GET_DATA_LEAF_NO_PARENT_WITH_PROPERTIES_MM["properties"],
@@ -1122,17 +1122,17 @@ def test_partial_update_leaf_properties(self):
     def test_partial_update_leaf_with_properties_with_non_existent_unit_id(self):
         """Test updating a leaf catalogue category's properties to have a property with a non-existent unit ID."""
 
-        self.add_unit_value_and_id("mm", str(ObjectId()))
+        self.set_unit_value_and_id("mm", str(ObjectId()))
         catalogue_category_id = self.post_catalogue_category(CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES)
         self.patch_catalogue_category(
             catalogue_category_id, {"properties": [CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT]}
         )
         self.check_patch_catalogue_category_failed_with_detail(422, "The specified unit does not exist")
 
     def test_partial_update_leaf_with_properties_with_invalid_unit_id(self):
-        """Test updating a leaf catalogue category's properties to have a property with an invalid unit ID provided."""
+        """Test updating a leaf catalogue category's properties to have a property with an invalid unit ID."""
 
-        self.add_unit_value_and_id("mm", "invalid-id")
+        self.set_unit_value_and_id("mm", "invalid-id")
         catalogue_category_id = self.post_catalogue_category(CATALOGUE_CATEGORY_POST_DATA_LEAF_NO_PARENT_NO_PROPERTIES)
         self.patch_catalogue_category(
             catalogue_category_id, {"properties": [CATALOGUE_CATEGORY_PROPERTY_DATA_NUMBER_NON_MANDATORY_WITH_MM_UNIT]}