Merge branch 'refactor-item-e2e-tests-#347' into refactor-units-tests-#…

…334

inventory_management_system_api/repositories/catalogue_item.py

@@ -2,8 +2,8 @@
 Module for providing a repository for managing catalogue items in a MongoDB database.
 """
 
-from datetime import datetime, timezone
 import logging
+from datetime import datetime, timezone
 from typing import List, Optional
 
 from bson import ObjectId
@@ -46,25 +46,6 @@ def create(self, catalogue_item: CatalogueItemIn, session: ClientSession = None)
         catalogue_item = self.get(str(result.inserted_id), session=session)
         return catalogue_item
 
-    def delete(self, catalogue_item_id: str, session: ClientSession = None) -> None:
-        """
-        Delete a catalogue item by its ID from a MongoDB database.
-
-        :param catalogue_item_id: The ID of the catalogue item to delete.
-        :param session: PyMongo ClientSession to use for database operations
-        :raises MissingRecordError: If the catalogue item doesn't exist.
-        """
-        catalogue_item_id = CustomObjectId(catalogue_item_id)
-        if self.has_child_elements(catalogue_item_id, session=session):
-            raise ChildElementsExistError(
-                f"Catalogue item with ID {str(catalogue_item_id)} has child elements and cannot be deleted"
-            )
-
-        logger.info("Deleting catalogue item with ID: %s from the database", catalogue_item_id)
-        result = self._catalogue_items_collection.delete_one({"_id": catalogue_item_id}, session=session)
-        if result.deleted_count == 0:
-            raise MissingRecordError(f"No catalogue item found with ID: {str(catalogue_item_id)}")
-
     def get(self, catalogue_item_id: str, session: ClientSession = None) -> Optional[CatalogueItemOut]:
         """
         Retrieve a catalogue item by its ID from a MongoDB database.
@@ -80,6 +61,29 @@ def get(self, catalogue_item_id: str, session: ClientSession = None) -> Optional
             return CatalogueItemOut(**catalogue_item)
         return None
 
+    def list(self, catalogue_category_id: Optional[str], session: ClientSession = None) -> List[CatalogueItemOut]:
+        """
+        Retrieve all catalogue items from a MongoDB database.
+
+        :param catalogue_category_id: The ID of the catalogue category to filter catalogue items by.
+        :param session: PyMongo ClientSession to use for database operations
+        :return: A list of catalogue items, or an empty list if no catalogue items are returned by the database.
+        """
+        query = {}
+        if catalogue_category_id:
+            catalogue_category_id = CustomObjectId(catalogue_category_id)
+            query["catalogue_category_id"] = catalogue_category_id
+
+        message = "Retrieving all catalogue items from the database"
+        if not query:
+            logger.info(message)
+        else:
+            logger.info("%s matching the provided catalogue category ID filter", message)
+            logger.debug("Provided catalogue category ID filter: %s", catalogue_category_id)
+
+        catalogue_items = self._catalogue_items_collection.find(query, session=session)
+        return [CatalogueItemOut(**catalogue_item) for catalogue_item in catalogue_items]
+
     def update(
         self, catalogue_item_id: str, catalogue_item: CatalogueItemIn, session: ClientSession = None
     ) -> CatalogueItemOut:
