Refactor remaining catalogue item repo unit tests #340

inventory_management_system_api/repositories/catalogue_item.py

@@ -84,25 +84,6 @@ def list(self, catalogue_category_id: Optional[str], session: ClientSession = No
         catalogue_items = self._catalogue_items_collection.find(query, session=session)
         return [CatalogueItemOut(**catalogue_item) for catalogue_item in catalogue_items]
 
-    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 update(
         self, catalogue_item_id: str, catalogue_item: CatalogueItemIn, session: ClientSession = None
     ) -> CatalogueItemOut:
@@ -123,6 +104,25 @@ def update(
         catalogue_item = self.get(str(catalogue_item_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 has_child_elements(self, catalogue_item_id: CustomObjectId, session: ClientSession = None) -> bool:
         """
         Check if a catalogue item has child elements based on its ID.

test/unit/repositories/test_catalogue_category.py

@@ -927,66 +927,6 @@ def test_update_with_invalid_id(self):
         self.check_update_failed_with_exception("Invalid ObjectId value 'invalid-id'")
 
 
-class HasChildElementsDSL(CatalogueCategoryRepoDSL):
-    """Base class for `has_child_elements` tests"""
-
-    _has_child_elements_catalogue_category_id: str
-    _has_child_elements_result: bool
-
-    def call_has_child_elements(self, catalogue_category_id: str) -> None:
-        """Calls the `CatalogueCategoryRepo` `has_child_elements` method.
-
-        :param catalogue_category_id: ID of the catalogue category to check.
-        """
-
-        self._has_child_elements_catalogue_category_id = catalogue_category_id
-        self._has_child_elements_result = self.catalogue_category_repository.has_child_elements(
-            CustomObjectId(catalogue_category_id), session=self.mock_session
-        )
-
-    def check_has_child_elements_success(self, expected_result: bool) -> None:
-        """Checks that a prior call to `call_has_child_elements` worked as expected
-
-        :param expected_result: The expected result returned by `has_child_elements`
-        """
-
-        self.check_has_child_elements_performed_expected_calls(self._has_child_elements_catalogue_category_id)
-
-        assert self._has_child_elements_result == expected_result
-
-
-class TestHasChildElements(HasChildElementsDSL):
-    """Tests for `has_child_elements`."""
-
-    def test_has_child_elements_with_no_children(self):
-        """Test `has_child_elements` when there are no child catalogue categories or catalogue items."""
-
-        self.mock_has_child_elements(child_catalogue_category_data=None, child_catalogue_item_data=None)
-        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
-        self.check_has_child_elements_success(expected_result=False)
-
-    def test_has_child_elements_with_child_catalogue_category(self):
-        """Test `has_child_elements` when there is a child catalogue category but no child catalogue items."""
-
-        self.mock_has_child_elements(
-            child_catalogue_category_data=CATALOGUE_CATEGORY_IN_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_A,
-            child_catalogue_item_data=None,
-        )
-        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
-        self.check_has_child_elements_success(expected_result=True)
-
-    def test_has_child_elements_with_child_catalogue_catalogue_item(self):
-        """Test `has_child_elements` when there are no child catalogue categories but there is a child catalogue
-        item."""
-
-        self.mock_has_child_elements(
-            child_catalogue_category_data=None,
-            child_catalogue_item_data=CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
-        )
-        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
-        self.check_has_child_elements_success(expected_result=True)
-
-
 class DeleteDSL(CatalogueCategoryRepoDSL):
     """Base class for `delete` tests."""
 
@@ -1114,6 +1054,66 @@ def test_delete_invalid_id(self):
         self.check_delete_failed_with_exception("Invalid ObjectId value 'invalid-id'")
 
 
+class HasChildElementsDSL(CatalogueCategoryRepoDSL):
+    """Base class for `has_child_elements` tests"""
+
+    _has_child_elements_catalogue_category_id: str
+    _has_child_elements_result: bool
+
+    def call_has_child_elements(self, catalogue_category_id: str) -> None:
+        """Calls the `CatalogueCategoryRepo` `has_child_elements` method.
+
+        :param catalogue_category_id: ID of the catalogue category to check.
+        """
+
+        self._has_child_elements_catalogue_category_id = catalogue_category_id
+        self._has_child_elements_result = self.catalogue_category_repository.has_child_elements(
+            CustomObjectId(catalogue_category_id), session=self.mock_session
+        )
+
+    def check_has_child_elements_success(self, expected_result: bool) -> None:
+        """Checks that a prior call to `call_has_child_elements` worked as expected
+
+        :param expected_result: The expected result returned by `has_child_elements`
+        """
+
+        self.check_has_child_elements_performed_expected_calls(self._has_child_elements_catalogue_category_id)
+
+        assert self._has_child_elements_result == expected_result
+
+
+class TestHasChildElements(HasChildElementsDSL):
+    """Tests for `has_child_elements`."""
+
+    def test_has_child_elements_with_no_children(self):
+        """Test `has_child_elements` when there are no child catalogue categories or catalogue items."""
+
+        self.mock_has_child_elements(child_catalogue_category_data=None, child_catalogue_item_data=None)
+        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
+        self.check_has_child_elements_success(expected_result=False)
+
+    def test_has_child_elements_with_child_catalogue_category(self):
+        """Test `has_child_elements` when there is a child catalogue category but no child catalogue items."""
+
+        self.mock_has_child_elements(
+            child_catalogue_category_data=CATALOGUE_CATEGORY_IN_DATA_NON_LEAF_NO_PARENT_NO_PROPERTIES_A,
+            child_catalogue_item_data=None,
+        )
+        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
+        self.check_has_child_elements_success(expected_result=True)
+
+    def test_has_child_elements_with_child_catalogue_catalogue_item(self):
+        """Test `has_child_elements` when there are no child catalogue categories but there is a child catalogue
+        item."""
+
+        self.mock_has_child_elements(
+            child_catalogue_category_data=None,
+            child_catalogue_item_data=CATALOGUE_ITEM_DATA_REQUIRED_VALUES_ONLY,
+        )
+        self.call_has_child_elements(catalogue_category_id=str(ObjectId()))
+        self.check_has_child_elements_success(expected_result=True)
+
+
 class CreatePropertyDSL(CatalogueCategoryRepoDSL):
     """Base class for `create_property` tests"""