Skip to content

Commit

Permalink
added ClubDiffSerializer
Browse files Browse the repository at this point in the history
  • Loading branch information
@Porcupine1
Porcupine1 committed Dec 24, 2024
1 parent 1724ec7 commit 0508c8b
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 118 deletions.
29 changes: 29 additions & 0 deletions backend/clubs/serializers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
156 changes: 61 additions & 95 deletions backend/clubs/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@
ClubApplicationSerializer,
ClubBoothSerializer,
ClubConstitutionSerializer,
ClubDiffSerializer,
ClubFairSerializer,
ClubListSerializer,
ClubMembershipSerializer,
Expand Down Expand Up @@ -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_<field>`: The most recent value of the field.
- `latest_approved_<field>`: 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):
"""
Expand Down Expand Up @@ -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):
Expand Down
55 changes: 32 additions & 23 deletions backend/tests/clubs/test_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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):
"""
Expand Down

0 comments on commit 0508c8b

Please sign in to comment.