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

Feature/attachment filename generator #836

Merged
merged 65 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
e6798c8
feat: Import code from old branch to avoid merge
miggol Sep 17, 2024
db4e2e0
wip: Working on attachments in new branch
miggol Sep 23, 2024
6edcf59
Merge remote-tracking branch 'origin/major/4' into feature/attachments-4
miggol Sep 23, 2024
0662e3d
fix: Mishmash of migrations
miggol Sep 23, 2024
83c899a
wip: Various fixes for attachments
miggol Sep 23, 2024
8bd9e1e
feat: Attachment renderable templates & updates
miggol Sep 24, 2024
e87b9ba
feat: Change var name and minor changes
miggol Oct 1, 2024
5232642
feat: Verbose name for Attachment.upload
miggol Oct 1, 2024
1833621
feat: Attachment views
miggol Oct 1, 2024
c1039b2
feat: Attachment form template
miggol Oct 1, 2024
4767f4c
wip: Minor changes
miggol Oct 1, 2024
2e9474e
feat: Add author field to Attachment
miggol Oct 1, 2024
c88c247
feat: Classmethod to initialize kind from an attachment
miggol Oct 1, 2024
d58ff27
feat: Provide manager object to children
miggol Oct 2, 2024
3454a92
feat: Download view for attachments
miggol Oct 2, 2024
f04a8dc
feat: Break out BaseAttachFormView to make attachment edit view
miggol Oct 2, 2024
cc6c496
fix: Move classes around to prevent circular imports
miggol Oct 7, 2024
7254db5
wip: Remove attachments manager for now
miggol Oct 7, 2024
e74ed5d
Merge remote-tracking branch 'origin/major/4' into feature/attachments-4
miggol Oct 7, 2024
66c8f79
feat: Initialize stepper outside of context-getting
miggol Oct 7, 2024
dcbe8b4
wip: Start managing attachments by Checkers
miggol Oct 7, 2024
be1de99
feat: Make sure slots know what they're attaching to
miggol Oct 7, 2024
aa0176b
fix: Minor fixes and comment updates
miggol Oct 7, 2024
987d916
cleanup: Remove old code and simplify object getting
miggol Oct 7, 2024
97db730
feat: Just fill in study slots by hand for now
miggol Oct 7, 2024
c8198d8
feat: Detach functionality for Attachments
miggol Oct 9, 2024
d2f4fdf
feat: Get Attachment subclasses without kind
miggol Oct 11, 2024
9c64aa4
feat: Make slots less dependent on kinds
miggol Oct 11, 2024
9131b70
feat: Detach view and links, editing flag for AttachFormView
miggol Oct 11, 2024
2682a7c
feat: Extra/optional attachments with DMP example
miggol Oct 11, 2024
54c71a6
feat: New desiredness flags
miggol Oct 11, 2024
4212ced
feat: New add_slot method
miggol Oct 11, 2024
3141555
feat: Template enhancements and cleanup
miggol Oct 11, 2024
d82222c
style: Black
miggol Oct 14, 2024
19ca5e1
style: djlint
miggol Oct 14, 2024
ec17850
add cdh_files/ to .gitignore
EdoStorm96 Oct 22, 2024
3b62436
Merge branch 'major/4' into feature/attachments-4
EdoStorm96 Oct 22, 2024
563154b
fix: create and implement attachment filename generator
EdoStorm96 Oct 23, 2024
3f76485
Merge branch 'major/4' into feature/attachment_filename_generator
EdoStorm96 Nov 25, 2024
f489256
feat: proper implementation of filename generator
EdoStorm96 Nov 25, 2024
a8edd10
fix: bug in Study.research_settings_contains_schools
EdoStorm96 Nov 25, 2024
f11758f
Merge branch 'major/4' into feature/attachment_filename_generator
miggol Dec 18, 2024
24b9194
Merge remote-tracking branch 'origin/major/4' into feature/attachment…
miggol Jan 6, 2025
dd68a5f
fix: Remove FnGen from model
miggol Jan 6, 2025
36b8d72
feat: Auto retrieve Kind from attachment if none provided
miggol Jan 6, 2025
c58e0bb
feat: Classmethod to get a slot from a proposal
miggol Jan 6, 2025
b76cdb4
wip: Get filename from slot instead of kind
miggol Jan 6, 2025
e065c2e
feat: Rename and finalize generate_filename() function
miggol Jan 6, 2025
1b89cc9
feat: Point methods in right direction
miggol Jan 6, 2025
135b4a7
style
miggol Jan 6, 2025
c75fe13
feat: Custom fn_part for kinds
miggol Jan 6, 2025
352fc48
feat: Add order variable for Slots and FnGen
miggol Jan 7, 2025
4b87f00
fix: Turn get_fn_part into classmethod
miggol Jan 7, 2025
9858dfa
fix: Kind derivation for empty slot matching
miggol Jan 7, 2025
ab40cdb
fix: Delete anachronistic migration
miggol Jan 7, 2025
14e2783
feat: Functions to enumerate slots
miggol Jan 7, 2025
72b751f
feat: Template changes to display normalized filename after <br />
miggol Jan 7, 2025
d2f3958
feat: Download filenames in PDF and DIFF are now the normalized versions
miggol Jan 7, 2025
70f7421
style
miggol Jan 7, 2025
eef2eae
Feat/fetc filename in pdf (#837)
EdoStorm96 Jan 8, 2025
43c3c5a
fix: Prevent collapsing of value in PDF
miggol Jan 8, 2025
feb821a
fix: Hyphenate kind in fetc filename
miggol Jan 8, 2025
da86b91
fix: Merge accident, double return statement
miggol Jan 8, 2025
a11756d
fix: Actually implement stepper.filled_slots
miggol Jan 8, 2025
1a1b913
fix: We can't assume all owners are represented in att_dict
miggol Jan 8, 2025
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 attachments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.urls import reverse
from main.utils import renderable


from cdh.files.db import FileField as CDHFileField

# Create your models here.
Expand Down
6 changes: 4 additions & 2 deletions attachments/templates/attachments/attachment_model.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

<a href="{% url "proposals:download_attachment_original" proposal_pk=proposal.pk attachment_pk=attachment.pk %}">
{{ attachment.upload.original_filename }}</a>
{% if include_normalized %}
<span class="ms-2o"> (<a href="{% url "proposals:download_attachment" proposal_pk=proposal.pk attachment_pk=attachment.pk %}">{% trans "FETC-bestandsnaam" %}</a>)</span>
{% if normalized_filename %}
<span class="ms-2o">
<br />
<a href="{% url "proposals:download_attachment" proposal_pk=proposal.pk attachment_pk=attachment.pk %}">{{ normalized_filename }}</a></span>
{% endif %}
119 changes: 118 additions & 1 deletion attachments/utils.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import mimetypes
from collections import Counter

from django.template.loader import get_template
from django.utils.translation import gettext as _
from django.urls import reverse
Expand Down Expand Up @@ -25,6 +28,14 @@ class AttachmentKind:
attached_field = "attachments"
desiredness = desiredness.OPTIONAL

@classmethod
def get_fn_part(cls):
if hasattr(cls, "fn_part"):
return cls.fn_part
# Capitalize DB name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really too familiar with classmethods ... But if I am reading this right, the kind will never have a fn_part attribute, since this method does not set the attribute. So the first if statement is redundant? Or does this happen automagically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the point is that we can give kinds a custom filename part without changing the db_name, which would be bad. I did this because I presume some filenames would get very long, and also are in English instead of Dutch. But in the end didn't decide on any custom names.

It's a classmethod because regular methods only work if the class is initialized i.e. has a self. However, we tend to use kinds as bare classes, just containing info. That's why it's a classmethod and as such it receives an uninitialized cls instead of a self, which is enough to get the attribute.

Note that cls and self are just conventions, they don't have to be called that way as long as they're the first argument.

parts = cls.db_name.split("_")
return "-".join([part.capitalize() for part in parts])


class AttachmentSlot(renderable):

Expand All @@ -37,15 +48,32 @@ def __init__(
kind=None,
force_desiredness=None,
optionality_group=None,
order=None,
):
self.attachment = attachment
self.attached_object = attached_object
self.kind = kind
self.order = order

if attachment and not kind:
# If an attachment was provided but no kind,
# attempt to get the kind from the attachment
self.kind = self.get_kind_from_attachment()
else:
self.kind = kind

self.force_desiredness = force_desiredness
self.optionality_group = optionality_group
if self.optionality_group:
self.optionality_group.members.append(self)

@classmethod
def from_proposal(attachment, proposal):
attached_object = attachment.get_owner_for_proposal(proposal)
return AttachmentSlot(
attached_object,
attachment=attachment,
)

def match(self, exclude=[]):
"""
Tries to find a matching attachment for this slot. If it finds one,
Expand All @@ -68,6 +96,16 @@ def match_and_set(self, exclude):
return self.attachment
return False

def get_kind_from_attachment(
self,
):
return get_kind_from_str(self.attachment.kind)

def get_fetc_filename(
self,
):
return generate_filename(self)

@property
def classes(self):
if self.required:
Expand Down Expand Up @@ -281,6 +319,85 @@ def merge_groups(slots):
return out


def generate_filename(slot):

proposal = slot.get_proposal()
chamber = proposal.reviewing_committee.name
lastname = proposal.created_by.last_name
refnum = proposal.reference_number
original_fn = slot.attachment.upload.original_filename
kind = slot.kind.get_fn_part()
order = slot.order

extension = (
"." + original_fn.split(".")[-1][-7:]
) # At most 7 chars seems reasonable

trajectory = None
if not type(slot.attached_object) is Proposal:
trajectory = "T" + str(slot.attached_object.order)

fn_parts = [
"FETC",
chamber,
refnum,
lastname,
trajectory,
kind,
order,
]

# Translations will trip up join(), so we convert them here.
# This will also remove parts that are None.
fn_parts = [str(p) for p in fn_parts if p]

return "-".join(fn_parts) + extension


def enumerate_slots(slots):
"""
Provides an order attribute to all attachment slots whose kind
appears more than once in the provided list.
"""
# Create seperate slot lists per attached_object
per_ao = sort_into_dict(
slots,
lambda x: x.attached_object,
).values()
# Assign orders to them separately
for ao_slots in per_ao:
assign_orders(ao_slots)


def sort_into_dict(iterable, key_func):
"""
Split iterable into separate lists in a dict whose keys
are the shared response to all its items' key_func(item).
"""
out_dict = {}
for item in iterable:
key = key_func(item)
if key not in out_dict:
out_dict[key] = [item]
else:
out_dict[key].append(item)
return out_dict


def assign_orders(slots):
# Count total kind occurrences
totals = Counter([slot.kind for slot in slots])
# Create counter to increment gradually
kind_counter = Counter()
# Loop through the slots
for slot in slots:
if totals[slot.kind] < 2:
# Skip slots with unique kinds
continue
kind_counter[slot.kind] += 1
slot.order = kind_counter[slot.kind]


def get_kind_from_str(db_name):
from attachments.kinds import ATTACHMENTS, OtherAttachment

Expand Down
9 changes: 7 additions & 2 deletions proposals/utils/pdf_diff_sections.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
get_extra_documents,
multi_sections,
KindRow,
AttachmentRow,
UploadDateRow,
)

Expand Down Expand Up @@ -576,8 +577,12 @@ def __init__(self, obj, sub_title, proposal):

def make_row_for_field(self, field):

# "kind" and "upload" require slots as their obj, other fields receive
# the attachment as their obj.
if field == "kind":
return KindRow(self.obj, self.obj.attachment, field)
if field == "upload":
return AttachmentRow(self.obj, field, self.proposal)
else:
obj = self.obj.attachment

Expand Down Expand Up @@ -737,7 +742,7 @@ def _all_slots_list(self, proposal):
from proposals.utils.stepper import Stepper

stepper = Stepper(proposal)
slots = [slot for slot in stepper.attachment_slots if slot.attachment]
slots = stepper.filled_slots

return slots

Expand Down Expand Up @@ -1008,7 +1013,7 @@ def get_all_attachment_sections_diff(self):

att_dict = self._get_matches_from_slots(self.old_slots, self.new_slots)

for owner_num in range(len(att_dict)):
for owner_num in att_dict.keys():
proposal = self.new_p
if owner_num == 0:
owner_obj = proposal
Expand Down
57 changes: 34 additions & 23 deletions proposals/utils/pdf_diff_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,11 @@ def __init__(self, obj, field, proposal=None):
self.proposal = proposal

def value(self):
return RowValue(self.obj, self.field, self.proposal).get_field_value()
val = RowValue(self.obj, self.field, self.proposal).get_field_value()
if val == "":
# Prevent collapsing of the value area
val = mark_safe("&nbsp;")
return val

def verbose_name(self):
if self.field in self.verbose_name_diff_field_dict:
Expand Down Expand Up @@ -231,6 +235,35 @@ def value(self):
return self.slot.kind.name


class AttachmentRow(Row):
"""
This row receives a slot as self.obj and also is basically fully custom.
"""

def get_verbose_name(self, field):
return _("Bestand")

def value(self):

if self.obj.attachment.upload:
output = format_html(
'<a href="{}">{}</a>',
settings.BASE_URL
+ reverse(
"proposals:download_attachment",
kwargs={
"proposal_pk": self.proposal.pk,
"attachment_pk": self.obj.attachment.pk,
},
),
self.obj.get_fetc_filename(),
)
else:
output = _("Niet aangeleverd")

return output


class UploadDateRow:
"""
Another very specific row case for attachments. This one just needed to be
Expand Down Expand Up @@ -280,8 +313,6 @@ def get_field_value(self):
return self.handle_user(value)
elif isinstance(value, Relation) or isinstance(value, Compensation):
return value.description
if value.__class__.__name__ == "FileWrapper":
return self.handle_attachment(value)
elif value.__class__.__name__ == "ManyRelatedManager":
if value.all().model == User:
return self.get_applicants_names(value)
Expand Down Expand Up @@ -316,26 +347,6 @@ def handle_field_file(self, field_file):

return output

def handle_attachment(self, filewrapper):

if filewrapper:
output = format_html(
'<a href="{}">{}</a>',
settings.BASE_URL
+ reverse(
"proposals:download_attachment_original",
kwargs={
"proposal_pk": self.proposal.pk,
"attachment_pk": self.obj.pk,
},
),
_("Download"),
)
else:
output = _("Niet aangeleverd")

return output

def yes_no_doubt(self, value):
from main.models import YesNoDoubt

Expand Down
12 changes: 10 additions & 2 deletions proposals/utils/stepper.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from interventions.forms import InterventionForm

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


Expand Down Expand Up @@ -82,7 +82,15 @@ def attachment_slots(
success = empty_slot.match_and_set(exclude=exclude)
if success:
extra_slots.append(empty_slot)
return self._attachment_slots + extra_slots
all_slots = self._attachment_slots + extra_slots
enumerate_slots(all_slots)
return all_slots

@property
def filled_slots(
self,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to use this in AttachmentsList.get_container(), but forgot. Please use it or get rid of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah I forgot to implement it lol

):
return [slot for slot in self.attachment_slots if slot.attachment]

def get_context_data(self):
context = super().get_context_data()
Expand Down
12 changes: 6 additions & 6 deletions proposals/views/attachment_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from reviews.mixins import HideStepperMixin
from django.utils.translation import gettext as _
from attachments.kinds import ATTACHMENTS, KIND_CHOICES
from attachments.utils import AttachmentKind, merge_groups
from attachments.utils import AttachmentKind, merge_groups, AttachmentSlot
from cdh.core import forms as cdh_forms
from django.utils.translation import gettext as _
from reviews.mixins import UsersOrGroupsAllowedMixin
Expand Down Expand Up @@ -369,14 +369,14 @@ def get_filename(self):
if self.original_filename:
return self.attachment.upload.original_filename
else:
return self.get_filename_from_kind()
return self.get_filename_from_slot()

def get_filename_from_kind(self):
self.kind = AttachmentKind.from_proposal(
self.proposal,
def get_filename_from_slot(self):
self.slot = AttachmentSlot.from_proposal(
self.attachment,
self.proposal,
)
return self.kind.name
return self.slot.get_fetc_filename()

def get_file_response(self):
attachment_file = self.attachment.upload.file
Expand Down
2 changes: 1 addition & 1 deletion reviews/templates/reviews/review_attachments.html
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ <h6>{{ slot.kind.name }}</h6>
</tr>
<tr>
<td>{% trans "Bestand" %}</td>
<td>{% include slot.attachment with proposal=proposal include_normalized=True %}</td>
<td>{% include slot.attachment with proposal=proposal normalized_filename=slot.get_fetc_filename %}</td>
</tr>
<tr>
<td>{% trans "Commentaar" %}</td>
Expand Down
3 changes: 1 addition & 2 deletions reviews/utils/attachment_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,8 @@ def get_containers(
from proposals.utils.stepper import Stepper

stepper = Stepper(self.proposal)
filled_slots = [slot for slot in stepper.attachment_slots if slot.attachment]
containers = DocList(
filled_slots,
stepper.filled_slots,
proposal=self.proposal,
).as_containers()
return containers
Expand Down
2 changes: 1 addition & 1 deletion reviews/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ def get_slots(
):
proposal = self.get_review().proposal
stepper = Stepper(proposal)
return [slot for slot in stepper.attachment_slots if slot.attachment]
return stepper.filled_slots

def per_object(
self,
Expand Down
Loading