From 0508c8b9efb6865acfe2116af26dcc28c7efdb2c Mon Sep 17 00:00:00 2001 From: Thomas Ngulube <47449914+Porcupine1@users.noreply.github.com> Date: Tue, 24 Dec 2024 01:37:24 -0500 Subject: [PATCH] added ClubDiffSerializer --- backend/clubs/serializers.py | 29 ++++++ backend/clubs/views.py | 156 ++++++++++++------------------ backend/tests/clubs/test_views.py | 55 ++++++----- 3 files changed, 122 insertions(+), 118 deletions(-) diff --git a/backend/clubs/serializers.py b/backend/clubs/serializers.py index 8f7a23426..0610b37e6 100644 --- a/backend/clubs/serializers.py +++ b/backend/clubs/serializers.py @@ -877,6 +877,31 @@ class Meta(ClubMinimalSerializer.Meta): fields = ClubMinimalSerializer.Meta.fields + ["files"] +class ClubDiffSerializer(serializers.ModelSerializer): + diff = serializers.SerializerMethodField() + + class Meta: + model = Club + fields = ["code"] + + def to_representation(self, instance): + fields = ["name", "description", "image"] + diff = {} + is_same = True + for field in fields: + diff[field] = { + "old": getattr(instance, f"latest_approved_{field}", None), + "new": getattr(instance, f"latest_{field}", None), + } + if diff[field]["old"] != diff[field]["new"]: + is_same = False + + if is_same: + return {instance.code: "No changes made since last approval"} + + return {instance.code: diff} + + class ClubListSerializer(serializers.ModelSerializer): """ The club list serializer returns a subset of the information that the full @@ -1070,6 +1095,10 @@ class Meta: } +class ClubHistorySerializer(serializers.ModelSerializer): + diff = serializers.SerializerMethodField("get_diff") + + class MembershipClubListSerializer(ClubListSerializer): """ The club list serializer, except return more detailed information about the diff --git a/backend/clubs/views.py b/backend/clubs/views.py index 4ee45c0f0..84506f528 100644 --- a/backend/clubs/views.py +++ b/backend/clubs/views.py @@ -162,6 +162,7 @@ ClubApplicationSerializer, ClubBoothSerializer, ClubConstitutionSerializer, + ClubDiffSerializer, ClubFairSerializer, ClubListSerializer, ClubMembershipSerializer, @@ -2060,6 +2061,38 @@ def perform_destroy(self, instance): context=context, ) + def _get_club_diff_queryset(self): + """ + Returns a queryset of clubs annotated with the latest and latest approved values + for specific fields (name, description, image) from their historical records. + + The annotations include: + - `latest_`: The most recent value of the field. + - `latest_approved_`: The most recent approved value of the field. + """ + latest_version_qs = Club.history.filter(code=OuterRef("code")).order_by( + "-history_date" + ) + + latest_approved_version_qs = Club.history.filter( + code=OuterRef("code"), approved=True + ).order_by("-history_date") + + return Club.objects.annotate( + latest_name=Subquery(latest_version_qs.values("name")[:1]), + latest_description=Subquery(latest_version_qs.values("description")[:1]), + latest_image=Subquery(latest_version_qs.values("image")[:1]), + latest_approved_name=Subquery( + latest_approved_version_qs.values("name")[:1] + ), + latest_approved_description=Subquery( + latest_approved_version_qs.values("description")[:1] + ), + latest_approved_image=Subquery( + latest_approved_version_qs.values("image")[:1] + ), + ) + @action(detail=True, methods=["GET"]) def club_detail_diff(self, request, *args, **kwargs): """ @@ -2118,144 +2151,77 @@ def club_detail_diff(self, request, *args, **kwargs): """ club = self.get_object() - latest_approved_version = ( - club.history.filter(approved=True) - .only("name", "description", "image") - .order_by("-history_date") - .first() - ) - - latest_version = ( - club.history.only("name", "description", "image") - .order_by("-history_date") - .first() - ) - - # 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) - 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({}) + club = self._get_club_diff_queryset().get(pk=club.pk) + serializer = ClubDiffSerializer(club) + return Response(serializer.data) @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 - properties: - club_code: + type: array + items: + type: object + additionalProperties: type: object - description: club code + description: Diff of the club fields. properties: name: type: object - description: Changes in the name field + description: Changes in the name field. properties: old: type: string - description: > - Old name of the club + nullable: true + description: Old name of the club. new: type: string - description: > - New name of the club + nullable: true + description: New name of the club. description: type: object description: > - Changes in the club description + Changes in the description field. properties: old: type: string + nullable: true description: > - Old description of the club + Old description of the club. new: type: string + nullable: true description: > - New description of the club + New description of the club. image: type: object - description: > - Changes in the image of the club + description: Changes in the image field. properties: old: type: string + nullable: true description: > - Old image URL of the club + Old image URL of the club. new: type: string + nullable: true description: > - New image URL of the club + 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 - } + pending_clubs = self._get_club_diff_queryset().filter( + approved=None, active=True ) + serializer = ClubDiffSerializer(pending_clubs, many=True) + return Response(serializer.data) @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 a82a2ae77..e829f898a 100644 --- a/backend/tests/clubs/test_views.py +++ b/backend/tests/clubs/test_views.py @@ -1399,7 +1399,10 @@ def test_club_detail_diff(self): reverse("clubs-club-detail-diff", args=(new_club.code,)) ) new_data1 = json.loads(resp3.content.decode("utf-8")) - self.assertEquals(new_data1, {}) + self.assertEqual( + new_data1["new-club"], + "No changes made since last approval", + ) def test_club_list_diff(self): """ @@ -1449,23 +1452,24 @@ def test_club_list_diff(self): # 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) + self.assertEqual(data[0]["club1"]["name"]["new"], "Club 1") + self.assertEqual(data[0]["club1"]["name"]["old"], None) + self.assertEqual( + data[0]["club1"]["description"]["new"], "This is the first club." + ) + self.assertEqual(data[0]["club1"]["description"]["old"], None) # Check club2 - self.assertEqual(data["club2"]["name"]["new"], "Club 2") - self.assertEqual(data["club2"]["name"]["old"], None) + self.assertEqual(data[1]["club2"]["name"]["new"], "Club 2") + self.assertEqual(data[1]["club2"]["name"]["old"], None) self.assertEqual( - data["club2"]["description"]["new"], "This is the second club." + data[1]["club2"]["description"]["new"], "This is the second club." ) - self.assertEqual(data["club2"]["description"]["old"], None) + self.assertEqual(data[1]["club2"]["description"]["old"], None) # club 3 should not be included, since currently approved - self.assertNotIn("club3", data) + self.assertNotEqual(3, len(data)) # Approve club1 club1.approved = True @@ -1489,23 +1493,28 @@ def test_club_list_diff(self): 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." + # club 1 should have old and new data + self.assertEqual( + new_data[0]["club1"]["description"]["new"], "updated description." + ) + self.assertEqual( + new_data[0]["club1"]["description"]["old"], "This is the first club." ) # 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) + self.assertEqual(new_data[0]["club1"]["name"]["new"], "Club 1") + self.assertEqual(data[0]["club1"]["name"]["old"], None) + self.assertEqual( + data[0]["club1"]["description"]["new"], "This is the first club." + ) + self.assertEqual(data[0]["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) + self.assertEqual(new_data[1]["club2"]["name"]["new"], "Club 2") + self.assertEqual(new_data[1]["club2"]["name"]["old"], None) # Club3 should still not be included - self.assertNotIn("club3", new_data) + self.assertNotEqual(3, len(new_data)) # Approve club1 to remove from pending list club1.approved = True @@ -1514,8 +1523,8 @@ def test_club_list_diff(self): 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) + self.assertEqual(1, len(new_data2)) + self.assertIn("club2", new_data2[0]) def test_club_modify_wrong_auth(self): """