Skip to content

Commit

Permalink
Feature/extra attachments (#743)
Browse files Browse the repository at this point in the history
* feat: Link to add extra attachment

* feat: KIND_CHOICES constant for... Kind choices

* feat: AttachFormView can now handle extra attachments

* feat: Match extra attachments to additional slots

* feat: Remove extra arg, kinds are now selectable when editing

* feat: Avoid using kinds to determine attached model

* fix: Missing detach form

* style: Black and djlint

* docs: Add comment to attachment_slots

* fix: Wrong object PK

* fix: Improve description of action in attach form

* feat: Much nicer styling on the attachments page

* fix: Untranslated string
  • Loading branch information
miggol authored Nov 12, 2024
1 parent b379ae1 commit 03d1828
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 46 deletions.
4 changes: 3 additions & 1 deletion attachments/kinds.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def num_recommended(self):
class OtherProposalAttachment(ProposalAttachmentKind):

db_name = "other"
name = _("Overige bestanden")
name = _("Overig bestand")
description = _("Voor alle overige soorten bestanden")

def num_required(self):
Expand All @@ -71,3 +71,5 @@ def num_suggested(self):
]

ATTACHMENTS = PROPOSAL_ATTACHMENTS + STUDY_ATTACHMENTS

KIND_CHOICES = list(((kind.db_name, kind.name) for kind in ATTACHMENTS))
3 changes: 2 additions & 1 deletion attachments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class Attachment(models.Model, renderable):
# whatever form needs them.
# From Django 5 onwards we can define a callable to get
# the choices which would be the preferred solution.
default=("", _("Gelieve selecteren")),
default=("other", _("Overig bestand")),
verbose_name=_("Type bestand"),
)
name = models.CharField(
max_length=50,
Expand Down
6 changes: 3 additions & 3 deletions attachments/templates/attachments/slot.html
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
{% load i18n %}

<div class="row mt-4 mb-4">
<div class="row mt-4 mb-4 p-3 border-3 border-bottom rounded {{classes}} bg-light">
<div class="col-2 align-middle">{{ slot.desiredness }}</div>
<div class="col-7 align-middle">
<h5>{{ slot.kind.name }}</h5>
{% if slot.attachment %}
{% include slot.attachment with proposal=proposal %}
{% else %}
Nog toe te voegen
{% trans "Nog toe te voegen" %}
{% endif %}
{{ kind.reason }}
</div>
<div class="col-3 align-middle">
<div class="col-3 align-middle text-end flex-end">
{% if slot.attachment %}
<a class="btn btn-primary" href="{{ slot.get_edit_url }}">{% trans "Wijzig" %}</a>
{% else %}
Expand Down
17 changes: 15 additions & 2 deletions attachments/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,24 @@ def __init__(
def match(self, exclude):
"""
Tries to fill this slot with an existing attachment that is not
in the exclusion set of already matched attachments.
in the exclusion set of already matched attachments. Returns True
or False depending on if the slot was succesfully matched.
"""
for instance in self.get_instances_for_slot():
if instance not in exclude:
self.attachment = instance
break
self.kind = get_kind_from_str(instance.kind)
return True
return False

@property
def classes(self):
if self.required:
if self.attachment:
return "border-success"
else:
return "border-warning"
return ""

@property
def desiredness(self):
Expand Down Expand Up @@ -80,6 +92,7 @@ def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context["slot"] = self
context["proposal"] = self.get_proposal()
context["classes"] = self.classes
return context

def get_proposal(self):
Expand Down
10 changes: 6 additions & 4 deletions proposals/templates/proposals/attach_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ <h3>{% trans "Maak of wijzig een bestand" %}</h3>
{% trans "de aanvraag" %} <em>{{ proposal.title }}</em>.
</p>
{% if not view.editing %}
{% blocktrans with name=kind_name trimmed %}
Je gaat hier een {{ name }} aan toevoegen. Wil je een ander soort bestand toevoegen? Ga dan terug naar de vorige pagina.
{% endblocktrans %}
{% if kind %}
{% blocktrans with name=kind.name trimmed %}
Je gaat hier een {{ name }} aan toevoegen. Wil je een ander soort bestand toevoegen? Ga dan terug naar de vorige pagina.
{% endblocktrans %}
{% endif %}
{% endif %}
</p>
</p>
{% endblock %}

{% block form-buttons %}
Expand Down
17 changes: 15 additions & 2 deletions proposals/templates/proposals/attachments.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,28 @@ <h3 class="mt-4">{% trans "Algemeen" %}</h3>
{% for slot in proposal_slots %}
{% include slot %}
{% endfor %}
{% for study, slots in study_slots.items %}
<hr />
<div class="text-center mb-5 mt-3">
<a class="btn btn-light align-bottom" href="{% url "proposals:attach_proposal" proposal.pk %}">
<img class="mb-1 me-2" src="{% static 'proposals/images/add.png' %}"
title="{% trans 'Toevoegen' %}" />
<h6 class="d-inline" style="--bs-btn-font-weight: 200; ">Voeg optioneel bestand toe aan aanvraag</h6>
</a>
</div>
{% for study, slots in study_slots.items %}
<h3>
{% trans "Traject " %} {{ study.order }}
{% if study.name %}: <em>{{ study.name }}</em>{% endif %}
</h3>
{% for slot in slots %}
{% include slot with manager=manager %}
{% endfor %}
<div class="text-center mb-5 mt-3">
<a class="btn btn-light align-bottom" href="{% url "proposals:attach_study" study.pk %}">
<img class="mb-1 me-2" src="{% static 'proposals/images/add.png' %}"
title="{% trans 'Toevoegen' %}" />
<h6 class="d-inline" style="--bs-btn-font-weight: 200; ">Voeg optioneel bestand toe aan traject {{study.order}}</h6>
</a>
</div>
{% endfor %}
{% block auto-form-render %}{% endblock %}
{% endblock %}
6 changes: 5 additions & 1 deletion proposals/templates/proposals/detach_form.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ <h3>{% trans "Verwijder een bestand" %}</h3>
{% endblocktrans %}
{% include object with proposal=proposal %}
</p>
</p>
<p>
{% blocktrans trimmed %}
Als je een nieuwe versie van dit document wil aanleveren, verwijder het dan niet, maar ga dan terug naar de vorige pagina en wijzig het daar. Zo blijft de geschiedenis van het document bewaard, en dat vergemakkelijkt het beoordeelproces aanzienlijk.
{% endblocktrans %}
</p>
{% endblock %}

{% block form-buttons %}
Expand Down
26 changes: 15 additions & 11 deletions proposals/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,38 +182,42 @@
ProposalAttachmentsView.as_view(),
name="attachments",
),
path(
"attach_proposal/<int:other_pk>/",
ProposalAttachView.as_view(owner_model=Proposal, extra=True, editing=False),
name="attach_proposal",
),
path(
"attach_study/<int:other_pk>/",
ProposalAttachView.as_view(owner_model=Study, extra=True, editing=False),
name="attach_study",
),
path(
"attach_proposal/<str:kind>/<int:other_pk>/",
ProposalAttachView.as_view(
owner_model=Proposal,
editing=False,
),
name="attach_proposal",
),
path(
"attach_study/<str:kind>/<int:other_pk>/",
ProposalAttachView.as_view(
owner_model=Study,
editing=False,
),
name="attach_study",
),
path(
"attach_proposal/extra/<int:other_pk>/",
ProposalAttachView.as_view(owner_model=Proposal, extra=True),
name="attach_proposal",
),
path(
"attach_study/extra/<int:other_pk>/",
ProposalAttachView.as_view(owner_model=Study, extra=True),
name="attach_study",
),
path(
"<int:proposal_pk>/detach/<int:attachment_pk>/",
ProposalDetachView.as_view(),
name="detach_file",
),
path(
"attachments/<int:other_pk>/edit/<int:attachment_pk>/",
ProposalUpdateAttachmentView.as_view(),
ProposalUpdateAttachmentView.as_view(
editing=True,
),
name="update_attachment",
),
path(
Expand Down
37 changes: 34 additions & 3 deletions proposals/utils/stepper.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
from interventions.forms import InterventionForm

from proposals.utils.validate_sessions_tasks import validate_sessions_tasks
from attachments.utils import AttachmentSlot
from attachments.kinds import desiredness


class Stepper(renderable):
Expand All @@ -37,9 +39,38 @@ def __init__(
self.request = request
self.items = []
self.current_item_ancestors = []
self.attachment_slots = []
self._attachment_slots = []
self.check_all(self.starting_checkers)

@property
def attachment_slots(
self,
):
"""
Appends unmatched attachments as extra slots to the internal
list _attachment_slots.
"""
extra_slots = []
# Get all attachable objects
objects = [self.proposal] + list(self.proposal.study_set.all())
for obj in objects:
success = True
while success:
# Keep appending extra slots as long as they can be matched to
# to an attachment for this object. We update the exclude list
# as we go to not duplicate them or end up in an infinite loop.
exclude = [s.attachment for s in self._attachment_slots] + [
s.attachment for s in extra_slots
]
empty_slot = AttachmentSlot(
obj,
force_desiredness=desiredness.EXTRA,
)
success = empty_slot.match(exclude=exclude)
if success:
extra_slots.append(empty_slot)
return self._attachment_slots + extra_slots

def get_context_data(self):
context = super().get_context_data()
# Provide the stepper bubble classes in order
Expand Down Expand Up @@ -167,9 +198,9 @@ def add_slot(self, slot):
here because the stepper has ownership of the already matched
attachments to be excluded from matching.
"""
exclude = [slot.attachment for slot in self.attachment_slots]
exclude = [slot.attachment for slot in self._attachment_slots]
slot.match(exclude)
self.attachment_slots.append(slot)
self._attachment_slots.append(slot)

def has_multiple_studies(
self,
Expand Down
42 changes: 24 additions & 18 deletions proposals/views/attachment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
from main.forms import ConditionalModelForm
from cdh.core import forms as cdh_forms
from django.http import FileResponse
from attachments.kinds import ATTACHMENTS
from attachments.kinds import ATTACHMENTS, KIND_CHOICES
from attachments.utils import AttachmentKind
from cdh.core import forms as cdh_forms
from django.utils.translation import gettext as _
from reviews.mixins import UsersOrGroupsAllowedMixin


Expand All @@ -28,6 +30,11 @@ class Meta:
"name",
"comments",
]
widgets = {
"kind": cdh_forms.BootstrapSelect(
choices=KIND_CHOICES,
)
}

def __init__(self, kind=None, other_object=None, extra=False, **kwargs):
self.kind = kind
Expand All @@ -38,8 +45,10 @@ def __init__(self, kind=None, other_object=None, extra=False, **kwargs):
elif type(other_object) is Study:
self._meta.model = StudyAttachment
super().__init__(**kwargs)
if not extra:
if kind is not None:
del self.fields["kind"]
else:
self.fields["kind"].default = ("other", _("Overig bestand"))

def save(
self,
Expand Down Expand Up @@ -94,12 +103,16 @@ class AttachFormView:
model = Attachment
form_class = AttachForm
template_name = "proposals/attach_form.html"
# The editing variable is set in the URLconf to determine
# if we're editing an existing file or adding a new one.
editing = True

def set_upload_field_label(self, form):
# Remind the user of what they're uploading
upload_field = form.fields["upload"]
kind = self.get_kind()
upload_field.label += f" ({kind.name})"
if kind:
upload_field.label += f" ({kind.name})"

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
Expand All @@ -108,7 +121,6 @@ def get_context_data(self, **kwargs):
if type(owner_object) is not Proposal:
context["study"] = self.get_owner_object()
context["kind"] = self.get_kind()
context["kind_name"] = self.get_kind().name
form = context["form"]
self.set_upload_field_label(form)
return context
Expand All @@ -126,8 +138,10 @@ def get_owner_object(self):
return owner_class.objects.get(pk=other_pk)

def get_kind(self):
kind_str = self.kwargs.get("kind")
return get_kind_from_str(kind_str)
kind_str = self.kwargs.get("kind", None)
if kind_str:
return get_kind_from_str(kind_str)
return None

def get_success_url(
self,
Expand All @@ -143,10 +157,10 @@ def get_form_kwargs(
kwargs = super().get_form_kwargs()
kwargs.update(
{
"kind": self.get_kind(),
"other_object": self.get_owner_object(),
}
)
kwargs["kind"] = self.get_kind()
return kwargs


Expand All @@ -162,10 +176,6 @@ class ProposalAttachView(
template_name = "proposals/attach_form.html"
extra = False

def get_kind(self):
kind_str = self.kwargs.get("kind")
return get_kind_from_str(kind_str)


class ProposalUpdateAttachmentView(
AttachFormView,
Expand All @@ -175,7 +185,6 @@ class ProposalUpdateAttachmentView(
model = Attachment
form_class = AttachForm
template_name = "proposals/attach_form.html"
editing = True

def get_object(
self,
Expand All @@ -186,15 +195,12 @@ def get_object(
return obj

def get_owner_object(self):
other_class = self.get_kind().attached_object
instance = self.get_object().get_correct_submodel()
attached_field = instance._meta.get_field("attached_to")
other_class = attached_field.related_model
other_pk = self.kwargs.get("other_pk")
return other_class.objects.get(pk=other_pk)

def get_kind(self):
obj = self.get_object()
kind_str = obj.kind
return get_kind_from_str(kind_str)


class DetachForm(
forms.Form,
Expand Down

0 comments on commit 03d1828

Please sign in to comment.