Skip to content

Commit 0ac1c57

Browse files
authored
Functionality for the deleting application attachments (#4624)
<!-- Thanks for contributing to Hypha! Please ensure your contributions pass all necessary linting/testing and that the appropriate documentation has been updated. --> <!-- Describe briefly what your pull request changes. If this is resolving an issue, please specify below via "Fixes #<Github Issue ID>" --> 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 <!-- If step does not require manual testing, skip/remove this section. Give a brief overview of the steps required for a user/dev to test this contribution. Important things to include: - Required user roles for where necessary (ie. "As a Staff Admin...") - Clear & validatable expected results (ie. "Confirm the submit button is now not clickable") - Language that can be understood by non-technical testers if being tested by users --> - [ ] 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
1 parent a32a999 commit 0ac1c57

8 files changed

Lines changed: 159 additions & 5 deletions

File tree

hypha/apply/activity/adapters/emails.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,6 @@ def handle_transition(self, new_phase, source, old_phase=None, **kwargs):
175175
old_index = list(dict(PHASES).keys()).index(old_phase.name)
176176
target_index = list(dict(PHASES).keys()).index(submission.status)
177177
is_forward = old_index < target_index
178-
print("NEW PHASE")
179-
print(new_phase.public_name)
180178

181179
kwargs["old_phase"] = old_phase.public_name
182180
kwargs["new_phase"] = new_phase.public_name

hypha/apply/funds/apps.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
from django.apps import AppConfig
2+
from django.core.signals import request_finished
23

34

45
class ApplyConfig(AppConfig):
56
name = "hypha.apply.funds"
7+
8+
def ready(self):
9+
# Connect the attachment deletion handler
10+
from . import signals
11+
12+
request_finished.connect(signals.delete_attachments)
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Generated by Django 4.2.24 on 2025-10-01 15:39
2+
3+
from django.db import migrations
4+
from django.core.files.storage import default_storage
5+
import sys
6+
from ..utils import delete_directory
7+
8+
9+
def delete_orphaned_attachments(apps, schema_editor):
10+
"""Remove all attachments not associated with an application"""
11+
12+
# TODO: This solution is not ideal but due to our unit tests writing to the filesystem
13+
# this can cause files belonging to the dev's local server to be deleted. Until
14+
# these can be better isolated, this signal will do nothing when pytest is running
15+
if "pytest" in sys.modules:
16+
return
17+
18+
ApplicationSubmission = apps.get_model("funds", "ApplicationSubmission")
19+
20+
submission_attachment_path = "submission"
21+
22+
folders_to_delete = []
23+
folders_to_check = []
24+
25+
if not default_storage.exists(submission_attachment_path):
26+
# If specified path doesn't exist, ignore
27+
# edge case that typically comes up in tests
28+
return
29+
30+
for folder in default_storage.listdir(submission_attachment_path)[0]:
31+
# `listdir` returns ([folders], [files]) ^
32+
try:
33+
folders_to_check.append(int(folder))
34+
except ValueError:
35+
# Folder name is not an int, therefore not a submission ID and can be deleted (an edge case)
36+
folders_to_delete.append(folder)
37+
38+
# Get a list of all undeleted submissions that have a folder
39+
valid_ids = set(
40+
ApplicationSubmission.objects.filter(id__in=folders_to_check).values_list(
41+
"id", flat=True
42+
)
43+
)
44+
45+
# Find the set difference and delete those folders
46+
folders_to_delete += list(set(folders_to_check) - valid_ids)
47+
48+
for folder in folders_to_delete:
49+
try:
50+
delete_directory(f"{submission_attachment_path}/{folder}")
51+
except FileNotFoundError:
52+
# Will get thrown when unit tests attempt to run migrations
53+
pass
54+
55+
56+
class Migration(migrations.Migration):
57+
dependencies = [
58+
("funds", "0130_alter_applicationsubmission_status"),
59+
]
60+
61+
operations = [migrations.RunPython(delete_orphaned_attachments)]

hypha/apply/funds/models/mixins.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ def extract_files(self):
161161
for field in self.form_fields:
162162
if isinstance(field.block, UploadableMediaBlock):
163163
files[field.id] = self.data(field.id) or []
164-
self.form_data.pop(field.id, None)
165164
return files
166165

167166
@classmethod

hypha/apply/funds/signals.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import sys
2+
3+
from django.core.files.storage import default_storage
4+
from django.db.models.signals import pre_delete
5+
from django.dispatch import receiver
6+
7+
from hypha.apply.funds.models.application_revisions import ApplicationRevision
8+
from hypha.apply.funds.models.submissions import ApplicationSubmission
9+
from hypha.apply.funds.utils import delete_directory
10+
11+
12+
@receiver(signal=pre_delete, sender=ApplicationSubmission)
13+
@receiver(signal=pre_delete, sender=ApplicationRevision)
14+
def delete_attachments(sender, instance=None, **kwargs):
15+
"""
16+
Before the deletion of any sub class of AccessFormData, ensure the files associated with it are deleted too.
17+
18+
This can include things like ApplicationSubmission & ApplicationRevision objects
19+
"""
20+
21+
# TODO: This solution is not ideal but due to our unit tests writing to the filesystem
22+
# this can cause files belonging to the dev's local server to be deleted. Until
23+
# these can be better isolated, this signal will do nothing when pytest is running
24+
if "pytest" in sys.modules:
25+
return
26+
27+
# This will be called anytime a deletion is ran, so ensure the object being deleted
28+
# can have attachments
29+
if issubclass(sender, ApplicationRevision):
30+
files = instance.extract_files()
31+
for value in files.values():
32+
if not isinstance(value, list): # Single file field
33+
value.delete()
34+
else: # Multiple file fields
35+
[sub_file.delete() for sub_file in value]
36+
elif issubclass(sender, ApplicationSubmission):
37+
submission_attachment_path = f"submission/{instance.id}"
38+
39+
if default_storage.exists(submission_attachment_path):
40+
delete_directory(submission_attachment_path)

hypha/apply/funds/utils.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import csv
2+
import os
23
import re
34
from datetime import datetime
45
from functools import reduce
@@ -8,6 +9,7 @@
89
from typing import Iterable
910

1011
import django_filters as filters
12+
from django.core.files.storage import default_storage
1113
from django.urls import reverse
1214
from django.utils.encoding import force_bytes
1315
from django.utils.html import strip_tags
@@ -267,3 +269,28 @@ def generate_invite_path(invite):
267269
kwargs={"uidb64": uid, "token": token},
268270
)
269271
return login_path
272+
273+
274+
def delete_directory(directory_path):
275+
"""Delete a full directory (empty or not)
276+
277+
Used in attachment cleanup when deleting submissions/revisions
278+
"""
279+
280+
directories, files = default_storage.listdir(directory_path)
281+
282+
for item in directories:
283+
item_path = os.path.join(directory_path, item)
284+
if default_storage.exists(item_path):
285+
# Recursively delete subdirectories
286+
delete_directory(item_path)
287+
288+
for item in files:
289+
item_path = os.path.join(directory_path, item)
290+
if default_storage.exists(item_path):
291+
# Delete files
292+
default_storage.delete(item_path)
293+
294+
if default_storage.exists(directory_path):
295+
# Delete the empty directory
296+
default_storage.delete(directory_path)

