Skip to content
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

💥 Make TemporaryFileUpload.submission non-nullable #4894

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
sergei-maertens marked this conversation as resolved.
Show resolved Hide resolved
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
Loading