diff --git a/commentaries/factories.py b/commentaries/factories.py index f0bae1a0912e9a18ceb0c6f079918c595534948b..805b0019dca52b6255a364891dfd4b8df680df17 100644 --- a/commentaries/factories.py +++ b/commentaries/factories.py @@ -29,5 +29,10 @@ class EmptyCommentaryFactory(CommentaryFactory): pub_DOI = None arxiv_identifier = None + class VettedCommentaryFactory(CommentaryFactory): vetted = True + + +class UnVettedCommentaryFactory(CommentaryFactory): + vetted = False diff --git a/commentaries/forms.py b/commentaries/forms.py index f0dcdf3a30c8497a8d19650abc9fda2c291bda45..22632aafe7c5a939b6704e2c6fb51af4cbac3e6a 100644 --- a/commentaries/forms.py +++ b/commentaries/forms.py @@ -5,20 +5,6 @@ from .models import Commentary from scipost.models import Contributor -COMMENTARY_ACTION_CHOICES = ( - (0, 'modify'), - (1, 'accept'), - (2, 'refuse (give reason below)'), - ) - -COMMENTARY_REFUSAL_CHOICES = ( - (0, '-'), - (-1, 'a commentary on this paper already exists'), - (-2, 'this paper cannot be traced'), - (-3, 'there exists a more revent version of this arXiv preprint'), - ) -commentary_refusal_dict = dict(COMMENTARY_REFUSAL_CHOICES) - class DOIToQueryForm(forms.Form): doi = forms.CharField(widget=forms.TextInput( @@ -32,6 +18,7 @@ class IdentifierToQueryForm(forms.Form): class RequestCommentaryForm(forms.ModelForm): + """Create new valid Commetary by user request""" existing_commentary = None class Meta: @@ -54,6 +41,7 @@ class RequestCommentaryForm(forms.ModelForm): self.fields['pub_abstract'].widget.attrs.update({'cols': 100}) def clean(self, *args, **kwargs): + """Check if form is valid and contains an unique identifier""" cleaned_data = super(RequestCommentaryForm, self).clean(*args, **kwargs) # Either Arxiv-ID or DOI is given @@ -92,6 +80,38 @@ class RequestCommentaryForm(forms.ModelForm): class VetCommentaryForm(forms.Form): + """Process an unvetted Commentary request. + + This form will provide fields to let the user + process a Commentary that is unvetted. On success, + the Commentary is either accepted or deleted from + the database. + + Keyword arguments: + commentary_id -- the Commentary.id to process (required) + user -- User instance of the vetting user (required) + + """ + ACTION_MODIFY = 0 + ACTION_ACCEPT = 1 + ACTION_REFUSE = 2 + COMMENTARY_ACTION_CHOICES = ( + (ACTION_MODIFY, 'modify'), + (ACTION_ACCEPT, 'accept'), + (ACTION_REFUSE, 'refuse (give reason below)'), + ) + REFUSAL_EMPTY = 0 + REFUSAL_PAPER_EXISTS = -1 + REFUSAL_UNTRACEBLE = -2 + REFUSAL_ARXIV_EXISTS = -3 + COMMENTARY_REFUSAL_CHOICES = ( + (REFUSAL_EMPTY, '-'), + (REFUSAL_PAPER_EXISTS, 'a commentary on this paper already exists'), + (REFUSAL_UNTRACEBLE, 'this paper cannot be traced'), + (REFUSAL_ARXIV_EXISTS, 'there exists a more revent version of this arXiv preprint'), + ) + COMMENTARY_REFUSAL_DICT = dict(COMMENTARY_REFUSAL_CHOICES) + action_option = forms.ChoiceField(widget=forms.RadioSelect, choices=COMMENTARY_ACTION_CHOICES, required=True, label='Action') @@ -99,6 +119,71 @@ class VetCommentaryForm(forms.Form): email_response_field = forms.CharField(widget=forms.Textarea( attrs={'rows': 5, 'cols': 40}), label='Justification (optional)', required=False) + def __init__(self, *args, **kwargs): + """Pop and save keyword arguments if set, return form instance""" + self.commentary_id = kwargs.pop('commentary_id', None) + self.user = kwargs.pop('user', None) + self.is_cleaned = False + return super(VetCommentaryForm, self).__init__(*args, **kwargs) + + def clean(self, *args, **kwargs): + """Check valid form and keyword arguments given""" + cleaned_data = super(VetCommentaryForm, self).clean(*args, **kwargs) + + # Check valid `commentary_id` + if not self.commentary_id: + self.add_error(None, 'No `commentary_id` provided') + return cleaned_data + else: + self.commentary = Commentary.objects.select_related('requested_by__user').get(pk=self.commentary_id) + + # Check valid `user` + if not self.user: + self.add_error(None, 'No `user` provided') + return cleaned_data + + self.is_cleaned = True + return cleaned_data + + def _form_is_cleaned(self): + """Raise ValueError if form isn't validated""" + if not self.is_cleaned: + raise ValueError(('VetCommentaryForm could not be processed ' + 'because the data didn\'t validate')) + + def get_commentary(self): + """Return Commentary if available""" + self._form_is_cleaned() + return self.commentary + + def get_refusal_reason(self): + """Return refusal reason""" + if self.commentary_is_refused(): + 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""" + 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(): + self.commentary.delete() + return None + class CommentarySearchForm(forms.Form): """Search for Commentary specified by user""" diff --git a/commentaries/models.py b/commentaries/models.py index a42005b4dae74ac4621960f27e9bfc11dcd4bbb0..903ed64694b73410a780c92581ba4df1243e8d2c 100644 --- a/commentaries/models.py +++ b/commentaries/models.py @@ -20,7 +20,6 @@ class CommentaryManager(models.Manager): def awaiting_vetting(self, **kwargs): return self.filter(vetted=False, **kwargs) - class Commentary(TimeStampedModel): """ A Commentary contains all the contents of a SciPost Commentary page for a given publication. diff --git a/commentaries/templates/commentaries/vet_commentary_email_accepted.html b/commentaries/templates/commentaries/vet_commentary_email_accepted.html new file mode 100644 index 0000000000000000000000000000000000000000..69ba77f82041bc55e7d95c3dc5b34f4e486de797 --- /dev/null +++ b/commentaries/templates/commentaries/vet_commentary_email_accepted.html @@ -0,0 +1,7 @@ +Dear {{commentary.requested_by.get_title}} {{commentary.requested_by.user.last_name}}, + +The Commentary Page you have requested, concerning publication with title {{commentary.pub_title}} by {{commentary.author_list}}, has been activated at https://scipost.org/commentary/'{{commentary.arxiv_or_DOI_string}}. +You are now welcome to submit your comments. + +Thank you for your contribution, +The SciPost Team. diff --git a/commentaries/templates/commentaries/vet_commentary_email_modified.html b/commentaries/templates/commentaries/vet_commentary_email_modified.html new file mode 100644 index 0000000000000000000000000000000000000000..f3ad85365c4ab2f4398dc9b9e644aeece54b7559 --- /dev/null +++ b/commentaries/templates/commentaries/vet_commentary_email_modified.html @@ -0,0 +1,7 @@ +Dear {{commentary.requested_by.get_title}} {{commentary.requested_by.user.last_name}}, + +The Commentary Page you have requested, concerning publication with title {{commentary.pub_title}} by {{commentary.author_list}}, has been activated (with slight modifications to your submitted details). +You are now welcome to submit your comments. + +Thank you for your contribution, +The SciPost Team. diff --git a/commentaries/templates/commentaries/vet_commentary_email_rejected.html b/commentaries/templates/commentaries/vet_commentary_email_rejected.html new file mode 100644 index 0000000000000000000000000000000000000000..b9d99fade03b2a34cfcb59728285f97b7e677c61 --- /dev/null +++ b/commentaries/templates/commentaries/vet_commentary_email_rejected.html @@ -0,0 +1,11 @@ +Dear {{commentary.requested_by.get_title}} {{commentary.requested_by.user.last_name}}, + +The Commentary Page you have requested, concerning publication with title {{commentary.pub_title}} by {{commentary.author_list}}, has not been activated for the following reason: {{refusal_reason}}. + +{% if further_explanation %} +Further explanations: +{{further_explanation}} +{% endif %} + +Thank you for your interest, +The SciPost Team. diff --git a/commentaries/test_forms.py b/commentaries/test_forms.py index 694ce19ec56c93503b879a51038f068b06947f7c..bd41e58a1a4105431445cd280f0ddb6fbb3589f0 100644 --- a/commentaries/test_forms.py +++ b/commentaries/test_forms.py @@ -1,14 +1,75 @@ -import factory - from django.test import TestCase from scipost.factories import UserFactory -from .factories import VettedCommentaryFactory -from .forms import RequestCommentaryForm +from .models import Commentary +from .factories import VettedCommentaryFactory, UnVettedCommentaryFactory +from .forms import RequestCommentaryForm, VetCommentaryForm from common.helpers import model_form_data +class TestVetCommentaryForm(TestCase): + fixtures = ['permissions', 'groups'] + + def setUp(self): + self.commentary = UnVettedCommentaryFactory.create() + self.user = UserFactory() + self.form_data = { + 'action_option': VetCommentaryForm.ACTION_ACCEPT, + 'refusal_reason': VetCommentaryForm.REFUSAL_EMPTY, + 'email_response_field': 'Lorem Ipsum' + } + + def test_valid_accepted_form(self): + """Test valid form data and return Commentary""" + form = VetCommentaryForm(self.form_data, commentary_id=self.commentary.id, user=self.user) + self.assertTrue(form.is_valid()) + self.assertFalse(Commentary.objects.vetted().exists()) + self.assertTrue(Commentary.objects.awaiting_vetting().exists()) + + # Accept Commentary in database + form.process_commentary() + self.assertTrue(Commentary.objects.vetted().exists()) + self.assertFalse(Commentary.objects.awaiting_vetting().exists()) + + def test_valid_modified_form(self): + """Test valid form data and delete Commentary""" + self.form_data['action_option'] = VetCommentaryForm.ACTION_MODIFY + form = VetCommentaryForm(self.form_data, commentary_id=self.commentary.id, user=self.user) + self.assertTrue(form.is_valid()) + self.assertFalse(Commentary.objects.vetted().exists()) + self.assertTrue(Commentary.objects.awaiting_vetting().exists()) + + # Delete the Commentary + form.process_commentary() + self.assertTrue(form.commentary_is_modified()) + self.assertFalse(Commentary.objects.awaiting_vetting().exists()) + + def test_valid_rejected_form(self): + """Test valid form data and delete Commentary""" + self.form_data['action_option'] = VetCommentaryForm.ACTION_REFUSE + self.form_data['refusal_reason'] = VetCommentaryForm.REFUSAL_UNTRACEBLE + form = VetCommentaryForm(self.form_data, commentary_id=self.commentary.id, user=self.user) + self.assertTrue(form.is_valid()) + self.assertFalse(Commentary.objects.vetted().exists()) + self.assertTrue(Commentary.objects.awaiting_vetting().exists()) + + # Delete the Commentary + form.process_commentary() + self.assertTrue(form.commentary_is_refused()) + self.assertFalse(Commentary.objects.awaiting_vetting().exists()) + + # Refusal choice is ok + refusal_reason_inserted = VetCommentaryForm.COMMENTARY_REFUSAL_DICT[\ + VetCommentaryForm.REFUSAL_UNTRACEBLE] + self.assertEqual(form.get_refusal_reason(), refusal_reason_inserted) + + def test_process_before_validation(self): + """Test response of form on processing before validation""" + form = VetCommentaryForm(self.form_data, commentary_id=self.commentary.id, user=self.user) + self.assertRaises(ValueError, form.process_commentary) + + class TestRequestCommentaryForm(TestCase): fixtures = ['permissions', 'groups'] @@ -17,6 +78,11 @@ class TestRequestCommentaryForm(TestCase): self.user = UserFactory() self.valid_form_data = model_form_data(factory_instance, RequestCommentaryForm) + def empty_and_return_form_data(self, key): + """Empty specific valid_form_data field and return""" + self.valid_form_data[key] = None + return self.valid_form_data + def test_valid_data_is_valid_for_arxiv(self): """Test valid form for Arxiv identifier""" form_data = self.valid_form_data @@ -31,12 +97,41 @@ class TestRequestCommentaryForm(TestCase): form = RequestCommentaryForm(form_data, user=self.user) self.assertTrue(form.is_valid()) - # def test_form_has_no_identifiers(self): - # """Test invalid form has no DOI nor Arxiv ID""" - # form_data = self.valid_form_data - # form_data['pub_DOI'] = '' - # form_data['arxiv_identifier'] = '' - # form = RequestCommentaryForm(form_data, user=self.user) - # form_response = form.is_valid() - # print(form_response) - # self.assertFormError(form_response, form, 'arxiv_identifier', None) + def test_form_has_no_identifiers(self): + """Test invalid form has no DOI nor Arxiv ID""" + form_data = self.valid_form_data + form_data['pub_DOI'] = '' + form_data['arxiv_identifier'] = '' + form = RequestCommentaryForm(form_data, user=self.user) + self.assertFalse(form.is_valid()) + self.assertTrue('arxiv_identifier' in form.errors) + self.assertTrue('pub_DOI' in form.errors) + + def test_form_with_duplicate_DOI(self): + """Test form response with already existing DOI""" + # Create a factory instance containing Arxiv ID and DOI + VettedCommentaryFactory.create() + + # Test duplicate DOI entry + form_data = self.empty_and_return_form_data('arxiv_identifier') + form = RequestCommentaryForm(form_data, user=self.user) + self.assertTrue('pub_DOI' in form.errors) + self.assertFalse(form.is_valid()) + + # Check is existing commentary is valid + existing_commentary = form.get_existing_commentary() + self.assertEqual(existing_commentary.pub_DOI, form_data['pub_DOI']) + + def test_form_with_duplicate_arxiv_id(self): + """Test form response with already existing Arxiv ID""" + VettedCommentaryFactory.create() + + # Test duplicate Arxiv entry + form_data = self.empty_and_return_form_data('pub_DOI') + form = RequestCommentaryForm(form_data, user=self.user) + self.assertTrue('arxiv_identifier' in form.errors) + self.assertFalse(form.is_valid()) + + # Check is existing commentary is valid + existing_commentary = form.get_existing_commentary() + self.assertEqual(existing_commentary.arxiv_identifier, form_data['arxiv_identifier']) diff --git a/commentaries/test_views.py b/commentaries/test_views.py index e6cc591f82f92345fe466d9dad9a23fe0d24063f..682a28b4b229b6ac21ae42325e0619f65f303f57 100644 --- a/commentaries/test_views.py +++ b/commentaries/test_views.py @@ -1,7 +1,7 @@ -from django.contrib.auth.models import Group from django.core.urlresolvers import reverse from django.test import TestCase + class RequestCommentaryTest(TestCase): """Test cases for `request_commentary` view method""" fixtures = ['permissions', 'groups', 'contributors'] diff --git a/commentaries/views.py b/commentaries/views.py index 506fd838fc126d015f7c7144b29e4f73a4340b0f..aa653f178663c94fbdecb8bca2d708691b73bfe3 100644 --- a/commentaries/views.py +++ b/commentaries/views.py @@ -12,10 +12,11 @@ from django.core.mail import EmailMessage from django.core.urlresolvers import reverse from django.http import HttpResponse from django.shortcuts import redirect +from django.template.loader import render_to_string from .models import Commentary from .forms import RequestCommentaryForm, DOIToQueryForm, IdentifierToQueryForm -from .forms import VetCommentaryForm, CommentarySearchForm, commentary_refusal_dict +from .forms import VetCommentaryForm, CommentarySearchForm from comments.models import Comment from comments.forms import CommentForm @@ -140,7 +141,7 @@ def prefill_using_DOI(request): @permission_required('scipost.can_request_commentary_pages', raise_exception=True) def prefill_using_identifier(request): - """ Probes arXiv with the identifier, to pre-fill the form. """ + """Probes arXiv with the identifier, to pre-fill the form""" if request.method == "POST": identifierform = IdentifierToQueryForm(request.POST) if identifierform.is_valid(): @@ -226,6 +227,7 @@ def prefill_using_identifier(request): @permission_required('scipost.can_vet_commentary_requests', raise_exception=True) def vet_commentary_requests(request): + """Show the first commentary thats awaiting vetting""" contributor = Contributor.objects.get(user=request.user) commentary_to_vet = Commentary.objects.awaiting_vetting().first() # only handle one at a time form = VetCommentaryForm() @@ -235,74 +237,49 @@ def vet_commentary_requests(request): @permission_required('scipost.can_vet_commentary_requests', raise_exception=True) def vet_commentary_request_ack(request, commentary_id): if request.method == 'POST': - form = VetCommentaryForm(request.POST) - commentary = Commentary.objects.get(pk=commentary_id) + form = VetCommentaryForm(request.POST, user=request.user, commentary_id=commentary_id) if form.is_valid(): - if form.cleaned_data['action_option'] == '1': - # accept the commentary as is - commentary.vetted = True - commentary.vetted_by = Contributor.objects.get(user=request.user) - commentary.latest_activity = timezone.now() - commentary.save() - email_text = ('Dear ' + title_dict[commentary.requested_by.title] + ' ' - + commentary.requested_by.user.last_name - + ', \n\nThe Commentary Page you have requested, ' - 'concerning publication with title ' - + commentary.pub_title + ' by ' + commentary.author_list - + ', has been activated at https://scipost.org/commentary/' - + str(commentary.arxiv_or_DOI_string) - + '. You are now welcome to submit your comments.' - '\n\nThank you for your contribution, \nThe SciPost Team.') - emailmessage = EmailMessage('SciPost Commentary Page activated', email_text, - 'SciPost commentaries <commentaries@scipost.org>', - [commentary.requested_by.user.email], - ['commentaries@scipost.org'], - reply_to=['commentaries@scipost.org']) - emailmessage.send(fail_silently=False) - elif form.cleaned_data['action_option'] == '0': - # re-edit the form starting from the data provided - form2 = 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}) - commentary.delete() - email_text = ('Dear ' + title_dict[commentary.requested_by.title] + ' ' - + commentary.requested_by.user.last_name - + ', \n\nThe Commentary Page you have requested, ' - 'concerning publication with title ' + commentary.pub_title - + ' by ' + commentary.author_list - + ', has been activated (with slight modifications to your submitted details).' - ' You are now welcome to submit your comments.' - '\n\nThank you for your contribution, \nThe SciPost Team.') - emailmessage = EmailMessage('SciPost Commentary Page activated', email_text, - 'SciPost commentaries <commentaries@scipost.org>', - [commentary.requested_by.user.email], - ['commentaries@scipost.org'], - reply_to=['commentaries@scipost.org']) - emailmessage.send(fail_silently=False) - context = {'form': form2 } + # Get commentary + commentary = form.get_commentary() + email_context = { + 'commentary': commentary + } + + # 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'] + + # Send email and process form + email_text = render_to_string(email_template, email_context) + 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) + 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) - elif form.cleaned_data['action_option'] == '2': - # the commentary request is simply rejected - email_text = ('Dear ' + title_dict[commentary.requested_by.title] + ' ' - + commentary.requested_by.user.last_name - + ', \n\nThe Commentary Page you have requested, ' - 'concerning publication with title ' - + commentary.pub_title + ' by ' + commentary.author_list - + ', has not been activated for the following reason: ' - + commentary_refusal_dict[int(form.cleaned_data['refusal_reason'])] - + '.\n\nThank you for your interest, \nThe SciPost Team.') - if form.cleaned_data['email_response_field']: - email_text += '\n\nFurther explanations: ' + form.cleaned_data['email_response_field'] - emailmessage = EmailMessage('SciPost Commentary Page activated', email_text, - 'SciPost commentaries <commentaries@scipost.org>', - [commentary.requested_by.user.email], - ['commentaries@scipost.org'], - reply_to=['comentaries@scipost.org']) - emailmessage.send(fail_silently=False) - commentary.delete() context = {'ack_header': 'SciPost Commentary request vetted.', 'followup_message': 'Return to the ', diff --git a/scipost/models.py b/scipost/models.py index 033c08ec50202e4cc88e200a4db4f8ee523a5be2..9fc4c36328d65f7573f4beec500951e27c832cbd 100644 --- a/scipost/models.py +++ b/scipost/models.py @@ -109,6 +109,9 @@ class Contributor(models.Model): def __str__(self): return '%s, %s' % (self.user.last_name, self.user.first_name) + def get_title(self): + return title_dict[self.title] + def is_currently_available(self): unav_periods = UnavailabilityPeriod.objects.filter(contributor=self)