add detail and list diff

backend/clubs/views.py

@@ -47,8 +47,10 @@
     ExpressionWrapper,
     F,
     Max,
+    OuterRef,
     Prefetch,
     Q,
+    Subquery,
     TextField,
     Value,
     When,
@@ -2059,7 +2061,7 @@ def perform_destroy(self, instance):
         )
 
     @action(detail=True, methods=["GET"])
-    def diff(self, request, *args, **kwargs):
+    def club_detail_diff(self, request, *args, **kwargs):
         """
         Return old and new data for a club that is pending approval.
         ---
@@ -2117,35 +2119,146 @@ def diff(self, request, *args, **kwargs):
         club = self.get_object()
 
         latest_approved_version = (
-            club.history.filter(approved=True).order_by("-history_date").first()
+            club.history.filter(approved=True)
+            .only("name", "description", "image")
+            .order_by("-history_date")
+            .first()
         )
-        latest_version = club.history.order_by("-history_date").first()
 
-        # if this is the first time the club is being approved
-        if not latest_approved_version:
-            return Response(
-                {
-                    club.code: {
-                        field: {"old": None, "new": getattr(latest_version, field)}
-                        for field in ["name", "description", "image"]
-                    }
-                }
-            )
+        latest_version = (
+            club.history.only("name", "description", "image")
+            .order_by("-history_date")
+            .first()
+        )
 
-        # if the current version is not approvedthe and it has been approved before
-        if not club.approved and latest_approved_version:
+        # if the current version is not approved, return a diff
+        # if the club has never be approved, for each field, it's old data is None
+        if not club.approved:
             return Response(
                 {
                     club.code: {
                         field: {
-                            "old": getattr(latest_approved_version, field),
+                            "old": (
+                                getattr(latest_approved_version, field)
+                                if latest_approved_version
+                                else None
+                            ),
                             "new": getattr(latest_version, field),
                         }
                         for field in ["name", "description", "image"]
                     }
                 }
             )
 
+        # if the current version is approved, no diff is returned
+        return Response({})
+
+    @action(detail=False, methods=["GET"])
+    def club_list_diff(self, request, *args, **kwargs):
+        """
+        Return old and new data for clubs that are pending approval.
+        ---
+        responses:
+            "200":
+                content:
+                    application/json:
+                        schema:
+                            type: object
+                            items:
+                                type: object
+                                properties:
+                                    code:
+                                        type: object
+                                        description: club code
+                                        properties:
+                                            name:
+                                                type: object
+                                                description: Changes in the name field
+                                                properties:
+                                                    old:
+                                                        type: string
+                                                        description: >
+                                                            Old name of the club
+                                                    new:
+                                                        type: string
+                                                        description: >
+                                                            New name of the club
+                                            description:
+                                                type: object
+                                                description: >
+                                                    Changes in the club description
+                                                properties:
+                                                    old:
+                                                        type: string
+                                                        description: >
+                                                            Old description of the club
+                                                    new:
+                                                        type: string
+                                                        description: >
+                                                            New description of the club
+                                            image:
+                                                type: object
+                                                description: >
+                                                    Changes in the image of the club
+                                                properties:
+                                                    old:
+                                                        type: string
+                                                        description: >
+                                                            Old image URL of the club
+                                                    new:
+                                                        type: string
+                                                        description: >
+                                                            New image URL of the club
+        ---
+        """
+        pending_clubs = Club.objects.filter(approved=None, active=True).only(
+            "code", "name", "description", "image"
+        )
+
+        # write subqueries
+        latest_versions_subquery = (
+            Club.history.filter(code=OuterRef("code"))
+            .order_by("-history_date")
+            .values("code")[:1]
+        )
+        latest_approved_versions_subquery = (
+            Club.history.filter(code=OuterRef("code"), approved=True)
+            .order_by("-history_date")
+            .values("code")[:1]
+        )
+
+        # get the latest versions of each club
+        latest_versions = Club.history.filter(
+            code__in=Subquery(latest_versions_subquery)
+        ).only("code", "name", "description", "image")
+
+        # get the latest approved versions of each club
+        latest_approved_versions = Club.history.filter(
+            code__in=Subquery(latest_approved_versions_subquery), approved=True
+        ).only("code", "name", "description", "image")
+
+        latest_versions_dict = {version.code: version for version in latest_versions}
+        latest_approved_versions_dict = {
+            version.code: version for version in latest_approved_versions
+        }
+
+        return Response(
+            {
+                club.code: {
+                    field: {
+                        "old": (
+                            getattr(latest_approved_versions_dict[club.code], field)
+                            if club.code in latest_approved_versions_dict
+                            else None
+                        ),
+                        "new": getattr(latest_versions_dict[club.code], field),
+                    }
+                    for field in ["name", "description", "image"]
+                }
+                for club in pending_clubs
+            }
+        )
+
     @action(detail=False, methods=["GET"])
     def fields(self, request, *args, **kwargs):
         """

