From 0100cbde19f220dcf706d56292ea984d8d25310e Mon Sep 17 00:00:00 2001 From: Geert Kapteijns <ghkapteijns@gmail.com> Date: Tue, 20 Dec 2016 23:10:38 +0100 Subject: [PATCH] Replace vet_thesislink_requests by FormView This does not work yet. I wrote some failing tests, but while I was doing that, I came to the conclusion that I want to make the route of the get request different from the route of the post request. --- theses/factories.py | 14 +++++- theses/forms.py | 42 +++++++++++------- .../theses/vet_thesislink_requests.html | 2 +- theses/test_forms.py | 22 +++++++++- theses/test_views.py | 23 +++++++--- theses/urls.py | 2 +- theses/views.py | 43 ++++++++++++++----- 7 files changed, 111 insertions(+), 37 deletions(-) diff --git a/theses/factories.py b/theses/factories.py index 75f999c53..bc57863b3 100644 --- a/theses/factories.py +++ b/theses/factories.py @@ -1,7 +1,11 @@ import factory -from .models import ThesisLink + +from common.helpers.factories import FormFactory from scipost.factories import ContributorFactory +from .models import ThesisLink +from .forms import VetThesisLinkForm + class ThesisLinkFactory(factory.django.DjangoModelFactory): class Meta: @@ -17,3 +21,11 @@ class ThesisLinkFactory(factory.django.DjangoModelFactory): defense_date = factory.Faker('date_time_this_century') abstract = factory.Faker('text') domain = 'ET' + + +class VetThesisLinkFormFactory(FormFactory): + class Meta: + model = VetThesisLinkForm + + action_option = VetThesisLinkForm.ACCEPT + # justification = factory.Faker('lorem') diff --git a/theses/forms.py b/theses/forms.py index 13b4a1793..05963cb32 100644 --- a/theses/forms.py +++ b/theses/forms.py @@ -3,18 +3,6 @@ from django import forms from .models import * from .helpers import past_years -THESIS_ACTION_CHOICES = ( - (0, 'modify'), - (1, 'accept'), - (2, 'refuse (give reason below)'), - ) - -THESIS_REFUSAL_CHOICES = ( - (0, '-'), - (-1, 'a link to this thesis already exists'), - (-2, 'the external link to this thesis does not work'), - ) - class RequestThesisLinkForm(forms.ModelForm): class Meta: @@ -29,13 +17,35 @@ class RequestThesisLinkForm(forms.ModelForm): class VetThesisLinkForm(forms.Form): - action_option = forms.ChoiceField(widget=forms.RadioSelect, - choices=THESIS_ACTION_CHOICES, - required=True, label='Action') + MODIFY = 0 + ACCEPT = 1 + REFUSE = 2 + THESIS_ACTION_CHOICES = ( + (MODIFY, 'modify'), + (ACCEPT, 'accept'), + (REFUSE, 'refuse (give reason below)'), + ) + + EMPTY_CHOICE = 0 + ALREADY_EXISTS = 1 + LINK_DOES_NOT_WORK = 2 + THESIS_REFUSAL_CHOICES = ( + (EMPTY_CHOICE, '---'), + (ALREADY_EXISTS, 'a link to this thesis already exists'), + (LINK_DOES_NOT_WORK, 'the external link to this thesis does not work'), + ) + + action_option = forms.ChoiceField( + widget=forms.RadioSelect, choices=THESIS_ACTION_CHOICES, required=True, label='Action') refusal_reason = forms.ChoiceField(choices=THESIS_REFUSAL_CHOICES, required=False) - email_response_field = forms.CharField(widget=forms.Textarea( + justification = forms.CharField(widget=forms.Textarea( attrs={'rows': 5, 'cols': 40}), label='Justification (optional)', required=False) + def vet_request(self, thesis_link): + print(self.cleaned_data) + if self.cleaned_data['action_option'] == VetThesisLinkForm.ACCEPT: + print('hoi') + class ThesisLinkSearchForm(forms.Form): author = forms.CharField(max_length=100, required=False, label="Author") diff --git a/theses/templates/theses/vet_thesislink_requests.html b/theses/templates/theses/vet_thesislink_requests.html index 4fd2a7d9a..c49008776 100644 --- a/theses/templates/theses/vet_thesislink_requests.html +++ b/theses/templates/theses/vet_thesislink_requests.html @@ -22,7 +22,7 @@ <p>{{ thesislink_to_vet.abstract }}</p> </div> <div class="col-4"> - <form action="{% url 'theses:vet_thesislink_request_ack' thesislink_id=thesislink_to_vet.id %}" method="post"> + <form method="post"> {% csrf_token %} {{ form.as_ul }} <input type="submit" value="Submit" /> diff --git a/theses/test_forms.py b/theses/test_forms.py index ce07aa621..145618459 100644 --- a/theses/test_forms.py +++ b/theses/test_forms.py @@ -2,8 +2,8 @@ import factory from django.test import TestCase -from .factories import ThesisLinkFactory -from .forms import RequestThesisLinkForm +from .factories import ThesisLinkFactory, VetThesisLinkFormFactory +from .forms import RequestThesisLinkForm, VetThesisLinkForm from common.helpers import model_form_data @@ -24,3 +24,21 @@ class TestRequestThesisLink(TestCase): form = RequestThesisLinkForm(form_data) form.is_valid() self.assertEqual(form.errors['domain'], ['This field is required.']) + + +class TestVetThesisLinkRequests(TestCase): + fixtures = ['permissions', 'groups'] + + def test_thesislink_gets_vetted_when_accepted(self): + thesis_link = ThesisLinkFactory() + form = VetThesisLinkFormFactory() + form.is_valid() + form.vet_request(thesis_link) + self.assertTrue(thesis_link.vetted) + + def test_thesislink_is_not_vetted_when_refused(self): + thesis_link = ThesisLinkFactory() + form = VetThesisLinkFormFactory(action_option=VetThesisLinkForm.REFUSE) + form.is_valid() + form.vet_request(thesis_link) + self.assertFalse(thesis_link.vetted) diff --git a/theses/test_views.py b/theses/test_views.py index 496e3ce31..faa7f14d5 100644 --- a/theses/test_views.py +++ b/theses/test_views.py @@ -6,9 +6,9 @@ from django.test.client import Client from django.contrib.auth.models import Group from django.urls import reverse -from .views import RequestThesisLink, vet_thesislink_requests -from scipost.factories import UserFactory -from .factories import ThesisLinkFactory +from .views import RequestThesisLink, VetThesisLinkRequests +from scipost.factories import UserFactory, ContributorFactory +from .factories import ThesisLinkFactory, VetThesisLinkFormFactory from .models import ThesisLink @@ -64,7 +64,7 @@ class TestVetThesisLinkRequests(TestCase): user = UserFactory() request.user = user self.assertRaises( - PermissionDenied, vet_thesislink_requests, request) + PermissionDenied, VetThesisLinkRequests.as_view(), request) def test_response_vetting_editor(self): # Create ThesisLink to vet. @@ -73,5 +73,18 @@ class TestVetThesisLinkRequests(TestCase): user = UserFactory() user.groups.add(Group.objects.get(name="Vetting Editors")) request.user = user - response = vet_thesislink_requests(request) + response = VetThesisLinkRequests.as_view()(request) self.assertEqual(response.status_code, 200) + + def test_thesislink_is_vetted_by_correct_contributor(self): + # TODO: how to make sure we are vetting the right thesis link? + contributor = ContributorFactory() + contributor.user.groups.add(Group.objects.get(name="Vetting Editors")) + post_data = VetThesisLinkFormFactory().data + + request = RequestFactory().post(self.target, post_data) + request.user = contributor.user + + response = VetThesisLinkRequests.as_view()(request) + + self.assertTrue(False) diff --git a/theses/urls.py b/theses/urls.py index 05839aa75..3d3c530e4 100644 --- a/theses/urls.py +++ b/theses/urls.py @@ -9,7 +9,7 @@ urlpatterns = [ url(r'^browse/(?P<discipline>[a-z]+)/(?P<nrweeksback>[0-9]+)/$', views.browse, name='browse'), url(r'^(?P<thesislink_id>[0-9]+)/$', views.thesis_detail, name='thesis'), url(r'^request_thesislink$', views.RequestThesisLink.as_view(), name='request_thesislink'), - url(r'^vet_thesislink_requests$', views.vet_thesislink_requests, + url(r'^vet_thesislink_requests$', views.VetThesisLinkRequests.as_view(), name='vet_thesislink_requests'), url(r'^vet_thesislink_request_ack/(?P<thesislink_id>[0-9]+)$', views.vet_thesislink_request_ack, name='vet_thesislink_request_ack'), diff --git a/theses/views.py b/theses/views.py index 15c6655c6..0dd1a2b6e 100644 --- a/theses/views.py +++ b/theses/views.py @@ -11,7 +11,7 @@ from django.core.urlresolvers import reverse, reverse_lazy from django.http import HttpResponse, HttpResponseRedirect from django.views.decorators.csrf import csrf_protect from django.db.models import Avg -from django.views.generic.edit import CreateView +from django.views.generic.edit import CreateView, FormView from django.utils.decorators import method_decorator from .models import * @@ -42,14 +42,35 @@ class RequestThesisLink(CreateView): return super(RequestThesisLink, self).form_valid(form) -@permission_required('scipost.can_vet_thesislink_requests', raise_exception=True) -def vet_thesislink_requests(request): - contributor = Contributor.objects.get(user=request.user) - thesislink_to_vet = ThesisLink.objects.filter( - vetted=False).first() # only handle one at a time - form = VetThesisLinkForm() - context = {'contributor': contributor, 'thesislink_to_vet': thesislink_to_vet, 'form': form} - return render(request, 'theses/vet_thesislink_requests.html', context) +@method_decorator(permission_required( + 'scipost.can_vet_thesislink_requests', raise_exception=True), name='dispatch') +class VetThesisLinkRequests(FormView): + form_class = VetThesisLinkForm + template_name = 'theses/vet_thesislink_requests.html' + # TODO: not right yet + success_url = reverse_lazy('theses:vet_thesislink_requests') + + def get_context_data(self, **kwargs): + context = super(VetThesisLinkRequests, self).get_context_data(**kwargs) + context['thesislink_to_vet'] = self.thesislink_to_vet() + return context + + def thesislink_to_vet(self): + return ThesisLink.objects.filter(vetted=False).first() + + def form_valid(self, form): + form.vet_request(self.thesislink_to_vet()) + return super(VetThesisLinkRequests, self).form_valid(form) + + +# @permission_required('scipost.can_vet_thesislink_requests', raise_exception=True) +# def vet_thesislink_requests(request): +# contributor = Contributor.objects.get(user=request.user) +# thesislink_to_vet = ThesisLink.objects.filter( +# vetted=False).first() # only handle one at a time +# form = VetThesisLinkForm() +# context = {'contributor': contributor, 'thesislink_to_vet': thesislink_to_vet, 'form': form} +# return render(request, 'theses/vet_thesislink_requests.html', context) @permission_required('scipost.can_vet_thesislink_requests', raise_exception=True) @@ -109,9 +130,9 @@ def vet_thesislink_request_ack(request, thesislink_id): + ', has not been activated for the following reason: ' + form.cleaned_data['refusal_reason'] + '.\n\nThank you for your interest, \nThe SciPost Team.') - if form.cleaned_data['email_response_field']: + if form.cleaned_data['justification']: email_text += '\n\nFurther explanations: ' + \ - form.cleaned_data['email_response_field'] + form.cleaned_data['justification'] emailmessage = EmailMessage('SciPost Thesis Link', email_text, 'SciPost Theses <theses@scipost.org>', [thesislink.requested_by.user.email], -- GitLab