From d37a2a037ef17ffa7e5c58fa85cc3879b5b890d8 Mon Sep 17 00:00:00 2001 From: Jorran de Wit <jorrandewit@outlook.com> Date: Sun, 12 Nov 2017 17:56:06 +0100 Subject: [PATCH] Add ProductionEvent attachments Authors will now be able to attach files to their feedback form for the proofs sent by the production team. The attachment is linked to the ProductionEvent that is created with the feedback. --- production/admin.py | 4 +- production/forms.py | 11 +++++- .../0033_productioneventattachment.py | 26 +++++++++++++ production/models.py | 23 ++++++++++++ .../partials/production_events.html | 10 ++++- .../production_stream_card_completed.html | 6 +-- production/templates/production/proofs.html | 3 +- production/urls.py | 2 + production/views.py | 37 +++++++++++++++++-- .../submissions/submission_detail.html | 16 ++++---- 10 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 production/migrations/0033_productioneventattachment.py diff --git a/production/admin.py b/production/admin.py index 2880659cd..e0452b589 100644 --- a/production/admin.py +++ b/production/admin.py @@ -2,7 +2,8 @@ from django.contrib import admin from guardian.admin import GuardedModelAdmin -from .models import ProductionStream, ProductionEvent, ProductionUser, Proofs +from .models import ProductionStream, ProductionEvent, ProductionUser, Proofs,\ + ProductionEventAttachment def event_count(obj): @@ -38,4 +39,5 @@ class ProductionProofsAdmin(admin.ModelAdmin): admin.site.register(Proofs, ProductionProofsAdmin) admin.site.register(ProductionUser) admin.site.register(ProductionEvent) +admin.site.register(ProductionEventAttachment) admin.site.register(ProductionStream, ProductionStreamAdmin) diff --git a/production/forms.py b/production/forms.py index 1768d6d12..dabd1d19d 100644 --- a/production/forms.py +++ b/production/forms.py @@ -3,7 +3,8 @@ import datetime from django import forms from . import constants -from .models import ProductionUser, ProductionStream, ProductionEvent, Proofs +from .models import ProductionUser, ProductionStream, ProductionEvent, Proofs,\ + ProductionEventAttachment from .signals import notify_stream_status_change today = datetime.datetime.today() @@ -144,6 +145,7 @@ class ProofsDecisionForm(forms.ModelForm): decision = forms.ChoiceField(choices=[(True, 'Accept Proofs for publication'), (False, 'Decline Proofs for publication')]) feedback = forms.CharField(required=False, widget=forms.Textarea) + feedback_attachment = forms.FileField(required=False) class Meta: model = Proofs @@ -173,9 +175,14 @@ class ProofsDecisionForm(forms.ModelForm): prodevent = ProductionEvent( stream=proofs.stream, event='status', - comments='Received feedback from the authors: {comments}'.format( + comments='<em>Received feedback from the authors:</em><br>{comments}'.format( comments=comments), noted_by=proofs.stream.supervisor ) prodevent.save() + if self.cleaned_data.get('feedback_attachment'): + attachment = ProductionEventAttachment( + attachment=self.cleaned_data['feedback_attachment'], + production_event=prodevent) + attachment.save() return proofs diff --git a/production/migrations/0033_productioneventattachment.py b/production/migrations/0033_productioneventattachment.py new file mode 100644 index 000000000..a9a7fdaa7 --- /dev/null +++ b/production/migrations/0033_productioneventattachment.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.4 on 2017-11-12 16:05 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +import production.models +import scipost.storage + + +class Migration(migrations.Migration): + + dependencies = [ + ('production', '0032_auto_20171010_1008'), + ] + + operations = [ + migrations.CreateModel( + name='ProductionEventAttachment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('attachment', models.FileField(storage=scipost.storage.SecureFileStorage(), upload_to=production.models.production_event_upload_location)), + ('production_event', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='attachments', to='production.ProductionEvent')), + ], + ), + ] diff --git a/production/models.py b/production/models.py index 65ace229b..12c461b7d 100644 --- a/production/models.py +++ b/production/models.py @@ -106,6 +106,29 @@ class ProductionEvent(models.Model): return self.stream.notification_name +def production_event_upload_location(instance, filename): + submission = instance.production_event.stream.submission + return 'UPLOADS/PRODSTREAMS/{year}/{arxiv}/{filename}'.format( + year=submission.submission_date.year, + arxiv=submission.arxiv_identifier_wo_vn_nr, + filename=filename) + + +class ProductionEventAttachment(models.Model): + """ + An ProductionEventAttachment is in general used by authors to reply to an Proofs version + with their version of the Proofs with comments. + """ + production_event = models.ForeignKey('production.ProductionEvent', on_delete=models.CASCADE, + related_name='attachments') + attachment = models.FileField(upload_to=production_event_upload_location, + storage=SecureFileStorage()) + + def get_absolute_url(self): + return reverse('production:production_event_attachment_pdf', + args=(self.production_event.stream.id, self.id,)) + + def proofs_upload_location(instance, filename): submission = instance.stream.submission return 'UPLOADS/PROOFS/{year}/{arxiv}/{filename}'.format( diff --git a/production/templates/production/partials/production_events.html b/production/templates/production/partials/production_events.html index 653f9c3df..74fbd57a5 100644 --- a/production/templates/production/partials/production_events.html +++ b/production/templates/production/partials/production_events.html @@ -34,10 +34,18 @@ {% if event.noted_to %} {{ event.noted_by.user.first_name }} {{ event.noted_by.user.last_name }} {{ event.comments|linebreaksbr }} {{ event.noted_to.user.first_name }} {{ event.noted_to.user.last_name }}. {% else %} - {{ event.comments|linebreaksbr }} + {{ event.comments|safe|linebreaksbr }} {% endif %} </p> {% endif %} + + {% if event.attachments.exists %} + <ul> + {% for attachment in event.attachments.all %} + <li><a href="{{ attachment.get_absolute_url }}" target="_blank">Download Attachment {{ forloop.counter }}</a></li> + {% endfor %} + </ul> + {% endif %} </li> {% empty %} <li>No events were found.</li> diff --git a/production/templates/production/partials/production_stream_card_completed.html b/production/templates/production/partials/production_stream_card_completed.html index e938b1d37..38f1c7fa6 100644 --- a/production/templates/production/partials/production_stream_card_completed.html +++ b/production/templates/production/partials/production_stream_card_completed.html @@ -54,8 +54,8 @@ <ul> {% for proofs in stream.proofs.all %} <li class="py-1"> - <a href="{% url 'production:proofs' stream_id=stream.id version=proofs.version %}">Version {{ proofs.version }}</a><br> - Uploaded by {{ proofs.uploaded_by.user.first_name }} {{ proofs.uploaded_by.user.last_name }}<br> + <a href="{% url 'production:proofs' stream_id=stream.id version=proofs.version %}">Version {{ proofs.version }}</a> · <span class="label label-secondary label-sm">{{ proofs.get_status_display }}</span><br> + Uploaded by: {{ proofs.uploaded_by.user.first_name }} {{ proofs.uploaded_by.user.last_name }}<br> Accessible for authors: {{ proofs.accessible_for_authors|yesno:'<strong>Yes</strong>,No'|safe }}<br> {% if perms.scipost.can_run_proofs_by_authors %} @@ -71,8 +71,6 @@ </ul> {% endif %} {% endif %} - - <span class="label label-secondary label-sm">{{ proofs.get_status_display }}</span> </li> {% empty %} <li>No Proofs found.</li> diff --git a/production/templates/production/proofs.html b/production/templates/production/proofs.html index bfbaa7937..b939acd97 100644 --- a/production/templates/production/proofs.html +++ b/production/templates/production/proofs.html @@ -38,8 +38,7 @@ </li> {% elif proofs.status == 'accepted_sup' %} <li><a href="{% url 'production:send_proofs' proofs.stream.id proofs.version %}">Send proofs to authors</a></li> - {% endif %} - {% if proofs.status != 'uploaded' %} + {% else %} <li><a href="{% url 'production:toggle_accessibility' proofs.stream.id proofs.version %}">{{ proofs.accessible_for_authors|yesno:'Hide,Make accessible' }} for authors</a></li> {% endif %} {% endif %} diff --git a/production/urls.py b/production/urls.py index c44ef6554..e0ecebf5d 100644 --- a/production/urls.py +++ b/production/urls.py @@ -21,6 +21,8 @@ urlpatterns = [ production_views.send_proofs, name='send_proofs'), url(r'^streams/(?P<stream_id>[0-9]+)/proofs/(?P<version>[0-9]+)/toggle_access$', production_views.toggle_accessibility, name='toggle_accessibility'), + url(r'^streams/(?P<stream_id>[0-9]+)/proofs/(?P<attachment_id>[0-9]+)/reply/pdf$', + production_views.production_event_attachment_pdf, name='production_event_attachment_pdf'), url(r'^streams/(?P<stream_id>[0-9]+)/events/add$', production_views.add_event, name='add_event'), url(r'^streams/(?P<stream_id>[0-9]+)/logs/add$', diff --git a/production/views.py b/production/views.py index cc7e0d406..f368999aa 100644 --- a/production/views.py +++ b/production/views.py @@ -18,7 +18,8 @@ from finances.forms import WorkLogForm from mails.views import MailEditingSubView from . import constants -from .models import ProductionUser, ProductionStream, ProductionEvent, Proofs +from .models import ProductionUser, ProductionStream, ProductionEvent, Proofs,\ + ProductionEventAttachment from .forms import ProductionEventForm, AssignOfficerForm, UserToOfficerForm,\ AssignSupervisorForm, StreamStatusForm, ProofsUploadForm, ProofsDecisionForm,\ AssignInvitationsOfficerForm @@ -492,7 +493,7 @@ def proofs_pdf(request, slug): # because now it will return 404 instead of a redirect to the login page. raise Http404 - proofs = Proofs.objects.get(id=proofs_slug_to_id(slug)) + proofs = get_object_or_404(Proofs, id=proofs_slug_to_id(slug)) stream = proofs.stream # Check if user has access! @@ -511,6 +512,34 @@ def proofs_pdf(request, slug): return response +def production_event_attachment_pdf(request, stream_id, attachment_id): + """ Open ProductionEventAttachment pdf. """ + if not request.user.is_authenticated: + # Don't use the decorator but this strategy, + # because now it will return 404 instead of a redirect to the login page. + raise Http404 + + stream = get_object_or_404(ProductionStream, id=stream_id) + attachment = get_object_or_404( + ProductionEventAttachment.objects.filter(production_event__stream=stream), + id=attachment_id) + + # Check if user has access! + checker = ObjectPermissionChecker(request.user) + access = checker.has_perm('can_work_for_stream', stream) and request.user.has_perm('scipost.can_view_production') + if not access and request.user.contributor: + access = request.user.contributor in stream.submission.authors.all() + if not access: + raise Http404 + + # Passed the test! The user may see the file! + content_type, encoding = mimetypes.guess_type(attachment.attachment.path) + content_type = content_type or 'application/octet-stream' + response = HttpResponse(attachment.attachment.read(), content_type=content_type) + response["Content-Encoding"] = encoding + return response + + @login_required @transaction.atomic def author_decision(request, slug): @@ -526,7 +555,7 @@ def author_decision(request, slug): if request.user.contributor not in proofs.stream.submission.authors.all(): raise Http404 - form = ProofsDecisionForm(request.POST or None, instance=proofs) + form = ProofsDecisionForm(request.POST or None, request.FILES or None, instance=proofs) if form.is_valid(): proofs = form.save() notify_stream_status_change(request.user, stream, False) @@ -595,7 +624,7 @@ def decision(request, stream_id, version, decision): ) prodevent.save() messages.success(request, 'Proofs have been {decision}.'.format(decision=decision)) - return redirect(stream.get_absolute_url()) + return redirect(reverse('production:proofs', args=(stream.id, proofs.version))) @is_production_user() diff --git a/submissions/templates/submissions/submission_detail.html b/submissions/templates/submissions/submission_detail.html index 0010c1e89..1e66d5504 100644 --- a/submissions/templates/submissions/submission_detail.html +++ b/submissions/templates/submissions/submission_detail.html @@ -105,13 +105,15 @@ <li> <a href="{{ proofs.get_absolute_url }}" target="_blank">Download version {{ proofs.version }}</a> · uploaded: {{ proofs.created|date:"DATE_FORMAT" }} · status: <span class="label label-secondary label-sm">{{ proofs.get_status_display }}</span> - {% if proofs.status == 'accepted_sup' and proofs_decision_form and is_author %} - <h3 class="mb-0 mt-2">Please advise the Production Team on your findings on Proofs version {{ proof.version }}</h3> - <form method="post" action="{% url 'production:author_decision' proofs.slug %}" class="my-2"> - {% csrf_token %} - {{ proofs_decision_form|bootstrap }} - <input class="btn btn-primary btn-sm" type="submit" value="Submit"> - </form> + {% if proofs.status == 'accepted_sup' or proofs.status == 'sent' %} + {% if proofs_decision_form and is_author %} + <h3 class="mb-0 mt-2">Please advise the Production Team on your findings on Proofs version {{ proofs.version }}</h3> + <form method="post" enctype="multipart/form-data" action="{% url 'production:author_decision' proofs.slug %}" class="my-2"> + {% csrf_token %} + {{ proofs_decision_form|bootstrap }} + <input class="btn btn-primary btn-sm" type="submit" value="Submit"> + </form> + {% endif %} {% endif %} </li> {% endfor %} -- GitLab