Move decorators and add validating form for starting reviews

[Finishes #131669611]
Closes gh-392

orb/decorators.py

@@ -20,27 +20,6 @@ def staff_test(user):
     raise PermissionDenied
 
 
-def content_reviewer(user):
-    """
-    Tests that user is content reviewer (or superuser)
-
-    Args:
-        user: the User object
-
-    Returns:
-        Boolean
-
-    Raises:
-        PermissionDenied
-
-    """
-    if not user.is_authenticated():
-        return False
-    if user.is_active and (user.userprofile.is_reviewer or user.is_staff):
-        return True
-    raise PermissionDenied
-
-
 def staff_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME, login_url=None):
     """
     Decorator for views that checks that the user is logged in and a staff member.
@@ -57,20 +36,3 @@ def staff_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME, login
     if function:
         return actual_decorator(function)
     return actual_decorator
-
-
-def reviewer_required(function=None, redirect_field_name=REDIRECT_FIELD_NAME, login_url=None):
-    """
-    Decorator for views that checks that the user is logged in and either a
-    staff member or a content reviewer.
-    """
-    actual_decorator = user_passes_test(
-        lambda u: content_reviewer(u),
-        login_url=login_url,
-        redirect_field_name=redirect_field_name
-    )
-    if function:
-        return actual_decorator(function)
-    return actual_decorator
-
-

orb/models.py

@@ -430,7 +430,7 @@ class UserProfile(TimestampBase):
     mailing = models.BooleanField(default=False, blank=False)
     crt_member = models.BooleanField(default=False, blank=False)
     mep_member = models.BooleanField(default=False, blank=False)
-    reviewer_roles = models.ManyToManyField('ReviewerRole', blank=True)
+    reviewer_roles = models.ManyToManyField('ReviewerRole', blank=True, related_name="profiles")
 
     profiles = ProfilesQueryset.as_manager()
     objects = profiles

orb/review/decorators.py

@@ -0,0 +1,44 @@
+from django.contrib.auth import REDIRECT_FIELD_NAME
+from django.contrib.auth.decorators import user_passes_test
+from django.core.exceptions import PermissionDenied
+
+
+def content_reviewer(user, include_staff=True):
+    """
+    Tests that user is content reviewer (or superuser)
+
+    Args:
+        user: the User object
+        include_staff: boolean for whether to also allow non-reviewer staff
+
+    Returns:
+        Boolean
+
+    Raises:
+        PermissionDenied
+
+    """
+    if not user.is_authenticated():
+        return False
+
+    is_staff = user.is_staff if include_staff else False
+
+    if user.is_active and (user.userprofile.is_reviewer or is_staff):
+        return True
+
+    raise PermissionDenied
+
+
+def reviewer_required(function=None, include_staff=True, redirect_field_name=REDIRECT_FIELD_NAME, login_url=None):
+    """
+    Decorator for views that checks that the user is logged in and either a
+    staff member or a content reviewer.
+    """
+    actual_decorator = user_passes_test(
+        lambda u: content_reviewer(u, include_staff=include_staff),
+        login_url=login_url,
+        redirect_field_name=redirect_field_name
+    )
+    if function:
+        return actual_decorator(function)
+    return actual_decorator

orb/review/forms.py

@@ -77,6 +77,32 @@ def save(self):
         return messages.SUCCESS, _("The resource has been rejected")
 
 
+class ReviewStartForm(forms.ModelForm):
+    """
+    Form class for validating a user starting a review on a resource.
+
+    Takes a resource, a user, and then validates that the role selected
+    belongs to the user and that it is available for a reivew.
+    """
+    class Meta:
+        model = ContentReview
+        fields = ['role']
+
+    def __init__(self, *args, **kwargs):
+        self.resource = kwargs.pop('resource')
+        self.reviewer = kwargs.pop('reviewer')
+        super(ReviewStartForm, self).__init__(*args, **kwargs)
+        self.fields['role'].queryset = ReviewerRole.objects.filter(
+            profiles__user=self.reviewer).exclude(reviews__in=self.resource.content_reviews.all())
+
+    def save(self):
+        instance = super(ReviewStartForm, self).save(commit=False)
+        instance.resource = self.resource
+        instance.reviewer = self.reviewer
+        instance.save()
+        return instance
+
+
 class ContentReviewForm(forms.ModelForm):
     """
     Form class for capturing the explanation for rejecting a submitted resource

orb/review/migrations/0002_auto_20161017_0013.py

@@ -0,0 +1,18 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('review', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='contentreview',
+            unique_together=set([('resource', 'role')]),
+        ),
+    ]

orb/review/models.py

@@ -109,7 +109,6 @@ class Meta:
         verbose_name = _("content review")
         verbose_name_plural = _("content reviews")
         unique_together = (
-            ('resource', 'reviewer'),
             ('resource', 'role'),
         )
 

orb/review/templatetags/review_tags.py

