From 17df3d4e17243644674b772421292f9676cd3f8f Mon Sep 17 00:00:00 2001 From: SaadYousaf Date: Thu, 27 Oct 2022 17:00:37 +0500 Subject: [PATCH] feat: add discussion reported and unreported events --- .../django_comment_client/base/views.py | 86 ++++++++++++++++++ lms/djangoapps/discussion/rest_api/api.py | 15 ++-- .../discussion/rest_api/tests/test_api.py | 87 ++++++++++++++++++- lms/djangoapps/discussion/views.py | 21 +---- 4 files changed, 182 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index c842b22714..6072ebd902 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -263,6 +263,92 @@ def track_comment_deleted_event(request, course, comment): track_forum_event(request, event_name, course, comment, event_data) +def track_thread_reported_event(request, course, thread): + """ + Send analytics event for a reported thread. + """ + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='reported') + event_data = { + 'body': thread.body[:TRACKING_MAX_FORUM_BODY], + 'content_type': 'Post', + 'commentable_id': thread.get('commentable_id', ''), + } + if hasattr(thread, 'username'): + event_data['target_username'] = thread.get('username', '') + add_truncated_title_to_event_data(event_data, thread.get('title', '')) + track_forum_event(request, event_name, course, thread, event_data) + + +def track_comment_reported_event(request, course, comment): + """ + Send analytics event for a reported response or comment. + """ + obj_type = 'comment' if comment.get('parent_id') else 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='reported') + event_data = { + 'body': comment.body[:TRACKING_MAX_FORUM_BODY], + 'commentable_id': comment.get('commentable_id', ''), + 'content_type': obj_type.capitalize(), + } + if hasattr(comment, 'username'): + event_data['target_username'] = comment.get('username', '') + track_forum_event(request, event_name, course, comment, event_data) + + +def track_thread_unreported_event(request, course, thread): + """ + Send analytics event for a unreported thread. + """ + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='unreported') + event_data = { + 'body': thread.body[:TRACKING_MAX_FORUM_BODY], + 'content_type': 'Post', + 'commentable_id': thread.get('commentable_id', ''), + 'reported_status_cleared': not bool(thread.get('abuse_flaggers', [])), + } + if hasattr(thread, 'username'): + event_data['target_username'] = thread.get('username', '') + add_truncated_title_to_event_data(event_data, thread.get('title', '')) + track_forum_event(request, event_name, course, thread, event_data) + + +def track_comment_unreported_event(request, course, comment): + """ + Send analytics event for a unreported response or comment. + """ + obj_type = 'comment' if comment.get('parent_id') else 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='unreported') + event_data = { + 'body': comment.body[:TRACKING_MAX_FORUM_BODY], + 'commentable_id': comment.get('commentable_id', ''), + 'content_type': obj_type.capitalize(), + 'reported_status_cleared': not bool(comment.get('abuse_flaggers', [])), + } + if hasattr(comment, 'username'): + event_data['target_username'] = comment.get('username', '') + track_forum_event(request, event_name, course, comment, event_data) + + +def track_discussion_reported_event(request, course, cc_content): + """ + Helper method for discussion reported events. + """ + if cc_content.type == 'thread': + track_thread_reported_event(request, course, cc_content) + else: + track_comment_reported_event(request, course, cc_content) + + +def track_discussion_unreported_event(request, course, cc_content): + """ + Helper method for discussion unreported events. + """ + if cc_content.type == 'thread': + track_thread_unreported_event(request, course, cc_content) + else: + track_comment_unreported_event(request, course, cc_content) + + def permitted(func): """ View decorator to verify the user is authorized to access this endpoint. diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 89b02eb661..768a56340a 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -34,7 +34,7 @@ from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.discussion.toggles import ENABLE_DISCUSSIONS_MFE, ENABLE_LEARNERS_TAB_IN_DISCUSSIONS_MFE from lms.djangoapps.discussion.toggles_utils import reported_content_email_notification_enabled -from lms.djangoapps.discussion.views import is_user_moderator +from lms.djangoapps.discussion.views import is_privileged_user from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, DiscussionTopicLink, Provider from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks from openedx.core.djangoapps.django_comment_common import comment_client @@ -81,7 +81,9 @@ from ..django_comment_client.base.views import ( track_thread_created_event, track_thread_deleted_event, track_thread_viewed_event, - track_voted_event + track_voted_event, + track_discussion_reported_event, + track_discussion_unreported_event, ) from ..django_comment_client.utils import ( get_group_id_for_user, @@ -1196,7 +1198,7 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con if field == "following": _handle_following_field(form_value, context["cc_requester"], cc_content) elif field == "abuse_flagged": - _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content) + _handle_abuse_flagged_field(form_value, context["cc_requester"], cc_content, request) elif field == "voted": _handle_voted_field(form_value, cc_content, api_content, request, context) elif field == "read": @@ -1215,11 +1217,13 @@ def _handle_following_field(form_value, user, cc_content): user.unfollow(cc_content) -def _handle_abuse_flagged_field(form_value, user, cc_content): +def _handle_abuse_flagged_field(form_value, user, cc_content, request): """mark or unmark thread/comment as abused""" course_key = CourseKey.from_string(cc_content.course_id) + course = get_course_with_access(request.user, 'load', course_key) if form_value: cc_content.flagAbuse(user, cc_content) + track_discussion_reported_event(request, course, cc_content) if ENABLE_DISCUSSIONS_MFE.is_enabled(course_key) and reported_content_email_notification_enabled( course_key): if cc_content.type == 'thread': @@ -1227,8 +1231,9 @@ def _handle_abuse_flagged_field(form_value, user, cc_content): else: comment_flagged.send(sender='flag_abuse_for_comment', user=user, post=cc_content) else: - remove_all = bool(is_user_moderator(course_key, User.objects.get(id=user.id))) + remove_all = bool(is_privileged_user(course_key, User.objects.get(id=user.id))) cc_content.unFlagAbuse(user, cc_content, remove_all) + track_discussion_unreported_event(request, course, cc_content) def _handle_voted_field(form_value, cc_content, api_content, request, context): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index b8c8a3d508..15e6fb8cfb 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -21,6 +21,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import CourseLocator from pytz import UTC from rest_framework.exceptions import PermissionDenied + from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase @@ -2830,7 +2831,8 @@ class UpdateThreadTest( @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack - def test_abuse_flagged(self, old_flagged, new_flagged): + @mock.patch("eventtracking.tracker.emit") + def test_abuse_flagged(self, old_flagged, new_flagged, mock_emit): """ Test attempts to edit the "abuse_flagged" field. @@ -2857,12 +2859,33 @@ class UpdateThreadTest( assert httpretty.last_request().method == 'PUT' assert parsed_body(httpretty.last_request()) == {'user_id': [str(self.user.id)]} + expected_event_name = 'edx.forum.thread.reported' if new_flagged else 'edx.forum.thread.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_thread', + 'content_type': 'Post', + 'commentable_id': 'original_topic', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT], + 'target_username': self.user.username, + 'title_truncated': False, + 'title': 'Original Title', + } + if not new_flagged: + expected_event_data['reported_status_cleared'] = False + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( (False, True), (True, True), ) @ddt.unpack - def test_thread_un_abuse_flag_for_moderator_role(self, is_author, remove_all): + @mock.patch("eventtracking.tracker.emit") + def test_thread_un_abuse_flag_for_moderator_role(self, is_author, remove_all, mock_emit): """ Test un-abuse flag for moderator role. @@ -2883,6 +2906,25 @@ class UpdateThreadTest( query_params.update({'all': ['True']}) assert parsed_body(httpretty.last_request()) == query_params + expected_event_name = 'edx.forum.thread.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_thread', + 'content_type': 'Post', + 'commentable_id': 'original_topic', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT, FORUM_ROLE_ADMINISTRATOR], + 'target_username': self.user.username, + 'title_truncated': False, + 'title': 'Original Title', + 'reported_status_cleared': False, + } + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + def test_invalid_field(self): self.register_thread() with pytest.raises(ValidationError) as assertion: @@ -3356,7 +3398,8 @@ class UpdateCommentTest( @ddt.data(*itertools.product([True, False], [True, False])) @ddt.unpack - def test_abuse_flagged(self, old_flagged, new_flagged): + @mock.patch("eventtracking.tracker.emit") + def test_abuse_flagged(self, old_flagged, new_flagged, mock_emit): """ Test attempts to edit the "abuse_flagged" field. @@ -3383,12 +3426,31 @@ class UpdateCommentTest( assert httpretty.last_request().method == 'PUT' assert parsed_body(httpretty.last_request()) == {'user_id': [str(self.user.id)]} + expected_event_name = 'edx.forum.response.reported' if new_flagged else 'edx.forum.response.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_comment', + 'content_type': 'Response', + 'commentable_id': 'dummy', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT], + 'target_username': self.user.username, + } + if not new_flagged: + expected_event_data['reported_status_cleared'] = False + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( (False, True), (True, True), ) @ddt.unpack - def test_comment_un_abuse_flag_for_moderator_role(self, is_author, remove_all): + @mock.patch("eventtracking.tracker.emit") + def test_comment_un_abuse_flag_for_moderator_role(self, is_author, remove_all, mock_emit): """ Test un-abuse flag for moderator role. @@ -3409,6 +3471,23 @@ class UpdateCommentTest( query_params.update({'all': ['True']}) assert parsed_body(httpretty.last_request()) == query_params + expected_event_name = 'edx.forum.response.unreported' + expected_event_data = { + 'body': 'Original body', + 'id': 'test_comment', + 'content_type': 'Response', + 'commentable_id': 'dummy', + 'url': '', + 'user_course_roles': [], + 'user_forums_roles': [FORUM_ROLE_STUDENT, FORUM_ROLE_ADMINISTRATOR], + 'target_username': self.user.username, + 'reported_status_cleared': False, + } + + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + @ddt.data( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index ce0ae405b0..a9ad7dce83 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -763,31 +763,16 @@ def is_privileged_user(course_key: CourseKey, user: User): """ Returns True if user has one of following course role Administrator, Moderator, Group Moderator, Community TA, - Global Staff, Course Instructor or Course Staff. + or Global Staff. """ forum_roles = [ FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, - FORUM_ROLE_ADMINISTRATOR + FORUM_ROLE_ADMINISTRATOR, ] has_course_role = Role.user_has_role_for_course(user, course_key, forum_roles) - return GlobalStaff().has_user(user) or is_course_staff(course_key, user) or has_course_role - - -def is_user_moderator(course_key: CourseKey, user: User): - """ - Returns True if user has one of following course role - Administrator, Moderator, Group Moderator, Community TA. - """ - forum_roles = [ - FORUM_ROLE_COMMUNITY_TA, - FORUM_ROLE_GROUP_MODERATOR, - FORUM_ROLE_MODERATOR, - FORUM_ROLE_ADMINISTRATOR - ] - has_course_role = Role.user_has_role_for_course(user, course_key, forum_roles) - return has_course_role + return GlobalStaff().has_user(user) or has_course_role class DiscussionBoardFragmentView(EdxFragmentView):