Address review comments, replacing set with update #417

inventory_management_system_api/repositories/usage_status.py

@@ -122,7 +122,7 @@ def _is_duplicate_usage_status(
         )
         return usage_status is not None
 
-    def _is_usage_status_in_item(self, usage_status_id: str, session: ClientSession = None) -> bool:
+    def _is_usage_status_in_item(self, usage_status_id: CustomObjectId, session: ClientSession = None) -> bool:
         """Checks to see if any of the items in the database have a specific usage status ID.
 
         :param usage_status_id: The ID of the usage status that is looked for.

inventory_management_system_api/routers/v1/setting.py

@@ -20,18 +20,18 @@
 
 @router.put(
     path="/spares_definition",
-    summary="Set the definition of a spare",
+    summary="Update the definition of a spare",
     response_description="Spares definition updated successfully",
 )
-def set_spares_definition(
+def update_spares_definition(
     spares_definition: SparesDefinitionPutSchema, setting_service: SettingServiceDep
 ) -> SparesDefinitionSchema:
     # pylint: disable=missing-function-docstring
-    logger.info("Setting spares definition")
+    logger.info("Updating spares definition")
     logger.debug("Spares definition data: %s", spares_definition)
 
     try:
-        updated_spares_definition = setting_service.set_spares_definition(spares_definition)
+        updated_spares_definition = setting_service.update_spares_definition(spares_definition)
         return SparesDefinitionSchema(**updated_spares_definition.model_dump())
     except (MissingRecordError, InvalidObjectIdError) as exc:
         message = "A specified usage status does not exist"

inventory_management_system_api/schemas/catalogue_category.py

@@ -6,7 +6,7 @@
 from numbers import Number
 from typing import Annotated, Any, List, Literal, Optional
 
-from pydantic import BaseModel, Field, conlist, field_validator
+from pydantic import BaseModel, Field, field_validator
 from pydantic_core.core_schema import ValidationInfo
 
 from inventory_management_system_api.schemas.mixins import CreatedModifiedSchemaMixin
@@ -28,7 +28,7 @@ class AllowedValuesListSchema(BaseModel):
     """
 
     type: Literal["list"]
-    values: conlist(Any, min_length=1)
+    values: list[Any] = Field(description="Value within the allowed values list", min_length=1)
 
 
 # Use discriminated union for any additional types of allowed values (so can use Pydantic's validation)

inventory_management_system_api/schemas/setting.py

@@ -2,7 +2,7 @@
 Module for defining the API schema models for representing settings.
 """
 
-from pydantic import BaseModel, Field, conlist, field_validator
+from pydantic import BaseModel, Field, field_validator
 
 from inventory_management_system_api.schemas.usage_status import UsageStatusSchema
 
@@ -15,21 +15,13 @@ class SparesDefinitionPutUsageStatusSchema(BaseModel):
     id: str = Field(description="The ID of the usage status.")
 
 
-class SparesDefinitionUsageStatusSchema(SparesDefinitionPutUsageStatusSchema):
-    """
-    Schema model for a usage status within a spares definition.
-    """
-
-    id: str = Field(description="The ID of the usage status.")
-
-
 class SparesDefinitionPutSchema(BaseModel):
     """
     Schema model for a spares definition update request.
     """
 
-    usage_statuses: conlist(SparesDefinitionPutUsageStatusSchema, min_length=1) = Field(
-        description="Usage statuses that classify items as a spare."
+    usage_statuses: list[SparesDefinitionPutUsageStatusSchema] = Field(
+        description="Usage statuses that classify items as a spare.", min_length=1
     )
 
     @field_validator("usage_statuses")

inventory_management_system_api/services/setting.py

@@ -35,9 +35,9 @@ def __init__(
         self._setting_repository = setting_repository
         self._usage_status_repository = usage_status_repository
 
-    def set_spares_definition(self, spares_definition: SparesDefinitionPutSchema) -> SparesDefinitionOut:
+    def update_spares_definition(self, spares_definition: SparesDefinitionPutSchema) -> SparesDefinitionOut:
         """
-        Sets the spares definition to a new value.
+        Updates the spares definition to a new value.
 
         :param spares_definition: The new spares definition.
         :return: The updated spares definition.

test/e2e/test_setting.py

@@ -17,8 +17,8 @@
 from httpx import Response
 
 
-class SetSparesDefinitionDSL(UsageStatusCreateDSL):
-    """Base class for set spares definition tests."""
+class UpdateSparesDefinitionDSL(UsageStatusCreateDSL):
+    """Base class for update spares definition tests."""
 
     _put_response_spares_definition: Response
 