@@ -73,4 +73,7 @@ def can_start_review(context, resource):
 
     """
     profile = context['user'].userprofile
+    if not profile.is_reviewer:
+        return False
+    # TODO this is wrong
     return not resource.content_reviews.filter(role__in=profile.reviewer_roles.all).exists()

orb/review/tests/test_forms.py

@@ -11,7 +11,7 @@
 from orb.models import Resource, ResourceCriteria
 from orb.models import UserProfile, ReviewerRole
 from orb.resources.tests.factory import resource_factory
-from orb.review.forms import AssignmentForm, ContentReviewForm
+from orb.review.forms import AssignmentForm, ContentReviewForm, ReviewStartForm
 from orb.review.models import ContentReview
 from .base import ReviewTestCase
 
@@ -36,6 +36,10 @@ def setUpClass(cls):
         cls.criteria_5 = ResourceCriteria.objects.create(description="E")
         cls.criteria_6 = ResourceCriteria.objects.create(description="F")
 
+    @classmethod
+    def tearDownClass(cls):
+        super(ReviewFormTests, cls).tearDownClass()
+
     def test_displayed_criteria(self):
         """Criteria should be based on user role"""
         form = ContentReviewForm(user=self.staff_user)
@@ -170,3 +174,59 @@ def test_save_reviews(self):
         form.save()
         self.assertEqual(count + 1, ContentReview.reviews.all().count())
         ContentReview.reviews.all().delete()
+
+
+class StartFormTests(ReviewTestCase):
+    """
+    The ReviewStartForm takes a resource, a user, and then validates
+    that the role selected belongs to the user and that it is
+    available for a reivew.
+    """
+    def test_non_reviewer(self):
+        form = ReviewStartForm(data={
+            'role': self.technical_role.pk,
+        }, resource=self.resource, reviewer=self.nonreviewer)
+        self.assertFalse(form.is_valid())
+
+    def test_reviewer_wrong_role(self):
+        form = ReviewStartForm(data={
+            'role': self.technical_role.pk,
+        }, resource=self.resource, reviewer=self.staff_user)
+        self.assertFalse(form.is_valid())
+
+    def test_reviewer_matching_role(self):
+        form = ReviewStartForm(data={
+            'role': self.technical_role.pk,
+        }, resource=self.resource, reviewer=self.reviewer)
+        self.assertTrue(form.is_valid())
+
+    def test_existing_assignment(self):
+        ContentReview.objects.create(
+            reviewer=self.reviewer,
+            resource=self.resource,
+            role=self.technical_role,
+        )
+        form = ReviewStartForm(data={
+            'role': self.technical_role.pk,
+        }, resource=self.resource, reviewer=self.reviewer)
+        self.assertFalse(form.is_valid())
+
+    def test_create_additional_review(self):
+        """If"""
+        ContentReview.objects.create(
+            reviewer=self.staff_user,
+            resource=self.resource,
+            role=self.medical_role,
+        )
+        form = ReviewStartForm(data={
+            'role': self.technical_role.pk,
+        }, resource=self.resource, reviewer=self.reviewer)
+        self.assertTrue(form.is_valid())
+
+    def test_create_content_review(self):
+        form = ReviewStartForm(data={
+            'role': self.medical_role.pk,
+        }, resource=self.resource, reviewer=self.staff_user)
+        self.assertTrue(form.is_valid())
+        self.assertTrue(form.save())
+

orb/review/views.py

@@ -9,9 +9,9 @@
 from django.shortcuts import redirect, render, get_object_or_404
 from django.utils.translation import ugettext_lazy as _
 
-from orb.decorators import reviewer_required
+from orb.review.decorators import reviewer_required
 from orb.models import Resource, ResourceCriteria, ReviewerRole
-from .forms import ReviewForm, ContentReviewForm, AssignmentForm, StaffReviewForm
+from .forms import ReviewForm, ContentReviewForm, AssignmentForm, StaffReviewForm, ReviewStartForm
 from .models import ContentReview
 
 
@@ -221,7 +221,7 @@ def staff_review(request, resource_id):
     })
 
 
-@reviewer_required
+@reviewer_required(include_staff=False)
 def start_assignment(request, resource_id):
     """
     View function that allows a user to assign themselves to review a resource
@@ -237,18 +237,16 @@ def start_assignment(request, resource_id):
     """
     resource = get_object_or_404(Resource, pk=resource_id)
     if request.method == 'POST':
-        try:
-            review = ContentReview.reviews.create(
-                resource=resource,
-                role=request.user.userprofile.reviewer_role,
-                reviewer=request.user,
-            )
-        except IntegrityError:
-            messages.error(request, _("There was an error starting the review."))
-            return redirect("orb_pending_resources")
-        return redirect(review.get_absolute_url())
+        form = ReviewStartForm(data=request.POST, resource=resource, reviewer=request.user)
+        if form.is_valid():
+            review = form.save()
+            return redirect(review.get_absolute_url())
+        messages.error(request, _("There was an error starting the review."))
+        return redirect("orb_pending_resources")
 
     return render(request, "orb/review/start_review.html", {
         'resource': resource,
-        'role': request.user.userprofile.reviewer_role,
+        'roles': ReviewerRole.objects.filter(
+            profiles__user=request.user,
+        ).exclude(reviews__in=resource.content_reviews.all()),
     })

orb/templates/orb/review/start_review.html

@@ -14,9 +14,16 @@ <h3>{{ resource.title }} (<a href="{% url 'orb_resource' resource.slug %}"
 {% endblocktrans %}</p>
 </div>
 
+
 <form method="POST" action=".">{% csrf_token %}
-<button class="btn btn-success" name="approved" value="1">{% trans 'Start' %}</button>
+    {%  for role in roles %}
+        <button class="btn btn-success" name="role" value="{{ role.pk }}">
+            {% blocktrans %}
+                Start {{ role }} Review
+            {% endblocktrans %}
+        </button>
+    {% endfor %}
 <a href="{% url 'orb_pending_resources' %}">{% trans 'Cancel' %}</a>
 </form>
 
-{% endblock %}
+{% endblock %}