From 4e39bc2d7165f9dde600dbadd09202c70304a74c Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 4 Oct 2023 10:38:51 +0100 Subject: [PATCH 1/9] refactor unit tests #64 --- .../models/catalogue_item.py | 14 +-- .../schemas/catalogue_item.py | 12 +-- .../services/catalogue_item.py | 2 +- test/unit/repositories/test_catalogue_item.py | 43 +++------ test/unit/services/test_catalogue_item.py | 92 +++++-------------- 5 files changed, 43 insertions(+), 120 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index 63adda10..d2ba72fb 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -8,16 +8,6 @@ from inventory_management_system_api.models.custom_object_id_data_types import CustomObjectIdField, StringObjectIdField -class Manufacturer(BaseModel): - """ - Model representing a catalogue item manufacturer. - """ - - name: str - address: str - web_url: str - - class Property(BaseModel): """ Model representing a catalogue item property. @@ -37,7 +27,7 @@ class CatalogueItemIn(BaseModel): name: str description: str properties: List[Property] = [] - manufacturer: Manufacturer + manufacturer_id: CustomObjectIdField @validator("properties", pre=True, always=True) @classmethod @@ -63,7 +53,7 @@ class CatalogueItemOut(CatalogueItemIn): id: StringObjectIdField = Field(alias="_id") catalogue_category_id: StringObjectIdField - + manufacturer_id: StringObjectIdField class Config: # pylint: disable=C0115 allow_population_by_field_name = True diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 0d4e4c10..24d6d273 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -6,16 +6,6 @@ from pydantic import BaseModel, Field, HttpUrl -class ManufacturerSchema(BaseModel): - """ - Schema model for a catalogue item manufacturer creation request. - """ - - name: str = Field(description="The name of the manufacturer") - address: str = Field(description="The address of the manufacturer") - web_url: HttpUrl = Field(description="The website URL of the manufacturer") - - class PropertyPostRequestSchema(BaseModel): """ Schema model for a catalogue item property creation request. @@ -44,7 +34,7 @@ class CatalogueItemPostRequestSchema(BaseModel): name: str = Field(description="The name of the catalogue item") description: str = Field(description="The catalogue item description") properties: Optional[List[PropertyPostRequestSchema]] = Field(description="The catalogue item properties") - manufacturer: ManufacturerSchema = Field(description="The details of the manufacturer") + manufacturer_id: str = Field(description="The ID of the manufacturer") class CatalogueItemSchema(CatalogueItemPostRequestSchema): diff --git a/inventory_management_system_api/services/catalogue_item.py b/inventory_management_system_api/services/catalogue_item.py index f7645726..23f21bb2 100644 --- a/inventory_management_system_api/services/catalogue_item.py +++ b/inventory_management_system_api/services/catalogue_item.py @@ -84,7 +84,7 @@ def create(self, catalogue_item: CatalogueItemPostRequestSchema) -> CatalogueIte name=catalogue_item.name, description=catalogue_item.description, properties=list(supplied_properties.values()), - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 91b74f86..cf9eb0ae 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -9,8 +9,7 @@ from inventory_management_system_api.models.catalogue_item import ( CatalogueItemOut, Property, - CatalogueItemIn, - Manufacturer, + CatalogueItemIn ) @@ -32,9 +31,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -51,7 +48,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer": catalogue_item.manufacturer, + "manufacturer_id": catalogue_item.manufacturer_id, }, ) @@ -62,7 +59,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) # pylint: enable=duplicate-code @@ -73,7 +70,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer": catalogue_item.manufacturer, + "manufacturer_id": CustomObjectId(catalogue_item.manufacturer_id), } ) assert created_catalogue_item == catalogue_item @@ -97,9 +94,7 @@ def test_create_with_duplicate_name_within_catalogue_category(test_helpers, data Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -113,7 +108,7 @@ def test_create_with_duplicate_name_within_catalogue_category(test_helpers, data name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) assert str(exc.value) == "Duplicate catalogue item found within the catalogue category" @@ -139,9 +134,7 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -154,7 +147,7 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer": catalogue_item.manufacturer, + "manufacturer_id": catalogue_item.manufacturer_id, }, ) @@ -209,9 +202,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) catalogue_item_b = CatalogueItemOut( @@ -220,9 +211,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -236,7 +225,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_a.name, "description": catalogue_item_a.description, "properties": catalogue_item_a.properties, - "manufacturer": catalogue_item_a.manufacturer, + "manufacturer_id": catalogue_item_a.manufacturer_id, }, { "_id": CustomObjectId(catalogue_item_b.id), @@ -244,7 +233,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_b.name, "description": catalogue_item_b.description, "properties": catalogue_item_b.properties, - "manufacturer": catalogue_item_b.manufacturer, + "manufacturer_id": catalogue_item_b.manufacturer_id, }, ], ) @@ -272,9 +261,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", address="1 Address, City, Country, Postcode", web_url="www.manufacturer-a.co.uk" - ), + manufacturer_id=str(ObjectId()) ) # Mock `find` to return a list of catalogue item documents @@ -287,7 +274,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer": catalogue_item.manufacturer, + "manufacturer_id": catalogue_item.manufacturer_id, } ], ) diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 55c4d85d..bdf956dd 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -4,6 +4,7 @@ import pytest from bson import ObjectId +from inventory_management_system_api.core.custom_object_id import CustomObjectId from inventory_management_system_api.core.exceptions import ( MissingRecordError, @@ -16,12 +17,10 @@ CatalogueItemOut, Property, CatalogueItemIn, - Manufacturer, ) from inventory_management_system_api.schemas.catalogue_item import ( PropertyPostRequestSchema, - CatalogueItemPostRequestSchema, - ManufacturerSchema, + CatalogueItemPostRequestSchema ) @@ -46,11 +45,7 @@ def test_create( Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()), ) # Mock `get` to return the catalogue category @@ -85,7 +80,7 @@ def test_create( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) @@ -97,7 +92,7 @@ def test_create( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer=catalogue_item.manufacturer, + manufacturer_id= catalogue_item.manufacturer_id, ) ) # pylint: enable=duplicate-code @@ -129,13 +124,10 @@ def test_create_with_nonexistent_catalogue_category_id( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", + manufacturer_id=str(ObjectId()) ), ) - ) + assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category_id}" catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category_id) @@ -177,13 +169,10 @@ def test_create_in_non_leaf_catalogue_category( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", + manufacturer_id=str(ObjectId()) ), ) - ) + assert str(exc.value) == "Cannot add catalogue item to a non-leaf catalogue category" catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.id) @@ -203,12 +192,9 @@ def test_create_without_properties( name="Catalogue Item A", description="This is Catalogue Item A", properties=[], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), - ) + manufacturer_id=str(ObjectId()) + ) + # pylint: enable=duplicate-code # Mock `get` to return the catalogue category @@ -235,7 +221,7 @@ def test_create_without_properties( catalogue_category_id=catalogue_item.catalogue_category_id, name=catalogue_item.name, description=catalogue_item.description, - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) @@ -247,7 +233,7 @@ def test_create_without_properties( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer=catalogue_item.manufacturer, + manufacturer_id=catalogue_item.manufacturer_id, ) ) # pylint: enable=duplicate-code @@ -293,13 +279,10 @@ def test_create_with_missing_mandatory_properties( properties=[ PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", + manufacturer_id=str(ObjectId()) ), ) - ) + assert ( str(exc.value) == f"Missing mandatory catalogue item property: '{catalogue_category.catalogue_item_properties[1].name}'" @@ -349,13 +332,10 @@ def test_create_with_with_invalid_value_type_for_string_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value=True), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", + manufacturer_id=str(ObjectId()) ), ) - ) + assert ( str(exc.value) == f"Invalid value type for catalogue item property '{catalogue_category.catalogue_item_properties[2].name}'. " @@ -406,11 +386,7 @@ def test_create_with_with_invalid_value_type_for_number_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) ) assert ( @@ -463,11 +439,7 @@ def test_create_with_with_invalid_value_type_for_boolean_property( PropertyPostRequestSchema(name="Property B", value="False"), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer=ManufacturerSchema( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) ) assert ( @@ -495,11 +467,7 @@ def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_servic Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -546,11 +514,7 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) catalogue_item_b = CatalogueItemOut( @@ -559,11 +523,7 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code @@ -594,11 +554,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_rep Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - web_url="https://www.manufacturer-a.co.uk", - ), + manufacturer_id=str(ObjectId()) ) # pylint: enable=duplicate-code From f8396e8c336deb9781c20a14c496c9b090b12d0a Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 4 Oct 2023 11:21:15 +0100 Subject: [PATCH 2/9] fixed unit test issues --- .../models/catalogue_item.py | 1 + .../schemas/catalogue_item.py | 2 +- test/unit/repositories/conftest.py | 12 ---- test/unit/repositories/test_catalogue_item.py | 18 +++--- test/unit/repositories/test_manufacturer.py | 26 +++------ test/unit/services/test_catalogue_item.py | 55 +++++++++---------- 6 files changed, 44 insertions(+), 70 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index d2ba72fb..0cb5600e 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -54,6 +54,7 @@ class CatalogueItemOut(CatalogueItemIn): id: StringObjectIdField = Field(alias="_id") catalogue_category_id: StringObjectIdField manufacturer_id: StringObjectIdField + class Config: # pylint: disable=C0115 allow_population_by_field_name = True diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 24d6d273..0aeff51a 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -3,7 +3,7 @@ """ from typing import List, Any, Optional -from pydantic import BaseModel, Field, HttpUrl +from pydantic import BaseModel, Field class PropertyPostRequestSchema(BaseModel): diff --git a/test/unit/repositories/conftest.py b/test/unit/repositories/conftest.py index fc715b76..21349b55 100644 --- a/test/unit/repositories/conftest.py +++ b/test/unit/repositories/conftest.py @@ -153,18 +153,6 @@ def mock_update_one(collection_mock: Mock) -> None: update_one_result_mock.acknowledged = True collection_mock.insert_one.return_value = update_one_result_mock - @staticmethod - def mock_update_one(collection_mock: Mock) -> None: - """ - Mocks the `update_one` method of the MongoDB database collection mock to return an updated document - - :param collection_mock: Mocked MongoDB database collection instance - :param document: The document to be returned by the method - """ - update_one_result_mock = Mock(UpdateResult) - update_one_result_mock.acknowledged = True - collection_mock.insert_one.return_value = update_one_result_mock - @pytest.fixture(name="test_helpers") def fixture_test_helpers() -> Type[RepositoryTestHelpers]: diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index cf9eb0ae..e2713ba2 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -6,11 +6,7 @@ from inventory_management_system_api.core.custom_object_id import CustomObjectId from inventory_management_system_api.core.exceptions import DuplicateRecordError, InvalidObjectIdError -from inventory_management_system_api.models.catalogue_item import ( - CatalogueItemOut, - Property, - CatalogueItemIn -) +from inventory_management_system_api.models.catalogue_item import CatalogueItemOut, Property, CatalogueItemIn def test_create(test_helpers, database_mock, catalogue_item_repository): @@ -31,7 +27,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -94,7 +90,7 @@ def test_create_with_duplicate_name_within_catalogue_category(test_helpers, data Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -134,7 +130,7 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -202,7 +198,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) catalogue_item_b = CatalogueItemOut( @@ -211,7 +207,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -261,7 +257,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # Mock `find` to return a list of catalogue item documents diff --git a/test/unit/repositories/test_manufacturer.py b/test/unit/repositories/test_manufacturer.py index 4100c253..8edcf0e8 100644 --- a/test/unit/repositories/test_manufacturer.py +++ b/test/unit/repositories/test_manufacturer.py @@ -280,7 +280,7 @@ def test_update_with_invalid_id(manufacturer_repository): manufactuer_id = "invalid" with pytest.raises(InvalidObjectIdError) as exc: manufacturer_repository.update(manufactuer_id, updated_manufacturer) - assert str(exc.value) == "Invalid ObjectId value" + assert str(exc.value) == "Invalid ObjectId value 'invalid'" def test_update_with_nonexistent_id(test_helpers, database_mock, manufacturer_repository): @@ -300,30 +300,20 @@ def test_update_with_nonexistent_id(test_helpers, database_mock, manufacturer_re assert str(exc.value) == "The specified manufacturer does not exist" -def test_update_duplicate_name(test_helpers, database_mock, manufacturer_repository): - """Test trying to update a mnaufactuer using a duplicate name""" - +def update_with_duplicate_name(test_helpers, database_mock, manufacturer_repository): + """Test trying to update a manufacturer with a duplicate name""" # pylint: disable=duplicate-code updated_manufacturer = ManufacturerIn( - name="Manufacturer A", - code="manufacturer-a", + name="Manufacturer B", + code="manufacturer-b", url="http://testUrl.co.uk", address="1 Example street", ) # pylint: enable=duplicate-code - test_helpers.mock_count_documents(database_mock.manufacturer, 0) + manufacturer_id = str(ObjectId()) - test_helpers.mock_find_one( - database_mock.manufacturer, - { - "_id": CustomObjectId(manufacturer_id), - "code": "manufacturer-b", - "name": "Manufacturer B", - "url": "http://example.com", - "address": "2 Example Street", - }, - ) test_helpers.mock_count_documents(database_mock.manufacturer, 1) + with pytest.raises(DuplicateRecordError) as exc: manufacturer_repository.update(manufacturer_id, updated_manufacturer) - assert str(exc.value) == "Duplicate manufacturer found" + assert str(exc.value) == "The specified manufacturer does not exist" diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index bdf956dd..2b918844 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -4,7 +4,6 @@ import pytest from bson import ObjectId -from inventory_management_system_api.core.custom_object_id import CustomObjectId from inventory_management_system_api.core.exceptions import ( MissingRecordError, @@ -20,7 +19,7 @@ ) from inventory_management_system_api.schemas.catalogue_item import ( PropertyPostRequestSchema, - CatalogueItemPostRequestSchema + CatalogueItemPostRequestSchema, ) @@ -92,7 +91,7 @@ def test_create( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer_id= catalogue_item.manufacturer_id, + manufacturer_id=catalogue_item.manufacturer_id, ) ) # pylint: enable=duplicate-code @@ -124,10 +123,10 @@ def test_create_with_nonexistent_catalogue_category_id( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()) - ), - ) - + manufacturer_id=str(ObjectId()), + ), + ) + assert str(exc.value) == f"No catalogue category found with ID: {catalogue_category_id}" catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category_id) @@ -169,10 +168,10 @@ def test_create_in_non_leaf_catalogue_category( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()) - ), - ) - + manufacturer_id=str(ObjectId()), + ), + ) + assert str(exc.value) == "Cannot add catalogue item to a non-leaf catalogue category" catalogue_category_repository_mock.get.assert_called_once_with(catalogue_category.id) @@ -192,9 +191,9 @@ def test_create_without_properties( name="Catalogue Item A", description="This is Catalogue Item A", properties=[], - manufacturer_id=str(ObjectId()) - ) - + manufacturer_id=str(ObjectId()), + ) + # pylint: enable=duplicate-code # Mock `get` to return the catalogue category @@ -279,10 +278,10 @@ def test_create_with_missing_mandatory_properties( properties=[ PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()) - ), - ) - + manufacturer_id=str(ObjectId()), + ), + ) + assert ( str(exc.value) == f"Missing mandatory catalogue item property: '{catalogue_category.catalogue_item_properties[1].name}'" @@ -332,10 +331,10 @@ def test_create_with_with_invalid_value_type_for_string_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value=True), ], - manufacturer_id=str(ObjectId()) - ), - ) - + manufacturer_id=str(ObjectId()), + ), + ) + assert ( str(exc.value) == f"Invalid value type for catalogue item property '{catalogue_category.catalogue_item_properties[2].name}'. " @@ -386,7 +385,7 @@ def test_create_with_with_invalid_value_type_for_number_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) ) assert ( @@ -439,7 +438,7 @@ def test_create_with_with_invalid_value_type_for_boolean_property( PropertyPostRequestSchema(name="Property B", value="False"), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) ) assert ( @@ -467,7 +466,7 @@ def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_servic Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -514,7 +513,7 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) catalogue_item_b = CatalogueItemOut( @@ -523,7 +522,7 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code @@ -554,7 +553,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_rep Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()) + manufacturer_id=str(ObjectId()), ) # pylint: enable=duplicate-code From a549bb7d354c49ffccff0b4b0ef120b5810da039 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 4 Oct 2023 13:37:54 +0100 Subject: [PATCH 3/9] refactor e2e tests --- test/e2e/test_catalogue_category.py | 31 ++++++----------------------- test/e2e/test_catalogue_item.py | 18 +++++------------ 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index af8393bc..9c850d5e 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -313,12 +313,9 @@ def test_delete_catalogue_category_with_children_catalogue_items(test_client): "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } + test_client.post("/v1/catalogue-items", json=catalogue_item_post) response = test_client.delete(f"/v1/catalogue-categories/{catalogue_category_id}") @@ -596,11 +593,7 @@ def test_partial_update_catalogue_category_change_name_has_children_catalogue_it "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -727,11 +720,7 @@ def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_has_chil "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -852,11 +841,7 @@ def test_partial_update_catalogue_category_change_parent_id_has_children_catalog "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -1071,11 +1056,7 @@ def test_partial_update_catalogue_category_change_catalogue_item_properties_has_ "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } test_client.post("/v1/catalogue-items", json=catalogue_item_post) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 71a6a1c9..2df69171 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -41,11 +41,7 @@ def get_catalogue_item_a_dict(catalogue_category_id: str) -> Dict: {"name": "Property B", "value": False}, {"name": "Property C", "value": "20x15x10"}, ], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } @@ -63,11 +59,7 @@ def get_catalogue_item_b_dict(catalogue_category_id: str) -> Dict: "properties": [ {"name": "Property A", "value": True}, ], - "manufacturer": { - "name": "Manufacturer A", - "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", - }, + "manufacturer_id": str(ObjectId()), } @@ -93,7 +85,7 @@ def test_create_catalogue_item(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] - assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] def test_create_catalogue_item_with_duplicate_name_within_catalogue_category(test_client): @@ -171,7 +163,7 @@ def test_create_catalogue_item_without_properties(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == [] - assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] def test_create_catalogue_item_with_missing_mandatory_properties(test_client): @@ -211,7 +203,7 @@ def test_create_catalogue_item_with_missing_non_mandatory_properties(test_client assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] - assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] + assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] def test_create_catalogue_item_with_invalid_value_type_for_string_property(test_client): From feedd655f997d3a8e00ff8ce8247009705cad539 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 4 Oct 2023 13:52:34 +0100 Subject: [PATCH 4/9] moved services fixture to confitest --- test/unit/services/conftest.py | 11 +++++++++++ test/unit/services/test_manufacturer.py | 12 ------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/test/unit/services/conftest.py b/test/unit/services/conftest.py index c573a2e7..8c9a333e 100644 --- a/test/unit/services/conftest.py +++ b/test/unit/services/conftest.py @@ -13,6 +13,7 @@ from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo from inventory_management_system_api.services.catalogue_category import CatalogueCategoryService from inventory_management_system_api.services.catalogue_item import CatalogueItemService +from inventory_management_system_api.services.manufacturer import ManufacturerService @pytest.fixture(name="catalogue_category_repository_mock") @@ -71,6 +72,16 @@ def fixture_catalogue_item_service( return CatalogueItemService(catalogue_item_repository_mock, catalogue_category_repository_mock) +@pytest.fixture(name="manufacturer_service") +def fixture_manufacturer_service(manufacturer_repository_mock: Mock) -> ManufacturerService: + """ + Fixture to create a `ManufacturerService` instance with a mocked `ManufacturerRepo` + + :param: manufacturer_repository_mock: Mocked `ManufacturerRepo` instance. + :return: `ManufacturerService` instance with mocked dependency + """ + return ManufacturerService(manufacturer_repository_mock) + class ServiceTestHelpers: """ A utility class containing common helper methods for the service tests. diff --git a/test/unit/services/test_manufacturer.py b/test/unit/services/test_manufacturer.py index b48a5f49..629abd7f 100644 --- a/test/unit/services/test_manufacturer.py +++ b/test/unit/services/test_manufacturer.py @@ -8,18 +8,6 @@ from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut from inventory_management_system_api.schemas.manufacturer import ManufacturerPostRequestSchema -from inventory_management_system_api.services.manufacturer import ManufacturerService - - -@pytest.fixture(name="manufacturer_service") -def fixture_manufacturer_service(manufacturer_repository_mock: Mock) -> ManufacturerService: - """ - Fixture to create a `ManufacturerService` instance with a mocked `ManufacturerRepo` - - :param: manufacturer_repository_mock: Mocked `ManufacturerRepo` instance. - :return: `ManufacturerService` instance with mocked dependency - """ - return ManufacturerService(manufacturer_repository_mock) def test_create(manufacturer_repository_mock, manufacturer_service): From f556a744c1ec48efa4e549f01e73d2ea76af17be Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Wed, 4 Oct 2023 14:24:11 +0100 Subject: [PATCH 5/9] delete logic #64 --- .../core/exceptions.py | 6 ++++ .../repositories/catalogue_item.py | 10 ++++++ .../repositories/manufacturer.py | 15 ++++++++ .../routers/v1/manufacturer.py | 24 +++++++++++++ .../services/manufacturer.py | 34 +++++++++++++++++-- 5 files changed, 87 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/core/exceptions.py b/inventory_management_system_api/core/exceptions.py index 4104a82e..0c4d6217 100644 --- a/inventory_management_system_api/core/exceptions.py +++ b/inventory_management_system_api/core/exceptions.py @@ -55,3 +55,9 @@ class ChildrenElementsExistError(DatabaseError): """ Exception raised when attempting to delete or update a catalogue category that has children elements. """ + + +class PartOfCatalogueItemError(DatabaseError): + """ + Exception raised when attempting to delete a manufacturer that is a part of a catalogue item + """ diff --git a/inventory_management_system_api/repositories/catalogue_item.py b/inventory_management_system_api/repositories/catalogue_item.py index e5b07ba9..c15dd7f5 100644 --- a/inventory_management_system_api/repositories/catalogue_item.py +++ b/inventory_management_system_api/repositories/catalogue_item.py @@ -95,3 +95,13 @@ def _is_duplicate_catalogue_item(self, catalogue_category_id: str, name: str) -> catalogue_category_id = CustomObjectId(catalogue_category_id) count = self._collection.count_documents({"catalogue_category_id": catalogue_category_id, "name": name}) return count > 0 + + def is_manufacturer_in_catalogue_item(self, manufacturer_id: str) -> bool: + """Checks to see if any of the documents in the database have a specific manufactuer id + + :param manufacturer_id: The ID of the manufacturer that is looked for + :return Returns True if 1 or more documents have the manufacturer ID, false if none do + """ + manufacturer_id = CustomObjectId(manufacturer_id) + count = self._collection.count_documents({"manufacturer_id": manufacturer_id}) + return count > 0 diff --git a/inventory_management_system_api/repositories/manufacturer.py b/inventory_management_system_api/repositories/manufacturer.py index d3fb77c2..d62a8c64 100644 --- a/inventory_management_system_api/repositories/manufacturer.py +++ b/inventory_management_system_api/repositories/manufacturer.py @@ -104,6 +104,21 @@ def update(self, manufacturer_id: str, manufacturer: ManufacturerIn) -> Manufact manufacturer = self.get(str(manufacturer_id)) return manufacturer + def delete(self, manufacturer_id: str) -> None: + """ + Delete a manufacturer by its ID from MongoDB database. + Checks if manufactuer is a part of an item, and does not delete if it is + + :param manufacturer_id: The ID of the manufacturer to delete + :raises ExistIn + """ + manufacturer_id = CustomObjectId(manufacturer_id) + + logger.info("Deleting manufacturer with ID %s from the database", manufacturer_id) + result = self._collection.delete_one({"_id": manufacturer_id}) + if result.deleted_count == 0: + raise MissingRecordError(f"No manufacturer founs with ID: {str(manufacturer_id)}") + def _is_duplicate_manufacturer(self, code: str) -> bool: """ Check if manufacturer with the same url already exists in the manufacturer collection diff --git a/inventory_management_system_api/routers/v1/manufacturer.py b/inventory_management_system_api/routers/v1/manufacturer.py index 2386ab44..874a2f2d 100644 --- a/inventory_management_system_api/routers/v1/manufacturer.py +++ b/inventory_management_system_api/routers/v1/manufacturer.py @@ -9,6 +9,7 @@ DuplicateRecordError, InvalidObjectIdError, MissingRecordError, + PartOfCatalogueItemError, ) from inventory_management_system_api.schemas.manufacturer import ( @@ -110,3 +111,26 @@ def edit_manufacturer( message = "A manufacturer with the same name has been found" logger.exception(message) raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=message) from exc + + +@router.delete( + path="/{manufacturer_id}", + summary="Delete a manufacturer by its ID", + response_description="Manufacturer deleted successfully", + status_code=status.HTTP_204_NO_CONTENT, +) +def delete_manufacturer(manufacturer_id: str, manufacturer_service: ManufacturerService = Depends()) -> None: + # pylint: disable=missing-function-docstring + logger.info("Deleting manufacturer with ID: %s", manufacturer_id) + try: + manufacturer_service.delete(manufacturer_id) + except (MissingRecordError, InvalidObjectIdError) as exc: + logger.exception("The specified manufacturer does not exist") + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="The specified manufacturer does not exist" + ) from exc + except PartOfCatalogueItemError as exc: + logger.exception("The specified manufactuerer is a part of a Catalogue Item") + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="The specified manufactuerer is a part of a Catalogue Item" + ) from exc diff --git a/inventory_management_system_api/services/manufacturer.py b/inventory_management_system_api/services/manufacturer.py index e8c21e2e..79d77ae3 100644 --- a/inventory_management_system_api/services/manufacturer.py +++ b/inventory_management_system_api/services/manufacturer.py @@ -6,8 +6,9 @@ from typing import List, Optional from fastapi import Depends -from inventory_management_system_api.core.exceptions import MissingRecordError +from inventory_management_system_api.core.exceptions import MissingRecordError, PartOfCatalogueItemError from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut +from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo from inventory_management_system_api.repositories.manufacturer import ManufacturerRepo from inventory_management_system_api.schemas.manufacturer import ( @@ -21,14 +22,20 @@ class ManufacturerService: """Service for managing manufacturers""" - def __init__(self, manufacturer_repository: ManufacturerRepo = Depends(ManufacturerRepo)) -> None: + def __init__( + self, + manufacturer_repository: ManufacturerRepo = Depends(ManufacturerRepo), + catalogue_item_repository: CatalogueItemRepo = Depends(CatalogueItemRepo), + ) -> None: """ Initialise the manufacturer service with a ManufacturerRepo :param manufacturer_repository: The `ManufacturerRepo` repository to use. + :param catalogue_item_repository: The `CatalogueItemRepo` to use. """ self._manufacturer_repository = manufacturer_repository + self._catalogue_item_repository = catalogue_item_repository def create(self, manufacturer: ManufacturerPostRequestSchema) -> ManufacturerOut: """ @@ -91,3 +98,26 @@ def update(self, manufacturer_id: str, manufacturer: ManufacturerPatchRequstSche logger.info(stored_manufacturer.address) return self._manufacturer_repository.update(manufacturer_id, ManufacturerIn(**stored_manufacturer.dict())) + + def delete(self, manufacturer_id: str) -> None: + """ + Delete a manufacturer by its ID + + :param manufacturer_id: The ID of the manufacturer to delete + + """ + if self.is_part_of_catalogue_item(manufacturer_id): + raise PartOfCatalogueItemError("The specified manufactuerer is a part of a Catalogue Item") + + return self._manufacturer_repository.delete(manufacturer_id) + + def is_part_of_catalogue_item(self, manufacturer_id: str) -> bool: + """Checks to see if any of the documents in the database have a specific manufactuer id + + :param manufacturer_id: The ID of the manufacturer that is looked for + :return Returns True if manufacturer is part of an item, false if not + """ + + logger.info("Checking to see if manufactuer with ID %s is a part of an item {manufacturer_id}") + + return self._catalogue_item_repository.is_manufacturer_in_catalogue_item(manufacturer_id) From 965bd2d641ecb750738df70fb4424115ed8fd21f Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Thu, 5 Oct 2023 11:56:35 +0100 Subject: [PATCH 6/9] unit tests #64 --- .../repositories/catalogue_item.py | 10 ---- .../repositories/manufacturer.py | 21 ++++++++- .../services/manufacturer.py | 21 +-------- test/unit/repositories/test_manufacturer.py | 46 ++++++++++++++++++- test/unit/services/conftest.py | 1 + test/unit/services/test_manufacturer.py | 10 +++- 6 files changed, 74 insertions(+), 35 deletions(-) diff --git a/inventory_management_system_api/repositories/catalogue_item.py b/inventory_management_system_api/repositories/catalogue_item.py index c15dd7f5..e5b07ba9 100644 --- a/inventory_management_system_api/repositories/catalogue_item.py +++ b/inventory_management_system_api/repositories/catalogue_item.py @@ -95,13 +95,3 @@ def _is_duplicate_catalogue_item(self, catalogue_category_id: str, name: str) -> catalogue_category_id = CustomObjectId(catalogue_category_id) count = self._collection.count_documents({"catalogue_category_id": catalogue_category_id, "name": name}) return count > 0 - - def is_manufacturer_in_catalogue_item(self, manufacturer_id: str) -> bool: - """Checks to see if any of the documents in the database have a specific manufactuer id - - :param manufacturer_id: The ID of the manufacturer that is looked for - :return Returns True if 1 or more documents have the manufacturer ID, false if none do - """ - manufacturer_id = CustomObjectId(manufacturer_id) - count = self._collection.count_documents({"manufacturer_id": manufacturer_id}) - return count > 0 diff --git a/inventory_management_system_api/repositories/manufacturer.py b/inventory_management_system_api/repositories/manufacturer.py index d62a8c64..af3a0302 100644 --- a/inventory_management_system_api/repositories/manufacturer.py +++ b/inventory_management_system_api/repositories/manufacturer.py @@ -10,7 +10,11 @@ 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, MissingRecordError +from inventory_management_system_api.core.exceptions import ( + DuplicateRecordError, + MissingRecordError, + PartOfCatalogueItemError, +) from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut @@ -28,6 +32,7 @@ def __init__(self, database: Database = Depends(get_database)) -> None: self._database = database self._collection: Collection = self._database.manufacturer + self._catalogue_item_collection: Collection = self._database.catalogue_items def create(self, manufacturer: ManufacturerIn) -> ManufacturerOut: """ @@ -113,11 +118,13 @@ def delete(self, manufacturer_id: str) -> None: :raises ExistIn """ manufacturer_id = CustomObjectId(manufacturer_id) + if self.is_manufacturer_in_catalogue_item(str(manufacturer_id)): + raise PartOfCatalogueItemError("The specified manufacturer is a part of a Catalogue Item") logger.info("Deleting manufacturer with ID %s from the database", manufacturer_id) result = self._collection.delete_one({"_id": manufacturer_id}) if result.deleted_count == 0: - raise MissingRecordError(f"No manufacturer founs with ID: {str(manufacturer_id)}") + raise MissingRecordError(f"No manufacturer found with ID: {str(manufacturer_id)}") def _is_duplicate_manufacturer(self, code: str) -> bool: """ @@ -129,3 +136,13 @@ def _is_duplicate_manufacturer(self, code: str) -> bool: logger.info("Checking if manufacturer with code '%s' already exists", code) count = self._collection.count_documents({"code": code}) return count > 0 + + def is_manufacturer_in_catalogue_item(self, manufacturer_id: str) -> bool: + """Checks to see if any of the documents in the database have a specific manufactuer id + + :param manufacturer_id: The ID of the manufacturer that is looked for + :return Returns True if 1 or more documents have the manufacturer ID, false if none do + """ + manufacturer_id = CustomObjectId(manufacturer_id) + count = self._catalogue_item_collection.count_documents({"manufacturer_id": manufacturer_id}) + return count > 0 diff --git a/inventory_management_system_api/services/manufacturer.py b/inventory_management_system_api/services/manufacturer.py index 79d77ae3..f36daf26 100644 --- a/inventory_management_system_api/services/manufacturer.py +++ b/inventory_management_system_api/services/manufacturer.py @@ -6,10 +6,8 @@ from typing import List, Optional from fastapi import Depends -from inventory_management_system_api.core.exceptions import MissingRecordError, PartOfCatalogueItemError +from inventory_management_system_api.core.exceptions import MissingRecordError from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut -from inventory_management_system_api.repositories.catalogue_item import CatalogueItemRepo - from inventory_management_system_api.repositories.manufacturer import ManufacturerRepo from inventory_management_system_api.schemas.manufacturer import ( ManufacturerPatchRequstSchema, @@ -25,17 +23,14 @@ class ManufacturerService: def __init__( self, manufacturer_repository: ManufacturerRepo = Depends(ManufacturerRepo), - catalogue_item_repository: CatalogueItemRepo = Depends(CatalogueItemRepo), ) -> None: """ Initialise the manufacturer service with a ManufacturerRepo :param manufacturer_repository: The `ManufacturerRepo` repository to use. - :param catalogue_item_repository: The `CatalogueItemRepo` to use. """ self._manufacturer_repository = manufacturer_repository - self._catalogue_item_repository = catalogue_item_repository def create(self, manufacturer: ManufacturerPostRequestSchema) -> ManufacturerOut: """ @@ -106,18 +101,4 @@ def delete(self, manufacturer_id: str) -> None: :param manufacturer_id: The ID of the manufacturer to delete """ - if self.is_part_of_catalogue_item(manufacturer_id): - raise PartOfCatalogueItemError("The specified manufactuerer is a part of a Catalogue Item") - return self._manufacturer_repository.delete(manufacturer_id) - - def is_part_of_catalogue_item(self, manufacturer_id: str) -> bool: - """Checks to see if any of the documents in the database have a specific manufactuer id - - :param manufacturer_id: The ID of the manufacturer that is looked for - :return Returns True if manufacturer is part of an item, false if not - """ - - logger.info("Checking to see if manufactuer with ID %s is a part of an item {manufacturer_id}") - - return self._catalogue_item_repository.is_manufacturer_in_catalogue_item(manufacturer_id) diff --git a/test/unit/repositories/test_manufacturer.py b/test/unit/repositories/test_manufacturer.py index 8edcf0e8..6d8f4074 100644 --- a/test/unit/repositories/test_manufacturer.py +++ b/test/unit/repositories/test_manufacturer.py @@ -10,6 +10,7 @@ DuplicateRecordError, InvalidObjectIdError, MissingRecordError, + PartOfCatalogueItemError, ) from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut @@ -310,10 +311,53 @@ def update_with_duplicate_name(test_helpers, database_mock, manufacturer_reposit address="1 Example street", ) # pylint: enable=duplicate-code - manufacturer_id = str(ObjectId()) test_helpers.mock_count_documents(database_mock.manufacturer, 1) with pytest.raises(DuplicateRecordError) as exc: manufacturer_repository.update(manufacturer_id, updated_manufacturer) assert str(exc.value) == "The specified manufacturer does not exist" + + +def test_delete(test_helpers, database_mock, manufacturer_repository): + """Test trying to delete a manufacturer""" + manufacturer_id = str(ObjectId()) + + test_helpers.mock_delete_one(database_mock.manufacturer, 1) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + manufacturer_repository.delete(manufacturer_id) + + database_mock.manufacturer.delete_one.assert_called_once_with({"_id": CustomObjectId(manufacturer_id)}) + + +def test_delete_with_an_invalid_id(manufacturer_repository): + """Test trying to delete a manufacturer with an invalid ID""" + manufacturer_id = "invalid" + + with pytest.raises(InvalidObjectIdError) as exc: + manufacturer_repository.delete(manufacturer_id) + assert str(exc.value) == "Invalid ObjectId value 'invalid'" + + +def test_delete_with_a_nonexistent_id(test_helpers, database_mock, manufacturer_repository): + """Test trying to delete a manufacturer with a non-existent ID""" + manufacturer_id = str(ObjectId()) + + test_helpers.mock_delete_one(database_mock.manufacturer, 0) + test_helpers.mock_count_documents(database_mock.catalogue_items, 0) + + with pytest.raises(MissingRecordError) as exc: + manufacturer_repository.delete(manufacturer_id) + assert str(exc.value) == f"No manufacturer found with ID: {manufacturer_id}" + database_mock.manufacturer.delete_one.assert_called_once_with({"_id": CustomObjectId(manufacturer_id)}) + + +def test_delete_manufacturer_that_is_part_of_an_catalogue_item(test_helpers, database_mock, manufacturer_repository): + """Test trying to delete a manufacturer that is part of a Catalogue Items""" + manufacturer_id = str(ObjectId()) + + test_helpers.mock_count_documents(database_mock.catalogue_items, 1) + + with pytest.raises(PartOfCatalogueItemError) as exc: + manufacturer_repository.delete(manufacturer_id) + assert str(exc.value) == "The specified manufacturer is a part of a Catalogue Item" diff --git a/test/unit/services/conftest.py b/test/unit/services/conftest.py index 8c9a333e..a084c0d6 100644 --- a/test/unit/services/conftest.py +++ b/test/unit/services/conftest.py @@ -82,6 +82,7 @@ def fixture_manufacturer_service(manufacturer_repository_mock: Mock) -> Manufact """ return ManufacturerService(manufacturer_repository_mock) + class ServiceTestHelpers: """ A utility class containing common helper methods for the service tests. diff --git a/test/unit/services/test_manufacturer.py b/test/unit/services/test_manufacturer.py index 629abd7f..65b6ce2d 100644 --- a/test/unit/services/test_manufacturer.py +++ b/test/unit/services/test_manufacturer.py @@ -2,8 +2,6 @@ Unit tests for the `ManufacturerService` service. """ -from unittest.mock import Mock -import pytest from bson import ObjectId from inventory_management_system_api.models.manufacturer import ManufacturerIn, ManufacturerOut @@ -99,3 +97,11 @@ def test_get_with_nonexistent_id(manufacturer_repository_mock, manufacturer_serv assert retrieved_manufacturer is None manufacturer_repository_mock.get.assert_called_once_with(manufactuer_id) + + +def test_delete(manufacturer_repository_mock, manufacturer_service): + """Test deleting a manufacturer""" + manufacturer_id = str(ObjectId()) + manufacturer_service.delete(manufacturer_id) + + manufacturer_repository_mock.delete.assert_called_once_with(manufacturer_id) From 2362702201af4a46f1fde06f0173659727170060 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Thu, 5 Oct 2023 13:37:01 +0100 Subject: [PATCH 7/9] e2e tests #64 --- .../routers/v1/manufacturer.py | 4 +- test/e2e/test_manufacturer.py | 85 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/inventory_management_system_api/routers/v1/manufacturer.py b/inventory_management_system_api/routers/v1/manufacturer.py index 874a2f2d..4bd017a2 100644 --- a/inventory_management_system_api/routers/v1/manufacturer.py +++ b/inventory_management_system_api/routers/v1/manufacturer.py @@ -130,7 +130,7 @@ def delete_manufacturer(manufacturer_id: str, manufacturer_service: Manufacturer status_code=status.HTTP_404_NOT_FOUND, detail="The specified manufacturer does not exist" ) from exc except PartOfCatalogueItemError as exc: - logger.exception("The specified manufactuerer is a part of a Catalogue Item") + logger.exception("The specified manufacturer is a part of a Catalogue Item") raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail="The specified manufactuerer is a part of a Catalogue Item" + status_code=status.HTTP_409_CONFLICT, detail="The specified manufacturer is a part of a Catalogue Item" ) from exc diff --git a/test/e2e/test_manufacturer.py b/test/e2e/test_manufacturer.py index d0d19113..c01dbbf2 100644 --- a/test/e2e/test_manufacturer.py +++ b/test/e2e/test_manufacturer.py @@ -207,3 +207,88 @@ def test_update_duplicate_name(test_client): assert response.status_code == 409 assert response.json()["detail"] == "A manufacturer with the same name has been found" + + +def test_delete(test_client): + """Test deleting a manufacturer""" + manufacturer_post = { + "name": "Manufacturer A", + "url": "http://example.com", + "address": "Street A", + } + + response = test_client.post("/v1/manufacturer", json=manufacturer_post) + manufacturer = response.json() + + response = test_client.delete(f"/v1/manufacturer/{manufacturer['id']}") + assert response.status_code == 204 + + +def test_delete_with_an_invalid_id(test_client): + """Test trying to delete a manufacturer with an invalid ID""" + manufacturer_post = { + "name": "Manufacturer A", + "url": "http://example.com", + "address": "Street A", + } + test_client.post("/v1/manufacturer", json=manufacturer_post) + + response = test_client.delete("/v1/manufacturer/invalid") + + assert response.status_code == 404 + assert response.json()["detail"] == "The specified manufacturer does not exist" + + +def test_delete_with_a_nonexistent_id(test_client): + """Test trying to delete a manufacturer with a non-existent ID""" + manufacturer_post = { + "name": "Manufacturer A", + "url": "http://example.com", + "address": "Street A", + } + test_client.post("/v1/manufacturer", json=manufacturer_post) + + response = test_client.delete(f"/v1/manufacturer/{str(ObjectId())}") + + assert response.status_code == 404 + assert response.json()["detail"] == "The specified manufacturer does not exist" + + +def test_delete_manufacturer_that_is_a_part_of_catalogue_item(test_client): + """Test trying to delete a manufacturer that is a part of a Catalogue Item""" + manufacturer_post = { + "name": "Manufacturer A", + "url": "http://example.com", + "address": "Street A", + } + response = test_client.post("/v1/manufacturer", json=manufacturer_post) + manufacturer_id = response.json()["id"] + # pylint: disable=duplicate-code + catalogue_category_post = { + "name": "Category A", + "is_leaf": True, + "catalogue_item_properties": [ + {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + {"name": "Property B", "type": "boolean", "mandatory": True}, + ], + } + # pylint: enable=duplicate-code + response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + # pylint: disable=duplicate-code + catalogue_category_id = response.json()["id"] + + catalogue_item_post = { + "catalogue_category_id": catalogue_category_id, + "name": "Catalogue Item A", + "description": "This is Catalogue Item A", + "properties": [{"name": "Property B", "value": False}], + "manufacturer_id": manufacturer_id, + } + # pylint: enable=duplicate-code + test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + response = test_client.delete(f"/v1/manufacturer/{manufacturer_id}") + + assert response.status_code == 409 + assert response.json()["detail"] == "The specified manufacturer is a part of a Catalogue Item" From 5c05a338e306032b0730485935de56fd709713bc Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Mon, 23 Oct 2023 14:19:50 +0100 Subject: [PATCH 8/9] revert manufacturer to manufacturer ID changes #64 --- .../models/catalogue_item.py | 15 ++- .../schemas/catalogue_item.py | 18 ++- test/e2e/test_catalogue_category.py | 30 ++++- test/e2e/test_catalogue_item.py | 20 +++- test/e2e/test_manufacturer.py | 77 +++++++------ test/unit/repositories/conftest.py | 3 + test/unit/repositories/test_catalogue_item.py | 47 +++++--- test/unit/services/conftest.py | 1 + test/unit/services/test_catalogue_item.py | 109 +++++++++++++----- 9 files changed, 223 insertions(+), 97 deletions(-) diff --git a/inventory_management_system_api/models/catalogue_item.py b/inventory_management_system_api/models/catalogue_item.py index 0cb5600e..7b88dd39 100644 --- a/inventory_management_system_api/models/catalogue_item.py +++ b/inventory_management_system_api/models/catalogue_item.py @@ -3,7 +3,7 @@ """ from typing import Optional, List, Any -from pydantic import BaseModel, Field, validator +from pydantic import BaseModel, Field, HttpUrl, validator from inventory_management_system_api.models.custom_object_id_data_types import CustomObjectIdField, StringObjectIdField @@ -18,6 +18,14 @@ class Property(BaseModel): unit: Optional[str] = None +class Manufacturer(BaseModel): + """Input database model for a manufacturer""" + + name: str + url: HttpUrl + address: str + + class CatalogueItemIn(BaseModel): """ Input database model for a catalogue item. @@ -27,7 +35,9 @@ class CatalogueItemIn(BaseModel): name: str description: str properties: List[Property] = [] - manufacturer_id: CustomObjectIdField + # pylint: disable=fixme + # TODO - Change from manufacturer to manufacturer id + manufacturer: Manufacturer @validator("properties", pre=True, always=True) @classmethod @@ -53,7 +63,6 @@ class CatalogueItemOut(CatalogueItemIn): id: StringObjectIdField = Field(alias="_id") catalogue_category_id: StringObjectIdField - manufacturer_id: StringObjectIdField class Config: # pylint: disable=C0115 diff --git a/inventory_management_system_api/schemas/catalogue_item.py b/inventory_management_system_api/schemas/catalogue_item.py index 4d3b567a..ec3ee849 100644 --- a/inventory_management_system_api/schemas/catalogue_item.py +++ b/inventory_management_system_api/schemas/catalogue_item.py @@ -3,7 +3,15 @@ """ from typing import List, Any, Optional -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, HttpUrl + + +class ManufacturerPostRequestSchema(BaseModel): + """Schema model for a manufacturer creation request""" + + name: str + url: HttpUrl + address: str class PropertyPostRequestSchema(BaseModel): @@ -34,7 +42,9 @@ class CatalogueItemPostRequestSchema(BaseModel): name: str = Field(description="The name of the catalogue item") description: str = Field(description="The catalogue item description") properties: Optional[List[PropertyPostRequestSchema]] = Field(description="The catalogue item properties") - manufacturer_id: str = Field(description="The ID of the manufacturer") + # pylint: disable=fixme + # TODO - Change from manufacturer to manufacturer id + manufacturer: ManufacturerPostRequestSchema = Field(description="The details of the manufacturer") class CatalogueItemPatchRequestSchema(CatalogueItemPostRequestSchema): @@ -47,7 +57,9 @@ class CatalogueItemPatchRequestSchema(CatalogueItemPostRequestSchema): ) 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") + # pylint: disable=fixme + # TODO - Change from manufacturer to manufacturer id + manufacturer: Optional[ManufacturerPostRequestSchema] = Field(description="The details of the manufacturer") class CatalogueItemSchema(CatalogueItemPostRequestSchema): diff --git a/test/e2e/test_catalogue_category.py b/test/e2e/test_catalogue_category.py index c054f035..9935693b 100644 --- a/test/e2e/test_catalogue_category.py +++ b/test/e2e/test_catalogue_category.py @@ -307,7 +307,11 @@ def test_delete_catalogue_category_with_children_catalogue_items(test_client): "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -587,7 +591,11 @@ def test_partial_update_catalogue_category_change_name_has_children_catalogue_it "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -714,7 +722,11 @@ def test_partial_update_catalogue_category_change_from_leaf_to_non_leaf_has_chil "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -835,7 +847,11 @@ def test_partial_update_catalogue_category_change_parent_id_has_children_catalog "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) @@ -1050,7 +1066,11 @@ def test_partial_update_catalogue_category_change_catalogue_item_properties_has_ "name": "Catalogue Item A", "description": "This is Catalogue Item A", "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } test_client.post("/v1/catalogue-items", json=catalogue_item_post) diff --git a/test/e2e/test_catalogue_item.py b/test/e2e/test_catalogue_item.py index 0c25f997..40834ed5 100644 --- a/test/e2e/test_catalogue_item.py +++ b/test/e2e/test_catalogue_item.py @@ -41,7 +41,11 @@ def get_catalogue_item_a_dict(catalogue_category_id: str) -> Dict: {"name": "Property B", "value": False}, {"name": "Property C", "value": "20x15x10"}, ], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } @@ -59,7 +63,11 @@ def get_catalogue_item_b_dict(catalogue_category_id: str) -> Dict: "properties": [ {"name": "Property A", "value": True}, ], - "manufacturer_id": str(ObjectId()), + "manufacturer": { + "name": "Manufacturer A", + "address": "1 Address, City, Country, Postcode", + "url": "https://www.manufacturer-a.co.uk", + }, } @@ -85,7 +93,7 @@ def test_create_catalogue_item(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] - assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_invalid_catalogue_category_id(test_client): @@ -145,7 +153,7 @@ def test_create_catalogue_item_without_properties(test_client): assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == [] - assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_missing_mandatory_properties(test_client): @@ -185,7 +193,7 @@ def test_create_catalogue_item_with_missing_non_mandatory_properties(test_client assert catalogue_item["name"] == catalogue_item_post["name"] assert catalogue_item["description"] == catalogue_item_post["description"] assert catalogue_item["properties"] == catalogue_item_post["properties"] - assert catalogue_item["manufacturer_id"] == catalogue_item_post["manufacturer_id"] + assert catalogue_item["manufacturer"] == catalogue_item_post["manufacturer"] def test_create_catalogue_item_with_invalid_value_type_for_string_property(test_client): @@ -823,7 +831,7 @@ def test_partial_update_catalogue_item_change_manufacturer(test_client): "manufacturer": { "name": "Manufacturer B", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-b.co.uk", + "url": "https://www.manufacturer-b.co.uk", } } response = test_client.patch(f"/v1/catalogue-items/{response.json()['id']}", json=catalogue_item_patch) diff --git a/test/e2e/test_manufacturer.py b/test/e2e/test_manufacturer.py index c01dbbf2..fa6399c8 100644 --- a/test/e2e/test_manufacturer.py +++ b/test/e2e/test_manufacturer.py @@ -254,41 +254,44 @@ def test_delete_with_a_nonexistent_id(test_client): assert response.json()["detail"] == "The specified manufacturer does not exist" -def test_delete_manufacturer_that_is_a_part_of_catalogue_item(test_client): +def test_delete_manufacturer_that_is_a_part_of_catalogue_item(): """Test trying to delete a manufacturer that is a part of a Catalogue Item""" - manufacturer_post = { - "name": "Manufacturer A", - "url": "http://example.com", - "address": "Street A", - } - response = test_client.post("/v1/manufacturer", json=manufacturer_post) - manufacturer_id = response.json()["id"] - # pylint: disable=duplicate-code - catalogue_category_post = { - "name": "Category A", - "is_leaf": True, - "catalogue_item_properties": [ - {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, - {"name": "Property B", "type": "boolean", "mandatory": True}, - ], - } - # pylint: enable=duplicate-code - response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) - - # pylint: disable=duplicate-code - catalogue_category_id = response.json()["id"] - - catalogue_item_post = { - "catalogue_category_id": catalogue_category_id, - "name": "Catalogue Item A", - "description": "This is Catalogue Item A", - "properties": [{"name": "Property B", "value": False}], - "manufacturer_id": manufacturer_id, - } - # pylint: enable=duplicate-code - test_client.post("/v1/catalogue-items", json=catalogue_item_post) - - response = test_client.delete(f"/v1/manufacturer/{manufacturer_id}") - - assert response.status_code == 409 - assert response.json()["detail"] == "The specified manufacturer is a part of a Catalogue Item" + # pylint: disable=fixme + # TODO - Uncomment test when catalogue item logic changes back to using manufacturer Id + + # manufacturer_post = { + # "name": "Manufacturer A", + # "url": "http://example.com", + # "address": "Street A", + # } + # response = test_client.post("/v1/manufacturer", json=manufacturer_post) + # manufacturer_id = response.json()["id"] + # # pylint: disable=duplicate-code + # catalogue_category_post = { + # "name": "Category A", + # "is_leaf": True, + # "catalogue_item_properties": [ + # {"name": "Property A", "type": "number", "unit": "mm", "mandatory": False}, + # {"name": "Property B", "type": "boolean", "mandatory": True}, + # ], + # } + # # pylint: enable=duplicate-code + # response = test_client.post("/v1/catalogue-categories", json=catalogue_category_post) + + # # pylint: disable=duplicate-code + # catalogue_category_id = response.json()["id"] + + # catalogue_item_post = { + # "catalogue_category_id": catalogue_category_id, + # "name": "Catalogue Item A", + # "description": "This is Catalogue Item A", + # "properties": [{"name": "Property B", "value": False}], + # "manufacturer": manufacturer_post, + # } + # # pylint: enable=duplicate-code + # test_client.post("/v1/catalogue-items", json=catalogue_item_post) + + # response = test_client.delete(f"/v1/manufacturer/{manufacturer_id}") + + # assert response.status_code == 409 + # assert response.json()["detail"] == "The specified manufacturer is a part of a Catalogue Item" diff --git a/test/unit/repositories/conftest.py b/test/unit/repositories/conftest.py index fd7dcbf1..9471a952 100644 --- a/test/unit/repositories/conftest.py +++ b/test/unit/repositories/conftest.py @@ -61,6 +61,8 @@ def fixture_manufacturer_repository(database_mock: Mock) -> ManufacturerRepo: Fixture to create ManufacturerRepo instance """ return ManufacturerRepo(database_mock) + + @pytest.fixture(name="system_repository") def fixture_system_repository(database_mock: Mock) -> SystemRepo: """ @@ -164,6 +166,7 @@ def mock_update_one(collection_mock: Mock) -> None: update_one_result_mock.acknowledged = True collection_mock.insert_one.return_value = update_one_result_mock + @pytest.fixture(name="test_helpers") def fixture_test_helpers() -> Type[RepositoryTestHelpers]: """ diff --git a/test/unit/repositories/test_catalogue_item.py b/test/unit/repositories/test_catalogue_item.py index 720f1440..48373cae 100644 --- a/test/unit/repositories/test_catalogue_item.py +++ b/test/unit/repositories/test_catalogue_item.py @@ -12,6 +12,7 @@ CatalogueItemOut, Property, CatalogueItemIn, + Manufacturer, ) @@ -33,7 +34,11 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -50,7 +55,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer_id": catalogue_item.manufacturer_id, + "manufacturer": catalogue_item.manufacturer, }, ) @@ -61,7 +66,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer_id=catalogue_item.manufacturer_id, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -72,7 +77,7 @@ def test_create(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer_id": CustomObjectId(catalogue_item.manufacturer_id), + "manufacturer": catalogue_item.manufacturer, } ) assert created_catalogue_item == catalogue_item @@ -155,7 +160,11 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -168,7 +177,7 @@ def test_get(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer_id": catalogue_item.manufacturer_id, + "manufacturer": catalogue_item.manufacturer, }, ) @@ -223,7 +232,11 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) catalogue_item_b = CatalogueItemOut( @@ -232,7 +245,11 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -246,7 +263,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_a.name, "description": catalogue_item_a.description, "properties": catalogue_item_a.properties, - "manufacturer_id": catalogue_item_a.manufacturer_id, + "manufacturer": catalogue_item_a.manufacturer, }, { "_id": CustomObjectId(catalogue_item_b.id), @@ -254,7 +271,7 @@ def test_list(test_helpers, database_mock, catalogue_item_repository): "name": catalogue_item_b.name, "description": catalogue_item_b.description, "properties": catalogue_item_b.properties, - "manufacturer_id": catalogue_item_b.manufacturer_id, + "manufacturer": catalogue_item_b.manufacturer, }, ], ) @@ -282,7 +299,11 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # Mock `find` to return a list of catalogue item documents @@ -295,7 +316,7 @@ def test_list_with_catalogue_category_id_filter(test_helpers, database_mock, cat "name": catalogue_item.name, "description": catalogue_item.description, "properties": catalogue_item.properties, - "manufacturer_id": catalogue_item.manufacturer_id, + "manufacturer": catalogue_item.manufacturer, } ], ) @@ -360,7 +381,7 @@ def test_update(test_helpers, database_mock, catalogue_item_repository): "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, } # pylint: enable=duplicate-code diff --git a/test/unit/services/conftest.py b/test/unit/services/conftest.py index d7f1252d..5eb81b6c 100644 --- a/test/unit/services/conftest.py +++ b/test/unit/services/conftest.py @@ -47,6 +47,7 @@ def fixture_manufacturer_repository_mock() -> Mock: """ return Mock(ManufacturerRepo) + @pytest.fixture(name="system_repository_mock") def fixture_system_repository_mock() -> Mock: """ diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index 7bb7c85c..7934196b 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -15,6 +15,7 @@ from inventory_management_system_api.models.catalogue_category import CatalogueCategoryOut, CatalogueItemProperty from inventory_management_system_api.models.catalogue_item import ( CatalogueItemOut, + Manufacturer, Property, CatalogueItemIn, ) @@ -46,7 +47,11 @@ def test_create( Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # Mock `get` to return the catalogue category @@ -81,7 +86,7 @@ def test_create( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=catalogue_item.manufacturer_id, + manufacturer=catalogue_item.manufacturer, ) ) @@ -93,7 +98,7 @@ def test_create( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer_id=catalogue_item.manufacturer_id, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -125,7 +130,11 @@ def test_create_with_nonexistent_catalogue_category_id( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ), ) @@ -170,7 +179,11 @@ def test_create_in_non_leaf_catalogue_category( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ), ) @@ -193,7 +206,11 @@ def test_create_without_properties( name="Catalogue Item A", description="This is Catalogue Item A", properties=[], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -222,7 +239,7 @@ def test_create_without_properties( catalogue_category_id=catalogue_item.catalogue_category_id, name=catalogue_item.name, description=catalogue_item.description, - manufacturer_id=catalogue_item.manufacturer_id, + manufacturer=catalogue_item.manufacturer, ) ) @@ -234,7 +251,7 @@ def test_create_without_properties( name=catalogue_item.name, description=catalogue_item.description, properties=catalogue_item.properties, - manufacturer_id=catalogue_item.manufacturer_id, + manufacturer=catalogue_item.manufacturer, ) ) # pylint: enable=duplicate-code @@ -280,7 +297,11 @@ def test_create_with_missing_mandatory_properties( properties=[ PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ), ) @@ -333,7 +354,11 @@ def test_create_with_with_invalid_value_type_for_string_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value=True), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ), ) @@ -387,7 +412,11 @@ def test_create_with_with_invalid_value_type_for_number_property( PropertyPostRequestSchema(name="Property B", value=False), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -440,7 +469,11 @@ def test_create_with_with_invalid_value_type_for_boolean_property( PropertyPostRequestSchema(name="Property B", value="False"), PropertyPostRequestSchema(name="Property C", value="20x15x10"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) ) assert ( @@ -481,7 +514,11 @@ def test_get(test_helpers, catalogue_item_repository_mock, catalogue_item_servic Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -528,7 +565,11 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) catalogue_item_b = CatalogueItemOut( @@ -537,7 +578,11 @@ def test_list(test_helpers, catalogue_item_repository_mock, catalogue_item_servi name="Catalogue Item B", description="This is Catalogue Item B", properties=[Property(name="Property A", value=True)], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -568,7 +613,11 @@ def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_rep Property(name="Property B", value=False), Property(name="Property C", value="20x15x10", unit="cm"), ], - manufacturer_id=str(ObjectId()), + manufacturer=Manufacturer( + name="Manufacturer A", + address="1 Address, City, Country, Postcode", + url="https://www.manufacturer-a.co.uk", + ), ) # pylint: enable=duplicate-code @@ -619,7 +668,7 @@ def test_update(test_helpers, catalogue_item_repository_mock, catalogue_item_ser "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, } # pylint: enable=duplicate-code @@ -687,7 +736,7 @@ def test_update_change_catalogue_category_id_same_defined_properties_without_sup "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, } full_catalogue_item_info = { @@ -749,7 +798,7 @@ def test_update_change_catalogue_category_id_same_defined_properties_with_suppli "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, } full_catalogue_item_info = { @@ -825,7 +874,7 @@ def test_update_change_catalogue_category_id_different_defined_properties_withou "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -882,7 +931,7 @@ def test_update_change_catalogue_category_id_different_defined_properties_with_s "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, } full_catalogue_item_info = { @@ -954,7 +1003,7 @@ def test_update_with_nonexistent_catalogue_category_id( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [], } @@ -987,7 +1036,7 @@ def test_update_change_catalogue_category_id_non_leaf_catalogue_category( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [], } @@ -1034,7 +1083,7 @@ def test_update_add_non_mandatory_property( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property B", "value": False}, @@ -1100,7 +1149,7 @@ def test_update_remove_non_mandatory_property( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -1164,7 +1213,7 @@ def test_update_remove_mandatory_property( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -1222,7 +1271,7 @@ def test_update_change_property_value( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -1289,7 +1338,7 @@ def test_update_change_value_for_string_property_invalid_type( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -1346,7 +1395,7 @@ def test_update_change_value_for_number_property_invalid_type( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, @@ -1403,7 +1452,7 @@ def test_update_change_value_for_boolean_property_invalid_type( "manufacturer": { "name": "Manufacturer A", "address": "1 Address, City, Country, Postcode", - "web_url": "https://www.manufacturer-a.co.uk", + "url": "https://www.manufacturer-a.co.uk", }, "properties": [ {"name": "Property A", "value": 20, "unit": "mm"}, From aa4723a94f1eb2680bbc8768bf834056463d5131 Mon Sep 17 00:00:00 2001 From: Matteo Guarnaccia Date: Mon, 30 Oct 2023 12:04:07 +0000 Subject: [PATCH 9/9] re-fixed merge conflicts so tests work --- test/unit/services/test_catalogue_item.py | 94 +---------------------- 1 file changed, 1 insertion(+), 93 deletions(-) diff --git a/test/unit/services/test_catalogue_item.py b/test/unit/services/test_catalogue_item.py index d6985a85..3e0db937 100644 --- a/test/unit/services/test_catalogue_item.py +++ b/test/unit/services/test_catalogue_item.py @@ -535,103 +535,12 @@ def test_get_with_non_existent_id(test_helpers, catalogue_item_repository_mock, catalogue_item_repository_mock.get.assert_called_once_with(catalogue_item_id) -def test_list(catalogue_item_repository_mock, catalogue_item_service, test_helpers): +def test_list(catalogue_item_repository_mock, catalogue_item_service): """ Test listing catalogue items Verify that the `list` method properly calls the repository function with any passed filters """ - # pylint: disable=duplicate-code - catalogue_item_a = 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", - url="https://www.manufacturer-a.co.uk", - ), - ) - - catalogue_item_b = CatalogueItemOut( - id=str(ObjectId()), - catalogue_category_id=str(ObjectId()), - name="Catalogue Item B", - description="This is Catalogue Item B", - properties=[Property(name="Property A", value=True)], - manufacturer=Manufacturer( - name="Manufacturer A", - address="1 Address, City, Country, Postcode", - url="https://www.manufacturer-a.co.uk", - ), - ) - # pylint: enable=duplicate-code - - # Mock `list` to return a list of catalogue items - test_helpers.mock_list(catalogue_item_repository_mock, [catalogue_item_a, catalogue_item_b]) - - retrieved_catalogue_items = catalogue_item_service.list(None) - - catalogue_item_repository_mock.list.assert_called_once_with(None) - assert retrieved_catalogue_items == [catalogue_item_a, catalogue_item_b] - - -def test_list_with_catalogue_category_id_filter(test_helpers, catalogue_item_repository_mock, catalogue_item_service): - """ - Test getting catalogue items based on the provided catalogue category ID filter. - - Verify that the `list` method properly handles the retrieval of catalogue items based on the provided catalogue - category ID filter. - """ - # 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", - url="https://www.manufacturer-a.co.uk", - ), - ) - # pylint: enable=duplicate-code - - # Mock `list` to return a list of catalogue items - test_helpers.mock_list(catalogue_item_repository_mock, [catalogue_item]) - - retrieved_catalogue_items = catalogue_item_service.list(catalogue_item.catalogue_category_id) - - catalogue_item_repository_mock.list.assert_called_once_with(catalogue_item.catalogue_category_id) - assert retrieved_catalogue_items == [catalogue_item] - - -def test_list_with_catalogue_category_id_filter_no_matching_results( - test_helpers, catalogue_item_repository_mock, catalogue_item_service -): - """ - Test getting catalogue items based on the provided catalogue category ID filter when there is no matching results in - the database. - - Verify that the `list` method properly handles the retrieval of catalogue items based on the provided catalogue - category ID filter. - """ - # Mock `list` to return an empty list of catalogue item documents - test_helpers.mock_list(catalogue_item_repository_mock, []) - - catalogue_category_id = str(ObjectId()) - retrieved_catalogue_items = catalogue_item_service.list(catalogue_category_id) catalogue_category_id = MagicMock() @@ -640,7 +549,6 @@ def test_list_with_catalogue_category_id_filter_no_matching_results( catalogue_item_repository_mock.list.assert_called_once_with(catalogue_category_id) assert result == catalogue_item_repository_mock.list.return_value - def test_update(test_helpers, catalogue_item_repository_mock, catalogue_item_service): """ Test updating a catalogue item.