@@ -100,28 +104,24 @@ def update(
         catalogue_item = self.get(str(catalogue_item_id), session=session)
         return catalogue_item
 
-    def list(self, catalogue_category_id: Optional[str], session: ClientSession = None) -> List[CatalogueItemOut]:
+    def delete(self, catalogue_item_id: str, session: ClientSession = None) -> None:
         """
-        Retrieve all catalogue items from a MongoDB database.
+        Delete a catalogue item by its ID from a MongoDB database.
 
-        :param catalogue_category_id: The ID of the catalogue category to filter catalogue items by.
+        :param catalogue_item_id: The ID of the catalogue item to delete.
         :param session: PyMongo ClientSession to use for database operations
-        :return: A list of catalogue items, or an empty list if no catalogue items are returned by the database.
+        :raises MissingRecordError: If the catalogue item doesn't exist.
         """
-        query = {}
-        if catalogue_category_id:
-            catalogue_category_id = CustomObjectId(catalogue_category_id)
-            query["catalogue_category_id"] = catalogue_category_id
-
-        message = "Retrieving all catalogue items from the database"
-        if not query:
-            logger.info(message)
-        else:
-            logger.info("%s matching the provided catalogue category ID filter", message)
-            logger.debug("Provided catalogue category ID filter: %s", catalogue_category_id)
+        catalogue_item_id = CustomObjectId(catalogue_item_id)
+        if self.has_child_elements(catalogue_item_id, session=session):
+            raise ChildElementsExistError(
+                f"Catalogue item with ID {str(catalogue_item_id)} has child elements and cannot be deleted"
+            )
 
-        catalogue_items = self._catalogue_items_collection.find(query, session=session)
-        return [CatalogueItemOut(**catalogue_item) for catalogue_item in catalogue_items]
+        logger.info("Deleting catalogue item with ID: %s from the database", catalogue_item_id)
+        result = self._catalogue_items_collection.delete_one({"_id": catalogue_item_id}, session=session)
+        if result.deleted_count == 0:
+            raise MissingRecordError(f"No catalogue item found with ID: {str(catalogue_item_id)}")
 
     def has_child_elements(self, catalogue_item_id: CustomObjectId, session: ClientSession = None) -> bool:
         """

inventory_management_system_api/repositories/item.py

@@ -51,20 +51,6 @@ def create(self, item: ItemIn, session: ClientSession = None) -> ItemOut:
         item = self.get(str(result.inserted_id), session=session)
         return item
 
-    def delete(self, item_id: str, session: ClientSession = None) -> None:
-        """
-        Delete an item by its ID from a MongoDB database.
-
-        :param item_id: The ID of the item to delete.
-        :param session: PyMongo ClientSession to use for database operations
-        :raises MissingRecordError: If the item doesn't exist
-        """
-        item_id = CustomObjectId(item_id)
-        logger.info("Deleting item with ID: %s from the database", item_id)
-        result = self._items_collection.delete_one({"_id": item_id}, session=session)
-        if result.deleted_count == 0:
-            raise MissingRecordError(f"No item found with ID: {str(item_id)}")
-
     def get(self, item_id: str, session: ClientSession = None) -> Optional[ItemOut]:
         """
         Retrieve an item by its ID from a MongoDB database.
@@ -127,6 +113,20 @@ def update(self, item_id: str, item: ItemIn, session: ClientSession = None) -> I
         item = self.get(str(item_id), session=session)
         return item
 
+    def delete(self, item_id: str, session: ClientSession = None) -> None:
+        """
+        Delete an item by its ID from a MongoDB database.
+
+        :param item_id: The ID of the item to delete.
+        :param session: PyMongo ClientSession to use for database operations
+        :raises MissingRecordError: If the item doesn't exist
+        """
+        item_id = CustomObjectId(item_id)
+        logger.info("Deleting item with ID: %s from the database", item_id)
+        result = self._items_collection.delete_one({"_id": item_id}, session=session)
+        if result.deleted_count == 0:
+            raise MissingRecordError(f"No item found with ID: {str(item_id)}")
+
     def insert_property_to_all_in(
         self, catalogue_item_ids: List[ObjectId], property_in: PropertyIn, session: ClientSession = None
     ):

inventory_management_system_api/services/catalogue_item.py

@@ -97,14 +97,6 @@ def create(self, catalogue_item: CatalogueItemPostSchema) -> CatalogueItemOut:
             )
         )
 
