Skip to content

Commit

Permalink
Merge pull request #4894 from open-formulieren/cleanup/3283-temporary…
Browse files Browse the repository at this point in the history
…-file-upload-submission-not-nullable

💥 Make TemporaryFileUpload.submission non-nullable
  • Loading branch information
sergei-maertens authored Dec 11, 2024
2 parents 895b800 + 6adb90d commit 53bf9d8
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 38 deletions.
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ RUN mkdir /app/bin /app/log /app/media /app/private_media /app/certifi_ca_bundle
COPY \
./bin/check_celery_worker_liveness.py \
./bin/report_component_problems.py \
./bin/check_temporary_uploads.py \
./bin/

# prevent writing to the container layer, which would degrade performance.
Expand Down
33 changes: 33 additions & 0 deletions bin/check_temporary_uploads.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env python
import sys
from pathlib import Path

import django

SRC_DIR = Path(__file__).parent.parent / "src"
sys.path.insert(0, str(SRC_DIR.resolve()))


def check_for_null_submissions_in_temporary_uploads():
from openforms.submissions.models import TemporaryFileUpload

problematic_uploads = TemporaryFileUpload.objects.filter(submission__isnull=True)
if problematic_uploads.exists():
print("There are still legacy temporary uploads. You must clear those first.")
return False

return True


def main(skip_setup=False) -> bool:
from openforms.setup import setup_env

if not skip_setup:
setup_env()
django.setup()

return check_for_null_submissions_in_temporary_uploads()


if __name__ == "__main__":
main()
12 changes: 12 additions & 0 deletions docs/installation/upgrade-300.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,15 @@ Removal of /api/v2/location/get-street-name-and-city endpoint

The /api/v2/location/get-street-name-and-city was deprecated for some time,
and is now removed in favor of the /api/v2/geo/address-autocomplete endpoint.

Legacy temporary file uploads no longer supported
=================================================

Before Open Forms 2.7.0, temporary file uploads (as created by the file upload form
component) would not be related to the submission they were uploaded in. We call these
legacy temporary file uploads, and support for them has been removed in Open Forms 3.0.

The setting ``TEMPORARY_UPLOADS_REMOVED_AFTER_DAYS`` controls how long file uploads are
kept. Ensure that this many days have passed since the last legacy upload before
upgrading to Open Forms 3.0, otherwise you will run into database errors during the
upgrade.
5 changes: 1 addition & 4 deletions src/openforms/formio/components/vanilla.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,7 @@ def validate(self, attrs: dict[str, Any]) -> dict[str, Any]:
{"originalName": _("Name does not match the uploaded file.")}
)

if (
not temporary_upload.legacy
and temporary_upload.submission != self.context["submission"]
):
if temporary_upload.submission != self.context["submission"]:
raise serializers.ValidationError({"url": _("Invalid URL.")})

with temporary_upload.content.open("rb") as infile:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 4.2.17 on 2024-12-09 15:04

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
("submissions", "0012_alter_submission_price"),
]

operations = [
migrations.RemoveConstraint(
model_name="temporaryfileupload",
name="non_legacy_submission_not_null",
),
migrations.RemoveField(
model_name="temporaryfileupload",
name="legacy",
),
migrations.AlterField(
model_name="temporaryfileupload",
name="submission",
field=models.ForeignKey(
help_text="Submission the temporary file upload belongs to.",
on_delete=django.db.models.deletion.CASCADE,
to="submissions.submission",
verbose_name="submission",
),
),
]
16 changes: 0 additions & 16 deletions src/openforms/submissions/models/submission_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from django.core.files.base import File
from django.db import models
from django.db.models import Q
from django.utils import timezone
from django.utils.translation import gettext_lazy as _

Expand Down Expand Up @@ -49,17 +48,9 @@ def select_prune(self, age: timedelta):

class TemporaryFileUpload(DeleteFileFieldFilesMixin, models.Model):
uuid = models.UUIDField(_("UUID"), unique=True, default=uuid.uuid4)
legacy = models.BooleanField(
_("legacy"),
default=False,
help_text=_("Whether the instance is linked to a submission instance."),
)
# TODO DeprecationWarning null=True is a transitional state, and should be removed
# at some point:
submission = models.ForeignKey(
"submissions.Submission",
on_delete=models.CASCADE,
null=True,
verbose_name=_("submission"),
help_text=_("Submission the temporary file upload belongs to."),
)
Expand All @@ -83,13 +74,6 @@ class TemporaryFileUpload(DeleteFileFieldFilesMixin, models.Model):
class Meta:
verbose_name = _("temporary file upload")
verbose_name_plural = _("temporary file uploads")
constraints = [
models.CheckConstraint(
check=(Q(legacy=False) & Q(submission__isnull=False))
| (Q(legacy=True) & Q(submission__isnull=True)),
name="non_legacy_submission_not_null",
),
]


class SubmissionFileAttachmentQuerySet(
Expand Down
18 changes: 0 additions & 18 deletions src/openforms/submissions/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from unittest.mock import patch

from django.core.exceptions import ValidationError
from django.db import IntegrityError
from django.test import TestCase, override_settings, tag

from freezegun import freeze_time
Expand All @@ -23,7 +22,6 @@
SubmissionFileAttachmentFactory,
SubmissionReportFactory,
SubmissionStepFactory,
TemporaryFileUploadFactory,
)


Expand Down Expand Up @@ -589,19 +587,3 @@ def test_names_do_not_break_pdf_saving_to_disk(self):
report.generate_submission_report_pdf()

self.assertTrue(report.content.storage.exists(report.content.name))


class TemporaryFileUploadTests(TestCase):
def test_legacy_check_constraint(self):
with self.assertRaises(IntegrityError):
TemporaryFileUploadFactory.create(
submission=None,
legacy=False,
)

def test_non_legacy_check_constraint(self):
with self.assertRaises(IntegrityError):
TemporaryFileUploadFactory.create(
submission=SubmissionFactory.create(),
legacy=True,
)
6 changes: 6 additions & 0 deletions src/openforms/upgrades/upgrade_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ def run_checks(self) -> bool:
# If your current version falls outside of a supported range, you need to do another
# upgrade path (first) or there is no upgrade path at all.
UPGRADE_PATHS = {
"3.0": UpgradeConstraint(
valid_ranges={
VersionRange(minimum="2.8.0"),
},
scripts=["check_temporary_uploads"],
),
"2.8": UpgradeConstraint(
valid_ranges={
VersionRange(minimum="2.7.4"),
Expand Down

0 comments on commit 53bf9d8

Please sign in to comment.