diff --git a/backend/clubs/views.py b/backend/clubs/views.py index 7d1b53afd..4ee45c0f0 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, @@ -2058,6 +2060,203 @@ def perform_destroy(self, instance): context=context, ) + @action(detail=True, methods=["GET"]) + def club_detail_diff(self, request, *args, **kwargs): + """ + Return old and new data for a club that is pending approval. + --- + responses: + "200": + content: + application/json: + schema: + type: object + properties: + club_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 + --- + """ + 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({}) + + @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: 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 0750c611b..a82a2ae77 100644 --- a/backend/tests/clubs/test_views.py +++ b/backend/tests/clubs/test_views.py @@ -1306,6 +1306,217 @@ 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_club_detail_diff(self): + """ + Test that diff returns the correct old and new club for a club in approval queue + """ + + # create club that requires approval + new_club = Club.objects.create( + code="new-club", + name="New Club", + description="We're the spirits of open source.", + approved=None, + active=True, + email="example@example.com", + ) + + # add officer to club + Membership.objects.create( + person=self.user4, club=new_club, role=Membership.ROLE_OFFICER + ) + + # login to officer user + self.client.login(username=self.user4.username, password="test") + + # New club should not have any old data + 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) + + # approve club + new_club.approved = True + new_club.save(update_fields=["approved"]) + self.assertTrue(new_club.approved) + + # 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(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 + 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"], + "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): """ Outsiders should not be able to modify a club.