-    def delete(self, catalogue_item_id: str) -> None:
-        """
-        Delete a catalogue item by its ID.
-
-        :param catalogue_item_id: The ID of the catalogue item to delete.
-        """
-        return self._catalogue_item_repository.delete(catalogue_item_id)
-
     def get(self, catalogue_item_id: str) -> Optional[CatalogueItemOut]:
         """
         Retrieve a catalogue item by its ID.
@@ -124,6 +116,7 @@ def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]:
         return self._catalogue_item_repository.list(catalogue_category_id)
 
     # pylint:disable=too-many-branches
+    # pylint:disable=too-many-locals
     def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchSchema) -> CatalogueItemOut:
         """
         Update a catalogue item by its ID.
@@ -169,15 +162,20 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchSchem
                     stored_catalogue_item.catalogue_category_id
                 )
 
+                # Ensure the properties are the same in every way ignoring the ids
+                invalid_action_error_message = (
+                    "Cannot move catalogue item to a category with different properties without "
+                    "specifying the new properties"
+                )
+                if len(current_catalogue_category.properties) != len(catalogue_category.properties):
+                    raise InvalidActionError(invalid_action_error_message)
+
                 old_to_new_id_map = {}
                 for current_catalogue_category_prop, catalogue_category_prop in zip(
                     current_catalogue_category.properties, catalogue_category.properties
                 ):
                     if not current_catalogue_category_prop.is_equal_without_id(catalogue_category_prop):
-                        raise InvalidActionError(
-                            "Cannot move catalogue item to a category with different properties without "
-                            "specifying the new properties"
-                        )
+                        raise InvalidActionError(invalid_action_error_message)
                     old_to_new_id_map[current_catalogue_category_prop.id] = catalogue_category_prop.id
 
                 # The IDs of the properties need to be updated to those of the new catalogue category
@@ -213,3 +211,11 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchSchem
             catalogue_item_id,
             CatalogueItemIn(**{**stored_catalogue_item.model_dump(), **update_data}),
         )
+
+    def delete(self, catalogue_item_id: str) -> None:
+        """
+        Delete a catalogue item by its ID.
+
+        :param catalogue_item_id: The ID of the catalogue item to delete.
+        """
+        return self._catalogue_item_repository.delete(catalogue_item_id)

inventory_management_system_api/services/item.py

@@ -97,13 +97,14 @@ def create(self, item: ItemPostSchema) -> ItemOut:
             ItemIn(**{**item.model_dump(), "properties": properties, "usage_status": usage_status.value})
         )
 
-    def delete(self, item_id: str) -> None:
+    def get(self, item_id: str) -> Optional[ItemOut]:
         """
-        Delete an item by its ID.
+        Retrieve an item by its ID
 
-        :param item_id: The ID of the item to delete.
+        :param item_id: The ID of the item to retrieve
+        :return: The retrieved item, or `None` if not found
         """
-        return self._item_repository.delete(item_id)
+        return self._item_repository.get(item_id)
 
     def list(self, system_id: Optional[str], catalogue_item_id: Optional[str]) -> List[ItemOut]:
         """
@@ -115,15 +116,6 @@ def list(self, system_id: Optional[str], catalogue_item_id: Optional[str]) -> Li
         """
         return self._item_repository.list(system_id, catalogue_item_id)
 
-    def get(self, item_id: str) -> Optional[ItemOut]:
-        """
-        Retrieve an item by its ID
-
-        :param item_id: The ID of the item to retrieve
-        :return: The retrieved item, or `None` if not found
-        """
-        return self._item_repository.get(item_id)
-
     def update(self, item_id: str, item: ItemPatchSchema) -> ItemOut:
         """
         Update an item by its ID.
@@ -182,11 +174,19 @@ def update(self, item_id: str, item: ItemPatchSchema) -> ItemOut:
 
         return self._item_repository.update(item_id, ItemIn(**{**stored_item.model_dump(), **update_data}))
 
+    def delete(self, item_id: str) -> None:
+        """
+        Delete an item by its ID.
+
+        :param item_id: The ID of the item to delete.
+        """
+        return self._item_repository.delete(item_id)
+
     def _merge_missing_properties(
         self, properties: List[PropertyOut], supplied_properties: List[PropertyPostSchema]
     ) -> List[PropertyPostSchema]:
         """
