From e61342d8576042725e053e3439af7cb2ba330909 Mon Sep 17 00:00:00 2001 From: George Katsikas <giorgakis.katsikas@gmail.com> Date: Tue, 12 Mar 2024 12:39:55 +0100 Subject: [PATCH] allow creation of orphaned subsidyattachments fixes #218 --- scipost_django/finances/forms.py | 4 +- ...0029_subsidyattachment_git_url_and_more.py | 34 +++++++++++++++ scipost_django/finances/models.py | 42 +++++++++++++++---- .../templates/finances/subsidy_list.html | 3 ++ scipost_django/finances/urls.py | 7 +++- scipost_django/finances/views.py | 22 +++++----- 6 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 scipost_django/finances/migrations/0029_subsidyattachment_git_url_and_more.py diff --git a/scipost_django/finances/forms.py b/scipost_django/finances/forms.py index bd3d71021..34aac0d01 100644 --- a/scipost_django/finances/forms.py +++ b/scipost_django/finances/forms.py @@ -292,6 +292,7 @@ class SubsidyAttachmentForm(forms.ModelForm): fields = ( "subsidy", "attachment", + "git_url", "kind", "date", "description", @@ -316,9 +317,10 @@ class SubsidyAttachmentForm(forms.ModelForm): def clean_attachment(self): attachment = self.cleaned_data["attachment"] + existing_attachment = getattr(self.instance, "attachment", None) # Allow already uploaded attachments - if hasattr(self.instance, "attachment") and not attachment is None: + if existing_attachment and attachment is not None: return attachment filename_regex = ( diff --git a/scipost_django/finances/migrations/0029_subsidyattachment_git_url_and_more.py b/scipost_django/finances/migrations/0029_subsidyattachment_git_url_and_more.py new file mode 100644 index 000000000..75f83a543 --- /dev/null +++ b/scipost_django/finances/migrations/0029_subsidyattachment_git_url_and_more.py @@ -0,0 +1,34 @@ +# Generated by Django 4.2.10 on 2024-03-12 10:52 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + dependencies = [ + ("contenttypes", "0002_remove_content_type_name"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ("finances", "0028_alter_subsidy_status"), + ] + + operations = [ + migrations.AddField( + model_name="subsidyattachment", + name="git_url", + field=models.URLField( + blank=True, help_text="URL to the file's location in GitLab" + ), + ), + migrations.AlterField( + model_name="subsidyattachment", + name="subsidy", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="attachments", + to="finances.subsidy", + ), + ) + ] diff --git a/scipost_django/finances/models.py b/scipost_django/finances/models.py index 67e1bed47..931f24a4f 100644 --- a/scipost_django/finances/models.py +++ b/scipost_django/finances/models.py @@ -4,6 +4,8 @@ __license__ = "AGPL v3" import datetime import os +from re import I +from typing import TYPE_CHECKING from django.conf import settings from django.contrib.contenttypes.models import ContentType @@ -22,6 +24,9 @@ from .utils import id_to_slug from scipost.storage import SecureFileStorage +if TYPE_CHECKING: + from organizations.models import Organization + class Subsidy(models.Model): """ @@ -43,7 +48,7 @@ class Subsidy(models.Model): after which the object of the Subsidy is officially terminated. """ - organization = models.ForeignKey( + organization = models.ForeignKey["Organization"]( "organizations.Organization", on_delete=models.CASCADE ) subsidy_type = models.CharField(max_length=256, choices=SUBSIDY_TYPES) @@ -188,10 +193,13 @@ class SubsidyPayment(models.Model): return self.proof_of_payment.date if self.proof_of_payment else None -def subsidy_attachment_path(instance, filename): +def subsidy_attachment_path(instance: "SubsidyAttachment", filename: str) -> str: """ Save the uploaded SubsidyAttachments to country-specific folders. """ + if instance.subsidy is None: + return "uploads/finances/subsidies/orphaned/%s" % filename + return "uploads/finances/subsidies/{0}/{1}/{2}".format( instance.subsidy.date_from.strftime("%Y"), instance.subsidy.organization.country, @@ -224,15 +232,21 @@ class SubsidyAttachment(models.Model): (VISIBILITY_FINADMINONLY, "SciPost FinAdmin only"), ) - subsidy = models.ForeignKey( + subsidy = models.ForeignKey["Subsidy"]( "finances.Subsidy", related_name="attachments", + null=True, blank=True, on_delete=models.CASCADE, ) attachment = models.FileField( - upload_to=subsidy_attachment_path, storage=SecureFileStorage() + upload_to=subsidy_attachment_path, + storage=SecureFileStorage(), + ) + + git_url = models.URLField( + blank=True, help_text="URL to the file's location in GitLab" ) kind = models.CharField( @@ -259,7 +273,7 @@ class SubsidyAttachment(models.Model): def get_absolute_url(self): if self.subsidy: return reverse( - "finances:subsidy_attachment", args=(self.subsidy.id, self.id) + "finances:subsidy_attachment", kwargs={"attachment_id": self.id} ) @property @@ -284,15 +298,29 @@ class SubsidyAttachment(models.Model): # Delete attachment files with same name if they exist, allowing replacement without name change @receiver(pre_save, sender=SubsidyAttachment) -def delete_old_attachment_file(sender, instance, **kwargs): +def delete_old_attachment_file(sender, instance: SubsidyAttachment, **kwargs): """ Replace existing file on update if a new one is provided. + Move file to the new location if the subsidy changes. """ if instance.pk and instance.attachment: old = SubsidyAttachment.objects.get(pk=instance.pk) - if old.attachment and old.attachment != instance.attachment: + if old is None or old.attachment is None: + return + + # Delete old file if it is replaced + if old.attachment != instance.attachment: old.attachment.delete(save=False) + # Move file to new location if subsidy changes + if old.subsidy != instance.subsidy: + old_relative_path = old.attachment.name + new_relative_path = subsidy_attachment_path(instance, instance.filename) + + instance.attachment.storage.save(new_relative_path, instance.attachment) + instance.attachment.storage.delete(old_relative_path) + instance.attachment.name = new_relative_path + @receiver(models.signals.post_delete, sender=SubsidyAttachment) def auto_delete_file_on_delete(sender, instance, **kwargs): diff --git a/scipost_django/finances/templates/finances/subsidy_list.html b/scipost_django/finances/templates/finances/subsidy_list.html index fbcd0c91d..0f844698f 100644 --- a/scipost_django/finances/templates/finances/subsidy_list.html +++ b/scipost_django/finances/templates/finances/subsidy_list.html @@ -26,6 +26,9 @@ <li> <a href="{% url 'finances:subsidy_create' %}">Add a Subsidy</a> </li> + <li> + <a href="{% url 'finances:subsidyattachment_create' %}">Add a SubsidyAttachment</a> + </li> <li> <a href="{% url 'finances:subsidies_old' %}" target="_blank">Go to the old list page</a> </li> diff --git a/scipost_django/finances/urls.py b/scipost_django/finances/urls.py index dd9ff6128..02bfde69f 100644 --- a/scipost_django/finances/urls.py +++ b/scipost_django/finances/urls.py @@ -119,6 +119,11 @@ urlpatterns = [ views.SubsidyAttachmentCreateView.as_view(), name="subsidyattachment_create", ), + path( + "subsidies/attachments/add/", + views.SubsidyAttachmentCreateView.as_view(), + name="subsidyattachment_create", + ), path( "subsidies/attachments/<int:pk>/update/", views.SubsidyAttachmentUpdateView.as_view(), @@ -130,7 +135,7 @@ urlpatterns = [ name="subsidyattachment_delete", ), path( - "subsidies/<int:subsidy_id>/attachments/<int:attachment_id>", + "subsidies/attachments/<int:attachment_id>", views.subsidy_attachment, name="subsidy_attachment", ), diff --git a/scipost_django/finances/views.py b/scipost_django/finances/views.py index 1846ef697..de0186c44 100644 --- a/scipost_django/finances/views.py +++ b/scipost_django/finances/views.py @@ -315,7 +315,7 @@ class SubsidyAutocompleteView(autocomplete.Select2QuerySetView): | Q(date_until__year__icontains=self.q) ) return qs - + def get_result_label(self, item): return format_html( "{}<br>{} -> {} [{}]", @@ -494,13 +494,17 @@ class SubsidyAttachmentCreateView(PermissionsMixin, CreateView): return context def get_initial(self): - subsidy = get_object_or_404(Subsidy, pk=self.kwargs.get("subsidy_id")) - return {"subsidy": subsidy} + subsidy_id = self.kwargs.get("subsidy_id") + if subsidy_id is not None: + subsidy = get_object_or_404(Subsidy, pk=self.kwargs.get("subsidy_id")) + return {"subsidy": subsidy} + return {} def get_success_url(self): - return reverse_lazy( - "finances:subsidy_details", kwargs={"pk": self.object.subsidy.id} - ) + if subsidy := self.object.subsidy: + return reverse_lazy("finances:subsidy_details", kwargs={"pk": subsidy.id}) + + return reverse_lazy("finances:subsidies") class SubsidyAttachmentUpdateView(PermissionsMixin, UpdateView): @@ -544,10 +548,8 @@ class SubsidyAttachmentDeleteView(PermissionsMixin, DeleteView): ) -def subsidy_attachment(request, subsidy_id, attachment_id): - attachment = get_object_or_404( - SubsidyAttachment.objects, subsidy__id=subsidy_id, id=attachment_id - ) +def subsidy_attachment(request, attachment_id): + attachment = get_object_or_404(SubsidyAttachment.objects, id=attachment_id) if not (request.user.is_authenticated and attachment.visible_to_user(request.user)): raise PermissionDenied content_type, encoding = mimetypes.guess_type(attachment.attachment.path) -- GitLab