hypha/apply/stream_forms/files.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
class StreamFieldDataEncoder(DjangoJSONEncoder):
99
def default(self, o):
10-
if isinstance(o, StreamFieldFile):
10+
if isinstance(o, File):
1111
return {
1212
"name": o.name,
1313
"filename": o.filename,

hypha/apply/stream_forms/testing/factories.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import factory
66
import wagtail_factories
7+
from django.core.files.base import File
78
from django.core.files.uploadedfile import SimpleUploadedFile
89
from django.core.serializers.json import DjangoJSONEncoder
910
from wagtail.blocks import RichTextBlock, StructValue
@@ -242,6 +243,17 @@ def make_answer(cls, params=None):
242243
return cls.choices[0]
243244

244245

246+
class UploadedFile(SimpleUploadedFile):
247+
"""Utilized to make functionality closer to that of `StreamFieldFile`
248+
249+
Requires a `filename` attribute which is pulled from the existing `_name`
250+
"""
251+
252+
def __init__(self, name, content, content_type=...):
253+
super().__init__(name, content, content_type)
254+
self.filename = self._name
255+
256+
245257
class UploadableMediaFactory(FormFieldBlockFactory):
246258
default_value = factory.django.FileField()
247259

@@ -252,7 +264,7 @@ def make_answer(cls, params=None):
252264
if params.get("filename") is None:
253265
params["filename"] = "test_example.pdf"
254266
file_name, file = cls.default_value._make_content(params)
255-
return SimpleUploadedFile(file_name, file.read())
267+
return UploadedFile(file_name, file.read())
256268

257269

258270
class ImageFieldBlockFactory(UploadableMediaFactory):
@@ -276,6 +288,16 @@ def make_answer(cls, params=None):
276288
return [UploadableMediaFactory.make_answer() for _ in range(2)]
277289

278290

291+
class StreamFieldDataEncoder(DjangoJSONEncoder):
292+
def default(self, o):
293+
if isinstance(o, File):
294+
return {
295+
"name": o.name,
296+
"filename": o.filename,
297+
}
298+
return super().default(o)
299+
300+
279301
class StreamFieldUUIDFactory(wagtail_factories.StreamFieldFactory):
280302
def evaluate(self, instance, step, extra):
281303
params = self.build_form(extra)

0 commit comments

Comments
 (0)