-
Notifications
You must be signed in to change notification settings - Fork 67
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
WIP start of external supplementary files #1749
WIP start of external supplementary files #1749
Conversation
Preprints - Links to external supplementary files openlibhums#1590
Can one of the admins verify this patch? |
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.
Thanks @tingletech ,
Hope you are getting along with Janeway!
Looks like a good approach to me, the only thing I would consider would be to model the relationship between the articles and the supplementary files as a one-to-many rather than a many-to-many, as noted inline, unless you found some other reason to model it this way.
@@ -281,6 +281,12 @@ class Preprint(models.Model): | |||
null=True, | |||
on_delete=models.SET_NULL, | |||
) | |||
submission_supplementary_files = models.ManyToManyField( |
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 think a one-to-many relationship works best here, even if it leads to some duplication across multiple articles. Some examples:
- removing a supplementary file from a preprint: We need to check all the pointers from the supplementary file or risk having orphan supplementary files.
- Ordering supplementary files: If we want the authors to re-order the supplementary files, we can do so with an attribute on the
SupplementaryPreprintFile
model. With a M2M we would need an intermediate model that holds this attribute.
'PreprintSupplementaryFile', | ||
related_name='submission_supplementary_files', | ||
blank=True, | ||
null=True, |
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.
If we end up going for a M2M, we don't need to set null=True
@@ -533,6 +539,11 @@ def download_url(self): | |||
) | |||
|
|||
|
|||
class PreprintSupplementaryFile(models.Model): | |||
preprint = models.ForeignKey(Preprint) |
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.
This would do the job if we go for a one-to-many relatioship. It's not needed on a M2M, since django will create the intermediate table and keys automatically
yea, you are right -- a preprint can have multiple supplementary files potentially -- but two different preprints probably would very rarely share the same supplementary files. |
Preprints - Links to external supplementary files #1590
I didn't get to far on this, but I wanted to get some feedback to see if this looks like it is in the right direction.