-
Notifications
You must be signed in to change notification settings - Fork 1
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
Changes from 59 commits
e6798c8
db4e2e0
6edcf59
0662e3d
83c899a
8bd9e1e
e87b9ba
5232642
1833621
c1039b2
4767f4c
2e9474e
c88c247
d58ff27
3454a92
f04a8dc
cc6c496
7254db5
e74ed5d
66c8f79
dcbe8b4
be1de99
aa0176b
987d916
97db730
c8198d8
d2f4fdf
9c64aa4
9131b70
2682a7c
54c71a6
4212ced
3141555
d82222c
19ca5e1
ec17850
3b62436
563154b
3f76485
f489256
a8edd10
f11758f
24b9194
dd68a5f
36b8d72
c58e0bb
b76cdb4
e065c2e
1b89cc9
135b4a7
c75fe13
352fc48
4b87f00
9858dfa
ab40cdb
14e2783
72b751f
d2f3958
70f7421
eef2eae
43c3c5a
feb821a
da86b91
a11756d
1a1b913
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
|
@@ -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 | ||
parts = cls.db_name.split("_") | ||
return "_".join([part.capitalize() for part in parts]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would prefer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought I would differentiate them as the kind is one single fn part. But you're right it does look better with a hyphen. |
||
|
||
|
||
class AttachmentSlot(renderable): | ||
|
||
|
@@ -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, | ||
|
@@ -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: | ||
|
@@ -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 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1093,6 +1093,7 @@ def make_stepper_item(self): | |
self.stepper, | ||
) | ||
return item | ||
return [TranslationChecker] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think this is a typo There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well spotted, must have happened when merging the base. |
||
|
||
|
||
class TranslationChecker( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
||
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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() | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 uninitializedcls
instead of aself
, which is enough to get the attribute.Note that
cls
andself
are just conventions, they don't have to be called that way as long as they're the first argument.