From 852e6dd357b4f57d40a2be421e5abcfe987e4e13 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:44:05 +0100 Subject: [PATCH 01/22] Allow for catalogue items with same name to be created #9 --- .../repositories/catalogue_item.py | 19 -------- test/e2e/test_catalogue_item.py | 18 -------- test/unit/repositories/test_catalogue_item.py | 45 +------------------ 3 files changed, 1 insertion(+), 81 deletions(-) diff --git a/inventory_management_system_api/repositories/catalogue_item.py b/inventory_management_system_api/repositories/catalogue_item.py index e5b07ba9..806cb380 100644 --- a/inventory_management_system_api/repositories/catalogue_item.py +++ b/inventory_management_system_api/repositories/catalogue_item.py @@ -10,7 +10,6 @@ from inventory_management_system_api.core.custom_object_id import CustomObjectId from inventory_management_system_api.core.database import get_database -from inventory_management_system_api.core.exceptions import DuplicateRecordError from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, CatalogueItemIn logger = logging.getLogger() @@ -34,15 +33,9 @@ def create(self, catalogue_item: CatalogueItemIn) -> CatalogueItemOut: """ Create a new catalogue item in a MongoDB database. - The method checks if a duplicate catalogue item is found within the catalogue category. - :param catalogue_item: The catalogue item to be created. :return: The created catalogue item. - :raises DuplicateRecordError: If a duplicate catalogue item is found within the catalogue category. """ - if self._is_duplicate_catalogue_item(str(catalogue_item.catalogue_category_id), catalogue_item.name): - raise DuplicateRecordError("Duplicate catalogue item found within the catalogue category") - logger.info("Inserting the new catalogue item into the database") result = self._collection.insert_one(catalogue_item.dict()) catalogue_item = self.get(str(result.inserted_id)) @@ -83,15 +76,3 @@ def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]: catalogue_items = self._collection.find(query) return [CatalogueItemOut(**catalogue_item) for catalogue_item in catalogue_items] - - def _is_duplicate_catalogue_item(self, catalogue_category_id: str, name: str) -> bool: - """ - Check if a catalogue item with the same name already exists within the catalogue category. - - :param catalogue_category_id: The ID of the catalogue category to check for duplicates in. - :return: `True` if a duplicate catalogue item is found, `False` otherwise. - """ - logger.info("Checking if catalogue item with name '%s' already exists within the category", name) - catalogue_category_id = CustomObjectId(catalogue_category_id) - count = self._collection.count_documents({"catalogue_category_id": catalogue_category_id, "name": name}) - return count > 0 diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 71a6a1c9..847f432a 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -96,24 +96,6 @@ def test_create_catalogue_item(test_client): assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] -def test_create_catalogue_item_with_duplicate_name_within_catalogue_category(test_client): - """ - Test creating a catalogue item with a duplicate name within the catalogue category. - """ - response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) - catalogue_category = response.json() - - catalogue_item_post = get_catalogue_item_a_dict(catalogue_category["id"]) - test_client.post("/v1/catalogue-items", json=catalogue_item_post) - - response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) - - assert response.status_code == 409 - assert ( - response.json()["detail"] == "A catalogue item with the same name already exists within the catalogue category" - ) - - def test_create_catalogue_item_with_invalid_catalogue_category_id(test_client): """ Test creating a catalogue item with an invalid catalogue category id. diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 91b74f86..5f445207 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -5,7 +5,7 @@ from bson import ObjectId 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.core.exceptions import InvalidObjectIdError from inventory_management_system_api.models.catalogue_item import ( CatalogueItemOut, Property, @@ -79,49 +79,6 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): assert created_catalogue_item == catalogue_item -def test_create_with_duplicate_name_within_catalogue_category(test_helpers, database_mock, catalogue_item_repository): - """ - Test creating a catalogue item with a duplicate name within the catalogue category. - - Verify that the `create` method properly handles a catalogue item with a duplicate name, finds that there is a - duplicate catalogue item, and does not create the catalogue item. - """ - # 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=[ - Property(name="Property A", value=20, unit="mm"), - 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 - - # Mock `count_documents` to return 1 (duplicate catalogue item found within the catalogue category) - test_helpers.mock_count_documents(database_mock.catalogue_items, 1) - - with pytest.raises(DuplicateRecordError) as exc: - catalogue_item_repository.create( - CatalogueItemIn( - catalogue_category_id=catalogue_item.catalogue_category_id, - 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" - database_mock.catalogue_items.count_documents.assert_called_once_with( - {"catalogue_category_id": CustomObjectId(catalogue_item.catalogue_category_id), "name": catalogue_item.name} - ) - - def test_get(test_helpers, database_mock, catalogue_item_repository): """ Test getting a catalogue item. From 719720d9836b2b09b4f8fea193729efdd292ab29 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:48:02 +0100 Subject: [PATCH 02/22] Define catalogue item patch endpoint schema model #9 --- .../schemas/catalogue_item.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 0d4e4c10..4e6d2b48 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -47,6 +47,19 @@ class CatalogueItemPostRequestSchema(BaseModel): manufacturer: ManufacturerSchema = Field(description="The details of the manufacturer") +class CatalogueItemPatchRequestSchema(CatalogueItemPostRequestSchema): + """ + Schema model for a catalogue item update request. + """ + + catalogue_category_id: Optional[str] = Field( + description="The ID of the catalogue category that the catalogue item belongs to" + ) + name: Optional[str] = Field(description="The name of the catalogue item") + description: Optional[str] = Field(description="The catalogue item description") + manufacturer: Optional[ManufacturerSchema] = Field(description="The details of the manufacturer") + + class CatalogueItemSchema(CatalogueItemPostRequestSchema): """ Schema model for a catalogue item response. From fba2acc106922d89e1ed136387450185cb8aca25 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:48:29 +0100 Subject: [PATCH 03/22] Implement a repo method for updating a catalogue item #9 --- .../repositories/catalogue_category.py | 2 +- .../repositories/catalogue_item.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/inventory_management_system_api/repositories/catalogue_category.py b/inventory_management_system_api/repositories/catalogue_category.py index 6ec5dfa2..4ee061d5 100644 --- a/inventory_management_system_api/repositories/catalogue_category.py +++ b/inventory_management_system_api/repositories/catalogue_category.py @@ -96,7 +96,7 @@ 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): + def update(self, catalogue_category_id: str, catalogue_category: CatalogueCategoryIn) -> CatalogueCategoryOut: """ Update a catalogue category by its ID in a MongoDB database. diff --git a/inventory_management_system_api/repositories/catalogue_item.py b/inventory_management_system_api/repositories/catalogue_item.py index 806cb380..c7720f4c 100644 --- a/inventory_management_system_api/repositories/catalogue_item.py +++ b/inventory_management_system_api/repositories/catalogue_item.py @@ -55,6 +55,22 @@ def get(self, catalogue_item_id: str) -> Optional[CatalogueItemOut]: return CatalogueItemOut(**catalogue_item) return None + def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemIn) -> CatalogueItemOut: + """ + Update a catalogue item by its ID in a MongoDB database. + + :param catalogue_item_id: The ID of the catalogue item to update. + :param catalogue_item: The catalogue item containing the update data. + :return: The updated catalogue item. + """ + catalogue_item_id = CustomObjectId(catalogue_item_id) + # TODO - (when the relevant item logic is implemented) check if catalogue item has children elements if the + # `catalogue_category_id` is being updated. + logger.info("Updating catalogue item with ID: %s in the database", catalogue_item_id) + self._collection.update_one({"_id": catalogue_item_id}, {"$set": catalogue_item.dict()}) + catalogue_item = self.get(str(catalogue_item_id)) + return catalogue_item + def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]: """ Retrieve all catalogue items from a MongoDB. From cb4f77fcb9b276f07707faa53cc76cb83978f8b4 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:49:01 +0100 Subject: [PATCH 04/22] Implement a service method for updating a catalogue item #9 --- .../services/catalogue_item.py | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index f7645726..810f71cc 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -18,7 +18,10 @@ 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.schemas.catalogue_category import CatalogueItemPropertyType -from inventory_management_system_api.schemas.catalogue_item import CatalogueItemPostRequestSchema +from inventory_management_system_api.schemas.catalogue_item import ( + CatalogueItemPostRequestSchema, + CatalogueItemPatchRequestSchema, +) logger = logging.getLogger() @@ -206,3 +209,71 @@ def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]: :return: A list of catalogue items, or an empty list if no catalogue items are retrieved. """ return self._catalogue_item_repository.list(catalogue_category_id) + + def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchRequestSchema) -> CatalogueItemOut: + """ + Update a catalogue item by its ID. + + :param catalogue_item_id: The ID of the catalogue item to update. + :param catalogue_item: The catalogue item containing the fields that need to be updated. + :return: The updated catalogue item. + """ + update_data = catalogue_item.dict(exclude_unset=True) + + stored_catalogue_item = self.get(catalogue_item_id) + if not stored_catalogue_item: + raise MissingRecordError(f"No catalogue item found with ID: {catalogue_item_id}") + + if "name" in update_data: + stored_catalogue_item.name = update_data["name"] + if "description" in update_data: + stored_catalogue_item.description = update_data["description"] + if "manufacturer" in update_data: + stored_catalogue_item.manufacturer = update_data["manufacturer"] + + catalogue_category = None + if ( + "catalogue_category_id" in update_data + and update_data["catalogue_category_id"] != stored_catalogue_item.catalogue_category_id + ): + stored_catalogue_item.catalogue_category_id = update_data["catalogue_category_id"] + catalogue_category = self._catalogue_category_repository.get(stored_catalogue_item.catalogue_category_id) + if not catalogue_category: + raise MissingRecordError( + f"No catalogue category found with ID: {stored_catalogue_item.catalogue_category_id}" + ) + + if catalogue_category.is_leaf is False: + raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") + + # TODO - Refactor this + if "properties" not in update_data: + update_data["properties"] = stored_catalogue_item.properties + + if "properties" in update_data: + if not catalogue_category: + catalogue_category = self._catalogue_category_repository.get( + stored_catalogue_item.catalogue_category_id + ) + + defined_properties = { + defined_property.name: defined_property.dict() + for defined_property in catalogue_category.catalogue_item_properties + } + supplied_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) + supplied_properties = self._filter_matching_catalogue_item_properties( + defined_properties, supplied_properties + ) + self._add_catalogue_item_property_units(defined_properties, supplied_properties) + self._validate_catalogue_item_property_values(defined_properties, supplied_properties) + + stored_catalogue_item.properties = list(supplied_properties.values()) + + return self._catalogue_item_repository.update( + catalogue_item_id, CatalogueItemIn(**stored_catalogue_item.dict()) + ) From a4bef3bd9ab63e68471fac6214b66ecd32e68677 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:50:33 +0100 Subject: [PATCH 05/22] Implement an endpoint for partially updating a catalogue item #9 --- .../routers/v1/catalogue_item.py | 56 +++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/inventory_management_system_api/routers/v1/catalogue_item.py b/inventory_management_system_api/routers/v1/catalogue_item.py index 5b1e93e1..1914537b 100644 --- a/inventory_management_system_api/routers/v1/catalogue_item.py +++ b/inventory_management_system_api/routers/v1/catalogue_item.py @@ -10,12 +10,15 @@ from inventory_management_system_api.core.exceptions import ( MissingRecordError, InvalidObjectIdError, - DuplicateRecordError, NonLeafCategoryError, InvalidCatalogueItemPropertyTypeError, MissingMandatoryCatalogueItemProperty, ) -from inventory_management_system_api.schemas.catalogue_item import CatalogueItemSchema, CatalogueItemPostRequestSchema +from inventory_management_system_api.schemas.catalogue_item import ( + CatalogueItemSchema, + CatalogueItemPostRequestSchema, + CatalogueItemPatchRequestSchema, +) from inventory_management_system_api.services.catalogue_item import CatalogueItemService logger = logging.getLogger() @@ -52,18 +55,15 @@ def get_catalogue_item( ) -> CatalogueItemSchema: # pylint: disable=missing-function-docstring logger.info("Getting catalogue item with ID: %s", catalogue_item_id) + message = "A catalogue item with such ID was not found" try: catalogue_item = catalogue_item_service.get(catalogue_item_id) if not catalogue_item: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="The requested catalogue item was not found" - ) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) return CatalogueItemSchema(**catalogue_item.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 item was not found" - ) from exc + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) from exc @router.post( @@ -85,13 +85,47 @@ def create_catalogue_item( logger.exception(str(exc)) raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc)) from exc except (MissingRecordError, InvalidObjectIdError) as exc: - message = "The specified catalogue category ID does not exist in the database" + message = "The specified 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: - message = "A catalogue item with the same name already exists within the catalogue category" + except NonLeafCategoryError as exc: + message = "Adding a catalogue item to a non-leaf catalogue category is not allowed" logger.exception(message) raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc + + +@router.patch( + path="/{catalogue_item_id}", + summary="Update a catalogue item partially by ID", + response_description="Catalogue item updated successfully", +) +def partial_update_catalogue_item( + catalogue_item: CatalogueItemPatchRequestSchema, + catalogue_item_id: str = Path(description="The ID of the catalogue item to update"), + catalogue_item_service: CatalogueItemService = Depends(), +) -> CatalogueItemSchema: + # pylint: disable=missing-function-docstring + logger.info("Partially updating catalogue item with ID: %s", catalogue_item_id) + logger.debug("Catalogue item data: %s", catalogue_item) + try: + updated_catalogue_item = catalogue_item_service.update(catalogue_item_id, catalogue_item) + return CatalogueItemSchema(**updated_catalogue_item.dict()) + except (InvalidCatalogueItemPropertyTypeError, MissingMandatoryCatalogueItemProperty) as exc: + logger.exception(str(exc)) + raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=str(exc)) from exc + except (MissingRecordError, InvalidObjectIdError) as exc: + if ( + catalogue_item.catalogue_category_id + and catalogue_item.catalogue_category_id in str(exc) + or "catalogue category" in str(exc).lower() + ): + message = "The specified 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 item with such ID was not found" + logger.exception(message) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=message) from exc except NonLeafCategoryError as exc: message = "Adding a catalogue item to a non-leaf catalogue category is not allowed" logger.exception(message) From 8494f278f989d33ca0edab228f7b8b87812957e4 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 5 Oct 2023 13:51:16 +0100 Subject: [PATCH 06/22] Write e2e tests for the catalogue item partial update endpoint #9 --- test/e2e/test_catalogue_item.py | 433 +++++++++++++++++++++++++++++++- 1 file changed, 429 insertions(+), 4 deletions(-) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 847f432a..a87d5747 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -104,7 +104,7 @@ def test_create_catalogue_item_with_invalid_catalogue_category_id(test_client): response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) assert response.status_code == 422 - assert response.json()["detail"] == "The specified catalogue category ID does not exist in the database" + assert response.json()["detail"] == "The specified catalogue category ID does not exist" def test_create_catalogue_item_with_nonexistent_catalogue_category_id(test_client): @@ -115,7 +115,7 @@ def test_create_catalogue_item_with_nonexistent_catalogue_category_id(test_clien response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) assert response.status_code == 422 - assert response.json()["detail"] == "The specified catalogue category ID does not exist in the database" + assert response.json()["detail"] == "The specified catalogue category ID does not exist" def test_create_catalogue_item_in_non_leaf_catalogue_category(test_client): @@ -280,7 +280,7 @@ def test_get_catalogue_item_with_invalid_id(test_client): response = test_client.get("/v1/catalogue-items/invalid") assert response.status_code == 404 - assert response.json()["detail"] == "The requested catalogue item was not found" + assert response.json()["detail"] == "A catalogue item with such ID was not found" def test_get_catalogue_item_with_nonexistent_id(test_client): @@ -290,7 +290,7 @@ def test_get_catalogue_item_with_nonexistent_id(test_client): response = test_client.get(f"/v1/catalogue-items/{str(ObjectId())}") assert response.status_code == 404 - assert response.json()["detail"] == "The requested catalogue item was not found" + assert response.json()["detail"] == "A catalogue item with such ID was not found" def test_get_catalogue_items(test_client): @@ -384,3 +384,428 @@ def test_get_catalogue_items_with_invalid_catalogue_category_id_filter(test_clie catalogue_items = response.json() assert len(catalogue_items) == 0 + + +def test_partial_update_catalogue_item(test_client): + """ + Test changing the name and description of a catalogue item. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = {"name": "Catalogue Item B", "description": "This is Catalogue Item B"} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 200 + + catalogue_item = response.json() + + catalogue_item_post["properties"][0]["unit"] = "mm" + catalogue_item_post["properties"][1]["unit"] = None + catalogue_item_post["properties"][2]["unit"] = "cm" + assert catalogue_item["catalogue_category_id"] == catalogue_category_id + assert catalogue_item["name"] == catalogue_item_patch["name"] + assert catalogue_item["description"] == catalogue_item_patch["description"] + assert catalogue_item["properties"] == catalogue_item_post["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + + +def test_partial_update_catalogue_item_invalid_id(test_client): + """ + Test updating a catalogue item with an invalid ID. + """ + catalogue_item_patch = {"name": "Catalogue Item B", "description": "This is Catalogue Item B"} + + response = test_client.patch("/v1/catalogue-items/invalid", json=catalogue_item_patch) + + assert response.status_code == 404 + assert response.json()["detail"] == "A catalogue item with such ID was not found" + + +def test_partial_update_catalogue_item_nonexistent_id(test_client): + """ + Test updating a catalogue item with a nonexistent ID. + """ + catalogue_item_patch = {"name": "Catalogue Item B", "description": "This is Catalogue Item B"} + + response = test_client.patch(f"/v1/catalogue-items/{str(ObjectId())}", json=catalogue_item_patch) + + assert response.status_code == 404 + assert response.json()["detail"] == "A catalogue item with such ID was not found" + + +def test_partial_update_catalogue_item_has_children_items(): + """ + Test updating a catalogue item which has children items. + """ + # TODO - Implement this test when the relevant item logic is implemented. Extra test cases may be needed. + + +def test_partial_update_catalogue_item_change_catalogue_category_id(test_client): + """ + Test moving a catalogue item to another catalogue category. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category_a = response.json() + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category_b = response.json() + + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_a["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "catalogue_category_id": catalogue_category_b["id"], + "properties": [{"name": "Property A", "value": True}], + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 200 + + catalogue_item = response.json() + + catalogue_item_patch["properties"][0]["unit"] = None + assert catalogue_item["catalogue_category_id"] == catalogue_item_patch["catalogue_category_id"] + assert catalogue_item["name"] == catalogue_item_post["name"] + assert catalogue_item["description"] == catalogue_item_post["description"] + assert catalogue_item["properties"] == catalogue_item_patch["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + + +def test_partial_update_catalogue_item_change_catalogue_category_id_without_properties(test_client): + """ + Test moving a catalogue item to another catalogue category without supplying any catalogue item properties. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category_a = response.json() + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category_b = response.json() + + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_a["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = {"catalogue_category_id": catalogue_category_b["id"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "Missing mandatory catalogue item property: 'Property A'" + + +def test_partial_update_catalogue_item_change_catalogue_category_id_missing_mandatory_properties(test_client): + """ + Test moving a catalogue item to another catalogue category with missing mandatory catalogue item properties. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category_a = response.json() + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category_b = response.json() + + catalogue_item_post = get_catalogue_item_b_dict(catalogue_category_b["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "catalogue_category_id": catalogue_category_a["id"], + "properties": [{"name": "Property A", "value": 20}], + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "Missing mandatory catalogue item property: 'Property B'" + + +def test_partial_update_catalogue_item_change_catalogue_category_id_missing_non_mandatory_properties(test_client): + """ + Test moving a catalogue item to another catalogue category with missing non-mandatory catalogue item properties. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category_a = response.json() + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category_b = response.json() + + catalogue_item_post = get_catalogue_item_b_dict(catalogue_category_b["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "catalogue_category_id": catalogue_category_a["id"], + "properties": [ + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10"}, + ], + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + catalogue_item = response.json() + + catalogue_item_patch["properties"][0]["unit"] = None + catalogue_item_patch["properties"][1]["unit"] = "cm" + assert catalogue_item["catalogue_category_id"] == catalogue_item_patch["catalogue_category_id"] + assert catalogue_item["name"] == catalogue_item_post["name"] + assert catalogue_item["description"] == catalogue_item_post["description"] + assert catalogue_item["properties"] == catalogue_item_patch["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + + +def test_partial_update_catalogue_item_change_catalogue_category_id_invalid_id(test_client): + """ + Test changing the catalogue category ID of a catalogue item to an invalid ID. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + + catalogue_item_post = get_catalogue_item_a_dict(response.json()["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "catalogue_category_id": "invalid", + "properties": [{"name": "Property A", "value": 20}], + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "The specified catalogue category ID does not exist" + + +def test_partial_update_catalogue_item_change_catalogue_category_id_nonexistent_id(test_client): + """ + Test changing the catalogue category ID of a catalogue item to a nonexistent ID. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + + catalogue_item_post = get_catalogue_item_a_dict(response.json()["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "catalogue_category_id": str(ObjectId()), + "properties": [{"name": "Property A", "value": 20}], + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "The specified catalogue category ID does not exist" + + +def test_partial_update_catalogue_item_change_catalogue_category_id_non_leaf_catalogue_category(test_client): + """ + Test moving a catalogue item to a non-leaf catalogue category. + """ + catalogue_category_post_a = {"name": "Category A", "is_leaf": False} + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post_a) + catalogue_category_a = response.json() + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category_b = response.json() + + catalogue_item_post = get_catalogue_item_b_dict(catalogue_category_b["id"]) + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = {"catalogue_category_id": catalogue_category_a["id"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 409 + assert response.json()["detail"] == "Adding a catalogue item to a non-leaf catalogue category is not allowed" + + +def test_partial_update_catalogue_item_change_catalogue_category_id_has_children_items(): + """ + Test moving a catalogue item with children items to another catalogue category. + """ + # TODO - Implement this test when the relevant item logic is implemented. + + +def test_partial_update_catalogue_item_add_non_mandatory_property(test_client): + """ + Test adding a non-mandatory catalogue item property and a value. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + catalogue_item_properties = catalogue_item_post["properties"].copy() + + # Delete the non-mandatory property so that the catalogue item is created without it + del catalogue_item_post["properties"][0] + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = {"properties": catalogue_item_properties} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 200 + + catalogue_item = response.json() + + catalogue_item_patch["properties"][0]["unit"] = "mm" + catalogue_item_patch["properties"][1]["unit"] = None + catalogue_item_patch["properties"][2]["unit"] = "cm" + 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"] == catalogue_item_patch["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + + +def test_partial_update_catalogue_item_remove_non_mandatory_property(test_client): + """ + Test removing a non-mandatory catalogue item property and its value.. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + # Delete the non-mandatory property + del catalogue_item_post["properties"][0] + catalogue_item_patch = {"properties": catalogue_item_post["properties"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 200 + + catalogue_item = response.json() + + catalogue_item_patch["properties"][0]["unit"] = None + catalogue_item_patch["properties"][1]["unit"] = "cm" + 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"] == catalogue_item_patch["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + + +def test_partial_update_catalogue_item_remove_mandatory_property(test_client): + """ + Test removing a mandatory catalogue item property and its value. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + del catalogue_item_post["properties"][1] + catalogue_item_patch = {"properties": catalogue_item_post["properties"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert response.json()["detail"] == "Missing mandatory catalogue item property: 'Property B'" + + +def test_partial_update_catalogue_item_change_value_for_string_property_invalid_type(test_client): + """ + Test changing the value of a string catalogue item property to an invalid type. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_post["properties"][2]["value"] = True + catalogue_item_patch = {"properties": catalogue_item_post["properties"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert ( + response.json()["detail"] + == "Invalid value type for catalogue item property 'Property C'. Expected type: string." + ) + + +def test_partial_update_catalogue_item_change_value_for_number_property_invalid_type(test_client): + """ + Test changing the value of a number catalogue item property to an invalid type. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_post["properties"][0]["value"] = "20" + catalogue_item_patch = {"properties": catalogue_item_post["properties"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert ( + response.json()["detail"] + == "Invalid value type for catalogue item property 'Property A'. Expected type: number." + ) + + +def test_partial_update_catalogue_item_change_value_for_boolean_property_invalid_type(test_client): + """ + Test changing the value of a boolean catalogue item property to an invalid type. + """ + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_A) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_a_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_post["properties"][1]["value"] = "False" + catalogue_item_patch = {"properties": catalogue_item_post["properties"]} + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 422 + assert ( + response.json()["detail"] + == "Invalid value type for catalogue item property 'Property B'. Expected type: boolean." + ) + + +def test_partial_update_catalogue_item_change_manufacturer(test_client): + """ + Test updating the manufacturer details of a catalogue item. + """ + # TODO - Modify this test to use manufacturer ID when the relevant manufacturer logic is implemented + response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) + catalogue_category = response.json() + + catalogue_category_id = catalogue_category["id"] + catalogue_item_post = get_catalogue_item_b_dict(catalogue_category_id) + + response = test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + catalogue_item_patch = { + "manufacturer": { + "name": "Manufacturer B", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-b.co.uk", + } + } + response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) + + assert response.status_code == 200 + catalogue_item = response.json() + + catalogue_item_post["properties"][0]["unit"] = None + 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"] == catalogue_item_post["properties"] + assert catalogue_item["manufacturer"] == catalogue_item_patch["manufacturer"] + + +def test_partial_update_catalogue_item_change_manufacturer_id_invalid_id(): + """ + Test changing the manufacturer ID of a catalogue item to an invalid ID. + """ + # TODO - Implement this test when the relevant manufacturer logic is implemented + + +def test_partial_update_catalogue_item_change_manufacturer_id_nonexistent_id(): + """ + Test changing the manufacturer ID of a catalogue item to a nonexistent ID. + """ + # TODO - Implement this test when the relevant manufacturer logic is implemented From 2f8ffda44f6d5b5eef486ffc0b0935565b0d71f4 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 11 Oct 2023 08:14:42 +0000 Subject: [PATCH 07/22] Create a production Dockerfile #76 --- Dockerfile.prod | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 Dockerfile.prod diff --git a/Dockerfile.prod b/Dockerfile.prod new file mode 100644 index 00000000..e0a32c81 --- /dev/null +++ b/Dockerfile.prod @@ -0,0 +1,38 @@ +FROM python:3.10-alpine3.18 + +WORKDIR /inventory-management-system-api-run + +COPY README.md pyproject.toml ./ +# Copy inventory_management_system_api source files +COPY inventory_management_system_api/ inventory_management_system_api/ + +RUN set -eux; \ + \ + # Install pip dependencies \ + python -m pip install --no-cache-dir .; \ + \ + # Create loging.ini from its .example file \ + cp inventory_management_system_api/logging.example.ini inventory_management_system_api/logging.ini; \ + \ + # Create a non-root user to run as \ + addgroup -S inventory-management-system-api; \ + adduser -S -D -G inventory-management-system-api -H -h /inventory-management-system-api-run inventory-management-system-api; \ + \ + # Create a log file \ + touch inventory-management-system-api.log; \ + # Change ownership of log file - app will need to write to it + chown -R inventory-management-system-api:inventory-management-system-api inventory-management-system-api.log; + +USER inventory-management-system-api + +ENV API__TITLE="Inventory Management System API" +ENV API__DESCRIPTION="This is the API for the Inventory Management System" +ENV DATABASE__PROTOCOL=mongodb +ENV DATABASE__USERNAME=root +ENV DATABASE__PASSWORD=example +ENV DATABASE__HOSTNAME=localhost +ENV DATABASE__PORT=27017 +ENV DATABASE__NAME=ims + +CMD ["uvicorn", "inventory_management_system_api.main:app", "--host", "0.0.0.0", "--port", "8000"] +EXPOSE 8000 From 97730c49dca8f7ebbf84c1774c03f9e8c2666428 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 11 Oct 2023 08:26:28 +0000 Subject: [PATCH 08/22] Create a CI job to build image #76 --- .github/workflows/.ci.yml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/.github/workflows/.ci.yml b/.github/workflows/.ci.yml index 7a799eb5..7ac3fd40 100644 --- a/.github/workflows/.ci.yml +++ b/.github/workflows/.ci.yml @@ -91,3 +91,35 @@ jobs: - name: Run e2e tests run: DATABASE__NAME="test-ims" pytest test/e2e/ --cov + + docker: + # This job triggers only if all the other jobs succeed and does different things depending on the context. + # The job builds the Docker image in all cases and also pushes the image to Harbor only if something is + # pushed to the develop branch. + needs: [linting, unit-tests, e2e-tests] + name: Docker + runs-on: ubuntu-latest + steps: + - name: Check out repo + uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0 + + - name: Login to Harbor + uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0 + with: + registry: harbor.stfc.ac.uk/inventory-management-system + username: ${{ secrets.HARBOR_USERNAME }} + password: ${{ secrets.HARBOR_TOKEN }} + + - name: Extract metadata (tags, labels) for Docker + id: meta + uses: docker/metadata-action@96383f45573cb7f253c731d3b3ab81c87ef81934 # v5.0.0 + with: + images: harbor.stfc.ac.uk/inventory-management-system/ims-api + + - name: Build Docker image + uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # v5.0.0 + with: + context: . + push: false + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} From cecdb3a43a4bd40da05c897280734172f6fe6eac Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 11 Oct 2023 08:28:48 +0000 Subject: [PATCH 09/22] Configure CI job to push image to Harbor #76 --- .github/workflows/.ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/.ci.yml b/.github/workflows/.ci.yml index 7ac3fd40..8594a12a 100644 --- a/.github/workflows/.ci.yml +++ b/.github/workflows/.ci.yml @@ -116,10 +116,10 @@ jobs: with: images: harbor.stfc.ac.uk/inventory-management-system/ims-api - - name: Build Docker image + - name: ${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' && 'Build and push Docker image to Harbor' || 'Build Docker image' }} uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # v5.0.0 with: context: . - push: false + push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' }} tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} + labels: ${{ steps.meta.outputs.labels }} \ No newline at end of file From 1e9c0a6c3e92eb850d2053609201793e31aaa32b Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 11 Oct 2023 08:55:43 +0000 Subject: [PATCH 10/22] Add newline at end of file #76 --- .github/workflows/.ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/.ci.yml b/.github/workflows/.ci.yml index 8594a12a..2d719129 100644 --- a/.github/workflows/.ci.yml +++ b/.github/workflows/.ci.yml @@ -122,4 +122,4 @@ jobs: context: . push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' }} tags: ${{ steps.meta.outputs.tags }} - labels: ${{ steps.meta.outputs.labels }} \ No newline at end of file + labels: ${{ steps.meta.outputs.labels }} From 90d1e3f0f5acc219b37de4060d9cbce61440e6b3 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:53:09 +0100 Subject: [PATCH 11/22] Implement a high-level method for processing catalogue item properties #9 --- .../services/catalogue_item.py | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index 810f71cc..dc0420f8 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -4,7 +4,7 @@ """ import logging from numbers import Number -from typing import Optional, List, Dict +from typing import Optional, List, Dict, Union from fastapi import Depends @@ -14,6 +14,7 @@ InvalidCatalogueItemPropertyTypeError, MissingMandatoryCatalogueItemProperty, ) +from inventory_management_system_api.models.catalogue_category import CatalogueItemProperty from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, CatalogueItemIn from inventory_management_system_api.repositories.catalogue_category import CatalogueCategoryRepo from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo @@ -21,6 +22,7 @@ from inventory_management_system_api.schemas.catalogue_item import ( CatalogueItemPostRequestSchema, CatalogueItemPatchRequestSchema, + PropertyPostRequestSchema, ) logger = logging.getLogger() @@ -91,6 +93,53 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte ) ) + def _process_catalogue_item_properties( + self, defined_properties: List[CatalogueItemProperty], supplied_properties: List[PropertyPostRequestSchema] + ) -> List[Dict]: + """ + Process and validate supplied catalogue item properties based on defined catalogue item properties. It checks + for missing mandatory catalogue item properties, files the matching catalogue item properties, adds the property + units, and finally validates the property values. + + The `supplied_properties_dict` dictionary may get modified as part of the processing and validation. + + :param defined_properties: The list of defined catalogue item property objects. + :param supplied_properties: The list of supplied catalogue item property objects. + :return: A list of processed and validated supplied catalogue item properties. + """ + # Convert properties to dictionaries for easier lookups + defined_properties_dict = self._create_catalogue_item_properties_dict(defined_properties) + supplied_properties_dict = self._create_catalogue_item_properties_dict(supplied_properties) + + # Some mandatory catalogue item properties may not have been supplied + self._check_missing_mandatory_catalogue_item_properties(defined_properties_dict, supplied_properties_dict) + # Catalogue item properties that have not been defined may have been supplied + supplied_properties_dict = self._filter_matching_catalogue_item_properties( + defined_properties_dict, supplied_properties_dict + ) + # Supplied catalogue item properties do not have units as we can't trust they would be correct + self._add_catalogue_item_property_units(defined_properties_dict, supplied_properties_dict) + # The values of the supplied catalogue item properties may not be of the expected types + self._validate_catalogue_item_property_values(defined_properties_dict, supplied_properties_dict) + + return list(supplied_properties_dict.values()) + + def _create_catalogue_item_properties_dict( + self, catalogue_item_properties: Union[List[CatalogueItemProperty], List[PropertyPostRequestSchema]] + ) -> Dict[str, Dict]: + """ + Convert a list of catalogue item property objects into a dictionary where the keys are the catalogue item + property names and the values are the catalogue item property dictionaries. + + :param catalogue_item_properties: The list of catalogue item property objects. + :return: A dictionary where the keys are the catalogue item property names and the values are the catalogue item + property dictionaries. + """ + return { + catalogue_item_property.name: catalogue_item_property.dict() + for catalogue_item_property in catalogue_item_properties + } + def _add_catalogue_item_property_units( self, defined_properties: Dict[str, Dict], @@ -100,7 +149,7 @@ def _add_catalogue_item_property_units( Add the units to the supplied properties. The supplied properties only contain a name and value so the units from the defined properties in the database - are added to the supplied properties. + are added to the supplied properties. This means that this method modifies the `supplied_properties` dictionary. :param defined_properties: The defined catalogue item properties stored as part of the catalogue category in the database. From 04d43722edde868df8e7dff13f2b87a9c6cc820d Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:54:37 +0100 Subject: [PATCH 12/22] Refactore create service method to use high-level process method #9 --- .../services/catalogue_item.py | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index dc0420f8..0a7264b1 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -52,9 +52,8 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte Create a new catalogue item. The method checks if the catalogue category exists in the database and raises a `MissingRecordError` if it does - not. It also checks if the category is not a leaf category and raises a `NonLeafCategoryError` if it is. It - then proceeds to check for missing mandatory catalogue item properties, adds the property units, and finally - validates the property values. + not. It also checks if the category is not a leaf category and raises a `NonLeafCategoryError` if it is. It then + processes the catalogue item properties. :param catalogue_item: The catalogue item to be created. :return: The created catalogue item. @@ -69,26 +68,16 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte if catalogue_category.is_leaf is False: raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") - defined_properties = { - defined_property.name: defined_property.dict() - for defined_property in catalogue_category.catalogue_item_properties - } - supplied_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) - supplied_properties = self._filter_matching_catalogue_item_properties(defined_properties, supplied_properties) - self._add_catalogue_item_property_units(defined_properties, supplied_properties) - self._validate_catalogue_item_property_values(defined_properties, supplied_properties) + defined_properties = catalogue_category.catalogue_item_properties + supplied_properties = catalogue_item.properties if catalogue_item.properties else [] + supplied_properties = self._process_catalogue_item_properties(defined_properties, supplied_properties) return self._catalogue_item_repository.create( CatalogueItemIn( catalogue_category_id=catalogue_item.catalogue_category_id, name=catalogue_item.name, description=catalogue_item.description, - properties=list(supplied_properties.values()), + properties=supplied_properties, manufacturer=catalogue_item.manufacturer, ) ) From 832bfaee9fce29ccc99711ca9fafb820aad84ff9 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:55:19 +0100 Subject: [PATCH 13/22] Refactore update service method to use high-level process method #9 --- .../services/catalogue_item.py | 46 ++++++------------- 1 file changed, 15 insertions(+), 31 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index 0a7264b1..72693d5d 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -252,6 +252,11 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchReque """ Update a catalogue item by its ID. + The method checks if the catalogue item exists in the database and raises a `MissingRecordError` if it does + not. If the catalogue category ID is being updated, it checks if catalogue category with such ID exists and + raises a MissingRecordError` if it does not. It also checks if the category is not a leaf category and raises a + `NonLeafCategoryError` if it is. If the catalogue item properties are being updated, it also processes them. + :param catalogue_item_id: The ID of the catalogue item to update. :param catalogue_item: The catalogue item containing the fields that need to be updated. :return: The updated catalogue item. @@ -262,29 +267,21 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchReque if not stored_catalogue_item: raise MissingRecordError(f"No catalogue item found with ID: {catalogue_item_id}") - if "name" in update_data: - stored_catalogue_item.name = update_data["name"] - if "description" in update_data: - stored_catalogue_item.description = update_data["description"] - if "manufacturer" in update_data: - stored_catalogue_item.manufacturer = update_data["manufacturer"] - catalogue_category = None if ( "catalogue_category_id" in update_data - and update_data["catalogue_category_id"] != stored_catalogue_item.catalogue_category_id + and catalogue_item.catalogue_category_id != stored_catalogue_item.catalogue_category_id ): - stored_catalogue_item.catalogue_category_id = update_data["catalogue_category_id"] - catalogue_category = self._catalogue_category_repository.get(stored_catalogue_item.catalogue_category_id) + catalogue_category = self._catalogue_category_repository.get(catalogue_item.catalogue_category_id) if not catalogue_category: - raise MissingRecordError( - f"No catalogue category found with ID: {stored_catalogue_item.catalogue_category_id}" - ) + raise MissingRecordError(f"No catalogue category found with ID: {catalogue_item.catalogue_category_id}") if catalogue_category.is_leaf is False: raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") - # TODO - Refactor this + # If the catalogue category ID is updated but no catalogue item properties are supplied then the stored + # catalogue item properties should be used. They need to be processed and validated against the defined + # properties of the new catalogue category. if "properties" not in update_data: update_data["properties"] = stored_catalogue_item.properties @@ -294,24 +291,11 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchReque stored_catalogue_item.catalogue_category_id ) - defined_properties = { - defined_property.name: defined_property.dict() - for defined_property in catalogue_category.catalogue_item_properties - } - supplied_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) - supplied_properties = self._filter_matching_catalogue_item_properties( - defined_properties, supplied_properties - ) - self._add_catalogue_item_property_units(defined_properties, supplied_properties) - self._validate_catalogue_item_property_values(defined_properties, supplied_properties) - - stored_catalogue_item.properties = list(supplied_properties.values()) + defined_properties = catalogue_category.catalogue_item_properties + supplied_properties = catalogue_item.properties if catalogue_item.properties else [] + update_data["properties"] = self._process_catalogue_item_properties(defined_properties, supplied_properties) + stored_catalogue_item = stored_catalogue_item.copy(update=update_data) return self._catalogue_item_repository.update( catalogue_item_id, CatalogueItemIn(**stored_catalogue_item.dict()) ) From 4d1bb9c59dcf72465ab5fed91cf32b55183f36e8 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:55:53 +0100 Subject: [PATCH 14/22] Move private service methods to bottom of file #9 --- .../services/catalogue_item.py | 140 +++++++++--------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index 72693d5d..914bd2c0 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -82,6 +82,76 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte ) ) + def get(self, catalogue_item_id: str) -> Optional[CatalogueItemOut]: + """ + Retrieve a catalogue item by its ID. + + :param catalogue_item_id: The ID of the catalogue item to retrieve. + :return: The retrieved catalogue item, or `None` if not found. + """ + return self._catalogue_item_repository.get(catalogue_item_id) + + def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]: + """ + Retrieve all catalogue items. + + :param catalogue_category_id: The ID of the catalogue category to filter catalogue items by. + :return: A list of catalogue items, or an empty list if no catalogue items are retrieved. + """ + return self._catalogue_item_repository.list(catalogue_category_id) + + def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchRequestSchema) -> CatalogueItemOut: + """ + Update a catalogue item by its ID. + + The method checks if the catalogue item exists in the database and raises a `MissingRecordError` if it does + not. If the catalogue category ID is being updated, it checks if catalogue category with such ID exists and + raises a MissingRecordError` if it does not. It also checks if the category is not a leaf category and raises a + `NonLeafCategoryError` if it is. If the catalogue item properties are being updated, it also processes them. + + :param catalogue_item_id: The ID of the catalogue item to update. + :param catalogue_item: The catalogue item containing the fields that need to be updated. + :return: The updated catalogue item. + """ + update_data = catalogue_item.dict(exclude_unset=True) + + stored_catalogue_item = self.get(catalogue_item_id) + if not stored_catalogue_item: + raise MissingRecordError(f"No catalogue item found with ID: {catalogue_item_id}") + + catalogue_category = None + if ( + "catalogue_category_id" in update_data + and catalogue_item.catalogue_category_id != stored_catalogue_item.catalogue_category_id + ): + catalogue_category = self._catalogue_category_repository.get(catalogue_item.catalogue_category_id) + if not catalogue_category: + raise MissingRecordError(f"No catalogue category found with ID: {catalogue_item.catalogue_category_id}") + + if catalogue_category.is_leaf is False: + raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") + + # If the catalogue category ID is updated but no catalogue item properties are supplied then the stored + # catalogue item properties should be used. They need to be processed and validated against the defined + # properties of the new catalogue category. + if "properties" not in update_data: + update_data["properties"] = stored_catalogue_item.properties + + if "properties" in update_data: + if not catalogue_category: + catalogue_category = self._catalogue_category_repository.get( + stored_catalogue_item.catalogue_category_id + ) + + defined_properties = catalogue_category.catalogue_item_properties + supplied_properties = catalogue_item.properties if catalogue_item.properties else [] + update_data["properties"] = self._process_catalogue_item_properties(defined_properties, supplied_properties) + + stored_catalogue_item = stored_catalogue_item.copy(update=update_data) + return self._catalogue_item_repository.update( + catalogue_item_id, CatalogueItemIn(**stored_catalogue_item.dict()) + ) + def _process_catalogue_item_properties( self, defined_properties: List[CatalogueItemProperty], supplied_properties: List[PropertyPostRequestSchema] ) -> List[Dict]: @@ -229,73 +299,3 @@ def _filter_matching_catalogue_item_properties( matching_properties[supplied_property_name] = supplied_property return matching_properties - - def get(self, catalogue_item_id: str) -> Optional[CatalogueItemOut]: - """ - Retrieve a catalogue item by its ID. - - :param catalogue_item_id: The ID of the catalogue item to retrieve. - :return: The retrieved catalogue item, or `None` if not found. - """ - return self._catalogue_item_repository.get(catalogue_item_id) - - def list(self, catalogue_category_id: Optional[str]) -> List[CatalogueItemOut]: - """ - Retrieve all catalogue items. - - :param catalogue_category_id: The ID of the catalogue category to filter catalogue items by. - :return: A list of catalogue items, or an empty list if no catalogue items are retrieved. - """ - return self._catalogue_item_repository.list(catalogue_category_id) - - def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchRequestSchema) -> CatalogueItemOut: - """ - Update a catalogue item by its ID. - - The method checks if the catalogue item exists in the database and raises a `MissingRecordError` if it does - not. If the catalogue category ID is being updated, it checks if catalogue category with such ID exists and - raises a MissingRecordError` if it does not. It also checks if the category is not a leaf category and raises a - `NonLeafCategoryError` if it is. If the catalogue item properties are being updated, it also processes them. - - :param catalogue_item_id: The ID of the catalogue item to update. - :param catalogue_item: The catalogue item containing the fields that need to be updated. - :return: The updated catalogue item. - """ - update_data = catalogue_item.dict(exclude_unset=True) - - stored_catalogue_item = self.get(catalogue_item_id) - if not stored_catalogue_item: - raise MissingRecordError(f"No catalogue item found with ID: {catalogue_item_id}") - - catalogue_category = None - if ( - "catalogue_category_id" in update_data - and catalogue_item.catalogue_category_id != stored_catalogue_item.catalogue_category_id - ): - catalogue_category = self._catalogue_category_repository.get(catalogue_item.catalogue_category_id) - if not catalogue_category: - raise MissingRecordError(f"No catalogue category found with ID: {catalogue_item.catalogue_category_id}") - - if catalogue_category.is_leaf is False: - raise NonLeafCategoryError("Cannot add catalogue item to a non-leaf catalogue category") - - # If the catalogue category ID is updated but no catalogue item properties are supplied then the stored - # catalogue item properties should be used. They need to be processed and validated against the defined - # properties of the new catalogue category. - if "properties" not in update_data: - update_data["properties"] = stored_catalogue_item.properties - - if "properties" in update_data: - if not catalogue_category: - catalogue_category = self._catalogue_category_repository.get( - stored_catalogue_item.catalogue_category_id - ) - - defined_properties = catalogue_category.catalogue_item_properties - supplied_properties = catalogue_item.properties if catalogue_item.properties else [] - update_data["properties"] = self._process_catalogue_item_properties(defined_properties, supplied_properties) - - stored_catalogue_item = stored_catalogue_item.copy(update=update_data) - return self._catalogue_item_repository.update( - catalogue_item_id, CatalogueItemIn(**stored_catalogue_item.dict()) - ) From f90f0be0502949760cb4df0a693a6bf97c210aee Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 14:57:54 +0100 Subject: [PATCH 15/22] Refactor catalogue category update service method #9 --- .../services/catalogue_category.py | 30 ++++++------------- 1 file changed, 9 insertions(+), 21 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_category.py b/inventory_management_system_api/services/catalogue_category.py index 97e28a3a..f2710a01 100644 --- a/inventory_management_system_api/services/catalogue_category.py +++ b/inventory_management_system_api/services/catalogue_category.py @@ -113,32 +113,20 @@ def update( 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 "name" in update_data and catalogue_category.name != stored_catalogue_category.name: + update_data["code"] = self._generate_code(catalogue_category.name) + update_data["path"] = self._generate_path(stored_catalogue_category.parent_path, update_data["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_id" in update_data and catalogue_category.parent_id != stored_catalogue_category.parent_id: + parent_catalogue_category = self.get(catalogue_category.parent_id) if catalogue_category.parent_id else None + update_data["parent_path"] = parent_catalogue_category.path if parent_catalogue_category else "/" + code = update_data["code"] if "code" in update_data else stored_catalogue_category.code + update_data["path"] = self._generate_path(update_data["parent_path"], 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 "catalogue_item_properties" in update_data: - stored_catalogue_category.catalogue_item_properties = update_data["catalogue_item_properties"] - + stored_catalogue_category = stored_catalogue_category.copy(update=update_data) return self._catalogue_category_repository.update( catalogue_category_id, CatalogueCategoryIn(**stored_catalogue_category.dict()) ) From 8dad3a7b4b42c15601597793d51a03846f53b35f Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 16:07:37 +0100 Subject: [PATCH 16/22] Ignore pylint TODO warnings #9 --- .../repositories/catalogue_item.py | 1 + test/e2e/test_catalogue_item.py | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/inventory_management_system_api/repositories/catalogue_item.py b/inventory_management_system_api/repositories/catalogue_item.py index c7720f4c..84669643 100644 --- a/inventory_management_system_api/repositories/catalogue_item.py +++ b/inventory_management_system_api/repositories/catalogue_item.py @@ -64,6 +64,7 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemIn) -> Cat :return: The updated catalogue item. """ catalogue_item_id = CustomObjectId(catalogue_item_id) + # pylint: disable=fixme # TODO - (when the relevant item logic is implemented) check if catalogue item has children elements if the # `catalogue_category_id` is being updated. logger.info("Updating catalogue item with ID: %s in the database", catalogue_item_id) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index a87d5747..ae13a746 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -442,6 +442,7 @@ def test_partial_update_catalogue_item_has_children_items(): """ Test updating a catalogue item which has children items. """ + # pylint: disable=fixme # TODO - Implement this test when the relevant item logic is implemented. Extra test cases may be needed. @@ -610,6 +611,7 @@ def test_partial_update_catalogue_item_change_catalogue_category_id_has_children """ Test moving a catalogue item with children items to another catalogue category. """ + # pylint: disable=fixme # TODO - Implement this test when the relevant item logic is implemented. @@ -768,6 +770,7 @@ def test_partial_update_catalogue_item_change_manufacturer(test_client): """ Test updating the manufacturer details of a catalogue item. """ + # pylint: disable=fixme # TODO - Modify this test to use manufacturer ID when the relevant manufacturer logic is implemented response = test_client.post("/v1/catalogue-categories", json=CATALOGUE_CATEGORY_POST_B) catalogue_category = response.json() @@ -801,6 +804,7 @@ def test_partial_update_catalogue_item_change_manufacturer_id_invalid_id(): """ Test changing the manufacturer ID of a catalogue item to an invalid ID. """ + # pylint: disable=fixme # TODO - Implement this test when the relevant manufacturer logic is implemented @@ -808,4 +812,5 @@ def test_partial_update_catalogue_item_change_manufacturer_id_nonexistent_id(): """ Test changing the manufacturer ID of a catalogue item to a nonexistent ID. """ + # pylint: disable=fixme # TODO - Implement this test when the relevant manufacturer logic is implemented From 31b86ab4be68f915ca1721add709d53b2404c0e0 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:29:51 +0100 Subject: [PATCH 17/22] Configure CI job to use production Dockerfile #76 --- .github/workflows/.ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/.ci.yml b/.github/workflows/.ci.yml index 2d719129..17591f91 100644 --- a/.github/workflows/.ci.yml +++ b/.github/workflows/.ci.yml @@ -120,6 +120,7 @@ jobs: uses: docker/build-push-action@0565240e2d4ab88bba5387d719585280857ece09 # v5.0.0 with: context: . + file: ./Dockerfile.prod push: ${{ github.event_name == 'push' && github.ref == 'refs/heads/develop' }} tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} From dd832c611ac46c399e1aa90bddd8bc0df4236dd0 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Thu, 12 Oct 2023 17:30:02 +0100 Subject: [PATCH 18/22] Update README #76 --- README.md | 65 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 7d131de7..716f6a77 100644 --- a/README.md +++ b/README.md @@ -2,65 +2,80 @@ ## How to Run -This is a Python microservice created using FastAPI and requires a MongoDB instance to run against. If you are using -Docker to run the application, the `docker-compose.yml file` has already been configured to start a MongoDB instance -that can be accessed at `localhost:27017` using `root` as the username and `example` as the password. +This is a Python microservice created using FastAPI and requires a MongoDB instance to run against. ### Prerequisites - Docker installed (if you want to run the microservice inside Docker) - Python 3.10 (or above) and MongoDB 6.0 installed on your machine if you are not using Docker - [MongoDB Compass](https://www.mongodb.com/products/compass) installed (if you want to interact with the database using a GUI) +- This repository cloned ### Docker Setup +The easiest way to run the application with Docker for local development is using the `docker-compose.yml` file. It is +configured to spin up a MongoDB instance that can be accessed at `localhost:27017` using `root` as the username and +`example` as the password. It also starts the application in a reload mode using the `Dockerfile`. Use the `Dockerfile` +or `Dockerfile.prod` to run just the application itself in a container. The former is for local development and must +not be used in production. -1. Ensure that Docker is installed and running on your machine. -2. Clone the repository and navigate to the project directory: - ```bash - git clone git@github.com:ral-facilities/inventory-management-system-api.git - cd inventory-management-system-api -3. Create a `logging.ini` file. +Ensure that Docker is installed and running on your machine before proceeding. + +#### Using Docker Compose File +1. Create a `logging.ini` file from the example in the root of the project directory: ```bash cp logging.example.ini logging.ini ``` -4. Build and start the Docker containers: +2. Build and start the Docker containers: ```bash docker-compose up ``` - The microservice should now be running inside Docker at http://localhost:8000. The Swagger UI can be accessed - at http://localhost:8000/docs. + The microservice should now be running inside Docker at http://localhost:8000 and its Swagger UI could be accessed + at http://localhost:8000/docs. A MongoDB instance should also be running at http://localhost:27017. -### Local Setup +#### Using Dockerfiles + +1. Build an image using either the `Dockerfile` or `Dockerfile.prod` from the root of the project directory: + ```bash + docker build -f Dockerfile.prod -t ims_api_image . + ``` -1. Clone the repository and navigate to the project directory: +2. Start the container using the image built and map it to port `8000` locally: ```bash - git clone git@github.com:ral-facilities/inventory-management-system-api.git - cd inventory-management-system-api + docker run -p 8000:8000 --name ims_api_container ims_api_image ``` -2. Create a Python virtual environment and activate it: + or with values for the environment variables: + ```bash + docker run -p 8000:8000 --name ims_api_container --env DATABASE__NAME=test-ims ims_api_image + ``` + The microservice should now be running inside Docker at http://localhost:8000 and its Swagger UI could be accessed + at http://localhost:8000/docs. + +### Local Setup + +Ensure that MongoDB is installed and running on your machine before proceeding. + +1. Create a Python virtual environment and activate it in the root of the project directory: ```bash python -m venv venv source venv/bin/activate ``` -3. Install the required dependencies using pip: +2. Install the required dependencies using pip: ```bash pip install .[dev] ``` -4. Create a `.env` file using the `.env.example` file and modify the values inside accordingly. -5. Create a `logging.ini` file using the `logging.example.ini` file and modify it accordingly. -6. Ensure that MongoDB is running locally. If it's not installed, you can follow the official MongoDB installation guide - for your operating system. -7. Start the microservice using Uvicorn from the project directory: +3. Create a `.env` file using the `.env.example` file and modify the values inside accordingly. +4. Create a `logging.ini` file using the `logging.example.ini` file and modify it accordingly. +5. Start the microservice using Uvicorn: ```bash uvicorn inventory_management_system_api.main:app --log-config inventory_management_system_api/logging.ini --reload ``` The microservice should now be running locally at http://localhost:8000. The Swagger UI can be accessed at http://localhost:8000/docs. -8. To run the unit tests, run : +6. To run the unit tests, run : ```bash pytest test/unit/ ``` -9. To run the e2e tests, ensure that MongoDB is running locally and run: +7. To run the e2e tests, run: ```bash DATABASE__NAME="test-ims" pytest test/e2e/ ``` From feb87b54582f0d98b70135953e42afbae7366453 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:20:48 +0100 Subject: [PATCH 19/22] Fix a bug with using stored catalogue item properties in service method #9 --- .../services/catalogue_item.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index 914bd2c0..23b742df 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -135,7 +135,13 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchReque # catalogue item properties should be used. They need to be processed and validated against the defined # properties of the new catalogue category. if "properties" not in update_data: - update_data["properties"] = stored_catalogue_item.properties + # Create `PropertyPostRequestSchema` objects from the stored catalogue item properties and assign them + # to the `properties` field of `catalogue_item` before proceeding with processing and validation. + catalogue_item.properties = [ + PropertyPostRequestSchema(**prop.dict()) for prop in stored_catalogue_item.properties + ] + # Get the new `catalogue_item` state + update_data = catalogue_item.dict(exclude_unset=True) if "properties" in update_data: if not catalogue_category: @@ -144,9 +150,10 @@ def update(self, catalogue_item_id: str, catalogue_item: CatalogueItemPatchReque ) defined_properties = catalogue_category.catalogue_item_properties - supplied_properties = catalogue_item.properties if catalogue_item.properties else [] + supplied_properties = catalogue_item.properties update_data["properties"] = self._process_catalogue_item_properties(defined_properties, supplied_properties) + # Create a copy of `stored_catalogue_item`, updating its field values with the received partial updates stored_catalogue_item = stored_catalogue_item.copy(update=update_data) return self._catalogue_item_repository.update( catalogue_item_id, CatalogueItemIn(**stored_catalogue_item.dict()) From 355a4022c0c2e9895182ec0cbb5251707cfd0a52 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:21:56 +0100 Subject: [PATCH 20/22] Write unit tests for the update repo method #9 --- .../repositories/test_catalogue_category.py | 16 +--- test/unit/repositories/test_catalogue_item.py | 78 +++++++++++++++++++ 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/test/unit/repositories/test_catalogue_category.py b/test/unit/repositories/test_catalogue_category.py index 8653569f..ec1b78b6 100644 --- a/test/unit/repositories/test_catalogue_category.py +++ b/test/unit/repositories/test_catalogue_category.py @@ -2,7 +2,7 @@ """ Unit tests for the `CatalogueCategoryRepo` repository. """ -from unittest.mock import call +from unittest.mock import call, MagicMock import pytest from bson import ObjectId @@ -912,19 +912,11 @@ 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. + Verify that the `update` method properly handles the update of a catalogue category with an invalid 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=[], - ) - + update_catalogue_category = MagicMock() 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}'" diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 5f445207..bc6e2ce0 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -1,6 +1,8 @@ """ Unit tests for the `CatalogueItemRepo` repository. """ +from unittest.mock import MagicMock + import pytest from bson import ObjectId @@ -289,3 +291,79 @@ 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 'invalid'" + + +def test_update(test_helpers, database_mock, catalogue_item_repository): + """ + Test updating a catalogue item. + + Verify that the `update` method properly handles the catalogue item to be updated. + """ + # pylint: disable=duplicate-code + catalogue_item_info = { + "name": "Catalogue Item B", + "description": "This is Catalogue Item B", + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + } + # pylint: enable=duplicate-code + catalogue_item = CatalogueItemOut(id=str(ObjectId()), catalogue_category_id=str(ObjectId()), **catalogue_item_info) + + # Mock `update_one` to return an object for the updated catalogue item document + test_helpers.mock_update_one(database_mock.catalogue_items) + # Mock `find_one` to return the updated catalogue item document + test_helpers.mock_find_one( + database_mock.catalogue_items, + { + "_id": CustomObjectId(catalogue_item.id), + "catalogue_category_id": CustomObjectId(catalogue_item.catalogue_category_id), + **catalogue_item_info, + }, + ) + + catalogue_item_in = CatalogueItemIn( + catalogue_category_id=catalogue_item.catalogue_category_id, **catalogue_item_info + ) + updated_catalogue_item = catalogue_item_repository.update(catalogue_item.id, catalogue_item_in) + + database_mock.catalogue_items.update_one.assert_called_once_with( + {"_id": CustomObjectId(catalogue_item.id)}, + { + "$set": { + "catalogue_category_id": CustomObjectId(catalogue_item.catalogue_category_id), + **catalogue_item_in.dict(), + } + }, + ) + database_mock.catalogue_items.find_one.assert_called_once_with({"_id": CustomObjectId(catalogue_item.id)}) + assert updated_catalogue_item == catalogue_item + + +def test_update_with_invalid_id(catalogue_item_repository): + """ + Test updating a catalogue category with invalid ID. + + Verify that the `update` method properly handles the update of a catalogue category with an invalid ID. + """ + update_catalogue_item = MagicMock() + catalogue_item_id = "invalid" + + with pytest.raises(InvalidObjectIdError) as exc: + catalogue_item_repository.update(catalogue_item_id, update_catalogue_item) + assert str(exc.value) == f"Invalid ObjectId value '{catalogue_item_id}'" + + +def test_update_has_child_items(): + """ + Test updating a catalogue item with child items. + """ + # pylint: disable=fixme + # TODO - Implement this test when the relevant item logic is implemented. From c43500d682c66e65335a699f8d2fd54f0b99c7d1 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:30:18 +0100 Subject: [PATCH 21/22] Write unit tests for the update service method #9 --- test/unit/services/test_catalogue_category.py | 2 + test/unit/services/test_catalogue_item.py | 847 ++++++++++++++++++ 2 files changed, 849 insertions(+) diff --git a/test/unit/services/test_catalogue_category.py b/test/unit/services/test_catalogue_category.py index 6b0c123c..32eb23a6 100644 --- a/test/unit/services/test_catalogue_category.py +++ b/test/unit/services/test_catalogue_category.py @@ -642,6 +642,7 @@ def test_update_change_from_leaf_to_non_leaf( """ Test changing a catalogue category from leaf to non-leaf. """ + # pylint: disable=duplicate-code catalogue_category = CatalogueCategoryOut( id=str(ObjectId()), name="Category A", @@ -652,6 +653,7 @@ def test_update_change_from_leaf_to_non_leaf( parent_id=None, catalogue_item_properties=[], ) + # pylint: enable=duplicate-code # Mock `get` to return a catalogue category test_helpers.mock_get( diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 55c4d85d..04bbc7ee 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -1,3 +1,4 @@ +# pylint: disable=too-many-lines """ Unit tests for the `CatalogueCategoryService` service. """ @@ -22,6 +23,7 @@ PropertyPostRequestSchema, CatalogueItemPostRequestSchema, ManufacturerSchema, + CatalogueItemPatchRequestSchema, ) @@ -629,3 +631,848 @@ def test_list_with_catalogue_category_id_filter_no_matching_results( catalogue_item_repository_mock.list.assert_called_once_with(catalogue_category_id) assert retrieved_catalogue_items == [] + + +def test_update(test_helpers, catalogue_item_repository_mock, catalogue_item_service): + """ + Test updating a catalogue item. + + Verify that the `update` method properly handles the catalogue item to be updated. + """ + # pylint: disable=duplicate-code + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + } + # pylint: enable=duplicate-code + full_catalogue_item_info = { + **catalogue_item_info, + "name": "Catalogue Item B", + "description": "This is Catalogue Item B", + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut( + name="Catalogue Item A", + description="This is Catalogue Item A", + **catalogue_item_info, + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema(name=catalogue_item.name, description=catalogue_item.description), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_with_nonexistent_id(test_helpers, catalogue_item_repository_mock, catalogue_item_service): + """ + Test updating a catalogue item 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 return a catalogue item + test_helpers.mock_get(catalogue_item_repository_mock, None) + + catalogue_item_id = str(ObjectId()) + with pytest.raises(MissingRecordError) as exc: + catalogue_item_service.update(catalogue_item_id, CatalogueItemPatchRequestSchema(properties=[])) + assert str(exc.value) == f"No catalogue item found with ID: {catalogue_item_id}" + + +def test_update_change_catalogue_category_id_same_defined_properties_without_supplied_properties( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test moving a catalogue item to another catalogue category that has the same defined catalogue item properties when + no properties are supplied. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + } + full_catalogue_item_info = { + **catalogue_item_info, + "catalogue_category_id": str(ObjectId()), + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut( + catalogue_category_id=str(ObjectId()), + **catalogue_item_info, + ), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, CatalogueItemPatchRequestSchema(catalogue_category_id=catalogue_item.catalogue_category_id) + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_change_catalogue_category_id_same_defined_properties_with_supplied_properties( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test moving a catalogue item to another catalogue category that has the same defined catalogue item properties when + properties are supplied. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + } + full_catalogue_item_info = { + **catalogue_item_info, + "catalogue_category_id": str(ObjectId()), + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut( + catalogue_category_id=str(ObjectId()), + properties=[ + Property(name="Property A", value=1, unit="mm"), + Property(name="Property B", value=True), + Property(name="Property C", value="1x1x1", unit="cm"), + ], + **catalogue_item_info, + ), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + catalogue_category_id=catalogue_item.catalogue_category_id, + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties], + ), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_change_catalogue_category_id_different_defined_properties_without_supplied_properties( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test moving a catalogue item to another catalogue category that has different defined catalogue item properties when + no properties are supplied. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut( + catalogue_category_id=str(ObjectId()), + **catalogue_item_info, + ), + ) + catalogue_category_id = str(ObjectId()) + # 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=True, + path="/category-a", + parent_path="/", + parent_id=None, + catalogue_item_properties=[ + CatalogueItemProperty(name="Property A", type="boolean", mandatory=True), + ], + ), + ) + + with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: + catalogue_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(catalogue_category_id=catalogue_category_id), + ) + assert str(exc.value) == "Invalid value type for catalogue item property 'Property A'. Expected type: boolean." + + +def test_update_change_catalogue_category_id_different_defined_properties_with_supplied_properties( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test moving a catalogue item to another catalogue category that has different defined catalogue item properties when + properties are supplied. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + } + full_catalogue_item_info = { + **catalogue_item_info, + "catalogue_category_id": str(ObjectId()), + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut( + catalogue_category_id=str(ObjectId()), + properties=[{"name": "Property A", "value": True}], + **catalogue_item_info, + ), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + catalogue_category_id=catalogue_item.catalogue_category_id, + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties], + ), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_with_nonexistent_catalogue_category_id( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test updating a catalogue item with a non-existent catalogue category ID. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get(catalogue_item_repository_mock, CatalogueItemOut(**catalogue_item_info)) + # 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_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(catalogue_category_id=catalogue_category_id), + ) + assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category_id}" + + +def test_update_change_catalogue_category_id_non_leaf_catalogue_category( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test moving a catalogue item to a non-leaf catalogue category. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get(catalogue_item_repository_mock, CatalogueItemOut(**catalogue_item_info)) + catalogue_category_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_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 + + with pytest.raises(NonLeafCategoryError) as exc: + catalogue_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(catalogue_category_id=catalogue_category_id), + ) + assert str(exc.value) == "Cannot add catalogue item to a non-leaf catalogue category" + + +def test_update_add_non_mandatory_property( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test adding a non-mandatory catalogue item property and a value. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + full_catalogue_item_info = { + **catalogue_item_info, + "properties": [{"name": "Property A", "value": 20, "unit": "mm"}] + catalogue_item_info["properties"], + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties] + ), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_remove_non_mandatory_property( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test removing a non-mandatory catalogue item property and its value. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + full_catalogue_item_info = {**catalogue_item_info, "properties": catalogue_item_info["properties"][-2:]} + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties] + ), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_remove_mandatory_property( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test removing a mandatory catalogue item property and its value. + """ + full_catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**full_catalogue_item_info), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + + with pytest.raises(MissingMandatoryCatalogueItemProperty) as exc: + catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties[:2]] + ), + ) + assert str(exc.value) == f"Missing mandatory catalogue item property: '{catalogue_item.properties[2].name}'" + + +def test_update_change_property_value( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test updating a value of a property. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + full_catalogue_item_info = { + **catalogue_item_info, + "properties": [{"name": "Property A", "value": 1, "unit": "mm"}] + catalogue_item_info["properties"][-2:], + } + catalogue_item = CatalogueItemOut(**full_catalogue_item_info) + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + 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), + ], + ), + ) + # Mock `update` to return the updated catalogue item + test_helpers.mock_update(catalogue_item_repository_mock, catalogue_item) + + updated_catalogue_item = catalogue_item_service.update( + catalogue_item.id, + CatalogueItemPatchRequestSchema( + properties=[{"name": prop.name, "value": prop.value} for prop in catalogue_item.properties] + ), + ) + + catalogue_item_repository_mock.update.assert_called_once_with( + catalogue_item.id, CatalogueItemIn(**full_catalogue_item_info) + ) + assert updated_catalogue_item == catalogue_item + + +def test_update_change_value_for_string_property_invalid_type( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test changing the value of a string property to an invalid type. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_item_info["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), + ], + ), + ) + + properties = [{"name": prop["name"], "value": prop["value"]} for prop in catalogue_item_info["properties"]] + properties[2]["value"] = True + with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: + catalogue_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(properties=properties), + ) + assert str(exc.value) == "Invalid value type for catalogue item property 'Property C'. Expected type: string." + + +def test_update_change_value_for_number_property_invalid_type( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test changing the value of a number property to an invalid type. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_item_info["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), + ], + ), + ) + + properties = [{"name": prop["name"], "value": prop["value"]} for prop in catalogue_item_info["properties"]] + properties[0]["value"] = "20" + with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: + catalogue_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(properties=properties), + ) + assert str(exc.value) == "Invalid value type for catalogue item property 'Property A'. Expected type: number." + + +def test_update_change_value_for_boolean_property_invalid_type( + test_helpers, catalogue_category_repository_mock, catalogue_item_repository_mock, catalogue_item_service +): + """ + Test changing the value of a boolean property to an invalid type. + """ + catalogue_item_info = { + "id": str(ObjectId()), + "catalogue_category_id": str(ObjectId()), + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "web_url": "https://www.manufacturer-a.co.uk", + }, + "properties": [ + {"name": "Property A", "value": 20, "unit": "mm"}, + {"name": "Property B", "value": False}, + {"name": "Property C", "value": "20x15x10", "unit": "cm"}, + ], + } + + # Mock `get` to return a catalogue item + test_helpers.mock_get( + catalogue_item_repository_mock, + CatalogueItemOut(**catalogue_item_info), + ) + # Mock `get` to return a catalogue category + test_helpers.mock_get( + catalogue_category_repository_mock, + CatalogueCategoryOut( + id=catalogue_item_info["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), + ], + ), + ) + + properties = [{"name": prop["name"], "value": prop["value"]} for prop in catalogue_item_info["properties"]] + properties[1]["value"] = "False" + with pytest.raises(InvalidCatalogueItemPropertyTypeError) as exc: + catalogue_item_service.update( + catalogue_item_info["id"], + CatalogueItemPatchRequestSchema(properties=properties), + ) + assert str(exc.value) == "Invalid value type for catalogue item property 'Property B'. Expected type: boolean." From e5ac5a149b3e5b51a0d57c504c3fea5dd12d0194 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov <45173816+VKTB@users.noreply.github.com> Date: Wed, 18 Oct 2023 13:30:37 +0100 Subject: [PATCH 22/22] Fix failing e2e test #9 --- test/e2e/test_catalogue_item.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index ae13a746..2aa80ce7 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -492,7 +492,10 @@ def test_partial_update_catalogue_item_change_catalogue_category_id_without_prop response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) assert response.status_code == 422 - assert response.json()["detail"] == "Missing mandatory catalogue item property: 'Property A'" + assert ( + response.json()["detail"] + == "Invalid value type for catalogue item property 'Property A'. Expected type: boolean." + ) def test_partial_update_catalogue_item_change_catalogue_category_id_missing_mandatory_properties(test_client):