backend/tests/clubs/test_views.py

@@ -1306,7 +1306,7 @@ def test_club_list_filter(self):
             codes = [club["code"] for club in data]
             self.assertEqual(set(codes), set(query["results"]), (query, resp.content))
 
-    def test_diff(self):
+    def test_club_detail_diff(self):
         """
         Test that diff returns the correct old and new club for a club in approval queue
         """
@@ -1330,7 +1330,7 @@ def test_diff(self):
         self.client.login(username=self.user4.username, password="test")
 
         # New club should not have any old data
-        resp = self.client.get(reverse("clubs-diff", args=(new_club.code,)))
+        resp = self.client.get(reverse("clubs-club-detail-diff", args=(new_club.code,)))
         data = json.loads(resp.content.decode("utf-8"))
         self.assertEqual(data["new-club"]["name"]["new"], "New Club")
         self.assertEqual(data["new-club"]["name"]["old"], None)
@@ -1340,30 +1340,182 @@ def test_diff(self):
         new_club.save(update_fields=["approved"])
         self.assertTrue(new_club.approved)
 
-        # Make a change that edit description (requires reapproval)
-        resp = self.client.patch(
+        # make multiple changes to club
+        resp1 = self.client.patch(
             reverse("clubs-detail", args=(new_club.code,)),
             {"description": "We are open source, expect us."},
             content_type="application/json",
         )
         # Ensure change successful
-        self.assertIn(resp.status_code, [200, 201], resp.content)
+        self.assertIn(resp1.status_code, [200, 201], resp1.content)
+
+        resp1 = self.client.patch(
+            reverse("clubs-detail", args=(new_club.code,)),
+            {"name": "New Club 2"},
+            content_type="application/json",
+        )
+        # Ensure change successful
+        self.assertIn(resp1.status_code, [200, 201], resp1.content)
+
+        resp1 = self.client.patch(
+            reverse("clubs-detail", args=(new_club.code,)),
+            {"description": "Expect us. Expect us."},
+            content_type="application/json",
+        )
+        # Ensure change successful
+        self.assertIn(resp1.status_code, [200, 201], resp1.content)
 
         # Ensure club is marked as not approved
         new_club.refresh_from_db()
         self.assertFalse(new_club.approved)
 
         # Should now have old and new data
-        resp = self.client.get(reverse("clubs-diff", args=(new_club.code,)))
-        new_data = json.loads(resp.content.decode("utf-8"))
+        resp2 = self.client.get(
+            reverse("clubs-club-detail-diff", args=(new_club.code,))
+        )
+        new_data = json.loads(resp2.content.decode("utf-8"))
         self.assertEqual(
             new_data["new-club"]["description"]["new"],
-            "We are open source, expect us.",
+            "Expect us. Expect us.",
         )
         self.assertEqual(
             new_data["new-club"]["description"]["old"],
             "We're the spirits of open source.",
         )
+        self.assertEqual(
+            new_data["new-club"]["name"]["new"],
+            "New Club 2",
+        )
+        self.assertEqual(
+            new_data["new-club"]["name"]["old"],
+            "New Club",
+        )
+
+        # attempt to get of approved club
+        new_club.approved = True
+        new_club.save(update_fields=["approved"])
+        self.assertTrue(new_club.approved)
+        resp3 = self.client.get(
+            reverse("clubs-club-detail-diff", args=(new_club.code,))
+        )
+        new_data1 = json.loads(resp3.content.decode("utf-8"))
+        self.assertEquals(new_data1, {})
+
+    def test_club_list_diff(self):
+        """
+        Test that diff returns correct old and new data for all clubs pending approval.
+        """
+
+        # Create first club that requires approval
+        club1 = Club.objects.create(
+            code="club1",
+            name="Club 1",
+            description="This is the first club.",
+            approved=None,
+            active=True,
+        )
+
+        # Create second club that requires approval
+        club2 = Club.objects.create(
+            code="club2",
+            name="Club 2",
+            description="This is the second club.",
+            approved=None,
+            active=True,
+        )
+
+        # Create third club that is already approved
+        Club.objects.create(
+            code="club3",
+            name="Club 3",
+            description="This is the third club.",
+            approved=True,
+            active=True,
+        )
+
+        # Add officer to club 1
+        Membership.objects.create(
+            person=self.user1, club=club1, role=Membership.ROLE_OFFICER
+        )
+
+        # Add officer to club 1
+        Membership.objects.create(
+            person=self.user1, club=club2, role=Membership.ROLE_OFFICER
+        )
+
+        # officer login
+        self.client.login(username=self.user1.username, password="test")
+
+        # Check that new clubs 1 and 2 have no old data
+        resp = self.client.get(reverse("clubs-club-list-diff"))
+        data = json.loads(resp.content.decode("utf-8"))
+
+        # Check club1
+        self.assertEqual(data["club1"]["name"]["new"], "Club 1")
+        self.assertEqual(data["club1"]["name"]["old"], None)
+        self.assertEqual(data["club1"]["description"]["new"], "This is the first club.")
+        self.assertEqual(data["club1"]["description"]["old"], None)
+
+        # Check club2
+        self.assertEqual(data["club2"]["name"]["new"], "Club 2")
+        self.assertEqual(data["club2"]["name"]["old"], None)
+        self.assertEqual(
+            data["club2"]["description"]["new"], "This is the second club."
+        )
+        self.assertEqual(data["club2"]["description"]["old"], None)
+
+        # club 3 should not be included, since currently approved
+        self.assertNotIn("club3", data)
+
+        # Approve club1
+        club1.approved = True
+        club1.save(update_fields=["approved"])
+        self.assertTrue(club1.approved)
+
+        # Make changes to club1
+        resp1 = self.client.patch(
+            reverse("clubs-detail", args=(club1.code,)),
+            {"description": "updated description."},
+            content_type="application/json",
+        )
+        # Ensure change successful
+        self.assertIn(resp1.status_code, [200, 201], resp1.content)
+
+        # Club1 should now require reapproval and diff should have old and new data
+        club1.refresh_from_db()
+        self.assertFalse(club1.approved)
+
+        # Check diff again
+        resp2 = self.client.get(reverse("clubs-club-list-diff"))
+        new_data = json.loads(resp2.content.decode("utf-8"))
+
+        # should not be equal since hasn't been approved
+        self.assertNotEqual(
+            new_data["club1"]["description"]["new"], "updated description."
+        )
+
+        # should has same data as before
+        self.assertEqual(data["club1"]["name"]["new"], "Club 1")
+        self.assertEqual(data["club1"]["name"]["old"], None)
+        self.assertEqual(data["club1"]["description"]["new"], "This is the first club.")
+        self.assertEqual(data["club1"]["description"]["old"], None)
+
+        # Check that club2 remains in the pending list with no changes
+        self.assertEqual(new_data["club2"]["name"]["new"], "Club 2")
+        self.assertEqual(new_data["club2"]["name"]["old"], None)
+
+        # Club3 should still not be included
+        self.assertNotIn("club3", new_data)
+
+        # Approve club1 to remove from pending list
+        club1.approved = True
+        club1.save(update_fields=["approved"])
+        resp3 = self.client.get(reverse("clubs-club-list-diff"))
+        new_data2 = json.loads(resp3.content.decode("utf-8"))
+
+        # Check that club1 is no longer in the pending list
+        self.assertNotIn("club1", new_data2)
+        self.assertIn("club2", new_data2)
 
     def test_club_modify_wrong_auth(self):
         """