Allow Wharton Council officers to manage associated clubs

backend/clubs/permissions.py

@@ -163,6 +163,8 @@
         # user must be in club or parent club to perform non-view actions
         membership = find_membership_helper(request.user, obj)
         if membership is None:
+            if obj.is_wharton:
+                return check_wharton_council_officer(self, request)
             return False
         # user has to be an owner to delete a club, an officer to edit it
         if view.action in {"destroy"}:
@@ -278,8 +280,8 @@
 
 class ClubItemPermission(permissions.BasePermission):
     """
-    Officers and above can create/update/delete events or testimonials.
-    Everyone else can view and list events or testimonials.
+    Officers and above can create/update/delete applications or testimonials.
+    Everyone else can view and list applications or testimonials.
     """
 
     def has_permission(self, request, view):
@@ -298,7 +300,9 @@
                 return True
             obj = Club.objects.get(code=view.kwargs["club_code"])
             membership = find_membership_helper(request.user, obj)
-            return membership is not None and membership.role <= Membership.ROLE_OFFICER
+            return (
+                membership is not None and membership.role <= Membership.ROLE_OFFICER
+            ) or (obj.is_wharton and check_wharton_council_officer(self, request))
         else:
             return True
 
@@ -333,30 +337,30 @@
         return request.user.is_authenticated and request.user.is_superuser
 
 
+def check_wharton_council_officer(self, request):
+    WHARTON_COUNCIL_CLUB_CODE = "wharton-council"
+    if not request.user.is_authenticated:
+        return False
+    user = get_user_model().objects.filter(username=request.user).first()
+    if user is not None:
+        membership = Membership.objects.filter(
+            club__code=WHARTON_COUNCIL_CLUB_CODE, person=user
+        ).first()
+        if membership is not None:
+            return membership.role <= Membership.ROLE_OFFICER
+    return False
+
+
 class WhartonApplicationPermission(permissions.BasePermission):
     """
     Grants permission if the user is an officer of Wharton Council
     """
 
-    WHARTON_COUNCIL_CLUB_CODE = "wharton-council"
-
-    def check_wharton_council_officer(self, request):
-        if not request.user.is_authenticated:
-            return False
-        user = get_user_model().objects.filter(username=request.user).first()
-        if user is not None:
-            membership = Membership.objects.filter(
-                club__code=self.WHARTON_COUNCIL_CLUB_CODE, person=user
-            ).first()
-            if membership is not None:
-                return membership.role <= Membership.ROLE_OFFICER
-        return False
-
     def has_object_permission(self, request, view, obj):
-        return self.check_wharton_council_officer(request)
+        return check_wharton_council_officer(self, request)
 
     def has_permission(self, request, view):
-        return self.check_wharton_council_officer(request)
+        return check_wharton_council_officer(self, request)
 
 
 def DjangoPermission(perm):

backend/clubs/serializers.py

@@ -60,6 +60,7 @@
     Ticket,
     Year,
 )
+from clubs.permissions import ClubPermission
 from clubs.utils import clean
 
 
@@ -890,6 +891,15 @@ class Meta(ClubMinimalSerializer.Meta):
         fields = ClubMinimalSerializer.Meta.fields + ["files"]
 
 
+class FakeView(object):
+    """
+    Dummy view used for permissions checking by the UserPermissionAPIView.
+    """
+
+    def __init__(self, action):
+        self.action = action
+
+
 class ClubListSerializer(serializers.ModelSerializer):
     """
     The club list serializer returns a subset of the information that the full
@@ -1013,8 +1023,13 @@ def to_representation(self, instance):
         if instance.ghost and not instance.approved:
             user = self.context["request"].user
 
-            can_see_pending = user.has_perm("clubs.see_pending_clubs") or user.has_perm(
-                "clubs.manage_club"
+            # users that can edit the club can see its pending status
+            can_see_pending = (
+                user.has_perm("clubs.see_pending_clubs")
+                or user.has_perm("clubs.manage_club")
+                or ClubPermission().has_object_permission(
+                    self.context["request"], FakeView("update"), instance
+                )
             )
             is_member = (
                 user.is_authenticated

backend/clubs/views.py

@@ -173,6 +173,7 @@
     EventSerializer,
     EventWriteSerializer,
     ExternalMemberListSerializer,
+    FakeView,
     FavoriteSerializer,
     FavoriteWriteSerializer,
     FavouriteEventSerializer,
@@ -4524,15 +4525,6 @@ def get(self, request, *args, **kwargs):
         return response
 
 
-class FakeView(object):
-    """
-    Dummy view used for permissions checking by the UserPermissionAPIView.
-    """
-
-    def __init__(self, action):
-        self.action = action
-
-
 class UserGroupAPIView(APIView):
     """
     get: Return the major permission groups and their members on the site.

