-
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
fix: create and implement attachment filename generator #725
Closed
Closed
Changes from 38 commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
e6798c8
feat: Import code from old branch to avoid merge
miggol db4e2e0
wip: Working on attachments in new branch
miggol 6edcf59
Merge remote-tracking branch 'origin/major/4' into feature/attachments-4
miggol 0662e3d
fix: Mishmash of migrations
miggol 83c899a
wip: Various fixes for attachments
miggol 8bd9e1e
feat: Attachment renderable templates & updates
miggol e87b9ba
feat: Change var name and minor changes
miggol 5232642
feat: Verbose name for Attachment.upload
miggol 1833621
feat: Attachment views
miggol c1039b2
feat: Attachment form template
miggol 4767f4c
wip: Minor changes
miggol 2e9474e
feat: Add author field to Attachment
miggol c88c247
feat: Classmethod to initialize kind from an attachment
miggol d58ff27
feat: Provide manager object to children
miggol 3454a92
feat: Download view for attachments
miggol f04a8dc
feat: Break out BaseAttachFormView to make attachment edit view
miggol cc6c496
fix: Move classes around to prevent circular imports
miggol 7254db5
wip: Remove attachments manager for now
miggol e74ed5d
Merge remote-tracking branch 'origin/major/4' into feature/attachments-4
miggol 66c8f79
feat: Initialize stepper outside of context-getting
miggol dcbe8b4
wip: Start managing attachments by Checkers
miggol be1de99
feat: Make sure slots know what they're attaching to
miggol aa0176b
fix: Minor fixes and comment updates
miggol 987d916
cleanup: Remove old code and simplify object getting
miggol 97db730
feat: Just fill in study slots by hand for now
miggol c8198d8
feat: Detach functionality for Attachments
miggol d2f4fdf
feat: Get Attachment subclasses without kind
miggol 9c64aa4
feat: Make slots less dependent on kinds
miggol 9131b70
feat: Detach view and links, editing flag for AttachFormView
miggol 2682a7c
feat: Extra/optional attachments with DMP example
miggol 54c71a6
feat: New desiredness flags
miggol 4212ced
feat: New add_slot method
miggol 3141555
feat: Template enhancements and cleanup
miggol d82222c
style: Black
miggol 19ca5e1
style: djlint
miggol ec17850
add cdh_files/ to .gitignore
EdoStorm96 3b62436
Merge branch 'major/4' into feature/attachments-4
EdoStorm96 563154b
fix: create and implement attachment filename generator
EdoStorm96 3f76485
Merge branch 'major/4' into feature/attachment_filename_generator
EdoStorm96 f489256
feat: proper implementation of filename generator
EdoStorm96 a8edd10
fix: bug in Study.research_settings_contains_schools
EdoStorm96 f11758f
Merge branch 'major/4' into feature/attachment_filename_generator
miggol 24b9194
Merge remote-tracking branch 'origin/major/4' into feature/attachment…
miggol dd68a5f
fix: Remove FnGen from model
miggol 36b8d72
feat: Auto retrieve Kind from attachment if none provided
miggol c58e0bb
feat: Classmethod to get a slot from a proposal
miggol b76cdb4
wip: Get filename from slot instead of kind
miggol e065c2e
feat: Rename and finalize generate_filename() function
miggol 1b89cc9
feat: Point methods in right direction
miggol 135b4a7
style
miggol c75fe13
feat: Custom fn_part for kinds
miggol 352fc48
feat: Add order variable for Slots and FnGen
miggol 4b87f00
fix: Turn get_fn_part into classmethod
miggol 9858dfa
fix: Kind derivation for empty slot matching
miggol ab40cdb
fix: Delete anachronistic migration
miggol 14e2783
feat: Functions to enumerate slots
miggol 72b751f
feat: Template changes to display normalized filename after <br />
miggol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ certs/ | |
fetc/ldap_settings.py | ||
fetc/saml_settings.py | ||
*.sqlite3 | ||
cdh_files/ | ||
|
||
### Coverage ### | ||
.coverage | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
from django.contrib import admin | ||
|
||
# Register your models here. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
from django.apps import AppConfig | ||
|
||
|
||
class AttachmentsConfig(AppConfig): | ||
default_auto_field = "django.db.models.BigAutoField" | ||
name = "attachments" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
from django.utils.translation import gettext as _ | ||
from django.urls import reverse | ||
|
||
from proposals.models import Proposal | ||
from studies.models import Study | ||
from main.utils import renderable | ||
from attachments.models import ProposalAttachment, StudyAttachment | ||
from attachments.utils import AttachmentKind, desiredness | ||
|
||
|
||
class ProposalAttachmentKind(AttachmentKind): | ||
|
||
attached_object = Proposal | ||
attachment_class = ProposalAttachment | ||
|
||
|
||
class StudyAttachmentKind(AttachmentKind): | ||
|
||
attached_object = Study | ||
attachment_class = StudyAttachment | ||
|
||
|
||
class InformationLetter(StudyAttachmentKind): | ||
|
||
db_name = "information_letter" | ||
name = _("Informatiebrief") | ||
description = _("Omschrijving informatiebrief") | ||
desiredness = desiredness.REQUIRED | ||
|
||
|
||
class ConsentForm(AttachmentKind): | ||
|
||
db_name = "consent_form" | ||
name = _("Toestemmingsverklaring") | ||
description = _("Omschrijving toestemmingsverklaring") | ||
|
||
|
||
class DataManagementPlan(ProposalAttachmentKind): | ||
|
||
db_name = "dmp" | ||
name = _("Data Management Plan") | ||
description = _("Omschrijving DMP") | ||
|
||
def num_recommended(self): | ||
return 1 | ||
|
||
|
||
class OtherProposalAttachment(ProposalAttachmentKind): | ||
|
||
db_name = "other" | ||
name = _("Overige bestanden") | ||
description = _("Voor alle overige soorten bestanden") | ||
|
||
def num_required(self): | ||
return 0 | ||
|
||
def num_suggested(self): | ||
""" | ||
You may always add another miscellaneous file.""" | ||
return self.num_provided + 1 | ||
|
||
|
||
STUDY_ATTACHMENTS = [ | ||
InformationLetter, | ||
ConsentForm, | ||
] | ||
|
||
PROPOSAL_ATTACHMENTS = [ | ||
DataManagementPlan, | ||
OtherProposalAttachment, | ||
] | ||
|
||
ATTACHMENTS = PROPOSAL_ATTACHMENTS + STUDY_ATTACHMENTS |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
# Generated by Django 4.2.11 on 2024-09-23 17:00 | ||
|
||
import cdh.files.db.fields | ||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
import main.utils | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
initial = True | ||
|
||
dependencies = [ | ||
("proposals", "0053_auto_20240201_1557"), | ||
("files", "0004_auto_20210921_1014"), | ||
("studies", "0028_remove_study_sessions_number"), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name="Attachment", | ||
fields=[ | ||
( | ||
"id", | ||
models.BigAutoField( | ||
auto_created=True, | ||
primary_key=True, | ||
serialize=False, | ||
verbose_name="ID", | ||
), | ||
), | ||
( | ||
"kind", | ||
models.CharField( | ||
choices=[ | ||
("dmp", "Data Management Plan"), | ||
("other", "Overige bestanden"), | ||
("information_letter", "Informatiebrief"), | ||
("consent_form", "Toestemmingsverklaring"), | ||
], | ||
default=("", "Gelieve selecteren"), | ||
max_length=100, | ||
), | ||
), | ||
( | ||
"name", | ||
models.CharField( | ||
default="", | ||
help_text="Geef je bestand een omschrijvende naam, het liefst maar enkele woorden.", | ||
max_length=50, | ||
), | ||
), | ||
( | ||
"comments", | ||
models.TextField( | ||
default="", | ||
help_text="Geef hier je motivatie om dit bestand toe te voegen en waar je het voor gaat gebruiken tijdens je onderzoek. Eventuele opmerkingen voor de FETC kun je hier ook kwijt.", | ||
max_length=2000, | ||
), | ||
), | ||
( | ||
"parent", | ||
models.ForeignKey( | ||
default=None, | ||
null=True, | ||
on_delete=django.db.models.deletion.SET_NULL, | ||
related_name="children", | ||
to="attachments.attachment", | ||
), | ||
), | ||
( | ||
"upload", | ||
cdh.files.db.fields.FileField( | ||
filename_generator=cdh.files.db.fields._default_filename_generator, | ||
on_delete=django.db.models.deletion.CASCADE, | ||
to="files.file", | ||
), | ||
), | ||
], | ||
bases=(models.Model, main.utils.renderable), | ||
), | ||
migrations.CreateModel( | ||
name="StudyAttachment", | ||
fields=[ | ||
( | ||
"attachment_ptr", | ||
models.OneToOneField( | ||
auto_created=True, | ||
on_delete=django.db.models.deletion.CASCADE, | ||
parent_link=True, | ||
primary_key=True, | ||
serialize=False, | ||
to="attachments.attachment", | ||
), | ||
), | ||
( | ||
"attached_to", | ||
models.ManyToManyField( | ||
related_name="attachments", to="studies.study" | ||
), | ||
), | ||
], | ||
bases=("attachments.attachment",), | ||
), | ||
migrations.CreateModel( | ||
name="ProposalAttachment", | ||
fields=[ | ||
( | ||
"attachment_ptr", | ||
models.OneToOneField( | ||
auto_created=True, | ||
on_delete=django.db.models.deletion.CASCADE, | ||
parent_link=True, | ||
primary_key=True, | ||
serialize=False, | ||
to="attachments.attachment", | ||
), | ||
), | ||
( | ||
"attached_to", | ||
models.ManyToManyField( | ||
related_name="attachments", to="proposals.proposal" | ||
), | ||
), | ||
], | ||
bases=("attachments.attachment",), | ||
), | ||
] |
40 changes: 40 additions & 0 deletions
40
attachments/migrations/0002_attachment_author_alter_attachment_upload.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# Generated by Django 4.2.11 on 2024-10-01 13:43 | ||
|
||
import cdh.files.db.fields | ||
from django.conf import settings | ||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
migrations.swappable_dependency(settings.AUTH_USER_MODEL), | ||
("files", "0004_auto_20210921_1014"), | ||
("attachments", "0001_initial"), | ||
] | ||
|
||
operations = [ | ||
migrations.AddField( | ||
model_name="attachment", | ||
name="author", | ||
field=models.ForeignKey( | ||
default=None, | ||
null=True, | ||
on_delete=django.db.models.deletion.SET_NULL, | ||
related_name="created_attachments", | ||
to=settings.AUTH_USER_MODEL, | ||
), | ||
), | ||
migrations.AlterField( | ||
model_name="attachment", | ||
name="upload", | ||
field=cdh.files.db.fields.FileField( | ||
filename_generator=cdh.files.db.fields._default_filename_generator, | ||
help_text="Selecteer hier het bestand om toe te voegen.", | ||
on_delete=django.db.models.deletion.CASCADE, | ||
to="files.file", | ||
verbose_name="Bestand", | ||
), | ||
), | ||
] |
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
from django.db import models | ||
from django.contrib.auth import get_user_model | ||
from django.utils.translation import gettext as _ | ||
from main.utils import renderable | ||
from attachments.utils import attachment_filename_generator | ||
|
||
from cdh.files.db import FileField as CDHFileField | ||
|
||
class Attachment(models.Model, renderable): | ||
|
||
template_name = "attachments/attachment_model.html" | ||
author = models.ForeignKey( | ||
get_user_model(), | ||
related_name="created_attachments", | ||
null=True, | ||
on_delete=models.SET_NULL, | ||
default=None, | ||
) | ||
upload = CDHFileField( | ||
verbose_name=_("Bestand"), | ||
help_text=_("Selecteer hier het bestand om toe te voegen."), | ||
filename_generator=attachment_filename_generator, | ||
) | ||
parent = models.ForeignKey( | ||
"attachments.attachment", | ||
related_name="children", | ||
null=True, | ||
on_delete=models.SET_NULL, | ||
default=None, | ||
) | ||
kind = models.CharField( | ||
max_length=100, | ||
# It would be nice to be able to define fixed choices here. | ||
# But I haven't found a way to nicely import them from kinds.py | ||
# without circular imports. So for now we just set the choices in | ||
# 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")), | ||
) | ||
name = models.CharField( | ||
max_length=50, | ||
) | ||
comments = models.TextField( | ||
max_length=2000, | ||
default="", | ||
help_text=_( | ||
"Geef hier optioneel je motivatie om dit bestand toe te voegen en " | ||
"waar " | ||
"je het voor gaat gebruiken tijdens je onderzoek. Eventuele " | ||
"opmerkingen voor de FETC kun je hier ook kwijt." | ||
), | ||
) | ||
|
||
def get_correct_submodel(self): | ||
if self.__class__.__name__ != "Attachment": | ||
# In this case, we're already dealing with a submodel | ||
return self | ||
submodels = [StudyAttachment, ProposalAttachment] | ||
# By default, lowering the class name is how subclassed model | ||
# relation names are generated by Django. That's why the following | ||
# lines work. | ||
# However, if we use a different related_name or run into a name | ||
# collision we'd have to be smarter about getting the submodel. | ||
for submodel in submodels: | ||
key = submodel.__name__.lower() | ||
if hasattr(self, key): | ||
return getattr(self, key) | ||
raise KeyError("Couldn't find a matching submodel.") | ||
|
||
def detach(self, other_object): | ||
""" | ||
Remove an attachment from an owner object. If no other | ||
owner objects remain, delete the attachment. | ||
|
||
This method is simple enough to define for all submodels, | ||
assuming they use the attached_to attribute name. However, | ||
base Attachments do not have an attached_to attribute, so | ||
we have to defer to the submodel if detach is called on a | ||
base Attachment instance. | ||
""" | ||
if self.__class__.__name__ == "Attachment": | ||
attachment = self.get_correct_submodel() | ||
return attachment.detach(other_object) | ||
# The following part only runs if called from a submodel | ||
if self.attached_to.count() > 1: | ||
self.attached_to.remove(other_object) | ||
else: | ||
self.delete() | ||
|
||
def get_context_data(self, **kwargs): | ||
context = super().get_context_data(**kwargs) | ||
context["attachment"] = self | ||
return context | ||
|
||
|
||
class ProposalAttachment( | ||
Attachment, | ||
): | ||
attached_to = models.ManyToManyField( | ||
"proposals.Proposal", | ||
related_name="attachments", | ||
) | ||
|
||
def get_owner_for_proposal( | ||
self, | ||
proposal, | ||
): | ||
""" | ||
This method doesn't do much, it's just here to provide | ||
a consistent interface for getting owner objects. | ||
""" | ||
return proposal | ||
|
||
|
||
class StudyAttachment( | ||
Attachment, | ||
): | ||
attached_to = models.ManyToManyField( | ||
"studies.Study", | ||
related_name="attachments", | ||
) | ||
|
||
def get_owner_for_proposal( | ||
self, | ||
proposal, | ||
): | ||
""" | ||
Gets the owner study based on given proposal. | ||
""" | ||
return self.attached_to.get(proposal=proposal) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
<a href="{% url "proposals:download_attachment_original" proposal_pk=proposal.pk attachment_pk=attachment.pk %}"> | ||
{{ attachment.upload }}</a> |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Sorry, I didn't give you enough guidance on this. The filename generator should be plugged into the attachment download view. When the download view without
original_fn
is visited, it should provide the file as a download with the generated filename.We don't want to change the filenames in the field or on disk. Those filenames being (pseudo-)random is a good thing because they avoid collisions and provide a minor amount of security!
Also, because attachments can belong to multiple proposals it doesn't make sense to put much of that info in the on-disk filename. This avoids the issue where documents from copied proposals have the wrong refnum in the filename. Generating the filename based on the proposal argument passed to the download view is better.