From 93d8a123d3c62ec08124fa6424729ee3009d788d Mon Sep 17 00:00:00 2001 From: AsadAzam Date: Wed, 9 Feb 2022 11:02:11 +0500 Subject: [PATCH] fix: remove visibility of anonymous posts for ta (#29883) --- .../discussion/django_comment_client/utils.py | 19 ++++++-- lms/djangoapps/discussion/tests/test_views.py | 43 +++++++++++++++++-- lms/djangoapps/discussion/views.py | 19 +++++--- 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/discussion/django_comment_client/utils.py b/lms/djangoapps/discussion/django_comment_client/utils.py index 151277edd8..c135ecee85 100644 --- a/lms/djangoapps/discussion/django_comment_client/utils.py +++ b/lms/djangoapps/discussion/django_comment_client/utils.py @@ -711,7 +711,10 @@ def add_courseware_context(content_list, course, user, id_map=None): content.update({"courseware_url": url, "courseware_title": title}) -def prepare_content(content, course_key, is_staff=False, discussion_division_enabled=None, group_names_by_id=None): +def prepare_content( + content, course_key, is_staff=False, is_community_ta=False, + discussion_division_enabled=None, group_names_by_id=None +): """ This function is used to pre-process thread and comment models in various ways before adding them to the HTTP response. This includes fixing empty @@ -725,6 +728,7 @@ def prepare_content(content, course_key, is_staff=False, discussion_division_ena content (dict): A thread or comment. course_key (CourseKey): The course key of the course. is_staff (bool): Whether the user is a staff member. + is_community_ta (bool): Whether the user is a TA (community or grpup community TA). discussion_division_enabled (bool): Whether division of course discussions is enabled. Note that callers of this method do not need to provide this value (it defaults to None)-- it is calculated and then passed to recursive calls of this method. @@ -738,11 +742,17 @@ def prepare_content(content, course_key, is_staff=False, discussion_division_ena 'read', 'group_id', 'group_name', 'pinned', 'abuse_flaggers', 'stats', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type', 'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total', - 'endorsement', 'context', 'last_activity_at' + 'endorsement', 'context', 'last_activity_at', 'username', 'user_id' ] - if (content.get('anonymous') is False) and ((content.get('anonymous_to_peers') is False) or is_staff): - fields += ['username', 'user_id'] + is_anonymous = content.get('anonymous') + is_anonymous_to_peers = content.get('anonymous_to_peers') + # is_staff is true for both staff and TAs, is_user_staff will be true for staff members only + is_user_staff = is_staff and not is_community_ta + + if is_anonymous or (is_anonymous_to_peers and not is_user_staff): + fields.remove('username') + fields.remove('user_id') content = strip_none(extract(content, fields)) @@ -782,6 +792,7 @@ def prepare_content(content, course_key, is_staff=False, discussion_division_ena child, course_key, is_staff, + is_community_ta, discussion_division_enabled=discussion_division_enabled, group_names_by_id=group_names_by_id ) diff --git a/lms/djangoapps/discussion/tests/test_views.py b/lms/djangoapps/discussion/tests/test_views.py index 4b4df1f810..0fccb9c63e 100644 --- a/lms/djangoapps/discussion/tests/test_views.py +++ b/lms/djangoapps/discussion/tests/test_views.py @@ -155,6 +155,8 @@ def make_mock_thread_data( # lint-amnesty, pylint: disable=missing-function-doc group_name=None, commentable_id=None, is_commentable_divided=None, + anonymous=False, + anonymous_to_peers=False, ): data_commentable_id = ( commentable_id or course.discussion_topics.get('General', {}).get('id') or "dummy_commentable_id" @@ -169,6 +171,8 @@ def make_mock_thread_data( # lint-amnesty, pylint: disable=missing-function-doc "resp_skip": 25, "resp_limit": 5, "group_id": group_id, + "anonymous": anonymous, + "anonymous_to_peers": anonymous_to_peers, "context": ( ThreadContext.COURSE if get_team(data_commentable_id) is None else ThreadContext.STANDALONE ) @@ -220,7 +224,9 @@ def make_mock_perform_request_impl( # lint-amnesty, pylint: disable=missing-fun group_id=None, commentable_id=None, num_thread_responses=1, - thread_list=None + thread_list=None, + anonymous=False, + anonymous_to_peers=False, ): def mock_perform_request_impl(*args, **kwargs): url = args[1] @@ -237,7 +243,9 @@ def make_mock_perform_request_impl( # lint-amnesty, pylint: disable=missing-fun thread_id=thread_id, num_children=num_thread_responses, group_id=group_id, - commentable_id=commentable_id + commentable_id=commentable_id, + anonymous=anonymous, + anonymous_to_peers=anonymous_to_peers, ) elif "/users/" in url: res = { @@ -266,6 +274,8 @@ def make_mock_request_impl( # lint-amnesty, pylint: disable=missing-function-do commentable_id=None, num_thread_responses=1, thread_list=None, + anonymous=False, + anonymous_to_peers=False, ): impl = make_mock_perform_request_impl( course, @@ -274,7 +284,9 @@ def make_mock_request_impl( # lint-amnesty, pylint: disable=missing-function-do group_id=group_id, commentable_id=commentable_id, num_thread_responses=num_thread_responses, - thread_list=thread_list + thread_list=thread_list, + anonymous=anonymous, + anonymous_to_peers=anonymous_to_peers, ) def mock_request_impl(*args, **kwargs): @@ -407,6 +419,31 @@ class SingleThreadTestCase(ForumsEnableMixin, ModuleStoreTestCase): # lint-amne ) assert response.status_code == 405 + def test_post_anonymous_to_ta(self, mock_request): + text = "dummy content" + thread_id = "test_thread_id" + mock_request.side_effect = make_mock_request_impl(course=self.course, text=text, thread_id=thread_id, + anonymous_to_peers=True) + + request = RequestFactory().get( + "dummy_url", + HTTP_X_REQUESTED_WITH="XMLHttpRequest" + ) + request.user = self.student + request.user.is_community_ta = True + response = views.single_thread( + request, + str(self.course.id), + "dummy_discussion_id", + "test_thread_id" + ) + + assert response.status_code == 200 + response_data = json.loads(response.content.decode('utf-8')) + # user is community ta, so response must not have username and user_id fields + assert response_data['content'].get('username') is None + assert response_data['content'].get('user_id') is None + def test_not_found(self, mock_request): request = RequestFactory().get("dummy_url") request.user = self.student diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index cc5f02265f..9f3c836de1 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -227,6 +227,7 @@ def inline_discussion(request, course_key, discussion_id): with function_trace('determine_group_permissions'): is_staff = has_permission(request.user, 'openclose_thread', course.id) + is_community_ta = utils.is_user_community_ta(request.user, course.id) course_discussion_settings = CourseDiscussionSettings.get(course.id) group_names_by_id = get_group_names_by_id(course_discussion_settings) course_is_divided = course_discussion_settings.division_scheme is not CourseDiscussionSettings.NONE @@ -237,6 +238,7 @@ def inline_discussion(request, course_key, discussion_id): thread, course_key, is_staff, + is_community_ta, course_is_divided, group_names_by_id ) for thread in threads @@ -271,7 +273,9 @@ def forum_form_discussion(request, course_key): try: unsafethreads, query_params = get_threads(request, course, user_info) # This might process a search query is_staff = has_permission(request.user, 'openclose_thread', course.id) - threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads] + threads = [utils.prepare_content( + thread, course_key, is_staff, request.user.is_community_ta + ) for thread in unsafethreads] except cc.utils.CommentClientMaintenanceError: return HttpResponseServerError('Forum is in maintenance mode', status=status.HTTP_503_SERVICE_UNAVAILABLE) except ValueError: @@ -336,7 +340,7 @@ def single_thread(request, course_key, discussion_id, thread_id): user_info=user_info ) - content = utils.prepare_content(thread.to_dict(), course_key, is_staff) + content = utils.prepare_content(thread.to_dict(), course_key, is_staff, request.user.is_community_ta) with function_trace("add_courseware_context"): add_courseware_context([content], course, request.user) @@ -482,7 +486,8 @@ def _create_discussion_board_context(request, base_context, thread=None): thread_pages = query_params['num_pages'] root_url = request.path is_staff = has_permission(user, 'openclose_thread', course.id) - threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] + is_community_ta = utils.is_user_community_ta(request.user, course.id) + threads = [utils.prepare_content(thread, course_key, is_staff, is_community_ta) for thread in threads] with function_trace("get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, user, user_info) @@ -552,7 +557,8 @@ def create_user_profile_context(request, course_key, user_id): annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info) is_staff = has_permission(request.user, 'openclose_thread', course.id) - threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads] + is_community_ta = utils.is_user_community_ta(request.user, course.id) + threads = [utils.prepare_content(thread, course_key, is_staff, is_community_ta) for thread in threads] with function_trace("add_courseware_context"): add_courseware_context(threads, course, request.user) @@ -668,10 +674,13 @@ def followed_threads(request, course_key, user_id): ) if request.is_ajax(): is_staff = has_permission(request.user, 'openclose_thread', course.id) + is_community_ta = utils.is_user_community_ta(request.user, course.id) return utils.JsonResponse({ 'annotated_content_info': annotated_content_info, 'discussion_data': [ - utils.prepare_content(thread, course_key, is_staff) for thread in paginated_results.collection + utils.prepare_content( + thread, course_key, is_staff, is_community_ta + ) for thread in paginated_results.collection ], 'page': query_params['page'], 'num_pages': query_params['num_pages'],