-        Merges the properties defined in a catalogue item with those that should be overriden for an item in
+        Merges the properties defined in a catalogue item with those that should be overridden for an item in
         the order they are defined in the catalogue item.
 
         :param properties: The list of property objects from the catalogue item.

test/e2e/conftest.py

@@ -48,7 +48,7 @@ def replace_unit_values_with_ids_in_properties(properties_without_ids: list[dict
     unchanged.
 
     :param properties_without_ids: The list of properties without IDs. Each property is a dictionary
-                                   that may contain a 'unit' key with a unit value that needs to be
+                                   that may contain a `unit` key with a unit value that needs to be
                                    replaced by the unit ID.
     :param units: The list of units. Each unit is a dictionary containing 'id' and 'value' keys, where
                   ID is the unique identifier for the unit and 'value' is the unit value to match
@@ -108,11 +108,11 @@ def check_created_and_modified_times_updated_correctly(post_response: Response,
 
     @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
+        """Inserts unit IDs into some data that may have a 'properties' list within it while removing the unit value.
 
-        :param data: Dictionary of data that could have a 'properties' value within it
-        :param unit_value_id_dict: Dictionary of unit value and id pairs for unit id lookups
-        :return: The data with any needed unit IDs inserted
+        :param data: Dictionary of data that could have a 'properties' value within it.
+        :param unit_value_id_dict: Dictionary of unit value and ID pairs for unit ID lookups.
+        :return: The data with any needed unit IDs inserted.
         """
 
         if "properties" in data and data["properties"]:
@@ -128,3 +128,63 @@ def replace_unit_values_with_ids_in_properties(data: dict, unit_value_id_dict: d
                 new_properties.append(new_property)
             return {**data, "properties": new_properties}
         return data
+
+    @staticmethod
+    def add_unit_ids_to_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.
+
+        :param data: Dictionary of data that could have a 'properties' value within it.
+        :param unit_value_id_dict: Dictionary of unit value and ID pairs for unit ID lookups.
+        :return: The data with any needed unit IDs inserted.
+        """
+
+        if "properties" in data and data["properties"]:
+            new_properties = []
+            for prop in data["properties"]:
+                new_property = {**prop}
+                if "unit" in prop:
+                    if prop["unit"] is not None:
+                        new_property["unit_id"] = unit_value_id_dict[prop["unit"]]
+                    else:
+                        new_property["unit_id"] = None
+                new_properties.append(new_property)
+            return {**data, "properties": new_properties}
+        return data
+
+    @staticmethod
+    def replace_property_names_with_ids_in_properties(data: dict, property_name_id_dict: dict[str, str]) -> dict:
+        """Inserts property IDs into some data that may have a 'properties' list within it while removing the property
+        name.
+
+        :param data: Dictionary of data that could have a 'properties' value within it.
+        :param property_name_id_dict: Dictionary of property name and ID pairs for property ID lookups.
+        :return: The data with any needed property IDs inserted.
+        """
+
+        if "properties" in data and data["properties"]:
+            new_properties = []
+            for prop in data["properties"]:
+                new_property = {**prop}
+                new_property["id"] = property_name_id_dict[prop["name"]]
+                del new_property["name"]
+                new_properties.append(new_property)
+            return {**data, "properties": new_properties}
+        return data
+
+    @staticmethod
+    def add_property_ids_to_properties(data: dict, property_name_id_dict: dict[str, str]) -> dict:
+        """Inserts property IDs into some data that may have a 'properties' list within it.
+
+        :param data: Dictionary of data that could have a 'properties' value within it.
+        :param property_name_id_dict: Dictionary of property name and ID pairs for property ID lookups.
+        :return: The data with any needed property IDs inserted.
+        """
+
+        if "properties" in data and data["properties"]:
+            new_properties = []
+            for prop in data["properties"]:
+                new_property = {**prop}
+                new_property["id"] = property_name_id_dict[prop["name"]]
+                new_properties.append(new_property)
+            return {**data, "properties": new_properties}
+        return data