Fix bug when erasing attachments with replacements - #9388
Conversation
Errors raised inside the erase transaction are swallowed by the after_rollback callback. Rescue exceptions inside the transaction block, log the class and message at error level, then re-raise so the rollback and false return value are unchanged.
Errors raised inside the erase transaction are swallowed by the after_rollback callback. Rescue exceptions inside the transaction block, log the class and message at error level, then re-raise so the rollback and false return value are unchanged. Give the explicit rollbacks for failed attachment locking and event logging messages so they are meaningful when logged.
Erasing an attachment that had previously been replaced always failed. Setting erased_at flips replacing_or_replaced? to false, which switches the replaced_reason validation from requiring presence to requiring absence.
a043a9c to
1a79863
Compare
garethrees
left a comment
garethrees
left a comment
There was a problem hiding this comment.
This allows erasure, but doesn't retain the replacement.
Start with a replaced attachment
Erase the attachment
Gets erased successfully – yay!
But the replacement is gone
The aim is for us to be able to make redactions in file-appropriate software outside of Alaveteli and upload as a replacement to continue to publish the remainder of the useful information, while being able to fully erase the originals that contain the data that we've redatced.
After testing again I'm still seeing the replacement attachment retained. Must be something else going on here. |
|
I've uploaded a replacement here https://staging.alaveteli.org/admin/attachments/4225839/edit but now instead of it erasing the original and replacement, I just get "Could not erase this attachment. Request technical assistance." Both main body part and attachment are locked and masked. |
Oh sorry, I think the staging deployed error its currently on c5d806e. I'll get this live now. |
|
Deploy was failing due to this: mysociety/whatdotheyknow-theme@796aa20 |
|
Ah annoying thanks for fixing. Do we even need |
|
Yeah looks like I'm getting the same on staging ( |
|
Trying to workout what is wrong here. It seems to work: Screen.Recording.2026-08-04.at.09.43.24.mov |
Erasing an attachment will also purge any replacements. This only showed up in production because the purge runs in a background job, so the file survives locally where no queue is running. Erasure now skips purging the file and resetting the filename when a replacement is present. No attachment storage is erased in that case, so the erase_attachment event records no storage key rather than pointing at the replacement which is still in use.
Storage keys recorded on erasure events are used to ensure data is removed from backups. Replacing an attachment destroys the original as well, either by overwriting the blob in place when a body is supplied or by purging the old blob when a file is uploaded, but nothing recorded which key had held it. Capture the key before the replacing and log it as old_storage_key on the edit_attachment event.
Erasing an attachment now keeps its replacement published, so admins should be able to upload a further replacement afterwards. Because replacing? was false once erased_at was set, handle_replacements skipped the upload and replace! reported success while changing nothing. Ask content_erased? rather than erased? in replacing? and replacing_or_replaced?, so an attachment which still has content can be replaced and the replaced_reason validations continue to apply.
An erased attachment which still has its replacement published needs the replacement options available so admins can swap the file if there are ever issues with those they are publishing. Masking and locking stay hidden as both need the raw email.
2 participants
What does this do?
Why was this needed?
Couldn't perform admin on WDTK due to this issue
[skip changelog]