Skip to content
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

Delete related attachment & images when an entity is deleted #460

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

VKTB
Copy link
Collaborator

@VKTB VKTB commented Feb 13, 2025

Description

This PR introduces two new config values for the API to connect to and interact with an Object Storage API instance. I opted for a boolean ENV VAR (OBJECT_STORAGE__ENABLED) because this API can be run on its own without an Object Storage API instance. This was the simplest way to ensure the tests did not break as an Object Storage API instance will otherwise be required to make them pass. These new config values will need to be added to the Docker IMS deployment configurations.

I implemented an ObjectStorageAPIClient class whose methods are used in the services layer by the catalogue item, item, and system modules to delete any attachments and images associated to the entity that is being deleted using the same JWT access token supplied in the request. The entities in the IMS API database get deleted after the attachments and images are successfully deleted from the object storage. Unfortunately, there is no way to restore these if the delete database operation is not successful. Because the JWT access tokens have an expiry time, there could be cases where they are valid when received by the IMS API but expire when they are received by the Object Storage API. In cases like these, the IMS API will just return 403 Invalid token or expired token so the consumer will just need to retry their request with a valid access token. The IMS API will return 500 Something went wrong for any other errors that occur while communicating with the Object Storage API.

Testing instructions

You will need an Object Storage API instance running with some attachments and images in S3.

  • Review code
  • Check Actions build
  • Review changes to test coverage
  • Attachments and/or images in Object Storage API (and S3!) get deleted when an entity (catalogue item, item, and system) gets deleted.
  • 204 returned when there are no attachments and/or images associated with the entity that is being deleted.

Agile board tracking

closes #399

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 86.53846% with 7 lines in your changes missing coverage. Please review.

Project coverage is 97.57%. Comparing base (329a188) to head (13f69f2).

Files with missing lines Patch % Lines
...y_management_system_api/services/catalogue_item.py 66.66% 2 Missing ⚠️
inventory_management_system_api/services/item.py 66.66% 2 Missing ⚠️
inventory_management_system_api/services/system.py 66.66% 2 Missing ⚠️
inventory_management_system_api/core/config.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #460      +/-   ##
===========================================
- Coverage    97.91%   97.57%   -0.34%     
===========================================
  Files           48       49       +1     
  Lines         1723     1772      +49     
===========================================
+ Hits          1687     1729      +42     
- Misses          36       43       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -43,6 +43,8 @@ async def __call__(self, request: Request) -> str:
if not self._is_jwt_access_token_valid(credentials.credentials):
raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Invalid token or expired token")

request.state.token = credentials.credentials
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is a nice thing to do as there are not many examples when I searched for it on GitHub. The alternative would be to do request.headers.get("Authorization").split("Bearer ")[1] in the endpoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete related attachment & images when an entity is deleted
1 participant