From a2b1493c3afe62fa56542ef5b16f69dc52ad0c54 Mon Sep 17 00:00:00 2001
From: Jorran de Wit <jorrandewit@outlook.com>
Date: Fri, 19 May 2017 23:01:44 +0200
Subject: [PATCH] Fix vetting Commentaries bug
Due to the new Commentary request forms, which were
also used during vetting (choice=modify), the vetting
went wrong sometimes.
If the vetting action was 'modify', the user was redirected
to the old form which didn't exist anymore.
As a result of how the contruction of this modify structure, the Commentary
was first deleted and thus the data was lost. The requester did receive
a mail that his/her Commentary was accessible on SciPost however.
I fixed this by recontructing the vetting views. I combined the
vetting list and the vet request. I removed the action removing
the Commentary and made this a redirect to a new view which is
a modify page accessible only for EdCol Admins to modify+accept
the Commentary. Also, the mail for 'modify' vetting is also just sent
at *after* modification now.
---
commentaries/forms.py | 18 ++--
.../modify_commentary_request.html | 44 +++++++++
.../request_published_article.html | 2 +-
.../commentaries/vet_commentary_requests.html | 3 +-
commentaries/test_forms.py | 7 +-
commentaries/urls.py | 9 +-
commentaries/views.py | 89 +++++++++++--------
7 files changed, 123 insertions(+), 49 deletions(-)
create mode 100644 commentaries/templates/commentaries/modify_commentary_request.html
diff --git a/commentaries/forms.py b/commentaries/forms.py
index 37ec5748c..4e77b521a 100644
--- a/commentaries/forms.py
+++ b/commentaries/forms.py
@@ -103,8 +103,9 @@ class RequestCommentaryForm(forms.ModelForm):
def save(self, *args, **kwargs):
self.instance.parse_links_into_urls()
- self.instance.requested_by = self.requested_by
- return super().save(self, *args, **kwargs)
+ if self.requested_by:
+ self.instance.requested_by = self.requested_by
+ return super().save(*args, **kwargs)
class RequestArxivPreprintForm(RequestCommentaryForm):
@@ -240,6 +241,13 @@ class VetCommentaryForm(forms.Form):
raise ValueError(('VetCommentaryForm could not be processed '
'because the data didn\'t validate'))
+ def clean_refusal_reason(self):
+ """`refusal_reason` field is required if action==refuse."""
+ if self.commentary_is_refused():
+ if int(self.cleaned_data['refusal_reason']) == self.REFUSAL_EMPTY:
+ self.add_error('refusal_reason', 'Please, choose a reason for rejection.')
+ return self.cleaned_data['refusal_reason']
+
def get_commentary(self):
"""Return Commentary if available"""
self._form_is_cleaned()
@@ -251,25 +259,23 @@ class VetCommentaryForm(forms.Form):
return self.COMMENTARY_REFUSAL_DICT[int(self.cleaned_data['refusal_reason'])]
def commentary_is_accepted(self):
- self._form_is_cleaned()
return int(self.cleaned_data['action_option']) == self.ACTION_ACCEPT
def commentary_is_modified(self):
- self._form_is_cleaned()
return int(self.cleaned_data['action_option']) == self.ACTION_MODIFY
def commentary_is_refused(self):
- self._form_is_cleaned()
return int(self.cleaned_data['action_option']) == self.ACTION_REFUSE
def process_commentary(self):
"""Vet the commentary or delete it from the database"""
+ # Modified actions are not doing anything. Users are redirected to an edit page instead.
if self.commentary_is_accepted():
self.commentary.vetted = True
self.commentary.vetted_by = Contributor.objects.get(user=self.user)
self.commentary.save()
return self.commentary
- elif self.commentary_is_modified() or self.commentary_is_refused():
+ elif self.commentary_is_refused():
self.commentary.delete()
return None
diff --git a/commentaries/templates/commentaries/modify_commentary_request.html b/commentaries/templates/commentaries/modify_commentary_request.html
new file mode 100644
index 000000000..c44301217
--- /dev/null
+++ b/commentaries/templates/commentaries/modify_commentary_request.html
@@ -0,0 +1,44 @@
+{% extends 'scipost/_personal_page_base.html' %}
+
+{% load bootstrap %}
+
+{% block pagetitle %}: vet Commentary requests{% endblock pagetitle %}
+
+{% block breadcrumb_items %}
+ {{block.super}}
+ <span class="breadcrumb-item">Vet Commentary Page requests</span>
+{% endblock %}
+
+{% block content %}
+
+<div class="row">
+ <div class="col-12">
+ <h1>SciPost Commentary Page request to modify and accept:</h1>
+ </div>
+</div>
+
+<hr>
+<div class="row">
+ <div class="col-12">
+ {% include 'commentaries/_commentary_summary.html' with commentary=commentary %}
+ </div>
+</div>
+
+<div class="row">
+ <div class="col-12">
+ <h3 class="mt-4">Abstract:</h3>
+ <p>{{ commentary.pub_abstract }}</p>
+ </div>
+</div>
+
+<div class="row">
+ <div class="col-12">
+ <form action="{% url 'commentaries:modify_commentary_request' commentary_id=commentary.id %}" method="post">
+ {% csrf_token %}
+ {{ form|bootstrap }}
+ <input type="submit" class="btn btn-secondary" value="Submit and accept" />
+ </form>
+ </div>
+</div>
+
+{% endblock %}
diff --git a/commentaries/templates/commentaries/request_published_article.html b/commentaries/templates/commentaries/request_published_article.html
index b0bf7365a..f30f2a4c0 100644
--- a/commentaries/templates/commentaries/request_published_article.html
+++ b/commentaries/templates/commentaries/request_published_article.html
@@ -32,4 +32,4 @@
</div>
</div>
-{% endblock content%}
+{% endblock content %}
diff --git a/commentaries/templates/commentaries/vet_commentary_requests.html b/commentaries/templates/commentaries/vet_commentary_requests.html
index e4f853aa1..e6c770481 100644
--- a/commentaries/templates/commentaries/vet_commentary_requests.html
+++ b/commentaries/templates/commentaries/vet_commentary_requests.html
@@ -15,6 +15,7 @@
<div class="col-12">
{% if not commentary_to_vet %}
<h1>There are no Commentary Page requests for you to vet.</h1>
+ <h3><a href="{% url 'scipost:personal_page' %}">Return to personal page</a></h3>
{% else %}
<h1>SciPost Commentary Page request to vet:</h1>
@@ -27,7 +28,7 @@
</div>
<div class="col-md-5">
- <form action="{% url 'commentaries:vet_commentary_request_ack' commentary_id=commentary_to_vet.id %}" method="post">
+ <form action="{% url 'commentaries:vet_commentary_requests_submit' commentary_id=commentary_to_vet.id %}" method="post">
{% csrf_token %}
{{ form|bootstrap }}
<input type="submit" class="btn btn-secondary" value="Submit" />
diff --git a/commentaries/test_forms.py b/commentaries/test_forms.py
index 0516fc3af..64d6fda33 100644
--- a/commentaries/test_forms.py
+++ b/commentaries/test_forms.py
@@ -5,9 +5,10 @@ from django.test import TestCase
from common.helpers import model_form_data
from scipost.factories import UserFactory, ContributorFactory
-from .factories import VettedCommentaryFactory, UnvettedCommentaryFactory, UnvettedArxivPreprintCommentaryFactory
-from .forms import RequestCommentaryForm, RequestPublishedArticleForm, VetCommentaryForm, DOIToQueryForm, \
- ArxivQueryForm, RequestArxivPreprintForm
+from .factories import VettedCommentaryFactory, UnvettedCommentaryFactory,\
+ UnvettedArxivPreprintCommentaryFactory
+from .forms import RequestPublishedArticleForm, VetCommentaryForm, DOIToQueryForm,\
+ ArxivQueryForm, RequestArxivPreprintForm
from .models import Commentary
from common.helpers.test import add_groups_and_permissions
diff --git a/commentaries/urls.py b/commentaries/urls.py
index 03be372fb..7d3280cb2 100644
--- a/commentaries/urls.py
+++ b/commentaries/urls.py
@@ -24,12 +24,15 @@ urlpatterns = [
url(r'^request_commentary$', views.request_commentary, name='request_commentary'),
url(r'^request_commentary/published_article$', views.RequestPublishedArticle.as_view(),
name='request_published_article'),
- url(r'^request_commentary/arxiv_preprint$', views.RequestArxivPreprint.as_view(), name='request_arxiv_preprint'),
+ url(r'^request_commentary/arxiv_preprint$', views.RequestArxivPreprint.as_view(),
+ name='request_arxiv_preprint'),
url(r'^prefill_using_DOI$', views.prefill_using_DOI, name='prefill_using_DOI'),
url(r'^prefill_using_arxiv_identifier$', views.prefill_using_arxiv_identifier,
name='prefill_using_arxiv_identifier'),
url(r'^vet_commentary_requests$', views.vet_commentary_requests,
name='vet_commentary_requests'),
- url(r'^vet_commentary_request_ack/(?P<commentary_id>[0-9]+)$',
- views.vet_commentary_request_ack, name='vet_commentary_request_ack'),
+ url(r'^vet_commentary_requests/(?P<commentary_id>[0-9]+)$', views.vet_commentary_requests,
+ name='vet_commentary_requests_submit'),
+ url(r'^vet_commentary_requests/(?P<commentary_id>[0-9]+)/modify$',
+ views.modify_commentary_request, name='modify_commentary_request'),
]
diff --git a/commentaries/views.py b/commentaries/views.py
index f302df3aa..3dc877847 100644
--- a/commentaries/views.py
+++ b/commentaries/views.py
@@ -4,6 +4,7 @@ from django.contrib.auth.decorators import permission_required
from django.core.mail import EmailMessage
from django.core.urlresolvers import reverse, reverse_lazy
from django.db.models import Q
+from django.shortcuts import redirect
from django.template.loader import render_to_string
from django.views.generic.edit import CreateView
from django.views.generic.list import ListView
@@ -103,22 +104,14 @@ def prefill_using_arxiv_identifier(request):
@permission_required('scipost.can_vet_commentary_requests', raise_exception=True)
-def vet_commentary_requests(request):
+def vet_commentary_requests(request, commentary_id=None):
"""Show the first commentary thats awaiting vetting"""
- contributor = Contributor.objects.get(user=request.user)
- commentary_to_vet = (Commentary.objects.awaiting_vetting()
- .exclude(requested_by=contributor).first()) # only handle one at a time
- form = VetCommentaryForm()
- context = {'contributor': contributor, 'commentary_to_vet': commentary_to_vet, 'form': form}
- return render(request, 'commentaries/vet_commentary_requests.html', context)
-
-
-@permission_required('scipost.can_vet_commentary_requests', raise_exception=True)
-def vet_commentary_request_ack(request, commentary_id):
- # Security fix: Smart asses can vet their own commentary without this line.
- # Commentary itself not really being used.
- get_object_or_404((Commentary.objects.awaiting_vetting()
- .exclude(requested_by=request.user.contributor)), id=commentary_id)
+ queryset = Commentary.objects.awaiting_vetting().exclude(requested_by=request.user.contributor)
+ if commentary_id:
+ # Security fix: Smart asses can vet their own commentary without this line.
+ commentary_to_vet = get_object_or_404(queryset, id=commentary_id)
+ else:
+ commentary_to_vet = queryset.first()
form = VetCommentaryForm(request.POST or None, user=request.user, commentary_id=commentary_id)
if form.is_valid():
@@ -131,21 +124,14 @@ def vet_commentary_request_ack(request, commentary_id):
# Retrieve email_template for action
if form.commentary_is_accepted():
email_template = 'commentaries/vet_commentary_email_accepted.html'
- elif form.commentary_is_modified():
- email_template = 'commentaries/vet_commentary_email_modified.html'
-
- request_commentary_form = RequestCommentaryForm(initial={
- 'pub_title': commentary.pub_title,
- 'arxiv_link': commentary.arxiv_link,
- 'pub_DOI_link': commentary.pub_DOI_link,
- 'author_list': commentary.author_list,
- 'pub_date': commentary.pub_date,
- 'pub_abstract': commentary.pub_abstract
- })
elif form.commentary_is_refused():
email_template = 'commentaries/vet_commentary_email_rejected.html'
email_context['refusal_reason'] = form.get_refusal_reason()
email_context['further_explanation'] = form.cleaned_data['email_response_field']
+ elif form.commentary_is_modified():
+ # For a modified commentary, redirect to request_commentary_form
+ return redirect(reverse('commentaries:modify_commentary_request',
+ args=(commentary.id,)))
# Send email and process form
email_text = render_to_string(email_template, email_context)
@@ -159,16 +145,49 @@ def vet_commentary_request_ack(request, commentary_id):
emailmessage.send(fail_silently=False)
commentary = form.process_commentary()
- # For a modified commentary, redirect to request_commentary_form
- if form.commentary_is_modified():
- context = {'form': request_commentary_form}
- return render(request, 'commentaries/request_commentary.html', context)
+ messages.success(request, 'SciPost Commentary request vetted.')
+ return redirect(reverse('commentaries:vet_commentary_requests'))
+
+ context = {
+ 'commentary_to_vet': commentary_to_vet,
+ 'form': form
+ }
+ return render(request, 'commentaries/vet_commentary_requests.html', context)
+
+
+@permission_required('scipost.can_vet_commentary_requests', raise_exception=True)
+def modify_commentary_request(request, commentary_id):
+ """Modify a commentary request after vetting with status 'modified'."""
+ commentary = get_object_or_404((Commentary.objects.awaiting_vetting()
+ .exclude(requested_by=request.user.contributor)),
+ id=commentary_id)
+ form = RequestCommentaryForm(request.POST or None, instance=commentary)
+ if form.is_valid():
+ # Process commentary data
+ commentary = form.save(commit=False)
+ commentary.vetted = True
+ commentary.save()
+
+ # Send email and process form
+ email_template = 'commentaries/vet_commentary_email_modified.html'
+ email_text = render_to_string(email_template, {'commentary': commentary})
+ email_args = (
+ 'SciPost Commentary Page activated',
+ email_text,
+ commentary.requested_by.user.email,
+ ['commentaries@scipost.org']
+ )
+ emailmessage = EmailMessage(*email_args, reply_to=['commentaries@scipost.org'])
+ emailmessage.send(fail_silently=False)
+
+ messages.success(request, 'SciPost Commentary request modified and vetted.')
+ return redirect(reverse('commentaries:vet_commentary_requests'))
- context = {'ack_header': 'SciPost Commentary request vetted.',
- 'followup_message': 'Return to the ',
- 'followup_link': reverse('commentaries:vet_commentary_requests'),
- 'followup_link_label': 'Commentary requests page'}
- return render(request, 'scipost/acknowledgement.html', context)
+ context = {
+ 'commentary': commentary,
+ 'form': form
+ }
+ return render(request, 'commentaries/modify_commentary_request.html', context)
class CommentaryListView(ListView):
--
GitLab