From 85bb34da086c8706339b62d81b8c4c3ff851a0f2 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:44:25 +0100 Subject: [PATCH 01/16] Allow for categories without catalogue item properties to be created #3 --- .../models/catalogue_category.py | 20 +++++++++++++++- .../schemas/catalogue_category.py | 24 +++---------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_category.py b/inventory_management_system_api/models/catalogue_category.py index 9a37738f..d10010b4 100644 --- a/inventory_management_system_api/models/catalogue_category.py +++ b/inventory_management_system_api/models/catalogue_category.py @@ -4,7 +4,7 @@ from typing import Optional, List from bson import ObjectId -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, validator from inventory_management_system_api.core.custom_object_id import CustomObjectId @@ -73,6 +73,24 @@ class CatalogueCategoryIn(BaseModel): parent_id: Optional[CustomObjectIdField] = None catalogue_item_properties: List[CatalogueItemProperty] + @validator("catalogue_item_properties", pre=True, always=True) + @classmethod + def validate_catalogue_item_properties( + cls, catalogue_item_properties: List[CatalogueItemProperty] | None + ) -> List[CatalogueItemProperty] | List: + """ + Validator for the `catalogue_item_properties` field that runs always (even if the field is missing) and before + any Pydantic validation checks. + + If the value is `None`, it replaces it with an empty list. + + :param catalogue_item_properties: The list of catalogue item properties. + :return: The list of catalogue item properties or an empty list. + """ + if catalogue_item_properties is None: + catalogue_item_properties = [] + return catalogue_item_properties + class CatalogueCategoryOut(CatalogueCategoryIn): """ diff --git a/inventory_management_system_api/schemas/catalogue_category.py b/inventory_management_system_api/schemas/catalogue_category.py index e3ebc006..aba82638 100644 --- a/inventory_management_system_api/schemas/catalogue_category.py +++ b/inventory_management_system_api/schemas/catalogue_category.py @@ -4,7 +4,7 @@ from enum import Enum from typing import Optional, List, Any, Dict -from pydantic import BaseModel, validator, root_validator, Field +from pydantic import BaseModel, validator, Field class CatalogueItemPropertyType(str, Enum): @@ -57,28 +57,10 @@ class CatalogueCategoryPostRequestSchema(BaseModel): "children but if it is not then it can only have catalogue categories as children." ) parent_id: Optional[str] = Field(default=None, description="The ID of the parent catalogue category") - catalogue_item_properties: List[CatalogueItemPropertySchema] = Field( + catalogue_item_properties: Optional[List[CatalogueItemPropertySchema]] = Field( description="The properties that the catalogue items in this category could/should have" ) - @root_validator(pre=True) - @classmethod - def validate_values(cls, values): - """ - Root validator for the model which runs before the values are assigned to the fields. - - This is needed to make the `catalogue_item_properties` field not required if the catalogue category is not a - leaf. It assigns an empty list if the field is not present in the body. - - :param values: The values of the model fields. - :return: The values of the model fields. - """ - # Do not require the `catalogue_item_properties` field to be present in the body if the catalogue category is - # not a leaf. Assign an empty list - if "is_leaf" in values and not values["is_leaf"] and "catalogue_item_properties" not in values: - values["catalogue_item_properties"] = [] - return values - @validator("catalogue_item_properties") @classmethod def validate_catalogue_item_properties( @@ -95,7 +77,7 @@ def validate_catalogue_item_properties( :return: The list of catalogue item properties. :raises ValueError: If catalogue item properties are provided for a non-leaf catalogue category. """ - if "is_leaf" in values and not values["is_leaf"] and catalogue_item_properties: + if "is_leaf" in values and values["is_leaf"] is False and catalogue_item_properties: raise ValueError("Catalogue item properties not allowed for non-leaf catalogue category") return catalogue_item_properties From 034685e2dd71670b2488937d59aae217b25fb856 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:46:16 +0100 Subject: [PATCH 02/16] Define catalogue category patch endpoint schema model #3 --- .../schemas/catalogue_category.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/inventory_management_system_api/schemas/catalogue_category.py b/inventory_management_system_api/schemas/catalogue_category.py index aba82638..6dc905fc 100644 --- a/inventory_management_system_api/schemas/catalogue_category.py +++ b/inventory_management_system_api/schemas/catalogue_category.py @@ -82,6 +82,19 @@ def validate_catalogue_item_properties( return catalogue_item_properties +class CatalogueCategoryPatchRequestSchema(CatalogueCategoryPostRequestSchema): + """ + Schema model for a catalogue category update request. + """ + + name: Optional[str] = Field(description="The name of the catalogue category") + is_leaf: Optional[bool] = Field( + description="Whether the category is a leaf or not. If it is then it can only have catalogue items as " + "children but if it is not then it can only have catalogue categories as children." + ) + parent_id: Optional[str] = Field(description="The ID of the parent catalogue category") + + class CatalogueCategorySchema(CatalogueCategoryPostRequestSchema): """ Schema model for a catalogue category response. From 695d822fb17ae802e1583a917320ac2847e0ae2a Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:48:04 +0100 Subject: [PATCH 03/16] Modify _has_children_elements method to check for children catalogue items #3 --- .../repositories/catalogue_category.py | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/inventory_management_system_api/repositories/catalogue_category.py b/inventory_management_system_api/repositories/catalogue_category.py index bebd9468..015a1e95 100644 --- a/inventory_management_system_api/repositories/catalogue_category.py +++ b/inventory_management_system_api/repositories/catalogue_category.py @@ -32,7 +32,8 @@ def __init__(self, database: Database = Depends(get_database)) -> None: :param database: The database to use. """ self._database = database - self._collection: Collection = self._database.catalogue_categories + self._catalogue_categories_collection: Collection = self._database.catalogue_categories + self._catalogue_items_collection: Collection = self._database.catalogue_items def create(self, catalogue_category: CatalogueCategoryIn) -> CatalogueCategoryOut: """ @@ -40,7 +41,7 @@ def create(self, catalogue_category: CatalogueCategoryIn) -> CatalogueCategoryOu If a parent catalogue category is specified by `parent_id`, the method checks if that exists in the database and raises a `MissingRecordError` if it doesn't exist. It also checks if a duplicate catalogue - category is found within the parent catalogue category. + category is found within the parent catalogue category and raises a `DuplicateRecordError` if it is. :param catalogue_category: The catalogue category to be created. :return: The created catalogue category. @@ -49,20 +50,22 @@ def create(self, catalogue_category: CatalogueCategoryIn) -> CatalogueCategoryOu """ parent_id = str(catalogue_category.parent_id) if catalogue_category.parent_id else None if parent_id and not self.get(parent_id): - raise MissingRecordError(f"No catalogue category found with ID: {parent_id}") + raise MissingRecordError(f"No parent catalogue category found with ID: {parent_id}") if self._is_duplicate_catalogue_category(parent_id, catalogue_category.code): raise DuplicateRecordError("Duplicate catalogue category found within the parent catalogue category") logger.info("Inserting the new catalogue category into the database") - result = self._collection.insert_one(catalogue_category.dict()) + result = self._catalogue_categories_collection.insert_one(catalogue_category.dict()) catalogue_category = self.get(str(result.inserted_id)) return catalogue_category def delete(self, catalogue_category_id: str) -> None: """ - Delete a catalogue category by its ID from a MongoDB database. The method checks if the catalogue category has - children elements and raises a `ChildrenElementsExistError` if it does. + Delete a catalogue category by its ID from a MongoDB database. + + The method checks if the catalogue category has children elements and raises a `ChildrenElementsExistError` + if it does. :param catalogue_category_id: The ID of the catalogue category to delete. :raises ChildrenElementsExistError: If the catalogue category has children elements. @@ -75,7 +78,7 @@ def delete(self, catalogue_category_id: str) -> None: ) logger.info("Deleting catalogue category with ID: %s from the database", catalogue_category_id) - result = self._collection.delete_one({"_id": catalogue_category_id}) + result = self._catalogue_categories_collection.delete_one({"_id": catalogue_category_id}) if result.deleted_count == 0: raise MissingRecordError(f"No catalogue category found with ID: {str(catalogue_category_id)}") @@ -88,7 +91,7 @@ def get(self, catalogue_category_id: str) -> Optional[CatalogueCategoryOut]: """ catalogue_category_id = CustomObjectId(catalogue_category_id) logger.info("Retrieving catalogue category with ID: %s from the database", catalogue_category_id) - catalogue_category = self._collection.find_one({"_id": catalogue_category_id}) + catalogue_category = self._catalogue_categories_collection.find_one({"_id": catalogue_category_id}) if catalogue_category: return CatalogueCategoryOut(**catalogue_category) return None @@ -115,7 +118,7 @@ def list(self, path: Optional[str], parent_path: Optional[str]) -> List[Catalogu logger.info("%s matching the provided filter(s)", message) logger.debug("Provided filter(s): %s", query) - catalogue_categories = self._collection.find(query) + catalogue_categories = self._catalogue_categories_collection.find(query) return [CatalogueCategoryOut(**catalogue_category) for catalogue_category in catalogue_categories] def _is_duplicate_catalogue_category(self, parent_id: Optional[str], code: str) -> bool: @@ -126,22 +129,29 @@ def _is_duplicate_catalogue_category(self, parent_id: Optional[str], code: str) :param code: The code of the catalogue category to check for duplicates. :return: `True` if a duplicate catalogue category code is found, `False` otherwise. """ - logger.info("Checking if catalogue category with code '%s' already exists within the category", code) + logger.info("Checking if catalogue category with code '%s' already exists within the parent category", code) if parent_id: parent_id = CustomObjectId(parent_id) - count = self._collection.count_documents({"parent_id": parent_id, "code": code}) + count = self._catalogue_categories_collection.count_documents({"parent_id": parent_id, "code": code}) return count > 0 def _has_children_elements(self, catalogue_category_id: str) -> bool: """ Check if a catalogue category has children elements based on its ID. + Children elements in this case means whether or not a catalogue category has children catalogue categories or + children catalogue items. + :param catalogue_category_id: The ID of the catalogue category to check. :return: True if the catalogue category has children elements, False otherwise. """ catalogue_category_id = CustomObjectId(catalogue_category_id) logger.info("Checking if catalogue category with ID '%s' has children elements", catalogue_category_id) + # Check if it has catalogue categories query = {"parent_id": catalogue_category_id} - count = self._collection.count_documents(query) + count = self._catalogue_categories_collection.count_documents(query) + # Check if it has catalogue items + query = {"catalogue_category_id": catalogue_category_id} + count = count + self._catalogue_items_collection.count_documents(query) return count > 0 From 9ba49c5ddf0b12e8d01ef2bd7a268ca5aa85914f Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:49:12 +0100 Subject: [PATCH 04/16] Implement a repo method for updating a catalogue category #3 --- .../repositories/catalogue_category.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/inventory_management_system_api/repositories/catalogue_category.py b/inventory_management_system_api/repositories/catalogue_category.py index 015a1e95..6ec5dfa2 100644 --- a/inventory_management_system_api/repositories/catalogue_category.py +++ b/inventory_management_system_api/repositories/catalogue_category.py @@ -96,6 +96,46 @@ def get(self, catalogue_category_id: str) -> Optional[CatalogueCategoryOut]: return CatalogueCategoryOut(**catalogue_category) return None + def update(self, catalogue_category_id: str, catalogue_category: CatalogueCategoryIn): + """ + Update a catalogue category by its ID in a MongoDB database. + + The method checks if the catalogue category has children elements and raises a `ChildrenElementsExistError` + if it does. If a parent catalogue category is specified by `parent_id`, the method checks if that exists in the + database and raises a `MissingRecordError` if it doesn't exist. It also checks if a duplicate catalogue category + is found within the parent catalogue category and raises a `DuplicateRecordError` if it is. + + :param catalogue_category_id: The ID of the catalogue category to update. + :param catalogue_category: The catalogue category containing the update data. + :return: The updated catalogue category. + :raises ChildrenElementsExistError: If the catalogue category has children elements. + :raises MissingRecordError: If the parent catalogue category specified by `parent_id` doesn't exist. + :raises DuplicateRecordError: If a duplicate catalogue category is found within the parent catalogue category. + """ + catalogue_category_id = CustomObjectId(catalogue_category_id) + if self._has_children_elements(str(catalogue_category_id)): + raise ChildrenElementsExistError( + f"Catalogue category with ID {str(catalogue_category_id)} has children elements and cannot be updated" + ) + + parent_id = str(catalogue_category.parent_id) if catalogue_category.parent_id else None + if parent_id and not self.get(parent_id): + raise MissingRecordError(f"No parent catalogue category found with ID: {parent_id}") + + stored_catalogue_category = self.get(str(catalogue_category_id)) + if ( + catalogue_category.name != stored_catalogue_category.name + or parent_id != stored_catalogue_category.parent_id + ) and self._is_duplicate_catalogue_category(parent_id, catalogue_category.code): + raise DuplicateRecordError("Duplicate catalogue category found within the parent catalogue category") + + logger.info("Updating catalogue category with ID: %s in the database", catalogue_category_id) + self._catalogue_categories_collection.update_one( + {"_id": catalogue_category_id}, {"$set": catalogue_category.dict()} + ) + catalogue_category = self.get(str(catalogue_category_id)) + return catalogue_category + def list(self, path: Optional[str], parent_path: Optional[str]) -> List[CatalogueCategoryOut]: """ Retrieve catalogue categories from a MongoDB database based on the provided filters. From 792976ac026e3abf64f2a2f1c98c78eb567b547f Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:50:45 +0100 Subject: [PATCH 05/16] Implement a service method for updating a catalogue category #3 --- .../services/catalogue_category.py | 64 ++++++++++++++++++- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_category.py b/inventory_management_system_api/services/catalogue_category.py index dc1b489a..a5a83554 100644 --- a/inventory_management_system_api/services/catalogue_category.py +++ b/inventory_management_system_api/services/catalogue_category.py @@ -7,10 +7,13 @@ from fastapi import Depends -from inventory_management_system_api.core.exceptions import LeafCategoryError +from inventory_management_system_api.core.exceptions import LeafCategoryError, MissingRecordError from inventory_management_system_api.models.catalogue_category import CatalogueCategoryIn, CatalogueCategoryOut from inventory_management_system_api.repositories.catalogue_category import CatalogueCategoryRepo -from inventory_management_system_api.schemas.catalogue_category import CatalogueCategoryPostRequestSchema +from inventory_management_system_api.schemas.catalogue_category import ( + CatalogueCategoryPostRequestSchema, + CatalogueCategoryPatchRequestSchema, +) logger = logging.getLogger() @@ -37,7 +40,7 @@ def create(self, catalogue_category: CatalogueCategoryPostRequestSchema) -> Cata :param catalogue_category: The catalogue category to be created. :return: The created catalogue category. - :raises LeafCategoryError: If the parent catalogue category is a leaf catalogue category + :raises LeafCategoryError: If the parent catalogue category is a leaf catalogue category. """ parent_id = catalogue_category.parent_id parent_catalogue_category = self.get(parent_id) if parent_id else None @@ -87,6 +90,61 @@ def list(self, path: Optional[str], parent_path: Optional[str]) -> List[Catalogu """ return self._catalogue_category_repository.list(path, parent_path) + def update( + self, catalogue_category_id: str, catalogue_category: CatalogueCategoryPatchRequestSchema + ) -> CatalogueCategoryOut: + """ + Update a catalogue category by its ID. + + The method checks if a catalogue category with such ID exists and raises a `MissingRecordError` if it doesn't + exist. If a category is attempted to be moved to a leaf parent catalogue category then it checks if the parent + is a leaf catalogue category and raises a `LeafCategoryError` if it is. + + :param catalogue_category_id: The ID of the catalogue category to update. + :param catalogue_category: The catalogue category containing the fields that need to be updated. + :return: The updated catalogue category. + :raises MissingRecordError: If the catalogue category doesn't exist. + :raises LeafCategoryError: If the parent catalogue category to which the catalogue category is attempted to be + moved is a leaf catalogue category. + """ + update_data = catalogue_category.dict(exclude_unset=True) + + stored_catalogue_category = self._catalogue_category_repository.get(catalogue_category_id) + if not stored_catalogue_category: + raise MissingRecordError(f"No catalogue category found with ID: {catalogue_category_id}") + + if "name" in update_data and update_data["name"] != stored_catalogue_category.name: + stored_catalogue_category.name = update_data["name"] + stored_catalogue_category.code = self._generate_code(stored_catalogue_category.name) + stored_catalogue_category.path = self._generate_path( + stored_catalogue_category.parent_path, stored_catalogue_category.code + ) + + if "parent_id" in update_data and update_data["parent_id"] != stored_catalogue_category.parent_id: + stored_catalogue_category.parent_id = update_data["parent_id"] + parent_catalogue_category = ( + self.get(stored_catalogue_category.parent_id) if stored_catalogue_category.parent_id else None + ) + stored_catalogue_category.parent_path = parent_catalogue_category.path if parent_catalogue_category else "/" + stored_catalogue_category.path = self._generate_path( + stored_catalogue_category.parent_path, stored_catalogue_category.code + ) + + if parent_catalogue_category and parent_catalogue_category.is_leaf: + raise LeafCategoryError("Cannot add catalogue category to a leaf parent catalogue category") + + if "is_leaf" in update_data: + stored_catalogue_category.is_leaf = update_data["is_leaf"] + if stored_catalogue_category.is_leaf is False: + stored_catalogue_category.catalogue_item_properties = [] + + if "catalogue_item_properties" in update_data: + stored_catalogue_category.catalogue_item_properties = update_data["catalogue_item_properties"] + + return self._catalogue_category_repository.update( + catalogue_category_id, CatalogueCategoryIn(**stored_catalogue_category.dict()) + ) + def _generate_code(self, name: str) -> str: """ Generate a code for a catalogue category based on its name. This is used to maintain uniqueness and prevent From 5536e358e1fdf0be47231f4c0bee53950292e09a Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:54:59 +0100 Subject: [PATCH 06/16] Implement an endpoint for partially updating a catalogue category #3 --- .../core/custom_object_id.py | 4 +- .../core/exceptions.py | 2 +- .../routers/v1/catalogue_category.py | 57 ++++++++++++++++--- test/unit/core/test_custom_objectid.py | 8 +-- .../repositories/test_catalogue_category.py | 11 ++-- test/unit/repositories/test_catalogue_item.py | 4 +- 6 files changed, 65 insertions(+), 21 deletions(-) diff --git a/inventory_management_system_api/core/custom_object_id.py b/inventory_management_system_api/core/custom_object_id.py index 3b88ac80..8b597f1e 100644 --- a/inventory_management_system_api/core/custom_object_id.py +++ b/inventory_management_system_api/core/custom_object_id.py @@ -20,9 +20,9 @@ def __init__(self, value: str): :raises InvalidObjectIdError: If the string value is an invalid `ObjectId`. """ if not isinstance(value, str): - raise InvalidObjectIdError("ObjectId value must be a string") + raise InvalidObjectIdError(f"ObjectId value '{value}' must be a string") if not ObjectId.is_valid(value): - raise InvalidObjectIdError("Invalid ObjectId value") + raise InvalidObjectIdError(f"Invalid ObjectId value '{value}'") super().__init__(value) diff --git a/inventory_management_system_api/core/exceptions.py b/inventory_management_system_api/core/exceptions.py index 93c093bd..4104a82e 100644 --- a/inventory_management_system_api/core/exceptions.py +++ b/inventory_management_system_api/core/exceptions.py @@ -53,5 +53,5 @@ class MissingRecordError(DatabaseError): class ChildrenElementsExistError(DatabaseError): """ - Exception raised when attempting to delete a catalogue category that has children elements. + Exception raised when attempting to delete or update a catalogue category that has children elements. """ diff --git a/inventory_management_system_api/routers/v1/catalogue_category.py b/inventory_management_system_api/routers/v1/catalogue_category.py index e56330ed..35a6a2a3 100644 --- a/inventory_management_system_api/routers/v1/catalogue_category.py +++ b/inventory_management_system_api/routers/v1/catalogue_category.py @@ -17,6 +17,7 @@ from inventory_management_system_api.schemas.catalogue_category import ( CatalogueCategorySchema, CatalogueCategoryPostRequestSchema, + CatalogueCategoryPatchRequestSchema, ) from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService @@ -53,18 +54,15 @@ def get_catalogue_category( ) -> CatalogueCategorySchema: # pylint: disable=missing-function-docstring logger.info("Getting catalogue category with ID: %s", catalogue_category_id) + message = "A catalogue category with such ID was not found" try: catalogue_category = catalogue_category_service.get(catalogue_category_id) if not catalogue_category: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="The requested catalogue category was not found" - ) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) return CatalogueCategorySchema(**catalogue_category.dict()) except InvalidObjectIdError as exc: - logger.exception("The ID is not a valid ObjectId value") - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="The requested catalogue category was not found" - ) from exc + logger.exception(message) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) from exc @router.post( @@ -84,7 +82,7 @@ def create_catalogue_category( catalogue_category = catalogue_category_service.create(catalogue_category) return CatalogueCategorySchema(**catalogue_category.dict()) except (MissingRecordError, InvalidObjectIdError) as exc: - message = "The specified parent catalogue category ID does not exist in the database" + message = "The specified parent catalogue category ID does not exist" logger.exception(message) raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=message) from exc except DuplicateRecordError as exc: @@ -97,6 +95,49 @@ def create_catalogue_category( raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc +@router.patch( + path="/{catalogue_category_id}", + summary="Update a catalogue category partially by ID", + response_description="Catalogue category updated successfully", +) +def partial_update_catalogue_category( + catalogue_category: CatalogueCategoryPatchRequestSchema, + catalogue_category_id: str = Path(description="The ID of the catalogue category to update"), + catalogue_category_service: CatalogueCategoryService = Depends(), +) -> CatalogueCategorySchema: + # pylint: disable=missing-function-docstring + logger.info("Partially updating catalogue category with ID: %s", catalogue_category_id) + logger.debug("Catalogue category data: %s", catalogue_category) + try: + updated_catalogue_category = catalogue_category_service.update(catalogue_category_id, catalogue_category) + return CatalogueCategorySchema(**updated_catalogue_category.dict()) + except (MissingRecordError, InvalidObjectIdError) as exc: + if ( + catalogue_category.parent_id + and catalogue_category.parent_id in str(exc) + or "parent catalogue category" in str(exc).lower() + ): + message = "The specified parent catalogue category ID does not exist" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=message) from exc + + message = "A catalogue category with such ID was not found" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) from exc + except ChildrenElementsExistError as exc: + message = "Catalogue category has children elements and cannot be updated" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc + except DuplicateRecordError as exc: + message = "A catalogue category with the same name already exists within the parent catalogue category" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc + except LeafCategoryError as exc: + message = "Adding a catalogue category to a leaf parent catalogue category is not allowed" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc + + @router.delete( path="/{catalogue_category_id}", summary="Delete a catalogue category by ID", diff --git a/test/unit/core/test_custom_objectid.py b/test/unit/core/test_custom_objectid.py index 4dd6161e..211094c0 100644 --- a/test/unit/core/test_custom_objectid.py +++ b/test/unit/core/test_custom_objectid.py @@ -24,7 +24,7 @@ def test_invalid_object_id(): value = "invalid" with pytest.raises(InvalidObjectIdError) as exc: CustomObjectId(value) - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" def test_non_string_input(): @@ -34,7 +34,7 @@ def test_non_string_input(): value = 123 with pytest.raises(InvalidObjectIdError) as exc: CustomObjectId(value) - assert str(exc.value) == "ObjectId value must be a string" + assert str(exc.value) == f"ObjectId value '{value}' must be a string" def test_empty_string_input(): @@ -44,7 +44,7 @@ def test_empty_string_input(): value = "" with pytest.raises(InvalidObjectIdError) as exc: CustomObjectId(value) - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == f"Invalid ObjectId value '{value}'" def test_none_input(): @@ -54,4 +54,4 @@ def test_none_input(): value = None with pytest.raises(InvalidObjectIdError) as exc: CustomObjectId(value) - assert str(exc.value) == "ObjectId value must be a string" + assert str(exc.value) == f"ObjectId value '{value}' must be a string" diff --git a/test/unit/repositories/test_catalogue_category.py b/test/unit/repositories/test_catalogue_category.py index 6e8582f3..9c8787ee 100644 --- a/test/unit/repositories/test_catalogue_category.py +++ b/test/unit/repositories/test_catalogue_category.py @@ -210,7 +210,7 @@ def test_create_with_nonexistent_parent_id(test_helpers, database_mock, catalogu catalogue_item_properties=[], ) ) - assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category.parent_id}" + assert str(exc.value) == f"No parent catalogue category found with ID: {catalogue_category.parent_id}" database_mock.catalogue_categories.find_one.assert_called_once_with( {"_id": CustomObjectId(catalogue_category.parent_id)} ) @@ -290,8 +290,9 @@ def test_delete(test_helpers, database_mock, catalogue_category_repository): # Mock `delete_one` to return that one document has been deleted test_helpers.mock_delete_one(database_mock.catalogue_categories, 1) - # Mock count_documents to return 1 (children elements not found) + # Mock count_documents to return 0 (children elements not found) test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) catalogue_category_repository.delete(catalogue_category_id) @@ -308,6 +309,7 @@ def test_delete_with_children_elements(test_helpers, database_mock, catalogue_ca """ catalogue_category_id = str(ObjectId()) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) # Mock count_documents to return 1 (children elements found) test_helpers.mock_count_documents(database_mock.catalogue_categories, 1) @@ -326,7 +328,7 @@ def test_delete_with_invalid_id(catalogue_category_repository): """ with pytest.raises(InvalidObjectIdError) as exc: catalogue_category_repository.delete("invalid") - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" def test_delete_with_nonexistent_id(test_helpers, database_mock, catalogue_category_repository): @@ -342,6 +344,7 @@ def test_delete_with_nonexistent_id(test_helpers, database_mock, catalogue_categ # Mock count_documents to return 0 (children elements not found) test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) with pytest.raises(MissingRecordError) as exc: catalogue_category_repository.delete(catalogue_category_id) @@ -399,7 +402,7 @@ def test_get_with_invalid_id(catalogue_category_repository): """ with pytest.raises(InvalidObjectIdError) as exc: catalogue_category_repository.get("invalid") - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" def test_get_with_nonexistent_id(test_helpers, database_mock, catalogue_category_repository): diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 31408b6d..375cfaa2 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -155,7 +155,7 @@ def test_get_with_invalid_id(catalogue_item_repository): """ with pytest.raises(InvalidObjectIdError) as exc: catalogue_item_repository.get("invalid") - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" def test_get_with_nonexistent_id(test_helpers, database_mock, catalogue_item_repository): @@ -302,4 +302,4 @@ def test_list_with_invalid_catalogue_category_id_filter(catalogue_item_repositor """ with pytest.raises(InvalidObjectIdError) as exc: catalogue_item_repository.list("invalid") - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" From 688bc6a0e96c5758d3a78ea64243876681ec3d8f Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 16:56:18 +0100 Subject: [PATCH 07/16] Write e2e test for the catalogue category partial update endpoint #3 --- test/e2e/conftest.py | 12 + test/e2e/test_catalogue_category.py | 666 ++++++++++++++++++++++++++-- test/e2e/test_catalogue_item.py | 22 - 3 files changed, 651 insertions(+), 49 deletions(-) diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py index c26c893f..8e4308df 100644 --- a/test/e2e/conftest.py +++ b/test/e2e/conftest.py @@ -4,6 +4,7 @@ import pytest from fastapi.testclient import TestClient +from inventory_management_system_api.core.database import get_database from inventory_management_system_api.main import app @@ -15,3 +16,14 @@ def fixture_test_client() -> TestClient: :return: The test client. """ return TestClient(app) + + +@pytest.fixture(name="cleanup_database_collections", autouse=True) +def fixture_cleanup_database_collections(): + """ + Fixture to clean up the collections in the test database after the session finishes. + """ + database = get_database() + yield + database.catalogue_categories.delete_many({}) + database.catalogue_items.delete_many({}) diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index f5dc8c08..4f16c374 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -1,22 +1,9 @@ +# pylint: disable=too-many-lines """ End-to-End tests for the catalogue category router. """ -import pytest - from bson import ObjectId -from inventory_management_system_api.core.database import get_database - - -@pytest.fixture(name="cleanup_catalogue_categories", autouse=True) -def fixture_cleanup_catalogue_categories(): - """ - Fixture to clean up the catalogue categories collection in the test database after the session finishes. - """ - database = get_database() - yield - database.catalogue_categories.delete_many({}) - def test_create_catalogue_category(test_client): """ @@ -113,7 +100,7 @@ def test_create_catalogue_category_with_invalid_parent_id(test_client): response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) assert response.status_code == 422 - assert response.json()["detail"] == "The specified parent catalogue category ID does not exist in the database" + assert response.json()["detail"] == "The specified parent catalogue category ID does not exist" def test_create_catalogue_category_with_nonexistent_parent_id(test_client): @@ -125,7 +112,7 @@ def test_create_catalogue_category_with_nonexistent_parent_id(test_client): response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) assert response.status_code == 422 - assert response.json()["detail"] == "The specified parent catalogue category ID does not exist in the database" + assert response.json()["detail"] == "The specified parent catalogue category ID does not exist" def test_create_catalogue_category_with_leaf_parent_catalogue_category(test_client): @@ -190,6 +177,27 @@ def test_create_catalogue_category_with_disallowed_unit_value_for_boolean_catalo assert response.json()["detail"][0]["msg"] == "Unit not allowed for boolean catalogue item property 'Property A'" +def test_create_leaf_catalogue_category_without_catalogue_item_properties(test_client): + """ + Test creating a catalogue category. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": True} + + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + assert response.status_code == 201 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] + + def test_create_non_leaf_catalogue_category_with_catalogue_item_properties(test_client): """ Test creating a non-leaf catalogue category with catalogue item properties. @@ -239,9 +247,22 @@ def test_delete_catalogue_category_with_invalid_id(test_client): assert response.json()["detail"] == "A catalogue category with such ID was not found" -def test_delete_catalogue_category_with_children_elements(test_client): +def test_delete_catalogue_category_with_nonexistent_id(test_client): + """ + Test deleting a catalogue category with a nonexistent ID. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + response = test_client.delete(f"/v1/catalogue-categories/{str(ObjectId())}") + + assert response.status_code == 404 + assert response.json()["detail"] == "A catalogue category with such ID was not found" + + +def test_delete_catalogue_category_with_children_catalogue_categories(test_client): """ - Test deleting a catalogue category with children elements. + Test deleting a catalogue category with children catalogue categories. """ catalogue_category_post = {"name": "Category A", "is_leaf": False} parent_response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) @@ -265,17 +286,33 @@ def test_delete_catalogue_category_with_children_elements(test_client): assert response.json()["detail"] == "Catalogue category has children elements and cannot be deleted" -def test_delete_catalogue_category_with_nonexistent_id(test_client): +def test_delete_catalogue_category_with_children_catalogue_items(test_client): """ - Test deleting a catalogue category with a nonexistent ID. + Test deleting a catalogue category with children catalogue items. """ - catalogue_category_post = {"name": "Category A", "is_leaf": False} - test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) - response = test_client.delete(f"/v1/catalogue-categories/{str(ObjectId())}") + catalogue_category_id = response.json()["id"] + catalogue_item_post = { + "catalogue_category_id": catalogue_category_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) - assert response.status_code == 404 - assert response.json()["detail"] == "A catalogue category with such ID was not found" + response = test_client.delete(f"/v1/catalogue-categories/{catalogue_category_id}") + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be deleted" def test_get_catalogue_category(test_client): @@ -321,7 +358,7 @@ def test_get_catalogue_category_with_invalid_id(test_client): response = test_client.get("/v1/catalogue-categories/invalid") assert response.status_code == 404 - assert response.json()["detail"] == "The requested catalogue category was not found" + assert response.json()["detail"] == "A catalogue category with such ID was not found" def test_get_catalogue_category_with_nonexistent_id(test_client): @@ -331,7 +368,7 @@ def test_get_catalogue_category_with_nonexistent_id(test_client): response = test_client.get(f"/v1/catalogue-categories/{str(ObjectId())}") assert response.status_code == 404 - assert response.json()["detail"] == "The requested catalogue category was not found" + assert response.json()["detail"] == "A catalogue category with such ID was not found" def test_get_catalogue_categories(test_client): @@ -459,3 +496,578 @@ def test_get_catalogue_categories_with_path_and_parent_path_filters_no_matching_ catalogue_categories = list(response.json()) assert len(catalogue_categories) == 0 + + +def test_partial_update_catalogue_category_change_name(test_client): + """ + Test changing the name of a catalogue category. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"name": "Category B"} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_patch["name"] + assert catalogue_category["code"] == "category-b" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-b" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] + + +def test_partial_update_catalogue_category_change_name_duplicate(test_client): + """ + Test changing the name of a catalogue category to a name that already exists. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_post = {"name": "Category B", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"name": "Category A"} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert ( + response.json()["detail"] + == "A catalogue category with the same name already exists within the parent catalogue category" + ) + + +def test_partial_update_catalogue_category_change_name_has_children_catalogue_categories(test_client): + """ + Test changing the name of a catalogue category which has children catalogue categories. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_id = response.json()["id"] + catalogue_category_post = {"name": "Category B", "is_leaf": False, "parent_id": catalogue_category_id} + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"name": "Category A"} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_name_has_children_catalogue_items(test_client): + """ + Test changing the name of a catalogue category which has children catalogue items. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_id = response.json()["id"] + catalogue_item_post = { + "catalogue_category_id": catalogue_category_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_category_patch = {"name": "Category B"} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_from_non_leaf_to_leaf(test_client): + """ + Test changing a catalogue category from non-leaf to leaf. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = { + "is_leaf": True, + "catalogue_item_properties": [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}], + } + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_patch["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == catalogue_category_patch["catalogue_item_properties"] + + +def test_partial_update_catalogue_category_change_from_non_leaf_to_leaf_without_catalogue_item_properties(test_client): + """ + Test changing a catalogue category from non-leaf to leaf without supplying any catalogue item properties. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"is_leaf": True} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_patch["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] + + +def test_partial_update_catalogue_category_change_from_non_leaf_to_leaf_has_children_catalogue_categories(test_client): + """ + Test changing a catalogue category with children catalogue categories from non-leaf to leaf. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_id = response.json()["id"] + catalogue_category_post = {"name": "Category B", "is_leaf": False, "parent_id": catalogue_category_id} + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = { + "is_leaf": True, + "catalogue_item_properties": [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}], + } + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf(test_client): + """ + Test changing a catalogue category from leaf to non-leaf. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"is_leaf": False} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_patch["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] + + +def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_has_children_catalogue_items(test_client): + """ + Test changing a catalogue category with children catalogue items from leaf to non-leaf. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_id = response.json()["id"] + catalogue_item_post = { + "catalogue_category_id": catalogue_category_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_category_patch = {"is_leaf": False} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_with_catalogue_item_properties(test_client): + """ + Test changing a catalogue category from leaf to non-leaf while also changing its catalogue item properties. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = { + "is_leaf": False, + "catalogue_item_properties": [{"name": "Property B", "type": "boolean", "mandatory": True}], + } + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 422 + assert ( + response.json()["detail"][0]["msg"] == "Catalogue item properties not allowed for non-leaf catalogue category" + ) + + +def test_partial_update_catalogue_category_change_parent_id(test_client): + """ + Test moving a catalogue category to another parent catalogue category. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_a_id = response.json()["id"] + + catalogue_category_post = { + "name": "Category B", + "is_leaf": True, + "parent_id": catalogue_category_a_id, + "catalogue_item_properties": [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_b_id = response.json()["id"] + + catalogue_category_patch = {"parent_id": None} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_b_id}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-b" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-b" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] == catalogue_category_patch["parent_id"] + assert catalogue_category["catalogue_item_properties"] == catalogue_category_post["catalogue_item_properties"] + + +def test_partial_update_catalogue_category_change_parent_id_has_children_catalogue_categories(test_client): + """ + Test moving a catalogue category with children categories to another parent catalogue category. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_a_id = response.json()["id"] + catalogue_category_post = {"name": "Category B", "is_leaf": False, "parent_id": catalogue_category_a_id} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_b_id = response.json()["id"] + catalogue_category_post = {"name": "Category C", "is_leaf": False, "parent_id": catalogue_category_b_id} + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"parent_id": None} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_b_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_parent_id_has_children_catalogue_items(test_client): + """ + Test moving a catalogue category with children items to another parent catalogue category. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_a_id = response.json()["id"] + catalogue_category_post = {"name": "Category B", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_b_id = response.json()["id"] + catalogue_category_post = { + "name": "Category C", + "is_leaf": True, + "parent_id": catalogue_category_b_id, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_c_id = response.json()["id"] + + catalogue_item_post = { + "catalogue_category_id": catalogue_category_c_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_category_patch = {"parent_id": catalogue_category_a_id} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_b_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_change_parent_id_duplicate_name(test_client): + """ + Test moving a catalogue category to another parent catalogue category in which a category with the same name already + exists. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_a_id = response.json()["id"] + + catalogue_category_post = {"name": "Category B", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_b_id = response.json()["id"] + + catalogue_category_post = {"name": "Category A", "is_leaf": False, "parent_id": catalogue_category_b_id} + test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"parent_id": catalogue_category_b_id} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_a_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert ( + response.json()["detail"] + == "A catalogue category with the same name already exists within the parent catalogue category" + ) + + +def test_partial_update_catalogue_category_change_parent_id_leaf_parent_catalogue_category(test_client): + """ + Test moving a catalogue category to a leaf parent catalogue category. + """ + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_a_id = response.json()["id"] + + catalogue_category_post = {"name": "Category B", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_b_id = response.json()["id"] + + catalogue_category_patch = {"parent_id": catalogue_category_a_id} + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_b_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Adding a catalogue category to a leaf parent catalogue category is not allowed" + + +def test_partial_update_catalogue_category_change_parent_id_invalid_id(test_client): + """ + Test changing the parent ID of a catalogue category to an invalid ID. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"parent_id": "invalid"} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "The specified parent catalogue category ID does not exist" + + +def test_partial_update_catalogue_category_change_parent_id_nonexistent_id(test_client): + """ + Test changing the parent ID of a catalogue category to a nonexistent ID. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_category_patch = {"parent_id": str(ObjectId())} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "The specified parent catalogue category ID does not exist" + + +def test_partial_update_catalogue_category_add_catalogue_item_property(test_client): + """ + Test adding a catalogue item property. + """ + catalogue_item_properties = [{"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}] + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": catalogue_item_properties, + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_item_properties.append({"name": "Property B", "type": "boolean", "mandatory": True}) + catalogue_category_patch = {"catalogue_item_properties": catalogue_item_properties} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + catalogue_item_properties[1]["unit"] = None + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == catalogue_item_properties + + +def test_partial_update_catalogue_category_remove_catalogue_item_property(test_client): + """ + Test removing a catalogue item property. + """ + catalogue_item_properties = [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ] + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": catalogue_item_properties, + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_item_properties.pop(0) + catalogue_category_patch = {"catalogue_item_properties": catalogue_item_properties} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + catalogue_item_properties[0]["unit"] = None + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == catalogue_item_properties + + +def test_partial_update_catalogue_category_modify_catalogue_item_property(test_client): + """ + Test modifying a catalogue item property. + """ + catalogue_item_properties = [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ] + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": catalogue_item_properties, + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + catalogue_item_properties[1]["name"] = "Property C" + catalogue_category_patch = {"catalogue_item_properties": catalogue_item_properties} + response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) + + assert response.status_code == 200 + + catalogue_category = response.json() + + catalogue_item_properties[1]["unit"] = None + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == catalogue_item_properties + + +def test_partial_update_catalogue_category_change_catalogue_item_properties_has_children_catalogue_items(test_client): + """ + Test changing the catalogue item properties when a catalogue category has children catalogue items. + """ + catalogue_category_post = { + "name": "Category C", + "is_leaf": True, + "parent_id": None, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_id = response.json()["id"] + + catalogue_item_post = { + "catalogue_category_id": catalogue_category_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_category_patch = { + "catalogue_item_properties": [{"name": "Property B", "type": "string", "mandatory": False}] + } + response = test_client.patch(f"/v1/catalogue-categories/{catalogue_category_id}", json=catalogue_category_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Catalogue category has children elements and cannot be updated" + + +def test_partial_update_catalogue_category_invalid_id(test_client): + """ + Test updating a catalogue category with an invalid ID. + """ + catalogue_category_patch = {"name": "Category A", "is_leaf": False} + + response = test_client.patch("/v1/catalogue-categories/invalid", json=catalogue_category_patch) + + assert response.status_code == 404 + assert response.json()["detail"] == "A catalogue category with such ID was not found" + + +def test_partial_update_catalogue_category_nonexistent_id(test_client): + """ + Test updating a catalogue category with a nonexistent ID. + """ + catalogue_category_patch = {"name": "Category A", "is_leaf": False} + + response = test_client.patch(f"/v1/catalogue-categories/{str(ObjectId())}", json=catalogue_category_patch) + + assert response.status_code == 404 + assert response.json()["detail"] == "A catalogue category with such ID was not found" diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index e80ed2cd..6caa94af 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -3,10 +3,8 @@ """ from typing import Dict -import pytest from bson import ObjectId -from inventory_management_system_api.core.database import get_database CATALOGUE_CATEGORY_POST_A = { # pylint: disable=duplicate-code "name": "Category A", @@ -63,26 +61,6 @@ def get_catalogue_item_b_dict(catalogue_category_id: str) -> Dict: } -@pytest.fixture(name="cleanup_catalogue_categories", autouse=True) -def fixture_cleanup_catalogue_categories(): - """ - Fixture to clean up the catalogue categories collection in the test database after the session finishes. - """ - database = get_database() - yield - database.catalogue_categories.delete_many({}) - - -@pytest.fixture(name="cleanup_catalogue_items", autouse=True) -def fixture_cleanup_catalogue_items(): - """ - Fixture to clean up the catalogue items collection in the test database after the session finishes. - """ - database = get_database() - yield - database.catalogue_items.delete_many({}) - - def test_create_catalogue_item(test_client): """ Test creating a catalogue item. From 736cd0dfbc3fef0b14809341a09e5bf3c51af596 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 15 Sep 2023 17:17:58 +0100 Subject: [PATCH 08/16] Allow for categories without catalogue item properties to be created #3 --- .../models/catalogue_item.py | 18 ++++++- .../schemas/catalogue_item.py | 2 +- .../services/catalogue_item.py | 5 +- test/e2e/test_catalogue_item.py | 22 ++++++++ test/unit/services/test_catalogue_item.py | 54 +++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index da11dbdb..29289511 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -3,7 +3,7 @@ """ from typing import Optional, List, Any -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, validator from inventory_management_system_api.models.catalogue_category import CustomObjectIdField, StringObjectIdField @@ -28,6 +28,22 @@ class CatalogueItemIn(BaseModel): description: str properties: List[Property] + @validator("properties", pre=True, always=True) + @classmethod + def validate_properties(cls, properties: List[Property] | None) -> List[Property] | List: + """ + Validator for the `properties` field that runs always (even if the field is missing) and before any Pydantic + validation checks. + + If the value is `None`, it replaces it with an empty list. + + :param properties: The list of properties. + :return: The list of properties or an empty list. + """ + if properties is None: + properties = [] + return properties + class CatalogueItemOut(CatalogueItemIn): """ diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 998ecfca..233a3e1c 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -33,7 +33,7 @@ class CatalogueItemPostRequestSchema(BaseModel): ) name: str = Field(description="The name of the catalogue item") description: str = Field(description="The catalogue item description") - properties: List[PropertyPostRequestSchema] = Field(description="The catalogue item properties") + properties: Optional[List[PropertyPostRequestSchema]] = Field(description="The catalogue item properties") class CatalogueItemSchema(CatalogueItemPostRequestSchema): diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index 5b3c3071..a32e7c1e 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -61,7 +61,7 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte if not catalogue_category: raise MissingRecordError(f"No catalogue category found with ID: {catalogue_category_id}") - if not catalogue_category.is_leaf: + if catalogue_category.is_leaf is False: raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") defined_properties = { @@ -69,7 +69,8 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte for defined_property in catalogue_category.catalogue_item_properties } supplied_properties = { - supplied_property.name: supplied_property.dict() for supplied_property in catalogue_item.properties + supplied_property.name: supplied_property.dict() + for supplied_property in (catalogue_item.properties if catalogue_item.properties else []) } self._check_missing_mandatory_catalogue_item_properties(defined_properties, supplied_properties) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 6caa94af..0ea8b37d 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -140,6 +140,28 @@ def test_create_catalogue_item_in_non_leaf_catalogue_category(test_client): assert response.json()["detail"] == "Adding a catalogue item to a non-leaf catalogue category is not allowed" +def test_create_catalogue_item_without_properties(test_client): + """ + Test creating a catalogue item in leaf catalogue category that does not have catalogue item properties. + """ + catalogue_category_post = {"name": "Category A", "is_leaf": True} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + catalogue_category_id = response.json()["id"] + + catalogue_item_post = get_catalogue_item_b_dict(catalogue_category_id) + del catalogue_item_post["properties"] + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + assert response.status_code == 201 + + catalogue_item = response.json() + + assert catalogue_item["catalogue_category_id"] == catalogue_category_id + assert catalogue_item["name"] == catalogue_item_post["name"] + assert catalogue_item["description"] == catalogue_item_post["description"] + assert catalogue_item["properties"] == [] + + def test_create_catalogue_item_with_missing_mandatory_properties(test_client): """ Test creating a catalogue item with missing mandatory catalogue item properties. diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 7bbfb51a..5fcb042c 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -184,6 +184,60 @@ def test_create_in_non_leaf_catalogue_category(catalogue_category_repository_moc catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.id) +def test_create_without_properties( + catalogue_item_repository_mock, catalogue_category_repository_mock, catalogue_item_service +): + """ + Test creating a catalogue item without properties. + + Verify that the `create` method properly handles the catalogue item to be created without properties. + """ + # pylint: disable=duplicate-code + catalogue_item = CatalogueItemOut( + id=str(ObjectId()), + catalogue_category_id=str(ObjectId()), + name="Catalogue Item A", + description="This is Catalogue Item A", + properties=[], + ) + # pylint: enable=duplicate-code + + # Mock `get` to return the catalogue category + catalogue_category_repository_mock.get.return_value = CatalogueCategoryOut( + id=catalogue_item.catalogue_category_id, + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + # Mock `create` to return the created catalogue item + catalogue_item_repository_mock.create.return_value = catalogue_item + + created_catalogue_item = catalogue_item_service.create( + CatalogueItemPostRequestSchema( + catalogue_category_id=catalogue_item.catalogue_category_id, + name=catalogue_item.name, + description=catalogue_item.description, + ) + ) + + catalogue_category_repository_mock.get.assert_called_once_with(catalogue_item.catalogue_category_id) + # pylint: disable=duplicate-code + catalogue_item_repository_mock.create.assert_called_once_with( + CatalogueItemIn( + catalogue_category_id=catalogue_item.catalogue_category_id, + name=catalogue_item.name, + description=catalogue_item.description, + properties=catalogue_item.properties, + ) + ) + # pylint: enable=duplicate-code + assert created_catalogue_item == catalogue_item + + def test_create_with_missing_mandatory_properties(catalogue_category_repository_mock, catalogue_item_service): """ Test creating a catalogue item with missing mandatory catalogue item properties. From 94332a10a444e9133f318c953bbb4157192c9d74 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:48:23 +0100 Subject: [PATCH 09/16] Write unit tests for the update repo method #3 --- test/unit/repositories/conftest.py | 38 +- .../repositories/test_catalogue_category.py | 418 +++++++++++++++++- test/unit/repositories/test_catalogue_item.py | 2 - 3 files changed, 436 insertions(+), 22 deletions(-) diff --git a/test/unit/repositories/conftest.py b/test/unit/repositories/conftest.py index 6f308141..3f9c0453 100644 --- a/test/unit/repositories/conftest.py +++ b/test/unit/repositories/conftest.py @@ -9,7 +9,7 @@ from pymongo.collection import Collection from pymongo.cursor import Cursor from pymongo.database import Database -from pymongo.results import DeleteResult, InsertOneResult +from pymongo.results import DeleteResult, InsertOneResult, UpdateResult from inventory_management_system_api.repositories.catalogue_category import CatalogueCategoryRepo from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo @@ -51,11 +51,12 @@ def fixture_catalogue_item_repository(database_mock: Mock) -> CatalogueItemRepo: return CatalogueItemRepo(database_mock) -class TestHelpers: +class RepositoryTestHelpers: """ - A utility class containing common helper methods for tests. + A utility class containing common helper methods for the repository tests. - This class provides a set of static methods that encapsulate common functionality frequently used in tests. + This class provides a set of static methods that encapsulate common functionality frequently used in the repository + tests. """ @staticmethod @@ -66,7 +67,12 @@ def mock_count_documents(collection_mock: Mock, count: int) -> None: :param collection_mock: Mocked MongoDB database collection instance. :param count: The count value to be returned by the `count_documents` method. """ - collection_mock.count_documents.return_value = count + if collection_mock.count_documents.side_effect is None: + collection_mock.count_documents.side_effect = [count] + else: + counts = list(collection_mock.count_documents.side_effect) + counts.append(count) + collection_mock.count_documents.side_effect = counts @staticmethod def mock_delete_one(collection_mock: Mock, deleted_count: int) -> None: @@ -118,12 +124,28 @@ def mock_find_one(collection_mock: Mock, document: dict | None) -> None: :param collection_mock: Mocked MongoDB database collection instance. :param document: The document to be returned by the `find_one` method. """ - collection_mock.find_one.return_value = document + if collection_mock.find_one.side_effect is None: + collection_mock.find_one.side_effect = [document] + else: + documents = list(collection_mock.find_one.side_effect) + documents.append(document) + collection_mock.find_one.side_effect = documents + + @staticmethod + def mock_update_one(collection_mock: Mock) -> None: + """ + Mock the `update_one` method of the MongoDB database collection mock to return an `UpdateResult` object. + + :param collection_mock: Mocked MongoDB database collection instance. + """ + update_one_result_mock = Mock(UpdateResult) + update_one_result_mock.acknowledged = True + collection_mock.insert_one.return_value = update_one_result_mock @pytest.fixture(name="test_helpers") -def fixture_test_helpers() -> Type[TestHelpers]: +def fixture_test_helpers() -> Type[RepositoryTestHelpers]: """ Fixture to provide a TestHelpers class. """ - return TestHelpers + return RepositoryTestHelpers diff --git a/test/unit/repositories/test_catalogue_category.py b/test/unit/repositories/test_catalogue_category.py index 9c8787ee..885da192 100644 --- a/test/unit/repositories/test_catalogue_category.py +++ b/test/unit/repositories/test_catalogue_category.py @@ -1,3 +1,4 @@ +# pylint: disable=too-many-lines """ Unit tests for the `CatalogueCategoryRepo` repository. """ @@ -84,7 +85,75 @@ def test_create(test_helpers, database_mock, catalogue_category_repository): "catalogue_item_properties": catalogue_category.catalogue_item_properties, } ) - database_mock.catalogue_categories.find_one.assert_called_once_with({"_id": CustomObjectId(catalogue_category.id)}) + assert created_catalogue_category == catalogue_category + + +def test_create_leaf_category_without_catalogue_item_properties( + test_helpers, database_mock, catalogue_category_repository +): + """ + Test creating a leaf catalogue category without catalogue item properties. + + Verify that the `create` method properly handles the catalogue category to be created, checks that there is not a + duplicate catalogue category, and creates the catalogue category. + """ + # pylint: disable=duplicate-code + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + # pylint: enable=duplicate-code + + # Mock `count_documents` to return 0 (no duplicate catalogue category found within the parent catalogue category) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + # Mock `insert_one` to return an object for the inserted catalogue category document + test_helpers.mock_insert_one(database_mock.catalogue_categories, CustomObjectId(catalogue_category.id)) + # Mock `find_one` to return the inserted catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category.id), + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + }, + ) + + # pylint: disable=duplicate-code + created_catalogue_category = catalogue_category_repository.create( + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ) + ) + # pylint: enable=duplicate-code + + database_mock.catalogue_categories.insert_one.assert_called_once_with( + { + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + } + ) assert created_catalogue_category == catalogue_category @@ -211,9 +280,6 @@ def test_create_with_nonexistent_parent_id(test_helpers, database_mock, catalogu ) ) assert str(exc.value) == f"No parent catalogue category found with ID: {catalogue_category.parent_id}" - database_mock.catalogue_categories.find_one.assert_called_once_with( - {"_id": CustomObjectId(catalogue_category.parent_id)} - ) def test_create_with_duplicate_name_within_parent(test_helpers, database_mock, catalogue_category_repository): @@ -271,9 +337,6 @@ def test_create_with_duplicate_name_within_parent(test_helpers, database_mock, c ) # pylint: enable=duplicate-code assert str(exc.value) == "Duplicate catalogue category found within the parent catalogue category" - database_mock.catalogue_categories.find_one.assert_called_once_with( - {"_id": CustomObjectId(catalogue_category.parent_id)} - ) database_mock.catalogue_categories.count_documents.assert_called_once_with( {"parent_id": CustomObjectId(catalogue_category.parent_id), "code": catalogue_category.code} ) @@ -301,17 +364,39 @@ def test_delete(test_helpers, database_mock, catalogue_category_repository): ) -def test_delete_with_children_elements(test_helpers, database_mock, catalogue_category_repository): +def test_delete_with_children_catalogue_categories(test_helpers, database_mock, catalogue_category_repository): """ - Test deleting a catalogue category with children elements. + Test deleting a catalogue category with children catalogue categories. - Verify that the `delete` method properly handles the deletion of a catalogue category with children elements. + Verify that the `delete` method properly handles the deletion of a catalogue category with children catalogue + categories. """ catalogue_category_id = str(ObjectId()) - test_helpers.mock_count_documents(database_mock.catalogue_items, 0) - # Mock count_documents to return 1 (children elements found) + # Mock count_documents to return 1 (children catalogue categories found) test_helpers.mock_count_documents(database_mock.catalogue_categories, 1) + # Mock count_documents to return 0 (children catalogue items not found) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + + with pytest.raises(ChildrenElementsExistError) as exc: + catalogue_category_repository.delete(catalogue_category_id) + assert str(exc.value) == ( + f"Catalogue category with ID {catalogue_category_id} has children elements and cannot be deleted" + ) + + +def test_delete_with_children_catalogue_items(test_helpers, database_mock, catalogue_category_repository): + """ + Test deleting a catalogue category with children catalogue items. + + Verify that the `delete` method properly handles the deletion of a catalogue category with children catalogue items. + """ + catalogue_category_id = str(ObjectId()) + + # Mock count_documents to return 0 (children catalogue categories not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + # Mock count_documents to return 1 (children catalogue items found) + test_helpers.mock_count_documents(database_mock.catalogue_items, 1) with pytest.raises(ChildrenElementsExistError) as exc: catalogue_category_repository.delete(catalogue_category_id) @@ -652,3 +737,312 @@ def test_list_with_path_and_parent_path_filters_no_matching_results( database_mock.catalogue_categories.find.assert_called_once_with({"path": "/category-a", "parent_path": "/"}) assert retrieved_catalogue_categories == [] + + +def test_update(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category. + + Verify that the `update` method properly handles the catalogue category to be updated, checks that the catalogue + category does not have children elements, there is not a duplicate catalogue category, and updates the catalogue + category. + """ + # pylint: disable=duplicate-code + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category B", + code="category-b", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + # pylint: enable=duplicate-code + + # Mock count_documents to return 0 (children elements not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + # Mock `find_one` to return a catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category.id), + "name": "Category A", + "code": "category-a", + "is_leaf": catalogue_category.is_leaf, + "path": "/category-a", + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + }, + ) + # Mock `count_documents` to return 0 (no duplicate catalogue category found within the parent catalogue category) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + # Mock `update_one` to return an object for the updated catalogue category document + test_helpers.mock_update_one(database_mock.catalogue_categories) + # Mock `find_one` to return the updated catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category.id), + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + }, + ) + + # pylint: disable=duplicate-code + updated_catalogue_category = catalogue_category_repository.update( + catalogue_category.id, + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + # pylint: enable=duplicate-code + + database_mock.catalogue_categories.update_one.assert_called_once_with( + {"_id": CustomObjectId(catalogue_category.id)}, + { + "$set": { + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + } + }, + ) + database_mock.catalogue_categories.find_one.assert_has_calls( + [ + call({"_id": CustomObjectId(catalogue_category.id)}), + call({"_id": CustomObjectId(catalogue_category.id)}), + ] + ) + assert updated_catalogue_category == catalogue_category + + +def test_update_with_invalid_id(catalogue_category_repository): + """ + Test updating a catalogue category with invalid ID. + + Verify that the `update` method properly handles the update of a catalogue category with a nonexistent ID. + """ + update_catalogue_category = CatalogueCategoryIn( + name="Category B", + code="category-b", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + + catalogue_category_id = "invalid" + with pytest.raises(InvalidObjectIdError) as exc: + catalogue_category_repository.update(catalogue_category_id, update_catalogue_category) + assert str(exc.value) == f"Invalid ObjectId value '{catalogue_category_id}'" + + +def test_update_has_children_catalogue_categories(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category with children catalogue categories. + + Verify that the `update` method properly handles the update of a catalogue category with children catalogue + categories. + """ + update_catalogue_category = CatalogueCategoryIn( + name="Category B", + code="category-b", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + + # Mock count_documents to return 1 (children catalogue categories found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 1) + # Mock count_documents to return 0 (children catalogue items not found) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + + catalogue_category_id = str(ObjectId()) + with pytest.raises(ChildrenElementsExistError) as exc: + catalogue_category_repository.update(catalogue_category_id, update_catalogue_category) + assert ( + str(exc.value) + == f"Catalogue category with ID {str(catalogue_category_id)} has children elements and cannot be updated" + ) + + +def test_update_has_children_catalogue_items(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category with children catalogue items. + + Verify that the `update` method properly handles the update of a catalogue category with children catalogue items. + """ + update_catalogue_category = CatalogueCategoryIn( + name="Category B", + code="category-b", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + + # Mock count_documents to return 0 (children catalogue categories not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + # Mock count_documents to return 1 (children catalogue items found) + test_helpers.mock_count_documents(database_mock.catalogue_items, 1) + + catalogue_category_id = str(ObjectId()) + with pytest.raises(ChildrenElementsExistError) as exc: + catalogue_category_repository.update(catalogue_category_id, update_catalogue_category) + assert ( + str(exc.value) + == f"Catalogue category with ID {str(catalogue_category_id)} has children elements and cannot be updated" + ) + + +def test_update_with_nonexistent_parent_id(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category with non-existent parent ID. + + Verify that the `update` method properly handles the update of a catalogue category with non-existent parent ID. + """ + # pylint: disable=duplicate-code + update_catalogue_category = CatalogueCategoryIn( + name="Category A", + code="category-a", + is_leaf=False, + path="/category-a", + parent_path="/", + parent_id=str(ObjectId()), + catalogue_item_properties=[], + ) + # pylint: enable=duplicate-code + + # Mock count_documents to return 0 (children elements not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + # Mock `find_one` to not return a parent catalogue category document + test_helpers.mock_find_one(database_mock.catalogue_categories, None) + + with pytest.raises(MissingRecordError) as exc: + catalogue_category_repository.update(str(ObjectId()), update_catalogue_category) + assert str(exc.value) == f"No parent catalogue category found with ID: {update_catalogue_category.parent_id}" + + +def test_update_duplicate_name_within_parent(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category with a duplicate name within the parent catalogue category. + + Verify that the `update` method properly handles the update of a catalogue category with a duplicate name in a + parent catalogue category. + """ + # pylint: disable=duplicate-code + update_catalogue_category = CatalogueCategoryIn( + name="Category B", + code="category-B", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + # pylint: enable=duplicate-code + + # Mock count_documents to return 0 (children elements not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + catalogue_category_id = str(ObjectId()) + # Mock `find_one` to return a catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category_id), + "name": "Category A", + "code": "category-a", + "is_leaf": update_catalogue_category.is_leaf, + "path": "/category-a", + "parent_path": update_catalogue_category.parent_path, + "parent_id": update_catalogue_category.parent_id, + "catalogue_item_properties": update_catalogue_category.catalogue_item_properties, + }, + ) + # Mock `count_documents` to return 1 (duplicate catalogue category found within the parent catalogue category) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 1) + + with pytest.raises(DuplicateRecordError) as exc: + catalogue_category_repository.update(catalogue_category_id, update_catalogue_category) + assert str(exc.value) == "Duplicate catalogue category found within the parent catalogue category" + + +def test_update_duplicate_name_within_new_parent(test_helpers, database_mock, catalogue_category_repository): + """ + Test updating a catalogue category with a duplicate name within a new parent catalogue category. + + Verify that the `update` method properly handles the update of a catalogue category with a duplicate name in a new + parent catalogue category. + """ + update_catalogue_category = CatalogueCategoryIn( + name="Category A", + code="category-a", + is_leaf=True, + path="/category-b/category-a", + parent_path="/category-b", + parent_id=str(ObjectId()), + catalogue_item_properties=[], + ) + + # Mock count_documents to return 0 (children elements not found) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + # Mock `find_one` to return a parent catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": update_catalogue_category.parent_id, + "name": "Category B", + "code": "category-b", + "is_leaf": False, + "path": "/category-b", + "parent_path": "/", + "parent_id": None, + "catalogue_item_properties": [], + }, + ) + catalogue_category_id = str(ObjectId()) + # Mock `find_one` to return a catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category_id), + "name": update_catalogue_category.is_leaf, + "code": update_catalogue_category.code, + "is_leaf": update_catalogue_category.is_leaf, + "path": "/category-a", + "parent_path": "/", + "parent_id": None, + "catalogue_item_properties": update_catalogue_category.catalogue_item_properties, + }, + ) + # Mock `count_documents` to return 1 (duplicate catalogue category found within the parent catalogue category) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 1) + + with pytest.raises(DuplicateRecordError) as exc: + catalogue_category_repository.update(catalogue_category_id, update_catalogue_category) + assert str(exc.value) == "Duplicate catalogue category found within the parent catalogue category" diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 375cfaa2..befe2ebb 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -65,8 +65,6 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "properties": catalogue_item.properties, } ) - - database_mock.catalogue_items.find_one.assert_called_once_with({"_id": CustomObjectId(catalogue_item.id)}) assert created_catalogue_item == catalogue_item From 86bb13f35db0863acf751fead2058202f7b7be50 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 20 Sep 2023 15:48:44 +0100 Subject: [PATCH 10/16] Write unit tests for the update service method #3 --- .../services/catalogue_category.py | 2 +- test/unit/services/conftest.py | 106 +++++ test/unit/services/test_catalogue_category.py | 429 +++++++++++++++--- test/unit/services/test_catalogue_item.py | 146 +++--- 4 files changed, 546 insertions(+), 137 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_category.py b/inventory_management_system_api/services/catalogue_category.py index a5a83554..597d3bda 100644 --- a/inventory_management_system_api/services/catalogue_category.py +++ b/inventory_management_system_api/services/catalogue_category.py @@ -109,7 +109,7 @@ def update( """ update_data = catalogue_category.dict(exclude_unset=True) - stored_catalogue_category = self._catalogue_category_repository.get(catalogue_category_id) + stored_catalogue_category = self.get(catalogue_category_id) if not stored_catalogue_category: raise MissingRecordError(f"No catalogue category found with ID: {catalogue_category_id}") diff --git a/test/unit/services/conftest.py b/test/unit/services/conftest.py index 2a4836f8..9992cd52 100644 --- a/test/unit/services/conftest.py +++ b/test/unit/services/conftest.py @@ -1,11 +1,17 @@ """ Module for providing common test configuration and test fixtures. """ +from typing import Union, List, Type from unittest.mock import Mock import pytest +from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut +from inventory_management_system_api.models.catalogue_item import CatalogueItemOut from inventory_management_system_api.repositories.catalogue_category import CatalogueCategoryRepo +from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo +from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService +from inventory_management_system_api.services.catalogue_item import CatalogueItemService @pytest.fixture(name="catalogue_category_repository_mock") @@ -16,3 +22,103 @@ def fixture_catalogue_category_repository_mock() -> Mock: :return: Mocked CatalogueCategoryRepo instance. """ return Mock(CatalogueCategoryRepo) + + +@pytest.fixture(name="catalogue_item_repository_mock") +def fixture_catalogue_item_repository_mock() -> Mock: + """ + Fixture to create a mock of the `CatalogueItemRepo` dependency. + + :return: Mocked CatalogueItemRepo instance. + """ + return Mock(CatalogueItemRepo) + + +@pytest.fixture(name="catalogue_category_service") +def fixture_catalogue_category_service(catalogue_category_repository_mock: Mock) -> CatalogueCategoryService: + """ + Fixture to create a `CatalogueCategoryService` instance with a mocked `CatalogueCategoryRepo` dependency. + + :param catalogue_category_repository_mock: Mocked `CatalogueCategoryRepo` instance. + :return: `CatalogueCategoryService` instance with the mocked dependency. + """ + return CatalogueCategoryService(catalogue_category_repository_mock) + + +@pytest.fixture(name="catalogue_item_service") +def fixture_catalogue_item_service( + catalogue_item_repository_mock: Mock, catalogue_category_repository_mock: Mock +) -> CatalogueItemService: + """ + Fixture to create a `CatalogueItemService` instance with a mocked `CatalogueItemRepo` and `CatalogueCategoryRepo` + dependencies. + + :param catalogue_item_repository_mock: Mocked `CatalogueItemRepo` instance. + :param catalogue_category_repository_mock: Mocked `CatalogueCategoryRepo` instance. + :return: `CatalogueItemService` instance with the mocked dependency. + """ + return CatalogueItemService(catalogue_item_repository_mock, catalogue_category_repository_mock) + + +class ServiceTestHelpers: + """ + A utility class containing common helper methods for the service tests. + + This class provides a set of static methods that encapsulate common functionality frequently used in the service + tests. + """ + + @staticmethod + def mock_create(repository_mock: Mock, repo_obj: Union[CatalogueCategoryOut, CatalogueItemOut]) -> None: + """ + Mock the `create` method of the repository mock to return a repository object. + + :param repository_mock: Mocked repository instance. + :param repo_obj: The repository object to be returned by the `create` method. + """ + repository_mock.create.return_value = repo_obj + + @staticmethod + def mock_get(repository_mock: Mock, repo_obj: Union[CatalogueCategoryOut, CatalogueItemOut, None]) -> None: + """ + Mock the `get` method of the repository mock to return a specific repository object. + + :param repository_mock: Mocked repository instance. + :param repo_obj: The repository object to be returned by the `get` method. + """ + if repository_mock.get.side_effect is None: + repository_mock.get.side_effect = [repo_obj] + else: + repo_objs = list(repository_mock.get.side_effect) + repo_objs.append(repo_obj) + repository_mock.get.side_effect = repo_objs + + @staticmethod + def mock_list(repository_mock: Mock, repo_objs: List[Union[CatalogueCategoryOut, CatalogueItemOut]]) -> None: + """ + Mock the `list` method of the repository mock to return a specific list of repository objects. + objects. + + :param repository_mock: Mocked repository instance. + :param repo_objs: The list of repository objects to be returned by the `list` method. + """ + repository_mock.list.return_value = repo_objs + + @staticmethod + def mock_update(repository_mock: Mock, repo_obj: Union[CatalogueCategoryOut, CatalogueItemOut]) -> None: + """ + Mock the `update` method of the repository mock to return a repository object. + + :param repository_mock: Mocked repository instance. + :param repo_obj: The repository object to be returned by the `update` method. + """ + + repository_mock.update.return_value = repo_obj + + +@pytest.fixture(name="test_helpers") +def fixture_test_helpers() -> Type[ServiceTestHelpers]: + """ + Fixture to provide a TestHelpers class. + """ + return ServiceTestHelpers diff --git a/test/unit/services/test_catalogue_category.py b/test/unit/services/test_catalogue_category.py index 128c2917..6b0c123c 100644 --- a/test/unit/services/test_catalogue_category.py +++ b/test/unit/services/test_catalogue_category.py @@ -1,33 +1,22 @@ """ Unit tests for the `CatalogueCategoryService` service. """ -from unittest.mock import Mock - import pytest from bson import ObjectId -from inventory_management_system_api.core.exceptions import LeafCategoryError +from inventory_management_system_api.core.exceptions import LeafCategoryError, MissingRecordError from inventory_management_system_api.models.catalogue_category import ( CatalogueCategoryOut, CatalogueCategoryIn, CatalogueItemProperty, ) -from inventory_management_system_api.schemas.catalogue_category import CatalogueCategoryPostRequestSchema -from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService - - -@pytest.fixture(name="catalogue_category_service") -def fixture_catalogue_category_service(catalogue_category_repository_mock: Mock) -> CatalogueCategoryService: - """ - Fixture to create a `CatalogueCategoryService` instance with a mocked `CatalogueCategoryRepo` dependency. - - :param catalogue_category_repository_mock: Mocked `CatalogueCategoryRepo` instance. - :return: `CatalogueCategoryService` instance with the mocked dependency. - """ - return CatalogueCategoryService(catalogue_category_repository_mock) +from inventory_management_system_api.schemas.catalogue_category import ( + CatalogueCategoryPatchRequestSchema, + CatalogueCategoryPostRequestSchema, +) -def test_create(catalogue_category_repository_mock, catalogue_category_service): +def test_create(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test creating a catalogue category. @@ -48,7 +37,7 @@ def test_create(catalogue_category_repository_mock, catalogue_category_service): # pylint: enable=duplicate-code # Mock `create` to return the created catalogue category - catalogue_category_repository_mock.create.return_value = catalogue_category + test_helpers.mock_create(catalogue_category_repository_mock, catalogue_category) created_catalogue_category = catalogue_category_service.create( CatalogueCategoryPostRequestSchema( @@ -74,7 +63,7 @@ def test_create(catalogue_category_repository_mock, catalogue_category_service): assert created_catalogue_category == catalogue_category -def test_create_with_parent_id(catalogue_category_repository_mock, catalogue_category_service): +def test_create_with_parent_id(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test creating a catalogue category with a parent ID. @@ -98,19 +87,22 @@ def test_create_with_parent_id(catalogue_category_repository_mock, catalogue_cat # Mock `get` to return the parent catalogue category # pylint: disable=duplicate-code - catalogue_category_repository_mock.get.return_value = CatalogueCategoryOut( - id=catalogue_category.parent_id, - name="Category A", - code="category-a", - is_leaf=False, - path="/category-a", - parent_path="/", - parent_id=None, - catalogue_item_properties=[], + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.parent_id, + name="Category A", + code="category-a", + is_leaf=False, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ), ) # pylint: enable=duplicate-code # Mock `create` to return the created catalogue category - catalogue_category_repository_mock.create.return_value = catalogue_category + test_helpers.mock_create(catalogue_category_repository_mock, catalogue_category) created_catalogue_category = catalogue_category_service.create( CatalogueCategoryPostRequestSchema( @@ -121,7 +113,6 @@ def test_create_with_parent_id(catalogue_category_repository_mock, catalogue_cat ) ) - catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.parent_id) # pylint: disable=duplicate-code catalogue_category_repository_mock.create.assert_called_once_with( CatalogueCategoryIn( @@ -138,7 +129,7 @@ def test_create_with_parent_id(catalogue_category_repository_mock, catalogue_cat assert created_catalogue_category == catalogue_category -def test_create_with_whitespace_name(catalogue_category_repository_mock, catalogue_category_service): +def test_create_with_whitespace_name(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test creating a catalogue category name containing leading/trailing/consecutive whitespaces. @@ -161,7 +152,7 @@ def test_create_with_whitespace_name(catalogue_category_repository_mock, catalog # pylint: enable=duplicate-code # Mock `create` to return the created catalogue category - catalogue_category_repository_mock.create.return_value = catalogue_category + test_helpers.mock_create(catalogue_category_repository_mock, catalogue_category) created_catalogue_category = catalogue_category_service.create( CatalogueCategoryPostRequestSchema( @@ -187,10 +178,13 @@ def test_create_with_whitespace_name(catalogue_category_repository_mock, catalog assert created_catalogue_category == catalogue_category -def test_create_with_leaf_parent_catalogue_category(catalogue_category_repository_mock, catalogue_category_service): +def test_create_with_leaf_parent_catalogue_category( + test_helpers, catalogue_category_repository_mock, catalogue_category_service +): """ Test creating a catalogue category in a leaf parent catalogue category. """ + # pylint: disable=duplicate-code catalogue_category = CatalogueCategoryOut( id=str(ObjectId()), name="Category B", @@ -201,21 +195,25 @@ def test_create_with_leaf_parent_catalogue_category(catalogue_category_repositor parent_id=str(ObjectId()), catalogue_item_properties=[], ) + # pylint: enable=duplicate-code # Mock `get` to return the parent catalogue category # pylint: disable=duplicate-code - catalogue_category_repository_mock.get.return_value = CatalogueCategoryOut( - id=catalogue_category.parent_id, - name="Category A", - code="category-a", - is_leaf=True, - path="/category-a", - parent_path="/", - parent_id=None, - catalogue_item_properties=[ - CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), - CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), - ], + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.parent_id, + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + ], + ), ) # pylint: enable=duplicate-code @@ -229,7 +227,6 @@ def test_create_with_leaf_parent_catalogue_category(catalogue_category_repositor ) ) assert str(exc.value) == "Cannot add catalogue category to a leaf parent catalogue category" - catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.parent_id) def test_delete(catalogue_category_repository_mock, catalogue_category_service): @@ -245,7 +242,7 @@ def test_delete(catalogue_category_repository_mock, catalogue_category_service): catalogue_category_repository_mock.delete.assert_called_once_with(catalogue_category_id) -def test_get(catalogue_category_repository_mock, catalogue_category_service): +def test_get(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test getting a catalogue category. @@ -265,7 +262,7 @@ def test_get(catalogue_category_repository_mock, catalogue_category_service): # pylint: enable=duplicate-code # Mock `get` to return a catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) retrieved_catalogue_category = catalogue_category_service.get(catalogue_category.id) @@ -273,7 +270,7 @@ def test_get(catalogue_category_repository_mock, catalogue_category_service): assert retrieved_catalogue_category == catalogue_category -def test_get_with_nonexistent_id(catalogue_category_repository_mock, catalogue_category_service): +def test_get_with_nonexistent_id(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test getting a catalogue category with a nonexistent ID. @@ -282,7 +279,7 @@ def test_get_with_nonexistent_id(catalogue_category_repository_mock, catalogue_c catalogue_category_id = str(ObjectId()) # Mock `get` to not return a catalogue category - catalogue_category_repository_mock.get.return_value = None + test_helpers.mock_get(catalogue_category_repository_mock, None) retrieved_catalogue_category = catalogue_category_service.get(catalogue_category_id) @@ -290,7 +287,7 @@ def test_get_with_nonexistent_id(catalogue_category_repository_mock, catalogue_c catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category_id) -def test_list(catalogue_category_repository_mock, catalogue_category_service): +def test_list(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test getting catalogue categories. @@ -321,7 +318,7 @@ def test_list(catalogue_category_repository_mock, catalogue_category_service): # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue categories - catalogue_category_repository_mock.list.return_value = [catalogue_category_a, catalogue_category_b] + test_helpers.mock_list(catalogue_category_repository_mock, [catalogue_category_a, catalogue_category_b]) retrieved_catalogue_categories = catalogue_category_service.list(None, None) @@ -329,7 +326,7 @@ def test_list(catalogue_category_repository_mock, catalogue_category_service): assert retrieved_catalogue_categories == [catalogue_category_a, catalogue_category_b] -def test_list_with_path_filter(catalogue_category_repository_mock, catalogue_category_service): +def test_list_with_path_filter(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test getting catalogue categories based on the provided path filter. @@ -350,7 +347,7 @@ def test_list_with_path_filter(catalogue_category_repository_mock, catalogue_cat # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue categories - catalogue_category_repository_mock.list.return_value = [catalogue_category] + test_helpers.mock_list(catalogue_category_repository_mock, [catalogue_category]) retrieved_catalogue_categories = catalogue_category_service.list("/category-a", None) @@ -358,7 +355,7 @@ def test_list_with_path_filter(catalogue_category_repository_mock, catalogue_cat assert retrieved_catalogue_categories == [catalogue_category] -def test_list_with_parent_path_filter(catalogue_category_repository_mock, catalogue_category_service): +def test_list_with_parent_path_filter(test_helpers, catalogue_category_repository_mock, catalogue_category_service): """ Test getting catalogue categories based on the provided parent path filter. @@ -378,6 +375,7 @@ def test_list_with_parent_path_filter(catalogue_category_repository_mock, catalo ) # pylint: enable=duplicate-code + # pylint: disable=duplicate-code catalogue_category_b = CatalogueCategoryOut( id=str(ObjectId()), name="Category B", @@ -388,9 +386,10 @@ def test_list_with_parent_path_filter(catalogue_category_repository_mock, catalo parent_id=None, catalogue_item_properties=[], ) + # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue categories - catalogue_category_repository_mock.list.return_value = [catalogue_category_a, catalogue_category_b] + test_helpers.mock_list(catalogue_category_repository_mock, [catalogue_category_a, catalogue_category_b]) retrieved_catalogue_categories = catalogue_category_service.list(None, "/") @@ -398,7 +397,9 @@ def test_list_with_parent_path_filter(catalogue_category_repository_mock, catalo assert retrieved_catalogue_categories == [catalogue_category_a, catalogue_category_b] -def test_list_with_path_and_parent_path_filters(catalogue_category_repository_mock, catalogue_category_service): +def test_list_with_path_and_parent_path_filters( + test_helpers, catalogue_category_repository_mock, catalogue_category_service +): """ Test getting catalogue categories based on the provided path and parent path filters. @@ -419,7 +420,7 @@ def test_list_with_path_and_parent_path_filters(catalogue_category_repository_mo # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue categories - catalogue_category_repository_mock.list.return_value = [catalogue_category] + test_helpers.mock_list(catalogue_category_repository_mock, [catalogue_category]) retrieved_catalogue_categories = catalogue_category_service.list("/category-b", "/") @@ -428,7 +429,7 @@ def test_list_with_path_and_parent_path_filters(catalogue_category_repository_mo def test_list_with_path_and_parent_path_filters_no_matching_results( - catalogue_category_repository_mock, catalogue_category_service + test_helpers, catalogue_category_repository_mock, catalogue_category_service ): """ Test getting catalogue categories based on the provided path and parent path filters when there is no matching @@ -438,9 +439,317 @@ def test_list_with_path_and_parent_path_filters_no_matching_results( parent path filters. """ # Mock `list` to return an empty list of catalogue categories - catalogue_category_repository_mock.list.return_value = [] + test_helpers.mock_list(catalogue_category_repository_mock, []) retrieved_catalogue_categories = catalogue_category_service.list("/category-b", "/") catalogue_category_repository_mock.list.assert_called_once_with("/category-b", "/") assert retrieved_catalogue_categories == [] + + +def test_update(test_helpers, catalogue_category_repository_mock, catalogue_category_service): + """ + Test updating a catalogue category. + + Verify that the `update` method properly handles the catalogue category to be updated. + """ + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category B", + code="category-b", + is_leaf=True, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.id, + name="Category A", + code="category-a", + is_leaf=catalogue_category.is_leaf, + path="/category-a", + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + # Mock `update` to return the updated catalogue category + test_helpers.mock_update(catalogue_category_repository_mock, catalogue_category) + + updated_catalogue_category = catalogue_category_service.update( + catalogue_category.id, CatalogueCategoryPatchRequestSchema(name=catalogue_category.name) + ) + + # pylint: disable=duplicate-code + catalogue_category_repository_mock.update.assert_called_once_with( + catalogue_category.id, + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + # pylint: enable=duplicate-code + assert updated_catalogue_category == catalogue_category + + +def test_update_with_nonexistent_id(test_helpers, catalogue_category_repository_mock, catalogue_category_service): + """ + Test updating a catalogue category with a non-existent ID. + + Verify that the `update` method properly handles the catalogue category to be updated with a non-existent ID. + """ + # Mock `get` to not return a catalogue category + test_helpers.mock_get(catalogue_category_repository_mock, None) + + catalogue_category_id = str(ObjectId()) + with pytest.raises(MissingRecordError) as exc: + catalogue_category_service.update( + catalogue_category_id, CatalogueCategoryPatchRequestSchema(catalogue_item_properties=[]) + ) + assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category_id}" + + +def test_update_change_parent_id(test_helpers, catalogue_category_repository_mock, catalogue_category_service): + """ + Test moving a catalogue category to another parent catalogue category. + """ + + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category B", + code="category-b", + is_leaf=False, + path="/category-a/category-b", + parent_path="/category-a", + parent_id=str(ObjectId()), + catalogue_item_properties=[], + ) + + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.id, + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + # Mock `get` to return a parent catalogue category + # pylint: disable=duplicate-code + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=str(ObjectId()), + name="Category A", + code="category-a", + is_leaf=False, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ), + ) + # pylint: enable=duplicate-code + # Mock `update` to return the updated catalogue category + test_helpers.mock_update(catalogue_category_repository_mock, catalogue_category) + + updated_catalogue_category = catalogue_category_service.update( + catalogue_category.id, CatalogueCategoryPatchRequestSchema(parent_id=catalogue_category.parent_id) + ) + + catalogue_category_repository_mock.update.assert_called_once_with( + catalogue_category.id, + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + assert updated_catalogue_category == catalogue_category + + +def test_update_change_parent_id_leaf_parent_catalogue_category( + test_helpers, catalogue_category_repository_mock, catalogue_category_service +): + """ + Testing moving a catalogue category to a leaf parent catalogue category. + """ + catalogue_category_b_id = str(ObjectId()) + # Mock `get` to return a catalogue category + # pylint: disable=duplicate-code + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category_b_id, + name="Category B", + code="category-b", + is_leaf=False, + path="/category-b", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ), + ) + catalogue_category_a_id = str(ObjectId()) + # Mock `get` to return a parent catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category_b_id, + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + ], + ), + ) + # pylint: enable=duplicate-code + + with pytest.raises(LeafCategoryError) as exc: + catalogue_category_service.update( + catalogue_category_b_id, CatalogueCategoryPatchRequestSchema(parent_id=catalogue_category_a_id) + ) + assert str(exc.value) == "Cannot add catalogue category to a leaf parent catalogue category" + + +def test_update_change_from_leaf_to_non_leaf( + test_helpers, catalogue_category_repository_mock, catalogue_category_service +): + """ + Test changing a catalogue category from leaf to non-leaf. + """ + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category A", + code="category-a", + is_leaf=False, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.id, + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=True, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + ], + ), + ) + # Mock `update` to return the updated catalogue category + test_helpers.mock_update(catalogue_category_repository_mock, catalogue_category) + + updated_catalogue_category = catalogue_category_service.update( + catalogue_category.id, CatalogueCategoryPatchRequestSchema(is_leaf=False) + ) + + catalogue_category_repository_mock.update.assert_called_once_with( + catalogue_category.id, + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + assert updated_catalogue_category == catalogue_category + + +def test_update_change_catalogue_item_properties( + test_helpers, catalogue_category_repository_mock, catalogue_category_service +): + """ + Test updating a catalogue category. + + Verify that the `update` method properly handles the catalogue category to be updated. + """ + # pylint: disable=duplicate-code + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + ], + ) + # pylint: enable=duplicate-code + + # Mock `get` to return a catalogue category + # pylint: disable=duplicate-code + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_category.id, + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=[catalogue_category.catalogue_item_properties[1]], + ), + ) + # pylint: enable=duplicate-code + # Mock `update` to return the updated catalogue category + test_helpers.mock_update(catalogue_category_repository_mock, catalogue_category) + + updated_catalogue_category = catalogue_category_service.update( + catalogue_category.id, + CatalogueCategoryPatchRequestSchema(catalogue_item_properties=catalogue_category.catalogue_item_properties), + ) + + catalogue_category_repository_mock.update.assert_called_once_with( + catalogue_category.id, + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_category.catalogue_item_properties, + ), + ) + assert updated_catalogue_category == catalogue_category diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 5fcb042c..5cf61e7d 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -1,7 +1,6 @@ """ Unit tests for the `CatalogueCategoryService` service. """ -from unittest.mock import Mock import pytest from bson import ObjectId @@ -14,40 +13,15 @@ ) from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut, CatalogueItemProperty from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, Property, CatalogueItemIn -from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo from inventory_management_system_api.schemas.catalogue_item import ( PropertyPostRequestSchema, CatalogueItemPostRequestSchema, ) -from inventory_management_system_api.services.catalogue_item import CatalogueItemService -@pytest.fixture(name="catalogue_item_repository_mock") -def fixture_catalogue_item_repository_mock() -> Mock: - """ - Fixture to create a mock of the `CatalogueItemRepo` dependency. - - :return: Mocked CatalogueItemRepo instance. - """ - return Mock(CatalogueItemRepo) - - -@pytest.fixture(name="catalogue_item_service") -def fixture_catalogue_item_service( - catalogue_item_repository_mock: Mock, catalogue_category_repository_mock: Mock -) -> CatalogueItemService: - """ - Fixture to create a `CatalogueItemService` instance with a mocked `CatalogueItemRepo` and `CatalogueCategoryRepo` - dependencies. - - :param catalogue_item_repository_mock: Mocked `CatalogueItemRepo` instance. - :param catalogue_category_repository_mock: Mocked `CatalogueCategoryRepo` instance. - :return: `CatalogueItemService` instance with the mocked dependency. - """ - return CatalogueItemService(catalogue_item_repository_mock, catalogue_category_repository_mock) - - -def test_create(catalogue_item_repository_mock, catalogue_category_repository_mock, catalogue_item_service): +def test_create( + test_helpers, catalogue_item_repository_mock, catalogue_category_repository_mock, catalogue_item_service +): """ Test creating a catalogue item. @@ -69,23 +43,26 @@ def test_create(catalogue_item_repository_mock, catalogue_category_repository_mo ) # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = CatalogueCategoryOut( - id=catalogue_item.catalogue_category_id, - name="Category A", - code="category-a", - is_leaf=True, - path="/category-a", - parent_path="/", - parent_id=None, - catalogue_item_properties=[ - CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), - CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), - CatalogueItemProperty(name="Property C", type="string", unit="cm", mandatory=True), - ], + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_item.catalogue_category_id, + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + CatalogueItemProperty(name="Property C", type="string", unit="cm", mandatory=True), + ], + ), ) # pylint: enable=duplicate-code # Mock `create` to return the created catalogue item - catalogue_item_repository_mock.create.return_value = catalogue_item + test_helpers.mock_create(catalogue_item_repository_mock, catalogue_item) created_catalogue_item = catalogue_item_service.create( CatalogueItemPostRequestSchema( @@ -114,7 +91,9 @@ def test_create(catalogue_item_repository_mock, catalogue_category_repository_mo assert created_catalogue_item == catalogue_item -def test_create_with_nonexistent_catalogue_category_id(catalogue_category_repository_mock, catalogue_item_service): +def test_create_with_nonexistent_catalogue_category_id( + test_helpers, catalogue_category_repository_mock, catalogue_item_service +): """ Test creating a catalogue item with a nonexistent parent ID. @@ -124,7 +103,7 @@ def test_create_with_nonexistent_catalogue_category_id(catalogue_category_reposi catalogue_category_id = str(ObjectId()) # Mock `get` to not return a catalogue category - catalogue_category_repository_mock.get.return_value = None + test_helpers.mock_get(catalogue_category_repository_mock, None) with pytest.raises(MissingRecordError) as exc: catalogue_item_service.create( @@ -143,7 +122,9 @@ def test_create_with_nonexistent_catalogue_category_id(catalogue_category_reposi catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category_id) -def test_create_in_non_leaf_catalogue_category(catalogue_category_repository_mock, catalogue_item_service): +def test_create_in_non_leaf_catalogue_category( + test_helpers, catalogue_category_repository_mock, catalogue_item_service +): """ Test creating a catalogue item in a non-leaf catalogue category. @@ -165,7 +146,7 @@ def test_create_in_non_leaf_catalogue_category(catalogue_category_repository_moc # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) with pytest.raises(NonLeafCategoryError) as exc: catalogue_item_service.create( @@ -185,7 +166,7 @@ def test_create_in_non_leaf_catalogue_category(catalogue_category_repository_moc def test_create_without_properties( - catalogue_item_repository_mock, catalogue_category_repository_mock, catalogue_item_service + test_helpers, catalogue_item_repository_mock, catalogue_category_repository_mock, catalogue_item_service ): """ Test creating a catalogue item without properties. @@ -203,18 +184,23 @@ def test_create_without_properties( # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = CatalogueCategoryOut( - id=catalogue_item.catalogue_category_id, - name="Category A", - code="category-a", - is_leaf=True, - path="/category-a", - parent_path="/", - parent_id=None, - catalogue_item_properties=[], + # pylint: disable=duplicate-code + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_item.catalogue_category_id, + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ), ) + # pylint: enable=duplicate-code # Mock `create` to return the created catalogue item - catalogue_item_repository_mock.create.return_value = catalogue_item + test_helpers.mock_create(catalogue_item_repository_mock, catalogue_item) created_catalogue_item = catalogue_item_service.create( CatalogueItemPostRequestSchema( @@ -238,7 +224,9 @@ def test_create_without_properties( assert created_catalogue_item == catalogue_item -def test_create_with_missing_mandatory_properties(catalogue_category_repository_mock, catalogue_item_service): +def test_create_with_missing_mandatory_properties( + test_helpers, catalogue_category_repository_mock, catalogue_item_service +): """ Test creating a catalogue item with missing mandatory catalogue item properties. @@ -264,7 +252,7 @@ def test_create_with_missing_mandatory_properties(catalogue_category_repository_ # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) with pytest.raises(MissingMandatoryCatalogueItemProperty) as exc: catalogue_item_service.create( @@ -285,7 +273,7 @@ def test_create_with_missing_mandatory_properties(catalogue_category_repository_ def test_create_with_with_invalid_value_type_for_string_property( - catalogue_category_repository_mock, catalogue_item_service + test_helpers, catalogue_category_repository_mock, catalogue_item_service ): """ Test creating a catalogue item with invalid value type for a string catalogue item property. @@ -295,6 +283,7 @@ def test_create_with_with_invalid_value_type_for_string_property( missing mandatory catalogue item properties, finds invalid value type for a string catalogue item property, and does not create the catalogue item. """ + # pylint: disable=duplicate-code catalogue_category = CatalogueCategoryOut( id=str(ObjectId()), name="Category A", @@ -309,9 +298,10 @@ def test_create_with_with_invalid_value_type_for_string_property( CatalogueItemProperty(name="Property C", type="string", unit="cm", mandatory=True), ], ) + # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: catalogue_item_service.create( @@ -335,7 +325,7 @@ def test_create_with_with_invalid_value_type_for_string_property( def test_create_with_with_invalid_value_type_for_number_property( - catalogue_category_repository_mock, catalogue_item_service + test_helpers, catalogue_category_repository_mock, catalogue_item_service ): """ Test creating a catalogue item with invalid value type for a number catalogue item property. @@ -345,6 +335,7 @@ def test_create_with_with_invalid_value_type_for_number_property( missing mandatory catalogue item properties, finds invalid value type for a number catalogue item property, and does not create the catalogue item. """ + # pylint: disable=duplicate-code catalogue_category = CatalogueCategoryOut( id=str(ObjectId()), name="Category A", @@ -359,9 +350,10 @@ def test_create_with_with_invalid_value_type_for_number_property( CatalogueItemProperty(name="Property C", type="string", unit="cm", mandatory=True), ], ) + # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: catalogue_item_service.create( @@ -385,7 +377,7 @@ def test_create_with_with_invalid_value_type_for_number_property( def test_create_with_with_invalid_value_type_for_boolean_property( - catalogue_category_repository_mock, catalogue_item_service + test_helpers, catalogue_category_repository_mock, catalogue_item_service ): """ Test creating a catalogue item with invalid value type for a boolean catalogue item property. @@ -395,6 +387,7 @@ def test_create_with_with_invalid_value_type_for_boolean_property( missing mandatory catalogue item properties, finds invalid value type for a boolean catalogue item property, and does not create the catalogue item. """ + # pylint: disable=duplicate-code catalogue_category = CatalogueCategoryOut( id=str(ObjectId()), name="Category A", @@ -409,9 +402,10 @@ def test_create_with_with_invalid_value_type_for_boolean_property( CatalogueItemProperty(name="Property C", type="string", unit="cm", mandatory=True), ], ) + # pylint: enable=duplicate-code # Mock `get` to return the catalogue category - catalogue_category_repository_mock.get.return_value = catalogue_category + test_helpers.mock_get(catalogue_category_repository_mock, catalogue_category) with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: catalogue_item_service.create( @@ -434,7 +428,7 @@ def test_create_with_with_invalid_value_type_for_boolean_property( catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.id) -def test_get(catalogue_item_repository_mock, catalogue_item_service): +def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_service): """ Test getting a catalogue item. @@ -455,7 +449,7 @@ def test_get(catalogue_item_repository_mock, catalogue_item_service): # pylint: enable=duplicate-code # Mock `get` to return a catalogue item - catalogue_item_repository_mock.get.return_value = catalogue_item + test_helpers.mock_get(catalogue_item_repository_mock, catalogue_item) retrieved_catalogue_item = catalogue_item_service.get(catalogue_item.id) @@ -463,7 +457,7 @@ def test_get(catalogue_item_repository_mock, catalogue_item_service): assert retrieved_catalogue_item == catalogue_item -def test_get_with_nonexistent_id(catalogue_item_repository_mock, catalogue_item_service): +def test_get_with_nonexistent_id(test_helpers, catalogue_item_repository_mock, catalogue_item_service): """ Test getting a catalogue item with a nonexistent ID. @@ -472,7 +466,7 @@ def test_get_with_nonexistent_id(catalogue_item_repository_mock, catalogue_item_ catalogue_item_id = str(ObjectId()) # Mock `get` to not return a catalogue item - catalogue_item_repository_mock.get.return_value = None + test_helpers.mock_get(catalogue_item_repository_mock, None) retrieved_catalogue_item = catalogue_item_service.get(catalogue_item_id) @@ -480,7 +474,7 @@ def test_get_with_nonexistent_id(catalogue_item_repository_mock, catalogue_item_ catalogue_item_repository_mock.get.assert_called_once_with(catalogue_item_id) -def test_list(catalogue_item_repository_mock, catalogue_item_service): +def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_service): """ Test getting catalogue items. @@ -509,7 +503,7 @@ def test_list(catalogue_item_repository_mock, catalogue_item_service): # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue items - catalogue_item_repository_mock.list.return_value = [catalogue_item_a, catalogue_item_b] + test_helpers.mock_list(catalogue_item_repository_mock, [catalogue_item_a, catalogue_item_b]) retrieved_catalogue_items = catalogue_item_service.list(None) @@ -517,7 +511,7 @@ def test_list(catalogue_item_repository_mock, catalogue_item_service): assert retrieved_catalogue_items == [catalogue_item_a, catalogue_item_b] -def test_list_with_catalogue_category_id_filter(catalogue_item_repository_mock, catalogue_item_service): +def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_repository_mock, catalogue_item_service): """ Test getting catalogue items based on the provided catalogue category ID filter. @@ -539,7 +533,7 @@ def test_list_with_catalogue_category_id_filter(catalogue_item_repository_mock, # pylint: enable=duplicate-code # Mock `list` to return a list of catalogue items - catalogue_item_repository_mock.list.return_value = [catalogue_item] + test_helpers.mock_list(catalogue_item_repository_mock, [catalogue_item]) retrieved_catalogue_items = catalogue_item_service.list(catalogue_item.catalogue_category_id) @@ -548,7 +542,7 @@ def test_list_with_catalogue_category_id_filter(catalogue_item_repository_mock, def test_list_with_catalogue_category_id_filter_no_matching_results( - catalogue_item_repository_mock, catalogue_item_service + test_helpers, catalogue_item_repository_mock, catalogue_item_service ): """ Test getting catalogue items based on the provided catalogue category ID filter when there is no matching results in @@ -558,7 +552,7 @@ def test_list_with_catalogue_category_id_filter_no_matching_results( category ID filter. """ # Mock `list` to return an empty list of catalogue item documents - catalogue_item_repository_mock.list.return_value = [] + test_helpers.mock_list(catalogue_item_repository_mock, []) catalogue_category_id = str(ObjectId()) retrieved_catalogue_items = catalogue_item_service.list(catalogue_category_id) From ff5f256a41354091e35cbd824f785cd786e627e4 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Fri, 22 Sep 2023 09:24:17 +0100 Subject: [PATCH 11/16] Modify catalogue item to include manufacturer details #3 --- .../models/catalogue_item.py | 11 ++++ .../schemas/catalogue_item.py | 11 ++++ .../services/catalogue_item.py | 1 + test/e2e/test_catalogue_category.py | 25 ++++++++ test/e2e/test_catalogue_item.py | 13 ++++ test/unit/repositories/test_catalogue_item.py | 33 +++++++++- test/unit/services/test_catalogue_item.py | 60 ++++++++++++++++++- 7 files changed, 152 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index 29289511..49ef1033 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -8,6 +8,16 @@ from inventory_management_system_api.models.catalogue_category import CustomObjectIdField, StringObjectIdField +class Manufacturer(BaseModel): + """ + Model representing a catalogue item manufacturer. + """ + + name: str + address: str + web_url: Optional[str] = None + + class Property(BaseModel): """ Model representing a catalogue item property. @@ -27,6 +37,7 @@ class CatalogueItemIn(BaseModel): name: str description: str properties: List[Property] + manufacturer: Manufacturer @validator("properties", pre=True, always=True) @classmethod diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 233a3e1c..5120ee73 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -6,6 +6,16 @@ from pydantic import BaseModel, Field +class ManufacturerSchema(BaseModel): + """ + Schema model for a catalogue item manufacturer creation request. + """ + + name: str = Field(description="The name of the manufacturer") + address: str = Field(description="The address of the manufacturer") + web_url: Optional[str] = Field(description="The website URL of the manufacturer") + + class PropertyPostRequestSchema(BaseModel): """ Schema model for a catalogue item property creation request. @@ -34,6 +44,7 @@ class CatalogueItemPostRequestSchema(BaseModel): name: str = Field(description="The name of the catalogue item") description: str = Field(description="The catalogue item description") properties: Optional[List[PropertyPostRequestSchema]] = Field(description="The catalogue item properties") + manufacturer: ManufacturerSchema = Field(description="The details of the manufacturer") class CatalogueItemSchema(CatalogueItemPostRequestSchema): diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index a32e7c1e..f7645726 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -84,6 +84,7 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte name=catalogue_item.name, description=catalogue_item.description, properties=list(supplied_properties.values()), + manufacturer=catalogue_item.manufacturer, ) ) diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index 4f16c374..f1f59fe7 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -306,6 +306,11 @@ def test_delete_catalogue_category_with_children_catalogue_items(test_client): "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -584,6 +589,11 @@ def test_partial_update_catalogue_category_change_name_has_children_catalogue_it "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -710,6 +720,11 @@ def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_has_chil "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -823,6 +838,11 @@ def test_partial_update_catalogue_category_change_parent_id_has_children_catalog "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -1037,6 +1057,11 @@ def test_partial_update_catalogue_category_change_catalogue_item_properties_has_ "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 0ea8b37d..05c04a8a 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -41,6 +41,11 @@ def get_catalogue_item_a_dict(catalogue_category_id: str) -> Dict: {"name": "Property B", "value": False}, {"name": "Property C", "value": "20x15x10"}, ], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } @@ -58,6 +63,11 @@ def get_catalogue_item_b_dict(catalogue_category_id: str) -> Dict: "properties": [ {"name": "Property A", "value": True}, ], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "www.manufacturer-a.co.uk", + }, } @@ -83,6 +93,7 @@ def test_create_catalogue_item(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_duplicate_name_within_catalogue_category(test_client): @@ -160,6 +171,7 @@ def test_create_catalogue_item_without_properties(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == [] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_missing_mandatory_properties(test_client): @@ -199,6 +211,7 @@ def test_create_catalogue_item_with_missing_non_mandatory_properties(test_client assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_invalid_value_type_for_string_property(test_client): diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index befe2ebb..91b74f86 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -6,7 +6,12 @@ from inventory_management_system_api.core.custom_object_id import CustomObjectId from inventory_management_system_api.core.exceptions import DuplicateRecordError, InvalidObjectIdError -from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, Property, CatalogueItemIn +from inventory_management_system_api.models.catalogue_item import ( + CatalogueItemOut, + Property, + CatalogueItemIn, + Manufacturer, +) def test_create(test_helpers, database_mock, catalogue_item_repository): @@ -27,6 +32,9 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -43,6 +51,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, + "manufacturer": catalogue_item.manufacturer, }, ) @@ -53,6 +62,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -63,6 +73,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, + "manufacturer": catalogue_item.manufacturer, } ) assert created_catalogue_item == catalogue_item @@ -86,6 +97,9 @@ def test_create_with_duplicate_name_within_catalogue_category(test_helpers, data Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -99,6 +113,7 @@ def test_create_with_duplicate_name_within_catalogue_category(test_helpers, data name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, + manufacturer=catalogue_item.manufacturer, ) ) assert str(exc.value) == "Duplicate catalogue item found within the catalogue category" @@ -124,6 +139,9 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -136,6 +154,7 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, + "manufacturer": catalogue_item.manufacturer, }, ) @@ -190,6 +209,9 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) catalogue_item_b = CatalogueItemOut( @@ -198,6 +220,9 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -211,6 +236,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_a.name, "description": catalogue_item_a.description, "properties": catalogue_item_a.properties, + "manufacturer": catalogue_item_a.manufacturer, }, { "_id": CustomObjectId(catalogue_item_b.id), @@ -218,6 +244,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_b.name, "description": catalogue_item_b.description, "properties": catalogue_item_b.properties, + "manufacturer": catalogue_item_b.manufacturer, }, ], ) @@ -245,6 +272,9 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # Mock `find` to return a list of catalogue item documents @@ -257,6 +287,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, + "manufacturer": catalogue_item.manufacturer, } ], ) diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 5cf61e7d..ac9cccad 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -12,10 +12,16 @@ InvalidCatalogueItemPropertyTypeError, ) from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut, CatalogueItemProperty -from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, Property, CatalogueItemIn +from inventory_management_system_api.models.catalogue_item import ( + CatalogueItemOut, + Property, + CatalogueItemIn, + Manufacturer, +) from inventory_management_system_api.schemas.catalogue_item import ( PropertyPostRequestSchema, CatalogueItemPostRequestSchema, + ManufacturerSchema, ) @@ -40,6 +46,9 @@ def test_create( Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # Mock `get` to return the catalogue category @@ -74,6 +83,7 @@ def test_create( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=catalogue_item.manufacturer, ) ) @@ -85,6 +95,7 @@ def test_create( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -116,6 +127,11 @@ def test_create_with_nonexistent_catalogue_category_id( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category_id}" @@ -159,6 +175,11 @@ def test_create_in_non_leaf_catalogue_category( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert str(exc.value) == "Cannot add catalogue item to a non-leaf catalogue category" @@ -180,6 +201,9 @@ def test_create_without_properties( name="Catalogue Item A", description="This is Catalogue Item A", properties=[], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -207,6 +231,7 @@ def test_create_without_properties( catalogue_category_id=catalogue_item.catalogue_category_id, name=catalogue_item.name, description=catalogue_item.description, + manufacturer=catalogue_item.manufacturer, ) ) @@ -218,6 +243,7 @@ def test_create_without_properties( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -263,6 +289,11 @@ def test_create_with_missing_mandatory_properties( properties=[ PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -314,6 +345,11 @@ def test_create_with_with_invalid_value_type_for_string_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value=True), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -366,6 +402,11 @@ def test_create_with_with_invalid_value_type_for_number_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -418,6 +459,11 @@ def test_create_with_with_invalid_value_type_for_boolean_property( PropertyPostRequestSchema(name="Property B", value="False"), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], + manufacturer=ManufacturerSchema( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -445,6 +491,9 @@ def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_servic Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -491,6 +540,9 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) catalogue_item_b = CatalogueItemOut( @@ -499,6 +551,9 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code @@ -529,6 +584,9 @@ def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_rep Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], + manufacturer=Manufacturer( + name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + ), ) # pylint: enable=duplicate-code From d38ba86a4a6ba187780df963460c2e17f57166d3 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 28 Sep 2023 10:06:05 +0100 Subject: [PATCH 12/16] Move custom ObjectId data type classes to a new module #3 --- .../models/catalogue_category.py | 43 +---------------- .../models/catalogue_item.py | 2 +- .../models/custom_object_id_data_types.py | 46 +++++++++++++++++++ 3 files changed, 48 insertions(+), 43 deletions(-) create mode 100644 inventory_management_system_api/models/custom_object_id_data_types.py diff --git a/inventory_management_system_api/models/catalogue_category.py b/inventory_management_system_api/models/catalogue_category.py index d10010b4..08c56cde 100644 --- a/inventory_management_system_api/models/catalogue_category.py +++ b/inventory_management_system_api/models/catalogue_category.py @@ -3,50 +3,9 @@ """ from typing import Optional, List -from bson import ObjectId from pydantic import BaseModel, Field, validator -from inventory_management_system_api.core.custom_object_id import CustomObjectId - - -class CustomObjectIdField(ObjectId): - """ - Custom field for handling MongoDB ObjectId validation. - """ - - @classmethod - def __get_validators__(cls): - yield cls.validate - - @classmethod - def validate(cls, value: str) -> CustomObjectId: - """ - Validate if the string value is a valid `ObjectId`. - - :param value: The string value to be validated. - :return: The validated `ObjectId`. - """ - return CustomObjectId(value) - - -class StringObjectIdField(str): - """ - Custom field for handling MongoDB ObjectId as string. - """ - - @classmethod - def __get_validators__(cls): - yield cls.validate - - @classmethod - def validate(cls, value: ObjectId) -> str: - """ - Convert the `ObjectId` value to string. - - :param value: The `ObjectId` value to be converted. - :return: The converted `ObjectId` as a string. - """ - return str(value) +from inventory_management_system_api.models.custom_object_id_data_types import CustomObjectIdField, StringObjectIdField class CatalogueItemProperty(BaseModel): diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index 49ef1033..5b60ec7d 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -5,7 +5,7 @@ from pydantic import BaseModel, Field, validator -from inventory_management_system_api.models.catalogue_category import CustomObjectIdField, StringObjectIdField +from inventory_management_system_api.models.custom_object_id_data_types import CustomObjectIdField, StringObjectIdField class Manufacturer(BaseModel): diff --git a/inventory_management_system_api/models/custom_object_id_data_types.py b/inventory_management_system_api/models/custom_object_id_data_types.py new file mode 100644 index 00000000..c88b2412 --- /dev/null +++ b/inventory_management_system_api/models/custom_object_id_data_types.py @@ -0,0 +1,46 @@ +""" +Module for defining custom `ObjectId` data type classes used by Pydantic models. +""" +from bson import ObjectId + +from inventory_management_system_api.core.custom_object_id import CustomObjectId + + +class CustomObjectIdField(ObjectId): + """ + Custom data type for handling MongoDB ObjectId validation. + """ + + @classmethod + def __get_validators__(cls): + yield cls.validate + + @classmethod + def validate(cls, value: str) -> CustomObjectId: + """ + Validate if the string value is a valid `ObjectId`. + + :param value: The string value to be validated. + :return: The validated `ObjectId`. + """ + return CustomObjectId(value) + + +class StringObjectIdField(str): + """ + Custom data type for handling MongoDB ObjectId as string. + """ + + @classmethod + def __get_validators__(cls): + yield cls.validate + + @classmethod + def validate(cls, value: ObjectId) -> str: + """ + Convert the `ObjectId` value to string. + + :param value: The `ObjectId` value to be converted. + :return: The converted `ObjectId` as a string. + """ + return str(value) From 7d82c8d06e302ccca31b002c21a1522761734618 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 28 Sep 2023 14:54:50 +0100 Subject: [PATCH 13/16] Ignore properties if supplied for non-leaf category #3 --- .../models/catalogue_category.py | 19 +++-- .../schemas/catalogue_category.py | 20 ----- .../services/catalogue_category.py | 2 - test/e2e/test_catalogue_category.py | 34 ++++++--- .../repositories/test_catalogue_category.py | 75 ++++++++++++++++++- 5 files changed, 110 insertions(+), 40 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_category.py b/inventory_management_system_api/models/catalogue_category.py index 08c56cde..1d8c0d6b 100644 --- a/inventory_management_system_api/models/catalogue_category.py +++ b/inventory_management_system_api/models/catalogue_category.py @@ -1,7 +1,7 @@ """ Module for defining the database models for representing catalogue categories. """ -from typing import Optional, List +from typing import Optional, List, Dict, Any from pydantic import BaseModel, Field, validator @@ -30,24 +30,29 @@ class CatalogueCategoryIn(BaseModel): path: str parent_path: str parent_id: Optional[CustomObjectIdField] = None - catalogue_item_properties: List[CatalogueItemProperty] + catalogue_item_properties: List[CatalogueItemProperty] = [] @validator("catalogue_item_properties", pre=True, always=True) @classmethod def validate_catalogue_item_properties( - cls, catalogue_item_properties: List[CatalogueItemProperty] | None + cls, catalogue_item_properties: List[CatalogueItemProperty] | None, values: Dict[str, Any] ) -> List[CatalogueItemProperty] | List: """ - Validator for the `catalogue_item_properties` field that runs always (even if the field is missing) and before - any Pydantic validation checks. + Validator for the `catalogue_item_properties` field that runs after field assignment but before type validation. - If the value is `None`, it replaces it with an empty list. + If the value is `None`, it replaces it with an empty list allowing for catalogue categories without catalogue + item properties to be created. If the category is a non-leaf category and if catalogue item properties are + supplied, it replaces it with an empty list because they cannot have properties. :param catalogue_item_properties: The list of catalogue item properties. + :param values: The values of the model fields. :return: The list of catalogue item properties or an empty list. """ - if catalogue_item_properties is None: + if catalogue_item_properties is None or ( + "is_leaf" in values and values["is_leaf"] is False and catalogue_item_properties + ): catalogue_item_properties = [] + return catalogue_item_properties diff --git a/inventory_management_system_api/schemas/catalogue_category.py b/inventory_management_system_api/schemas/catalogue_category.py index 6dc905fc..6c26f047 100644 --- a/inventory_management_system_api/schemas/catalogue_category.py +++ b/inventory_management_system_api/schemas/catalogue_category.py @@ -61,26 +61,6 @@ class CatalogueCategoryPostRequestSchema(BaseModel): description="The properties that the catalogue items in this category could/should have" ) - @validator("catalogue_item_properties") - @classmethod - def validate_catalogue_item_properties( - cls, catalogue_item_properties: List[CatalogueItemPropertySchema], values: Dict[str, Any] - ) -> List[CatalogueItemPropertySchema]: - """ - Validator for the `catalogue_item_properties` field. - - It checks if the category is a non-leaf category and if catalogue item properties are present in the body. It - raises a `ValueError` if this is the case. - - :param catalogue_item_properties: The list of catalogue item properties. - :param values: The values of the model fields. - :return: The list of catalogue item properties. - :raises ValueError: If catalogue item properties are provided for a non-leaf catalogue category. - """ - if "is_leaf" in values and values["is_leaf"] is False and catalogue_item_properties: - raise ValueError("Catalogue item properties not allowed for non-leaf catalogue category") - return catalogue_item_properties - class CatalogueCategoryPatchRequestSchema(CatalogueCategoryPostRequestSchema): """ diff --git a/inventory_management_system_api/services/catalogue_category.py b/inventory_management_system_api/services/catalogue_category.py index 597d3bda..97e28a3a 100644 --- a/inventory_management_system_api/services/catalogue_category.py +++ b/inventory_management_system_api/services/catalogue_category.py @@ -135,8 +135,6 @@ def update( if "is_leaf" in update_data: stored_catalogue_category.is_leaf = update_data["is_leaf"] - if stored_catalogue_category.is_leaf is False: - stored_catalogue_category.catalogue_item_properties = [] if "catalogue_item_properties" in update_data: stored_catalogue_category.catalogue_item_properties = update_data["catalogue_item_properties"] diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index f1f59fe7..06a6755b 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -202,7 +202,7 @@ def test_create_non_leaf_catalogue_category_with_catalogue_item_properties(test_ """ Test creating a non-leaf catalogue category with catalogue item properties. """ - catalogue_category = { + catalogue_category_post = { "name": "Category A", "is_leaf": False, "catalogue_item_properties": [ @@ -211,12 +211,19 @@ def test_create_non_leaf_catalogue_category_with_catalogue_item_properties(test_ ], } - response = test_client.post("/v1/catalogue-categories", json=catalogue_category) + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) - assert response.status_code == 422 - assert ( - response.json()["detail"][0]["msg"] == "Catalogue item properties not allowed for non-leaf catalogue category" - ) + assert response.status_code == 201 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_post["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] def test_delete_catalogue_category(test_client): @@ -752,10 +759,17 @@ def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_with_cat } response = test_client.patch(f"/v1/catalogue-categories/{response.json()['id']}", json=catalogue_category_patch) - assert response.status_code == 422 - assert ( - response.json()["detail"][0]["msg"] == "Catalogue item properties not allowed for non-leaf catalogue category" - ) + assert response.status_code == 200 + + catalogue_category = response.json() + + assert catalogue_category["name"] == catalogue_category_post["name"] + assert catalogue_category["code"] == "category-a" + assert catalogue_category["is_leaf"] == catalogue_category_patch["is_leaf"] + assert catalogue_category["path"] == "/category-a" + assert catalogue_category["parent_path"] == "/" + assert catalogue_category["parent_id"] is None + assert catalogue_category["catalogue_item_properties"] == [] def test_partial_update_catalogue_category_change_parent_id(test_client): diff --git a/test/unit/repositories/test_catalogue_category.py b/test/unit/repositories/test_catalogue_category.py index 885da192..e5ad4444 100644 --- a/test/unit/repositories/test_catalogue_category.py +++ b/test/unit/repositories/test_catalogue_category.py @@ -138,7 +138,7 @@ def test_create_leaf_category_without_catalogue_item_properties( path=catalogue_category.path, parent_path=catalogue_category.parent_path, parent_id=catalogue_category.parent_id, - catalogue_item_properties=catalogue_category.catalogue_item_properties, + catalogue_item_properties=None, ) ) # pylint: enable=duplicate-code @@ -157,6 +157,79 @@ def test_create_leaf_category_without_catalogue_item_properties( assert created_catalogue_category == catalogue_category +def test_create_non_leaf_category_with_catalogue_item_properties( + test_helpers, database_mock, catalogue_category_repository +): + """ + Test creating a non-leaf catalogue category with catalogue item properties. + + Verify that the `create` method properly handles the catalogue category to be created, checks that there is not a + duplicate catalogue category, and creates the catalogue category. + """ + # pylint: disable=duplicate-code + catalogue_category = CatalogueCategoryOut( + id=str(ObjectId()), + name="Category A", + code="category-a", + is_leaf=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[], + ) + # pylint: enable=duplicate-code + + # Mock `count_documents` to return 0 (no duplicate catalogue category found within the parent catalogue category) + test_helpers.mock_count_documents(database_mock.catalogue_categories, 0) + # Mock `insert_one` to return an object for the inserted catalogue category document + test_helpers.mock_insert_one(database_mock.catalogue_categories, CustomObjectId(catalogue_category.id)) + # Mock `find_one` to return the inserted catalogue category document + test_helpers.mock_find_one( + database_mock.catalogue_categories, + { + "_id": CustomObjectId(catalogue_category.id), + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_category.catalogue_item_properties, + }, + ) + + catalogue_item_properties = [ + CatalogueItemProperty(name="Property A", type="number", unit="mm", mandatory=False), + CatalogueItemProperty(name="Property B", type="boolean", mandatory=True), + ] + # pylint: disable=duplicate-code + created_catalogue_category = catalogue_category_repository.create( + CatalogueCategoryIn( + name=catalogue_category.name, + code=catalogue_category.code, + is_leaf=catalogue_category.is_leaf, + path=catalogue_category.path, + parent_path=catalogue_category.parent_path, + parent_id=catalogue_category.parent_id, + catalogue_item_properties=catalogue_item_properties, + ) + ) + # pylint: enable=duplicate-code + + database_mock.catalogue_categories.insert_one.assert_called_once_with( + { + "name": catalogue_category.name, + "code": catalogue_category.code, + "is_leaf": catalogue_category.is_leaf, + "path": catalogue_category.path, + "parent_path": catalogue_category.parent_path, + "parent_id": catalogue_category.parent_id, + "catalogue_item_properties": catalogue_item_properties, + } + ) + assert created_catalogue_category == catalogue_category + + def test_create_with_parent_id(test_helpers, database_mock, catalogue_category_repository): """ Test creating a catalogue category with a parent ID. From f53490a0ee5d032371046a01c000f291788d07e2 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 28 Sep 2023 14:56:43 +0100 Subject: [PATCH 14/16] Make manufacturer URL mandatory #3 --- .../models/catalogue_item.py | 10 +++++----- .../schemas/catalogue_item.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index 5b60ec7d..63adda10 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -15,7 +15,7 @@ class Manufacturer(BaseModel): name: str address: str - web_url: Optional[str] = None + web_url: str class Property(BaseModel): @@ -36,17 +36,17 @@ class CatalogueItemIn(BaseModel): catalogue_category_id: CustomObjectIdField name: str description: str - properties: List[Property] + properties: List[Property] = [] manufacturer: Manufacturer @validator("properties", pre=True, always=True) @classmethod def validate_properties(cls, properties: List[Property] | None) -> List[Property] | List: """ - Validator for the `properties` field that runs always (even if the field is missing) and before any Pydantic - validation checks. + Validator for the `properties` field that runs after field assignment but before type validation. - If the value is `None`, it replaces it with an empty list. + If the value is `None`, it replaces it with an empty list allowing for catalogue items without properties to be + created. :param properties: The list of properties. :return: The list of properties or an empty list. diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 5120ee73..84781c54 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -13,7 +13,7 @@ class ManufacturerSchema(BaseModel): name: str = Field(description="The name of the manufacturer") address: str = Field(description="The address of the manufacturer") - web_url: Optional[str] = Field(description="The website URL of the manufacturer") + web_url: str = Field(description="The website URL of the manufacturer") class PropertyPostRequestSchema(BaseModel): From 6f6990d7c737f9256575291fff91db748986a10b Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:22:46 +0100 Subject: [PATCH 15/16] Modify manufacturer schema model to validate URLs #3 --- .../schemas/catalogue_item.py | 4 +-- test/e2e/test_catalogue_category.py | 10 +++--- test/e2e/test_catalogue_item.py | 4 +-- test/unit/services/test_catalogue_item.py | 36 ++++++++++++------- 4 files changed, 33 insertions(+), 21 deletions(-) diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 84781c54..0d4e4c10 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -3,7 +3,7 @@ """ from typing import List, Any, Optional -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, HttpUrl class ManufacturerSchema(BaseModel): @@ -13,7 +13,7 @@ class ManufacturerSchema(BaseModel): name: str = Field(description="The name of the manufacturer") address: str = Field(description="The address of the manufacturer") - web_url: str = Field(description="The website URL of the manufacturer") + web_url: HttpUrl = Field(description="The website URL of the manufacturer") class PropertyPostRequestSchema(BaseModel): diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index 06a6755b..af8393bc 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -316,7 +316,7 @@ def test_delete_catalogue_category_with_children_catalogue_items(test_client): "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -599,7 +599,7 @@ def test_partial_update_catalogue_category_change_name_has_children_catalogue_it "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -730,7 +730,7 @@ def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_has_chil "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -855,7 +855,7 @@ def test_partial_update_catalogue_category_change_parent_id_has_children_catalog "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -1074,7 +1074,7 @@ def test_partial_update_catalogue_category_change_catalogue_item_properties_has_ "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 05c04a8a..71a6a1c9 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -44,7 +44,7 @@ def get_catalogue_item_a_dict(catalogue_category_id: str) -> Dict: "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } @@ -66,7 +66,7 @@ def get_catalogue_item_b_dict(catalogue_category_id: str) -> Dict: "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "www.manufacturer-a.co.uk", + "web_url": "https://www.manufacturer-a.co.uk", }, } diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index ac9cccad..55c4d85d 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -47,7 +47,9 @@ def test_create( Property(name="Property C", value="20x15x10", unit="cm"), ], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) @@ -130,7 +132,7 @@ def test_create_with_nonexistent_catalogue_category_id( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -178,7 +180,7 @@ def test_create_in_non_leaf_catalogue_category( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -202,7 +204,9 @@ def test_create_without_properties( description="This is Catalogue Item A", properties=[], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) # pylint: enable=duplicate-code @@ -292,7 +296,7 @@ def test_create_with_missing_mandatory_properties( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -348,7 +352,7 @@ def test_create_with_with_invalid_value_type_for_string_property( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -405,7 +409,7 @@ def test_create_with_with_invalid_value_type_for_number_property( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -462,7 +466,7 @@ def test_create_with_with_invalid_value_type_for_boolean_property( manufacturer=ManufacturerSchema( name="Manufacturer A", address="1 Address, City, Country, Postcode", - web_url="www.manufacturer-a.co.uk", + web_url="https://www.manufacturer-a.co.uk", ), ) ) @@ -492,7 +496,9 @@ def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_servic Property(name="Property C", value="20x15x10", unit="cm"), ], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) # pylint: enable=duplicate-code @@ -541,7 +547,9 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi Property(name="Property C", value="20x15x10", unit="cm"), ], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) @@ -552,7 +560,9 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) # pylint: enable=duplicate-code @@ -585,7 +595,9 @@ def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_rep Property(name="Property C", value="20x15x10", unit="cm"), ], manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + web_url="https://www.manufacturer-a.co.uk", ), ) # pylint: enable=duplicate-code From 7d3dfb26e79862ae950f044714d714f6b8e0a16e Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 28 Sep 2023 16:42:14 +0100 Subject: [PATCH 16/16] Implement a custom handler to handle uncaught exceptions #3 --- inventory_management_system_api/main.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/main.py b/inventory_management_system_api/main.py index 50295e95..a3a7df48 100644 --- a/inventory_management_system_api/main.py +++ b/inventory_management_system_api/main.py @@ -3,10 +3,11 @@ """ import logging -from fastapi import FastAPI, Request +from fastapi import FastAPI, Request, status from fastapi.exception_handlers import request_validation_exception_handler from fastapi.exceptions import RequestValidationError from fastapi.middleware.cors import CORSMiddleware +from fastapi.responses import JSONResponse from inventory_management_system_api.core.config import config from inventory_management_system_api.core.logger_setup import setup_logger @@ -20,8 +21,22 @@ logger.info("Logging now setup") +@app.exception_handler(Exception) +async def custom_general_exception_handler(_: Request, exc: Exception) -> JSONResponse: + """ + Custom exception handler for FastAPI to handle uncaught exceptions. It logs the error and returns an appropriate + response. + + :param _: Unused + :param exc: The exception object that triggered this handler. + :return: A JSON response indicating that something went wrong. + """ + logger.exception(exc) + return JSONResponse(content={"detail": "Something went wrong"}, status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) + + @app.exception_handler(RequestValidationError) -async def validation_exception_handler(request: Request, exc: RequestValidationError): +async def custom_validation_exception_handler(request: Request, exc: RequestValidationError) -> JSONResponse: """ Custom exception handler for FastAPI to handle `RequestValidationError`. @@ -31,6 +46,7 @@ async def validation_exception_handler(request: Request, exc: RequestValidationE :param request: The incoming HTTP request that caused the validation error. :param exc: The exception object representing the validation error. + :return: A JSON response with validation error details. """ logger.exception(exc) return await request_validation_exception_handler(request, exc)