backend/tests/clubs/test_views.py

@@ -182,6 +182,22 @@ def setUp(self):
             email="example@example.com",
         )
 
+        self.wc = Club.objects.create(
+            code="wharton-council",
+            name="Wharton Council",
+            approved=True,
+            email="example@example.com",
+        )
+
+        # a lot of tests (e.g. directory) implicitly assume some constant of clubs
+        self.NUM_CLUBS = 2
+
+        self.wc_badge = Badge.objects.create(
+            club=self.wc,
+            label="Wharton Council",
+            description="Wharton Council",
+        )
+
         self.event1 = Event.objects.create(
             code="test-event",
             club=self.club1,
@@ -235,7 +251,7 @@ def test_directory(self):
         """
         resp = self.client.get(reverse("clubs-directory"))
         self.assertIn(resp.status_code, [200, 201], resp.content)
-        self.assertEqual(len(resp.data), 1)
+        self.assertEqual(len(resp.data), self.NUM_CLUBS)
 
     def test_advisor_visibility(self):
         """
@@ -1576,6 +1592,38 @@ def test_club_modify(self):
         self.assertEqual(data["description"], "We do stuff.")
         self.assertEqual(len(data["tags"]), 2)
 
+    def test_club_wc_modify(self):
+        """
+        Wharton Council officers should be able to modify associated clubs.
+        """
+        wc_club = Club.objects.create(
+            name="WC Member Club",
+            code="wc-club",
+            description="We love Wharton",
+        )
+        Membership.objects.create(
+            person=self.user4, club=self.wc, role=Membership.ROLE_OFFICER
+        )
+
+        self.client.login(username=self.user4.username, password="test")
+
+        # assert that we can't modify a non-WC club
+        resp = self.client.patch(
+            reverse("clubs-detail", args=(wc_club.code,)),
+            {"description": "We hate Wharton"},
+            content_type="application/json",
+        )
+        self.assertIn(resp.status_code, [400, 403], resp.content)
+
+        # assert that we can modify a WC club
+        wc_club.badges.add(self.wc_badge)
+        resp = self.client.patch(
+            reverse("clubs-detail", args=(wc_club.code,)),
+            {"description": "We hate Wharton"},
+            content_type="application/json",
+        )
+        self.assertIn(resp.status_code, [200, 201], resp.content)
+
     def test_club_archive_no_auth(self):
         """
         Unauthenticated users should not be able to archive a club.
@@ -1613,6 +1661,7 @@ def test_club_archive(self):
         self.club1.save()
 
         # ensure club was put back on clubs endpoint
+        cache.clear()
         resp = self.client.get(reverse("clubs-list"))
         self.assertIn(resp.status_code, [200], resp.content)
         codes = [club["code"] for club in resp.data]
@@ -2012,7 +2061,7 @@ def test_club_report_selects_one_field(self):
             reverse("clubs-list"), {"format": "xlsx", "fields": "name"}
         )
         self.assertEqual(200, res.status_code)
-        self.assertEqual(1, len(res.data))
+        self.assertEqual(self.NUM_CLUBS, len(res.data))
         self.assertTrue(isinstance(res.data[0], dict))
         self.assertEqual(1, len(res.data[0]))
 
@@ -2021,14 +2070,14 @@ def test_club_report_selects_few_fields(self):
             reverse("clubs-list"), {"format": "xlsx", "fields": "name,code"}
         )
         self.assertEqual(200, res.status_code)
-        self.assertEqual(1, len(res.data))
+        self.assertEqual(self.NUM_CLUBS, len(res.data))
         self.assertTrue(isinstance(res.data[0], dict))
         self.assertEqual(2, len(res.data[0]))
 
     def test_club_report_selects_all_fields(self):
         res = self.client.get(reverse("clubs-list"), {"format": "xlsx"})
         self.assertEqual(200, res.status_code)
-        self.assertEqual(1, len(res.data))
+        self.assertEqual(self.NUM_CLUBS, len(res.data))
         self.assertTrue(isinstance(res.data[0], dict))
         self.assertTrue(len(res.data[0]) > 2)