From 786dd03eeb7b2bf5f77a805eeb1c49432bbed697 Mon Sep 17 00:00:00 2001
From: "J.-S. Caux" <J.S.Caux@uva.nl>
Date: Fri, 26 Mar 2021 11:29:01 +0100
Subject: [PATCH] Move vote eligibility check to class method

---
 colleges/models/potential_fellowship.py | 12 ++++++++++++
 colleges/permissions.py                 | 14 --------------
 colleges/views.py                       |  3 +--
 scipost/models.py                       |  5 +++++
 4 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/colleges/models/potential_fellowship.py b/colleges/models/potential_fellowship.py
index 2c39230ce..40877b3c2 100644
--- a/colleges/models/potential_fellowship.py
+++ b/colleges/models/potential_fellowship.py
@@ -57,6 +57,18 @@ class PotentialFellowship(models.Model):
     def __str__(self):
         return '%s, %s' % (self.profile.__str__(), self.get_status_display())
 
+    def can_vote(self, user):
+        """
+        Determines whether user can vote on election for this PotentialFellow.
+        Qualifying conditions (either of the following):
+        * is Admin
+        * is in AdvisoryBoard for this College's Academic Field
+        * is a Senior Fellow in the College proposed
+        """
+        return (user.contributor.is_sp_admin() or
+                user.contributor.is_in_advisory_board and user.contributor.profile.acad_field == self.college.acad_field or
+                user.contributor.fellowships.senior().filter(college=self.college).exists())
+
     def latest_event_details(self):
         event = self.potentialfellowshipevent_set.order_by('-noted_on').first()
         if not event:
diff --git a/colleges/permissions.py b/colleges/permissions.py
index fdd9769c8..f27cf1a4f 100644
--- a/colleges/permissions.py
+++ b/colleges/permissions.py
@@ -32,17 +32,3 @@ def fellowship_or_admin_required():
                 return True
         raise PermissionDenied
     return user_passes_test(test)
-
-
-def can_vote_on_potential_fellowship_for_college(user, college):
-    """
-    Qualifying conditions (either of the following):
-    * is Admin
-    * is in AdvisoryBoard for this College's Academic Field
-    * is a Senior Fellow in this College
-    """
-    if is_in_group(user, 'SciPost Administrators'):
-        return True
-    if is_in_group(user, 'Advisory Board') and user.contributor.profile.acad_field == college.acad_field:
-        return True
-    return user.contributor.fellowships.senior().filter(college=college).exists()
diff --git a/colleges/views.py b/colleges/views.py
index 9edd490c6..e3e8bbe61 100644
--- a/colleges/views.py
+++ b/colleges/views.py
@@ -463,8 +463,7 @@ class PotentialFellowshipDetailView(PermissionsMixin, DetailView):
 @permission_required('scipost.can_vote_on_potentialfellowship', raise_exception=True)
 def vote_on_potential_fellowship(request, potfel_id, vote):
     potfel = get_object_or_404(PotentialFellowship, pk=potfel_id)
-    from colleges.permissions import can_vote_on_potential_fellowship_for_college
-    if not can_vote_on_potential_fellowship_for_college(request.user, potfel.college):
+    if not potfel.can_vote(request.user):
         raise Http404
     potfel.in_agreement.remove(request.user.contributor)
     potfel.in_abstain.remove(request.user.contributor)
diff --git a/scipost/models.py b/scipost/models.py
index 4fd2fd3ee..9c3f3f723 100644
--- a/scipost/models.py
+++ b/scipost/models.py
@@ -130,6 +130,11 @@ class Contributor(models.Model):
         return (self.user.groups.filter(name='Editorial Administrators').exists()
                 or self.user.is_superuser)
 
+    def is_in_advisory_board(self):
+        """Check if Contributor is in the Advisory Board."""
+        return (self.user.groups.filter(name='Advisory Board').exists()
+                or self.user.is_superuser)
+
     def is_active_fellow(self):
         """Check if Contributor is a member of the Editorial College."""
         return self.fellowships.active().exists() or self.user.is_superuser
-- 
GitLab