Skip to content

Functionality for the deleting application attachments#4624

Merged
wes-otf merged 7 commits into
mainfrom
fix/remove-orphaned-attachements
Oct 15, 2025
Merged

Functionality for the deleting application attachments#4624
wes-otf merged 7 commits into
mainfrom
fix/remove-orphaned-attachements

Conversation

@wes-otf

@wes-otf wes-otf commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

Fixes #4622. Deletes application attachments when an application is deleted and includes a migration to retroactively delete orphaned attachments.

Also removes a debug statement accidentally included in a PR

Test Steps

  • Ensure that attached files are deleted with their associated application
  • Ensure that previously deleted applications that had attachments still in the system have been deleted

@wes-otf wes-otf force-pushed the fix/remove-orphaned-attachements branch from eda7c6d to 53f8c41 Compare October 2, 2025 17:17
@wes-otf

wes-otf commented Oct 2, 2025

Copy link
Copy Markdown
Contributor Author

There's a few more things left on this PR:

  • Deleting of empty directories after an application is deleted (migration handles this)
  • Test passing! That's been the biggest hold up, I can't pin why the test_gets_draft_on_edit_submission test fails but feel close

@wes-otf wes-otf added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Oct 9, 2025
@wes-otf wes-otf marked this pull request as ready for review October 9, 2025 13:48
@wes-otf wes-otf force-pushed the fix/remove-orphaned-attachements branch from 8340aeb to 3b515a2 Compare October 9, 2025 15:29
Comment on lines +21 to +25
# TODO: This solution is not ideal but due to our unit tests writing to the filesystem
# this can cause files belonging to the dev's local server to be deleted. Until
# these can be better isolated, this signal will do nothing when pytest is running
if "pytest" in sys.modules:
return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This solution isn't ideal but I think until we can take a larger pass at our tests and making them more isolated it makes sense? When trying to run actual tests the files for applications on my local server were getting deleted, along with some weird stuff coming up in the unit tests that I couldn't recreate in a running instance.

@wes-otf

wes-otf commented Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

All worked well on test! I've tested both on local storage and AWS, all deleted the attachments as they were supposed to. Gonna merge this

@wes-otf wes-otf added Status: Tested - approved for live ✅ and removed Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Oct 15, 2025
@wes-otf wes-otf merged commit 0ac1c57 into main Oct 15, 2025
7 checks passed
@frjo frjo deleted the fix/remove-orphaned-attachements branch February 18, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Tested - approved for live ✅ Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Application attachments aren't deleted with the application

1 participant