From 2f036e2df6b4ed2af9fd7b014022b206e7a0bb84 Mon Sep 17 00:00:00 2001 From: Muhammad Adeel Tajamul <77053848+muhammadadeeltajamul@users.noreply.github.com> Date: Wed, 19 Apr 2023 19:55:10 +0500 Subject: [PATCH] feat: added edit_by_label and closed_by_label in threads response (#32070) --- .../discussion/rest_api/serializers.py | 38 +++++- .../discussion/rest_api/tests/test_api.py | 5 + .../rest_api/tests/test_serializers.py | 115 ++++++++++++++++++ .../discussion/rest_api/tests/test_views.py | 6 + .../discussion/rest_api/tests/utils.py | 2 + 5 files changed, 164 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index f90a137d76..015ed030ed 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -6,7 +6,7 @@ from urllib.parse import urlencode, urlunparse from django.conf import settings from django.contrib.auth import get_user_model -from django.core.exceptions import ValidationError +from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.models import TextChoices from django.urls import reverse from django.utils.html import strip_tags @@ -83,6 +83,7 @@ def get_context(course, request, thread=None): "ta_user_ids": ta_user_ids, "cc_requester": cc_requester, "has_moderation_privilege": has_moderation_privilege, + "is_global_staff": is_global_staff, } @@ -155,6 +156,7 @@ class _ContentSerializer(serializers.Serializer): anonymous_to_peers = serializers.BooleanField(default=False) last_edit = serializers.SerializerMethodField(required=False) edit_reason_code = serializers.CharField(required=False, validators=[validate_edit_reason_code]) + edit_by_label = serializers.SerializerMethodField(required=False) non_updatable_fields = set() @@ -204,13 +206,25 @@ class _ContentSerializer(serializers.Serializer): """ is_staff = user_id in self.context["course_staff_user_ids"] or user_id in self.context["moderator_user_ids"] is_ta = user_id in self.context["ta_user_ids"] + is_global_staff = self.context["is_global_staff"] return ( - "Staff" if is_staff else + "Staff" if (is_staff or is_global_staff) else "Community TA" if is_ta else None ) + def _get_user_label_from_username(self, username): + """ + Returns role label of user from username + Possible Role Labels: Staff, Community TA or None + """ + try: + user = User.objects.get(username=username) + return self._get_user_label(user.id) + except ObjectDoesNotExist: + return None + def get_author_label(self, obj): """ Returns the role label for the content author. @@ -282,6 +296,17 @@ class _ContentSerializer(serializers.Serializer): last_edit["reason"] = EDIT_REASON_CODES.get(reason_code) return last_edit + def get_edit_by_label(self, obj): + """ + Returns the role label for the last edit user. + """ + is_user_author = str(obj['user_id']) == str(self.context['request'].user.id) + is_user_privileged = _validate_privileged_access(self.context) + edit_history = obj.get("edit_history") + if (is_user_author or is_user_privileged) and edit_history: + last_edit = edit_history[-1] + return self._get_user_label_from_username(last_edit.get('editor_username')) + class ThreadSerializer(_ContentSerializer): """ @@ -316,6 +341,7 @@ class ThreadSerializer(_ContentSerializer): close_reason_code = serializers.CharField(required=False, validators=[validate_close_reason_code]) close_reason = serializers.SerializerMethodField() closed_by = serializers.SerializerMethodField() + closed_by_label = serializers.SerializerMethodField(required=False) non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS @@ -426,6 +452,14 @@ class ThreadSerializer(_ContentSerializer): if _validate_privileged_access(self.context) or is_user_author: return obj.get("closed_by") + def get_closed_by_label(self, obj): + """ + Returns the role label for the user who closed the post. + """ + is_user_author = str(obj['user_id']) == str(self.context['request'].user.id) + if is_user_author or _validate_privileged_access(self.context): + return self._get_user_label_from_username(obj.get("closed_by")) + def create(self, validated_data): thread = Thread(user_id=self.context["cc_requester"]["id"], **validated_data) thread.save() diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index d9188463fe..c8ab575df4 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -1472,6 +1472,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, }, { "id": "test_comment_2", @@ -1498,6 +1499,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu "anonymous": True, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, }, ] actual_comments = self.get_comment_list( @@ -2232,6 +2234,7 @@ class CreateCommentTest( "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } assert actual == expected expected_url = ( @@ -2328,6 +2331,7 @@ class CreateCommentTest( "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } assert actual == expected expected_url = ( @@ -3154,6 +3158,7 @@ class UpdateCommentTest( "child_count": 0, "can_delete": True, "last_edit": None, + "edit_by_label": None, } assert actual == expected assert parsed_body(httpretty.last_request()) == { diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index 6e7f02b553..570c7852e3 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -188,6 +188,8 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "pinned": True, "editable_fields": ["abuse_flagged", "copy_link", "following", "read", "voted"], "abuse_flagged_count": None, + "edit_by_label": None, + "closed_by_label": None, }) assert self.serialize(thread) == expected @@ -242,6 +244,118 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe serialized = self.serialize(thread_data) assert 'response_count' not in serialized + @ddt.data( + (FORUM_ROLE_MODERATOR, True), + (FORUM_ROLE_STUDENT, False), + ("author", True), + ) + @ddt.unpack + def test_closed_by_label_field(self, role, visible): + """ + Tests if closed by field is visible to author and priviledged users + """ + moderator = UserFactory() + request_role = FORUM_ROLE_STUDENT if role == "author" else role + author = self.user if role == "author" else self.author + self.create_role(FORUM_ROLE_MODERATOR, [moderator]) + self.create_role(request_role, [self.user]) + + thread = make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(author.id), + "username": author.username, + "title": "Test Title", + "body": "Test body", + "pinned": True, + "votes": {"up_count": 4}, + "comments_count": 5, + "unread_comments_count": 3, + "closed_by": moderator + }) + closed_by_label = "Staff" if visible else None + closed_by = moderator if visible else None + can_delete = role != FORUM_ROLE_STUDENT + editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"] + if role == "author": + editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type']) + elif role == FORUM_ROLE_MODERATOR: + editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned', + 'raw_body', 'title', 'topic_id', 'type']) + expected = self.expected_thread_data({ + "author": author.username, + "can_delete": can_delete, + "vote_count": 4, + "comment_count": 6, + "unread_comment_count": 3, + "pinned": True, + "editable_fields": sorted(editable_fields), + "abuse_flagged_count": None, + "edit_by_label": None, + "closed_by_label": closed_by_label, + "closed_by": closed_by, + }) + assert self.serialize(thread) == expected + + @ddt.data( + (FORUM_ROLE_MODERATOR, True), + (FORUM_ROLE_STUDENT, False), + ("author", True), + ) + @ddt.unpack + def test_edit_by_label_field(self, role, visible): + """ + Tests if closed by field is visible to author and priviledged users + """ + moderator = UserFactory() + request_role = FORUM_ROLE_STUDENT if role == "author" else role + author = self.user if role == "author" else self.author + self.create_role(FORUM_ROLE_MODERATOR, [moderator]) + self.create_role(request_role, [self.user]) + + thread = make_minimal_cs_thread({ + "id": "test_thread", + "course_id": str(self.course.id), + "commentable_id": "test_topic", + "user_id": str(author.id), + "username": author.username, + "title": "Test Title", + "body": "Test body", + "pinned": True, + "votes": {"up_count": 4}, + "edit_history": [{"editor_username": moderator}], + "comments_count": 5, + "unread_comments_count": 3, + "closed_by": None + }) + edit_by_label = "Staff" if visible else None + can_delete = role != FORUM_ROLE_STUDENT + last_edit = None if role == FORUM_ROLE_STUDENT else {"editor_username": moderator} + editable_fields = ["abuse_flagged", "copy_link", "following", "read", "voted"] + + if role == "author": + editable_fields.extend(['anonymous', 'raw_body', 'title', 'topic_id', 'type']) + elif role == FORUM_ROLE_MODERATOR: + editable_fields.extend(['close_reason_code', 'closed', 'edit_reason_code', 'pinned', + 'raw_body', 'title', 'topic_id', 'type']) + + expected = self.expected_thread_data({ + "author": author.username, + "can_delete": can_delete, + "vote_count": 4, + "comment_count": 6, + "unread_comment_count": 3, + "pinned": True, + "editable_fields": sorted(editable_fields), + "abuse_flagged_count": None, + "last_edit": last_edit, + "edit_by_label": edit_by_label, + "closed_by_label": None, + "closed_by": None, + }) + assert self.serialize(thread) == expected + @ddt.ddt class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase): @@ -318,6 +432,7 @@ class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase): "child_count": 0, "can_delete": False, "last_edit": None, + "edit_by_label": None, } assert self.serialize(comment) == expected diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 559b4ff8f6..56e690761d 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -1709,6 +1709,8 @@ class LearnerThreadViewAPITest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "votes": {"up_count": 4}, "comments_count": 5, "unread_comments_count": 3, + "closed_by_label": None, + "edit_by_label": None, })], "page": 1, "num_pages": 1, @@ -1968,6 +1970,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response_data.update(overrides or {}) return response_data @@ -2392,6 +2395,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response = self.client.post( self.url, @@ -2483,6 +2487,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response_data.update(overrides or {}) return response_data @@ -2672,6 +2677,7 @@ class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase "anonymous": False, "anonymous_to_peers": False, "last_edit": None, + "edit_by_label": None, } response = self.client.get(self.url) diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index e4b5285324..fd456674c3 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -499,7 +499,9 @@ class CommentsServiceMockMixin: "type": "discussion", "response_count": 0, "last_edit": None, + "edit_by_label": None, "closed_by": None, + "closed_by_label": None, "close_reason": None, "close_reason_code": None, }