@@ -90,36 +90,36 @@ def check_put_spares_definition_failed_with_validation_message(self, status_code
         assert self._put_response_spares_definition.json()["detail"][0]["msg"] == message
 
 
-class TestSetSparesDefinition(SetSparesDefinitionDSL):
-    """Tests for setting the spares definition."""
+class TestUpdateSparesDefinition(UpdateSparesDefinitionDSL):
+    """Tests for updating the spares definition."""
 
-    def test_set_spares_definition(self):
-        """Test setting the spares definition (for the first time)."""
+    def test_update_spares_definition(self):
+        """Test updating the spares definition (for the first time)."""
 
         self.put_spares_definition_and_post_prerequisites(SETTING_SPARES_DEFINITION_DATA_NEW_USED)
 
         self.check_put_spares_definition_success(SETTING_SPARES_DEFINITION_GET_DATA_NEW_USED)
 
-    def test_set_spares_definition_overwrite(self):
-        """Test setting the spares definition (when it has already been assigned before)."""
+    def test_update_spares_definition_overwrite(self):
+        """Test updating the spares definition (when it has already been assigned before)."""
 
         self.put_spares_definition_and_post_prerequisites(SETTING_SPARES_DEFINITION_DATA_NEW_USED)
         self.check_put_spares_definition_success(SETTING_SPARES_DEFINITION_GET_DATA_NEW_USED)
 
         self.put_spares_definition_and_post_prerequisites(SETTING_SPARES_DEFINITION_DATA_NEW)
         self.check_put_spares_definition_success(SETTING_SPARES_DEFINITION_GET_DATA_NEW)
 
-    def test_set_spares_definition_with_no_usage_statuses(self):
-        """Test setting the spares definition with no usage statuses given."""
+    def test_update_spares_definition_with_no_usage_statuses(self):
+        """Test updating the spares definition with no usage statuses given."""
 
         self.put_spares_definition({**SETTING_SPARES_DEFINITION_DATA_NEW_USED, "usage_statuses": []})
 
         self.check_put_spares_definition_failed_with_validation_message(
             422, "List should have at least 1 item after validation, not 0"
         )
 
-    def test_set_spares_definition_with_duplicate_usage_statuses(self):
-        """Test setting the spares definition with no usage statuses given."""
+    def test_update_spares_definition_with_duplicate_usage_statuses(self):
+        """Test updating the spares definition with no usage statuses given."""
 
         self.put_spares_definition_and_post_prerequisites(
             {
@@ -137,17 +137,17 @@ def test_set_spares_definition_with_duplicate_usage_statuses(self):
             f"{self.usage_status_value_id_dict[SETTING_SPARES_DEFINITION_DATA_NEW_USED['usage_statuses'][0]['value']]}",
         )
 
-    def test_set_spares_definition_with_non_existent_usage_status_id(self):
-        """Test setting the spares definition when there is a non-existent usage status ID."""
+    def test_update_spares_definition_with_non_existent_usage_status_id(self):
+        """Test updating the spares definition when there is a non-existent usage status ID."""
 
         self.post_usage_status(USAGE_STATUS_POST_DATA_NEW)
         self.add_usage_status_value_and_id(USAGE_STATUS_POST_DATA_USED["value"], str(ObjectId()))
         self.put_spares_definition(SETTING_SPARES_DEFINITION_DATA_NEW_USED)
 
         self.check_put_spares_definition_failed_with_detail(422, "A specified usage status does not exist")
 
-    def test_set_spares_definition_with_invalid_usage_status_id(self):
-        """Test setting the spares definition when there is an invalid usage status ID."""
+    def test_update_spares_definition_with_invalid_usage_status_id(self):
+        """Test updating the spares definition when there is an invalid usage status ID."""
 
         self.post_usage_status(USAGE_STATUS_POST_DATA_NEW)
         self.add_usage_status_value_and_id(USAGE_STATUS_POST_DATA_USED["value"], "invalid-id")

test/unit/services/test_setting.py

@@ -37,20 +37,20 @@ def setup(self, setting_repository_mock, usage_status_repository_mock, setting_s
         self.setting_service = setting_service
 
 
-class SetSparesDefinitionDSL(SettingServiceDSL):
-    """Base class for `set_spares_definition` tests."""
+class UpdateSparesDefinitionDSL(SettingServiceDSL):
+    """Base class for `update_spares_definition` tests."""
 
     _spares_definition_put: SparesDefinitionPutSchema
     _expected_spares_definition_in: SparesDefinitionIn
     _expected_spares_definition_out: MagicMock
-    _set_spares_definition: MagicMock
-    _set_spares_definition_exception: pytest.ExceptionInfo
+    _updated_spares_definition: MagicMock
+    _update_spares_definition_exception: pytest.ExceptionInfo
 
-    def mock_set_spares_definition(
+    def mock_update_spares_definition(
         self, spares_definition_data: dict, usage_statuses_out_data: list[Optional[dict]]
     ) -> None:
         """
-        Mocks repository methods appropriately to test the `set_spares_definition` service method.
+        Mocks repository methods appropriately to test the `update_spares_definition` service method.
 
         :param spares_definition_data: Dictionary containing the put data as would be required for a
                                        `SparesDefinitionPutSchema` but with any `id`'s replaced by the `value` as the
@@ -83,26 +83,26 @@ def mock_set_spares_definition(
         self._expected_spares_definition_out = MagicMock()
         ServiceTestHelpers.mock_upsert(self.mock_setting_repository, self._expected_spares_definition_out)
 
-    def call_set_spares_definition(self) -> None:
-        """Calls the `SettingService` `set_spares_definition` method with the appropriate data from a prior call to
-        `mock_set_spares_definition`."""
+    def call_update_spares_definition(self) -> None:
+        """Calls the `SettingService` `update_spares_definition` method with the appropriate data from a prior call to
+        `mock_update_spares_definition`."""
 
-        self._set_spares_definition = self.setting_service.set_spares_definition(self._spares_definition_put)
+        self._updated_spares_definition = self.setting_service.update_spares_definition(self._spares_definition_put)
 
-    def call_set_spares_definition_expecting_error(self, error_type: type[BaseException]) -> None:
+    def call_update_spares_definition_expecting_error(self, error_type: type[BaseException]) -> None:
         """
-        Calls the `SettingService` `set_spares_definition` method with the appropriate data from a prior call to
-        `mock_set_spares_definition` while expecting an error to be raised.
+        Calls the `SettingService` `update_spares_definition` method with the appropriate data from a prior call to
+        `mock_update_spares_definition` while expecting an error to be raised.
 
         :param error_type: Expected exception to be raised.
         """
 
         with pytest.raises(error_type) as exc:
-            self.setting_service.set_spares_definition(self._spares_definition_put)
-        self._set_spares_definition_exception = exc
+            self.setting_service.update_spares_definition(self._spares_definition_put)
+        self._update_spares_definition_exception = exc
 
-    def check_set_spares_definition_success(self) -> None:
-        """Checks that a prior call to `call_set_spares_definition` worked as expected."""
+    def check_update_spares_definition_success(self) -> None:
+        """Checks that a prior call to `call_update_spares_definition` worked as expected."""
 
         # Ensure obtained all of the required usage statuses
         self.mock_usage_status_repository.get.assert_has_calls(
@@ -116,39 +116,39 @@ def check_set_spares_definition_success(self) -> None:
             self._expected_spares_definition_in, SparesDefinitionOut
         )
 
-        assert self._set_spares_definition == self._expected_spares_definition_out
+        assert self._updated_spares_definition == self._expected_spares_definition_out
 
-    def check_set_spares_definition_failed_with_exception(self, message: str) -> None:
+    def check_update_spares_definition_failed_with_exception(self, message: str) -> None:
         """
-        Checks that a prior call to `call_set_spares_definition_expecting_error` worked as expected, raising an
+        Checks that a prior call to `call_update_spares_definition_expecting_error` worked as expected, raising an
         exception with the correct message.
 
         :param message: Expected message of the raised exception.
         """
 
         self.mock_setting_repository.upsert.assert_not_called()
 
-        assert str(self._set_spares_definition_exception.value) == message
+        assert str(self._update_spares_definition_exception.value) == message
 
 
-class TestSetSpareDefinition(SetSparesDefinitionDSL):
-    """Tests for setting the spares definition."""
+class TestUpdateSpareDefinition(UpdateSparesDefinitionDSL):
+    """Tests for updating the spares definition."""
 
-    def test_set_spare_definition(self):
-        """Test setting the spares definition."""
+    def test_update_spare_definition(self):
+        """Test updating the spares definition."""
 
-        self.mock_set_spares_definition(
+        self.mock_update_spares_definition(
             SETTING_SPARES_DEFINITION_DATA_NEW_USED, [USAGE_STATUS_OUT_DATA_NEW, USAGE_STATUS_OUT_DATA_USED]
         )
-        self.call_set_spares_definition()
-        self.check_set_spares_definition_success()
+        self.call_update_spares_definition()
+        self.check_update_spares_definition_success()
 
-    def test_set_spare_definition_with_non_existent_usage_status_id(self):
-        """Test setting the spares definition with a non-existent usage status ID."""
+    def test_update_spare_definition_with_non_existent_usage_status_id(self):
+        """Test updating the spares definition with a non-existent usage status ID."""
 
-        self.mock_set_spares_definition(SETTING_SPARES_DEFINITION_DATA_NEW_USED, [USAGE_STATUS_OUT_DATA_NEW, None])
-        self.call_set_spares_definition_expecting_error(MissingRecordError)
-        self.check_set_spares_definition_failed_with_exception(
+        self.mock_update_spares_definition(SETTING_SPARES_DEFINITION_DATA_NEW_USED, [USAGE_STATUS_OUT_DATA_NEW, None])
+        self.call_update_spares_definition_expecting_error(MissingRecordError)
+        self.check_update_spares_definition_failed_with_exception(
             # Pydantic Field confuses pylint
             # pylint: disable=unsubscriptable-object
             f"No usage status found with ID: {self._spares_definition_put.usage_statuses[1].id}"