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

Allow deletion of attachments where the asset does not exist in Asset Manager #2580

Merged

Conversation

ryanb-gds
Copy link
Contributor

@ryanb-gds ryanb-gds commented Apr 5, 2024

An attachment which does not have a corresponding asset in Asset Manager is effectively broken. Prior to this commit, it wasn't possible for users to delete these broken attachments.

Being unable to delete the broken attachments prevents users from discarding any draft documents associated with them.

To resolve this, we return a boolean "true" from the Attachment#destroy method when the asset cannot be found, as to all intents and purposes the asset is deleted anyway.

I haven't included a feature test for this as it feels like an edge case that is not valuable enough to cover with an expensive feature test.

It is not clear how the asset has been removed from Asset Manager whilst the attachment is still present.

Trello: https://trello.com/c/epa4vl02

Depends on alphagov/gds-api-adapters#1248 to be merged

@ryanb-gds ryanb-gds force-pushed the permit-deletion-of-attachments-missing-from-asset-manager branch from 5cdc6e4 to 039d430 Compare April 5, 2024 12:22
@ryanb-gds ryanb-gds marked this pull request as ready for review April 30, 2024 08:26
@ryanb-gds ryanb-gds force-pushed the permit-deletion-of-attachments-missing-from-asset-manager branch from 039d430 to 6637266 Compare April 30, 2024 08:52
… manager

An attachment which does not have a corresponding asset in Asset Manager is effectively broken. Prior to this commit, it wasn't possible for users to delete these broken attachments.

Being unable to delete the broken attachments prevents users from discarding any draft documents associated with them.

To resolve this, we return a boolean "true" from the Attachment#destroy method when the asset cannot be found, as to all intents and purposes the asset is deleted anyway.

It is not clear how the asset has been removed from Asset Manager whilst the attachment is still present.
@ryanb-gds ryanb-gds force-pushed the permit-deletion-of-attachments-missing-from-asset-manager branch from 6637266 to 83ed8c9 Compare April 30, 2024 08:59
@ryanb-gds ryanb-gds merged commit 9a1731d into main Apr 30, 2024
15 checks passed
@ryanb-gds ryanb-gds deleted the permit-deletion-of-attachments-missing-from-asset-manager branch April 30, 2024 10:05
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.

None yet

2 participants