From 2ec546e2f9dc5b422337d262255749168b0ec886 Mon Sep 17 00:00:00 2001 From: Thomas Ngulube <47449914+Porcupine1@users.noreply.github.com> Date: Sun, 13 Oct 2024 05:16:10 -0400 Subject: [PATCH] add detail and list diff --- backend/clubs/views.py | 145 +++++++++++++++++++++++--- backend/tests/clubs/test_views.py | 168 ++++++++++++++++++++++++++++-- 2 files changed, 289 insertions(+), 24 deletions(-) diff --git a/backend/clubs/views.py b/backend/clubs/views.py index ea38bd441..51d6bfa52 100644 --- a/backend/clubs/views.py +++ b/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,28 +2119,30 @@ 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"] @@ -2146,6 +2150,115 @@ def diff(self, request, *args, **kwargs): } ) + # 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): """ diff --git a/backend/tests/clubs/test_views.py b/backend/tests/clubs/test_views.py index 5f9a7230a..a82a2ae77 100644 --- a/backend/tests/clubs/test_views.py +++ b/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): """