Allow Wharton Council officers to manage associated clubs

backend/clubs/management/commands/sync.py

@@ -12,6 +12,7 @@
         "There should be no issues with repeatedly running this script. "
     )
     web_execute = True
+    dry_run = False  # Initialize for usage as standalone utility
 
     def add_arguments(self, parser):
         parser.add_argument(
@@ -95,43 +96,61 @@
             else:
                 self.stdout.write(f"Would have deleted {len(dups)} duplicate entries!")
 
-    def sync_badges(self):
+    def sync_badge(self, badge: Badge):
         """
-        Synchronizes badges based on parent child relationships.
-        Tends to favor adding objects to fix relationships instead of removing them.
+        Synchronizes a badge with its parent child relationships.
+        If a club has one of them (badge, parent), add the other.
         """
-        # add badges to parent child relationships
-        count = 0
-        for badge in Badge.objects.all():
-            if badge.org is not None:
-                self._visited = set()
-                count += self.recursively_add_badge(badge.org, badge)
-        self.stdout.write(
-            self.style.SUCCESS(f"Modified {count} club badge relationships.")
-        )
+        if badge.org is None:
+            return 0, 0
+
+        # add badge to parent child relationships
+        badge_add_count = 0
+        parent_child_add_count = 0
+        self._visited = set()
+        badge_add_count += self.recursively_add_badge(badge.org, badge)
 
         # if badge exist on child, link it to the parent directly
         # unless it is already indirectly linked
-        count = 0
+        for club in badge.club_set.all():
+            if club.pk == badge.org.pk:
+                continue
+
+            parent_club_codes = self.get_parent_club_codes(club)
+            if badge.org.code not in parent_club_codes:
+                if not self.dry_run:
+                    self.stdout.write(
+                        f"Adding {badge.org.name} as parent for {club.name}."
+                    )
+                    club.parent_orgs.add(badge.org)
+                else:
+                    self.stdout.write(
+                        f"Would have added {badge.org.name} "
+                        f"as a parent for {club.name}."
+                    )
+                parent_child_add_count += 1
+        return badge_add_count, parent_child_add_count
+
+    def sync_badges(self):
+        """
+        Synchronizes badges based on parent child relationships.
+        Uses parent child relationships as source of truth.
+        """
+        # remove badges with orgs from all clubs
+        for club in Club.objects.all():
+            club.badges = club.badges.filter(org__isnull=False)
+            club.save()
+
+        badge_add_count = 0
+        parent_child_add_count = 0
         for badge in Badge.objects.all():
-            if badge.org is not None:
-                for club in badge.club_set.all():
-                    if club.pk == badge.org.pk:
-                        continue
-
-                    parent_club_codes = self.get_parent_club_codes(club)
-                    if badge.org.code not in parent_club_codes:
-                        if not self.dry_run:
-                            self.stdout.write(
-                                f"Adding {badge.org.name} as parent for {club.name}."
-                            )
-                            club.parent_orgs.add(badge.org)
-                        else:
-                            self.stdout.write(
-                                f"Would have added {badge.org.name} "
-                                f"as a parent for {club.name}."
-                            )
-                        count += 1
+            b_count, pc_count = self.sync_badge(badge)
+            badge_add_count += b_count
+            parent_child_add_count += pc_count
+
         self.stdout.write(
-            self.style.SUCCESS(f"Modified {count} parent child relationships.")
+            self.style.SUCCESS(
+                f"Modified {badge_add_count} club badge relationships."
+                f"Modified {parent_child_add_count} parent child relationships."
+            )
         )

backend/clubs/models.py

@@ -1489,7 +1489,11 @@ class Badge(models.Model):
 
     # The organization that this badge represents (If this is the "SAC Funded" badge,
     # then this would link to SAC)
-    org = models.ForeignKey(Club, on_delete=models.CASCADE, blank=True, null=True)
+    # TODO: Editability can be loosened if correspondance betweent parent_orgs, badges
+    # of clubs with the badge is maintained when modifying this field
+    org = models.ForeignKey(
+        Club, on_delete=models.CASCADE, blank=True, null=True, editable=False
+    )
 
     # The fair that this badge is related to
     fair = models.ForeignKey(ClubFair, on_delete=models.CASCADE, blank=True, null=True)

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,14 @@ 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"
+            # admins, club members, and anyone with permission to edit the club
+            # can see its latest, potentially unapproved version
+            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

@@ -80,6 +80,7 @@
 from tatsu.exceptions import FailedParse
 
 from clubs.filters import RandomOrderingFilter, RandomPageNumberPagination
+from clubs.management.commands.sync import Command as SyncCommand
 from clubs.mixins import XLSXFormatterMixin
 from clubs.models import (
     AdminNote,
@@ -173,6 +174,7 @@
     EventSerializer,
     EventWriteSerializer,
     ExternalMemberListSerializer,
+    FakeView,
     FavoriteSerializer,
     FavoriteWriteSerializer,
     FavouriteEventSerializer,
@@ -4524,15 +4526,6 @@
         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.
@@ -7547,12 +7540,15 @@
         badge = get_object_or_404(Badge, pk=self.kwargs["badge_pk"])
         club = get_object_or_404(Club, code=request.data["club"])
         club.badges.add(badge)
+        SyncCommand().sync_badge(badge)
         return Response({"success": True})
 
     def destroy(self, request, *args, **kwargs):
         club = self.get_object()
         badge = get_object_or_404(Badge, pk=self.kwargs["badge_pk"])
         club.badges.remove(badge)
+        club.parent_orgs.remove(badge.org)
+        SyncCommand().sync_badge(badge)
         return Response({"success": True})
 
     def get_queryset(self):

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",
+        )
+
+        # some tests (e.g. directory) implicitly assume some constant number of clubs
+        self.NUM_CLUBS = 2
+
+        self.wc_badge = Badge.objects.create(
+            org=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,52 @@ def test_club_modify(self):
         self.assertEqual(data["description"], "We do stuff.")
         self.assertEqual(len(data["tags"]), 2)
 
+    def test_club_modify_child(self):
+        """
+        Officers of parent clubs should be able to modify child clubs
+        (e.g. clubs with the parent badge).
+        """
+        child_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-child club
+        resp = self.client.patch(
+            reverse("clubs-detail", args=(child_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 child club
+        self.client.login(username=self.user5.username, password="test")
+        resp = self.client.post(
+            reverse("badge-clubs-list", args=(self.wc_badge.id,)),
+            {"club": child_club.code},
+            content_type="application/json",
+        )
+        self.assertIn(resp.status_code, [200, 201], resp.content)
+        child_club.refresh_from_db()
+        self.assertEqual(child_club.badges.count(), 1)
+        self.assertEqual(child_club.badges.all()[0], self.wc_badge)
+        self.assertEqual(child_club.parent_orgs.count(), 1)
+        self.assertEqual(child_club.parent_orgs.all()[0], self.wc)
+
+        self.client.login(username=self.user4.username, password="test")
+        resp = self.client.patch(
+            reverse("clubs-detail", args=(child_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 +1675,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 +2075,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 +2084,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)