-
Notifications
You must be signed in to change notification settings - Fork 39
Functionality for the deleting application attachments #4624
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
53f8c41
Initial implementation for deleting application attachements
wes-otf 7c55728
Added `FileNotFound` exception handling for when tests are ran
wes-otf 674b4af
Added check for submission attachments directory
wes-otf 138b580
Add pytest bypass to delete signal
wes-otf 3b515a2
Revert factory, tried to include `-upload` attribute in hopes that'd …
wes-otf a49e746
Removed unneeded `base_location` in submission paths
wes-otf 6ffaaee
Added check to see if submission folder actually exists
wes-otf 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 hidden or 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 hidden or 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 |
|---|---|---|
| @@ -1,5 +1,12 @@ | ||
| from django.apps import AppConfig | ||
| from django.core.signals import request_finished | ||
|
|
||
|
|
||
| class ApplyConfig(AppConfig): | ||
| name = "hypha.apply.funds" | ||
|
|
||
| def ready(self): | ||
| # Connect the attachment deletion handler | ||
| from . import signals | ||
|
|
||
| request_finished.connect(signals.delete_attachments) |
61 changes: 61 additions & 0 deletions
61
hypha/apply/funds/migrations/0131_delete_orphaned_attachments.py
This file contains hidden or 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,61 @@ | ||
| # Generated by Django 4.2.24 on 2025-10-01 15:39 | ||
|
|
||
| from django.db import migrations | ||
| from django.core.files.storage import default_storage | ||
| import sys | ||
| from ..utils import delete_directory | ||
|
|
||
|
|
||
| def delete_orphaned_attachments(apps, schema_editor): | ||
| """Remove all attachments not associated with an application""" | ||
|
|
||
| # 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 | ||
|
|
||
| ApplicationSubmission = apps.get_model("funds", "ApplicationSubmission") | ||
|
|
||
| submission_attachment_path = "submission" | ||
|
|
||
| folders_to_delete = [] | ||
| folders_to_check = [] | ||
|
|
||
| if not default_storage.exists(submission_attachment_path): | ||
| # If specified path doesn't exist, ignore | ||
| # edge case that typically comes up in tests | ||
| return | ||
|
|
||
| for folder in default_storage.listdir(submission_attachment_path)[0]: | ||
| # `listdir` returns ([folders], [files]) ^ | ||
| try: | ||
| folders_to_check.append(int(folder)) | ||
| except ValueError: | ||
| # Folder name is not an int, therefore not a submission ID and can be deleted (an edge case) | ||
| folders_to_delete.append(folder) | ||
|
|
||
| # Get a list of all undeleted submissions that have a folder | ||
| valid_ids = set( | ||
| ApplicationSubmission.objects.filter(id__in=folders_to_check).values_list( | ||
| "id", flat=True | ||
| ) | ||
| ) | ||
|
|
||
| # Find the set difference and delete those folders | ||
| folders_to_delete += list(set(folders_to_check) - valid_ids) | ||
|
|
||
| for folder in folders_to_delete: | ||
| try: | ||
| delete_directory(f"{submission_attachment_path}/{folder}") | ||
| except FileNotFoundError: | ||
| # Will get thrown when unit tests attempt to run migrations | ||
| pass | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("funds", "0130_alter_applicationsubmission_status"), | ||
| ] | ||
|
|
||
| operations = [migrations.RunPython(delete_orphaned_attachments)] |
This file contains hidden or 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 hidden or 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,40 @@ | ||
| import sys | ||
|
|
||
| from django.core.files.storage import default_storage | ||
| from django.db.models.signals import pre_delete | ||
| from django.dispatch import receiver | ||
|
|
||
| from hypha.apply.funds.models.application_revisions import ApplicationRevision | ||
| from hypha.apply.funds.models.submissions import ApplicationSubmission | ||
| from hypha.apply.funds.utils import delete_directory | ||
|
|
||
|
|
||
| @receiver(signal=pre_delete, sender=ApplicationSubmission) | ||
| @receiver(signal=pre_delete, sender=ApplicationRevision) | ||
| def delete_attachments(sender, instance=None, **kwargs): | ||
| """ | ||
| Before the deletion of any sub class of AccessFormData, ensure the files associated with it are deleted too. | ||
|
|
||
| This can include things like ApplicationSubmission & ApplicationRevision objects | ||
| """ | ||
|
|
||
| # 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 | ||
|
|
||
| # This will be called anytime a deletion is ran, so ensure the object being deleted | ||
| # can have attachments | ||
| if issubclass(sender, ApplicationRevision): | ||
| files = instance.extract_files() | ||
| for value in files.values(): | ||
| if not isinstance(value, list): # Single file field | ||
| value.delete() | ||
| else: # Multiple file fields | ||
| [sub_file.delete() for sub_file in value] | ||
| elif issubclass(sender, ApplicationSubmission): | ||
| submission_attachment_path = f"submission/{instance.id}" | ||
|
|
||
| if default_storage.exists(submission_attachment_path): | ||
| delete_directory(submission_attachment_path) | ||
This file contains hidden or 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 hidden or 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 hidden or 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.
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.
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.