-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add endpoint to set spares definition #413 #421
Merged
Merged
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
bf5de8a
Initial functional implementation #413
joelvdavies 82cf9b5
Some clean up of unnecessary changes #413
joelvdavies 3788b5f
Return actual usage status data in response and ensure statuses exist…
joelvdavies 73de077
Prevent deletion of a usage status that is in the spares definition #413
joelvdavies a9855d0
Fix linting and existing tests #413
joelvdavies 73203b1
Initial SettingRepo unit tests #413
joelvdavies 8654cd5
Add remaining SettingRepo unit tests #413
joelvdavies adbb663
Prevent duplicates usage status IDs in the definition schema #413
joelvdavies d9085fb
Enforce at least one usage status and add SettingService unit tests #413
joelvdavies b154828
Add e2e tests for spares definition setting #413
joelvdavies 8216524
Reduce repetition of id_dicts and use consistent setup names in e2e t…
joelvdavies f737d09
Clear settings collection, use more than one usage status in tests an…
joelvdavies 6812453
Improve consistency of setting model names #413
joelvdavies c0d8e7b
Merge branch 'develop' into add-spares-definition-put-#413
joelvdavies d747c73
Address some review comments #413
joelvdavies ec7dd7c
Improve upsert comment #413
joelvdavies 78b48bd
Merge branch 'develop' into add-spares-definition-put-#413
joelvdavies 6e00914
Address review comments, replacing set with update #417
joelvdavies File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
""" | ||
Module for defining the database models for representing settings. | ||
""" | ||
|
||
from abc import ABC, abstractmethod | ||
from typing import ClassVar | ||
|
||
from pydantic import BaseModel, Field | ||
|
||
from inventory_management_system_api.models.custom_object_id_data_types import CustomObjectIdField, StringObjectIdField | ||
from inventory_management_system_api.models.usage_status import UsageStatusOut | ||
|
||
|
||
class SettingInBase(BaseModel, ABC): | ||
""" | ||
Base input database model for a setting. | ||
""" | ||
|
||
@property | ||
@staticmethod | ||
@abstractmethod | ||
def SETTING_ID() -> str: # pylint: disable=invalid-name | ||
"""ID of the setting. Ensures this value can be obtained from the class type itself as a static variable.""" | ||
|
||
|
||
class SettingOutBase(SettingInBase): | ||
""" | ||
Base output database model for a setting. | ||
""" | ||
|
||
id: StringObjectIdField = Field(alias="_id") | ||
|
||
|
||
class SparesDefinitionUsageStatusIn(BaseModel): | ||
""" | ||
Input database model for a usage status in a spares definition. | ||
""" | ||
|
||
id: CustomObjectIdField | ||
|
||
|
||
class SparesDefinitionUsageStatusOut(BaseModel): | ||
""" | ||
Output database model for a usage status in a spares definition. | ||
""" | ||
|
||
id: StringObjectIdField | ||
|
||
|
||
class SparesDefinitionIn(SettingInBase): | ||
""" | ||
Input database model for a spares definition. | ||
""" | ||
|
||
SETTING_ID: ClassVar[str] = "spares_definition" | ||
|
||
usage_statuses: list[SparesDefinitionUsageStatusIn] | ||
|
||
|
||
class SparesDefinitionOut(SparesDefinitionIn, SettingOutBase): | ||
""" | ||
Output database model for a spares definition. | ||
""" | ||
|
||
usage_statuses: list[UsageStatusOut] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
107 changes: 107 additions & 0 deletions
107
inventory_management_system_api/repositories/setting.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
""" | ||
Module for providing a repository for managing settings in a MongoDB database. | ||
""" | ||
|
||
import logging | ||
from typing import Optional, Type, TypeVar | ||
|
||
from pymongo.client_session import ClientSession | ||
from pymongo.collection import Collection | ||
|
||
from inventory_management_system_api.core.database import DatabaseDep | ||
from inventory_management_system_api.models.setting import SettingInBase, SettingOutBase, SparesDefinitionOut | ||
|
||
logger = logging.getLogger() | ||
|
||
# Template types for models inheriting from SettingIn/OutBase so this repo can be used generically for multiple settings | ||
SettingInBaseT = TypeVar("SettingInBaseT", bound=SettingInBase) | ||
SettingOutBaseT = TypeVar("SettingOutBaseT", bound=SettingOutBase) | ||
|
||
|
||
# Aggregation pipeline for getting the spares definition complete with usage status data | ||
SPARES_DEFINITION_GET_AGGREGATION_PIPELINE: list = [ | ||
# Only perform this on the relevant document | ||
{"$match": {"_id": SparesDefinitionOut.SETTING_ID}}, | ||
# Deconstruct the usage statuses so can go through them one by one | ||
{"$unwind": "$usage_statuses"}, | ||
# Find and store actual usage status data as 'statusDetails' | ||
{ | ||
"$lookup": { | ||
"from": "usage_statuses", | ||
"localField": "usage_statuses.id", | ||
"foreignField": "_id", | ||
"as": "statusDetails", | ||
} | ||
}, | ||
{"$unwind": "$statusDetails"}, | ||
# Merge the two sets of documents together | ||
{"$addFields": {"usage_statuses": {"$mergeObjects": ["$usage_statuses", "$statusDetails"]}}}, | ||
# Remove the temporary 'statusDetails' field as no longer needed | ||
{"$unset": "statusDetails"}, | ||
# Reconstruct the original document by merging with the original fields | ||
{ | ||
"$group": { | ||
"_id": "$_id", | ||
"usage_statuses": {"$push": "$usage_statuses"}, | ||
"otherFields": {"$first": "$$ROOT"}, | ||
} | ||
}, | ||
{"$replaceRoot": {"newRoot": {"$mergeObjects": ["$otherFields", {"usage_statuses": "$usage_statuses"}]}}}, | ||
] | ||
|
||
|
||
class SettingRepo: | ||
""" | ||
Repository for managing settings in a MongoDB database. | ||
""" | ||
|
||
def __init__(self, database: DatabaseDep) -> None: | ||
""" | ||
Initialize the `SettingRepo` with a MongoDB database instance. | ||
|
||
:param database: The database to use. | ||
""" | ||
self._database = database | ||
self._settings_collection: Collection = self._database.settings | ||
|
||
def upsert( | ||
self, setting: SettingInBaseT, out_model_type: Type[SettingOutBaseT], session: ClientSession = None | ||
) -> SettingOutBaseT: | ||
joelvdavies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
""" | ||
Assign a setting a MongoDB database. Will either update or insert the setting depending on whether it | ||
already exists. | ||
joelvdavies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
:param setting: Setting containing the fields to be updated. Also contains the ID for lookup. | ||
:param out_model_type: The output type of the setting's model. | ||
:param session: PyMongo ClientSession to use for database operations. | ||
:return: The updated setting. | ||
""" | ||
|
||
logger.info("Assigning setting with ID: %s in the database", setting.SETTING_ID) | ||
self._settings_collection.update_one( | ||
{"_id": setting.SETTING_ID}, {"$set": setting.model_dump(by_alias=True)}, upsert=True, session=session | ||
) | ||
|
||
return self.get(out_model_type=out_model_type, session=session) | ||
|
||
def get(self, out_model_type: Type[SettingOutBaseT], session: ClientSession = None) -> Optional[SettingOutBaseT]: | ||
""" | ||
Retrieve a setting from a MongoDB database. | ||
|
||
:param out_model_type: The output type of the setting's model. Also contains the ID for lookup. | ||
:param session: PyMongo ClientSession to use for database operations. | ||
:return: Retrieved setting or `None` if not found. | ||
""" | ||
|
||
if out_model_type is SparesDefinitionOut: | ||
# The spares definition contains a list of usage statuses - use an aggregate query here to obtain | ||
# the actual usage status entities instead of just their stored ID | ||
|
||
result = list(self._settings_collection.aggregate(SPARES_DEFINITION_GET_AGGREGATION_PIPELINE)) | ||
setting = result[0] if len(result) > 0 else None | ||
else: | ||
setting = self._settings_collection.find_one({"_id": out_model_type.SETTING_ID}, session=session) | ||
|
||
if setting is not None: | ||
return out_model_type(**setting) | ||
return None |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
""" | ||
Module for providing an API router which defines routes for managing settings using the `SettingService` service. | ||
""" | ||
|
||
import logging | ||
from typing import Annotated | ||
|
||
from fastapi import APIRouter, Depends, HTTPException, status | ||
|
||
from inventory_management_system_api.core.exceptions import InvalidObjectIdError, MissingRecordError | ||
from inventory_management_system_api.schemas.setting import SparesDefinitionPutSchema, SparesDefinitionSchema | ||
from inventory_management_system_api.services.setting import SettingService | ||
|
||
logger = logging.getLogger() | ||
|
||
router = APIRouter(prefix="/v1/settings", tags=["settings"]) | ||
|
||
SettingServiceDep = Annotated[SettingService, Depends(SettingService)] | ||
|
||
|
||
@router.put( | ||
path="/spares_definition", | ||
summary="Set the definition of a spare", | ||
joelvdavies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
response_description="Spares definition updated successfully", | ||
) | ||
def set_spares_definition( | ||
VKTB marked this conversation as resolved.
Show resolved
Hide resolved
|
||
spares_definition: SparesDefinitionPutSchema, setting_service: SettingServiceDep | ||
) -> SparesDefinitionSchema: | ||
# pylint: disable=missing-function-docstring | ||
logger.info("Setting spares definition") | ||
joelvdavies marked this conversation as resolved.
Show resolved
Hide resolved
|
||
logger.debug("Spares definition data: %s", spares_definition) | ||
|
||
try: | ||
updated_spares_definition = setting_service.set_spares_definition(spares_definition) | ||
return SparesDefinitionSchema(**updated_spares_definition.model_dump()) | ||
except (MissingRecordError, InvalidObjectIdError) as exc: | ||
message = "A specified usage status does not exist" | ||
logger.exception(message) | ||
raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=message) from exc |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should ensure that SettingOutBaseT is the same settings as the InBase (i.e Would not include SparesDefinitionIn and ExpiryDefinitionOut), but again it runs fine as it is without errors, so similar to the last comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having the union here would actually mean it can be any subtype of SettingOutBase or SettingInBase (https://stackoverflow.com/questions/59933946/difference-between-typevart-a-b-and-typevart-bound-uniona-b). I don't think there is a way to have them coupled like that unfortunately.