From cf5d276aea7ce3dd6805196f35b2af2f3014ddea Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Mon, 24 Aug 2015 17:31:53 +0200 Subject: [PATCH 1/2] Emit events when users vote on forum posts. --- lms/djangoapps/discussion_api/api.py | 24 +- .../django_comment_client/base/views.py | 243 +++++++++--------- 2 files changed, 126 insertions(+), 141 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 1e0923bdf6..2b3dc81405 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -25,13 +25,7 @@ from discussion_api.permissions import ( get_initializable_thread_fields, ) from discussion_api.serializers import CommentSerializer, ThreadSerializer, get_context -from django_comment_client.base.views import ( - THREAD_CREATED_EVENT_NAME, - get_comment_created_event_data, - get_comment_created_event_name, - get_thread_created_event_data, - track_forum_event, -) +from django_comment_client.base.views import track_comment_created_event, track_thread_created_event from django_comment_common.signals import ( thread_created, thread_edited, @@ -566,13 +560,7 @@ def create_thread(request, thread_data): api_thread = serializer.data _do_extra_actions(api_thread, cc_thread, thread_data.keys(), actions_form, context) - track_forum_event( - request, - THREAD_CREATED_EVENT_NAME, - course, - cc_thread, - get_thread_created_event_data(cc_thread, followed=actions_form.cleaned_data["following"]) - ) + track_thread_created_event(request, course, cc_thread, actions_form.cleaned_data["following"]) return api_thread @@ -616,13 +604,7 @@ def create_comment(request, comment_data): api_comment = serializer.data _do_extra_actions(api_comment, cc_comment, comment_data.keys(), actions_form, context) - track_forum_event( - request, - get_comment_created_event_name(cc_comment), - context["course"], - cc_comment, - get_comment_created_event_data(cc_comment, cc_thread["commentable_id"], followed=False) - ) + track_comment_created_event(request, context["course"], cc_comment, cc_thread["commentable_id"], followed=False) return api_comment diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 9c33cfa450..b84fffc460 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -49,10 +49,92 @@ import lms.lib.comment_client as cc log = logging.getLogger(__name__) TRACKING_MAX_FORUM_BODY = 2000 +_EVENT_NAME_TEMPLATE = 'edx.forum.{obj_type}.{action_name}' -THREAD_CREATED_EVENT_NAME = "edx.forum.thread.created" -RESPONSE_CREATED_EVENT_NAME = 'edx.forum.response.created' -COMMENT_CREATED_EVENT_NAME = 'edx.forum.comment.created' + +def track_forum_event(request, event_name, course, obj, data, id_map=None): + """ + Send out an analytics event when a forum event happens. Works for threads, + responses to threads, and comments on those responses. + """ + user = request.user + data['id'] = obj.id + commentable_id = data['commentable_id'] + + team = get_team(commentable_id) + if team is not None: + data.update(team_id=team.team_id) + + if id_map is None: + id_map = get_cached_discussion_id_map(course, [commentable_id], user) + if commentable_id in id_map: + data['category_name'] = id_map[commentable_id]["title"] + data['category_id'] = commentable_id + data['url'] = request.META.get('HTTP_REFERER', '') + data['user_forums_roles'] = [ + role.name for role in user.roles.filter(course_id=course.id) + ] + data['user_course_roles'] = [ + role.role for role in user.courseaccessrole_set.filter(course_id=course.id) + ] + + tracker.emit(event_name, data) + + +def track_created_event(request, event_name, course, obj, data): + if len(obj.body) > TRACKING_MAX_FORUM_BODY: + data['truncated'] = True + else: + data['truncated'] = False + data['body'] = obj.body[:TRACKING_MAX_FORUM_BODY] + track_forum_event(request, event_name, course, obj, data) + + +def track_thread_created_event(request, course, thread, followed): + event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='created') + event_data = { + 'commentable_id': thread.commentable_id, + 'group_id': thread.get("group_id"), + 'thread_type': thread.thread_type, + 'title': thread.title, + 'anonymous': thread.anonymous, + 'anonymous_to_peers': thread.anonymous_to_peers, + 'options': {'followed': followed}, + # There is a stated desire for an 'origin' property that will state + # whether this thread was created via courseware or the forum. + # However, the view does not contain that data, and including it will + # likely require changes elsewhere. + } + track_created_event(request, event_name, course, thread, event_data) + + +def track_comment_created_event(request, course, comment, commentable_id, followed): + obj_type = 'comment' if comment.get("parent_id") else 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='created') + event_data = { + 'discussion': {'id': comment.thread_id}, + 'commentable_id': commentable_id, + 'options': {'followed': followed}, + } + parent_id = comment.get('parent_id') + if parent_id: + event_data['response'] = {'id': parent_id} + track_created_event(request, event_name, course, comment, event_data) + + +def track_voted_event(request, course, obj, vote_value, undo_vote=False): + if isinstance(obj, cc.Thread): + obj_type = 'thread' + else: + obj_type = 'response' + event_name = _EVENT_NAME_TEMPLATE.format(obj_type=obj_type, action_name='voted') + event_data = { + 'commentable_id': obj.commentable_id, + 'target_username': obj.get('username'), + 'undo_vote': undo_vote, + 'vote_value': vote_value, + } + track_forum_event(request, event_name, course, obj, event_data) def permitted(fn): @@ -85,85 +167,6 @@ def ajax_content_response(request, course_key, content): }) -def track_forum_event(request, event_name, course, obj, data, id_map=None): - """ - Send out an analytics event when a forum event happens. Works for threads, - responses to threads, and comments on those responses. - """ - user = request.user - data['id'] = obj.id - commentable_id = data['commentable_id'] - - team = get_team(commentable_id) - if team is not None: - data.update(team_id=team.team_id) - - if id_map is None: - id_map = get_cached_discussion_id_map(course, [commentable_id], user) - - if commentable_id in id_map: - data['category_name'] = id_map[commentable_id]["title"] - data['category_id'] = commentable_id - if len(obj.body) > TRACKING_MAX_FORUM_BODY: - data['truncated'] = True - else: - data['truncated'] = False - - data['body'] = obj.body[:TRACKING_MAX_FORUM_BODY] - data['url'] = request.META.get('HTTP_REFERER', '') - data['user_forums_roles'] = [ - role.name for role in user.roles.filter(course_id=course.id) - ] - data['user_course_roles'] = [ - role.role for role in user.courseaccessrole_set.filter(course_id=course.id) - ] - - tracker.emit(event_name, data) - - -def get_thread_created_event_data(thread, followed): - """ - Get the event data payload for thread creation (excluding fields populated - by track_forum_event) - """ - return { - 'commentable_id': thread.commentable_id, - 'group_id': thread.get("group_id"), - 'thread_type': thread.thread_type, - 'title': thread.title, - 'anonymous': thread.anonymous, - 'anonymous_to_peers': thread.anonymous_to_peers, - 'options': {'followed': followed}, - # There is a stated desire for an 'origin' property that will state - # whether this thread was created via courseware or the forum. - # However, the view does not contain that data, and including it will - # likely require changes elsewhere. - } - - -def get_comment_created_event_name(comment): - """Get the appropriate event name for creating a response/comment""" - return COMMENT_CREATED_EVENT_NAME if comment.get("parent_id") else RESPONSE_CREATED_EVENT_NAME - - -def get_comment_created_event_data(comment, commentable_id, followed): - """ - Get the event data payload for comment creation (excluding fields populated - by track_forum_event) - """ - event_data = { - 'discussion': {'id': comment.thread_id}, - 'commentable_id': commentable_id, - 'options': {'followed': followed}, - } - - parent_id = comment.get("parent_id") - if parent_id: - event_data['response'] = {'id': parent_id} - - return event_data - - @require_POST @login_required @permitted @@ -234,12 +237,11 @@ def create_thread(request, course_id, commentable_id): cc_user = cc.User.from_django_user(user) cc_user.follow(thread) - event_data = get_thread_created_event_data(thread, follow) data = thread.to_dict() add_courseware_context([data], course, user) - track_forum_event(request, THREAD_CREATED_EVENT_NAME, course, thread, event_data) + track_thread_created_event(request, course, thread, follow) if request.is_ajax(): return ajax_content_response(request, course_key, data) @@ -330,9 +332,7 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None): cc_user = cc.User.from_django_user(request.user) cc_user.follow(comment.thread) - event_name = get_comment_created_event_name(comment) - event_data = get_comment_created_event_data(comment, comment.thread.commentable_id, followed) - track_forum_event(request, event_name, course, comment, event_data) + track_comment_created_event(request, course, comment, comment.thread.commentable_id, followed) if request.is_ajax(): return ajax_content_response(request, course_key, comment.to_dict()) @@ -456,6 +456,24 @@ def delete_comment(request, course_id, comment_id): return JsonResponse(prepare_content(comment.to_dict(), course_key)) +def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): + """ + Vote or unvote for a thread or a response. + """ + course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course = get_course_with_access(request.user, 'load', course_key) + user = cc.User.from_django_user(request.user) + if undo_vote: + user.unvote(obj) + # TODO(smarnach): Determine the value of the vote that is undone. Currently, you can + # only cast upvotes in the user interface, so it is assumed that the vote value is 'up'. + # (People could theoretically downvote by handcrafting AJAX requests.) + else: + user.vote(obj, value) + track_voted_event(request, course, obj, value, undo_vote) + return JsonResponse(prepare_content(obj.to_dict(), course_key)) + + @require_POST @login_required @permitted @@ -463,13 +481,10 @@ def vote_for_comment(request, course_id, comment_id, value): """ given a course_id and comment_id, """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = request.user - cc_user = cc.User.from_django_user(user) comment = cc.Comment.find(comment_id) - cc_user.vote(comment, value) - comment_voted.send(sender=None, user=user, post=comment) - return JsonResponse(prepare_content(comment.to_dict(), course_key)) + result = _vote_or_unvote(request, course_id, comment, value) + comment_voted.send(sender=None, user=request.user, post=comment) + return result @require_POST @@ -480,11 +495,7 @@ def undo_vote_for_comment(request, course_id, comment_id): given a course id and comment id, remove vote ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = cc.User.from_django_user(request.user) - comment = cc.Comment.find(comment_id) - user.unvote(comment) - return JsonResponse(prepare_content(comment.to_dict(), course_key)) + return _vote_or_unvote(request, course_id, cc.Comment.find(comment_id), undo_vote=True) @require_POST @@ -495,13 +506,21 @@ def vote_for_thread(request, course_id, thread_id, value): given a course id and thread id vote for this thread ajax only """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = request.user - cc_user = cc.User.from_django_user(user) thread = cc.Thread.find(thread_id) - cc_user.vote(thread, value) - thread_voted.send(sender=None, user=user, post=thread) - return JsonResponse(prepare_content(thread.to_dict(), course_key)) + result = _vote_or_unvote(request, course_id, thread, value) + thread_voted.send(sender=None, user=request.user, post=thread) + return result + + +@require_POST +@login_required +@permitted +def undo_vote_for_thread(request, course_id, thread_id): + """ + given a course id and thread id, remove users vote for thread + ajax only + """ + return _vote_or_unvote(request, course_id, cc.Thread.find(thread_id), undo_vote=True) @require_POST @@ -576,22 +595,6 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): return JsonResponse(prepare_content(comment.to_dict(), course_key)) -@require_POST -@login_required -@permitted -def undo_vote_for_thread(request, course_id, thread_id): - """ - given a course id and thread id, remove users vote for thread - ajax only - """ - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - user = cc.User.from_django_user(request.user) - thread = cc.Thread.find(thread_id) - user.unvote(thread) - - return JsonResponse(prepare_content(thread.to_dict(), course_key)) - - @require_POST @login_required @permitted From ef563e428574b8cdb633372eb3ef0ea793dcbaa7 Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Sat, 5 Sep 2015 23:24:07 +0200 Subject: [PATCH 2/2] Add tests for the discussion forum vote events. --- .../django_comment_client/base/tests.py | 34 +++++++++++++++++++ .../django_comment_client/base/views.py | 32 ++++++++++++++--- 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 3e0dfd8129..9f14f2daa4 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -1641,6 +1641,40 @@ class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin): self.assertEqual(name, event_name) self.assertEqual(event['team_id'], team.team_id) + @ddt.data( + ('vote_for_thread', 'thread_id', 'thread'), + ('undo_vote_for_thread', 'thread_id', 'thread'), + ('vote_for_comment', 'comment_id', 'response'), + ('undo_vote_for_comment', 'comment_id', 'response'), + ) + @ddt.unpack + @patch('eventtracking.tracker.emit') + @patch('lms.lib.comment_client.utils.requests.request') + def test_thread_voted_event(self, view_name, obj_id_name, obj_type, mock_request, mock_emit): + undo = view_name.startswith('undo') + + self._set_mock_request_data(mock_request, { + 'closed': False, + 'commentable_id': 'test_commentable_id', + 'username': 'gumprecht', + }) + request = RequestFactory().post('dummy_url', {}) + request.user = self.student + request.view_name = view_name + view_function = getattr(views, view_name) + kwargs = dict(course_id=unicode(self.course.id)) + kwargs[obj_id_name] = obj_id_name + if not undo: + kwargs.update(value='up') + view_function(request, **kwargs) + + self.assertTrue(mock_emit.called) + event_name, event = mock_emit.call_args[0] + self.assertEqual(event_name, 'edx.forum.{}.voted'.format(obj_type)) + self.assertEqual(event['target_username'], 'gumprecht') + self.assertEqual(event['undo_vote'], undo) + self.assertEqual(event['vote_value'], 'up') + class UsersEndpointTestCase(ModuleStoreTestCase, MockRequestSetupMixin): diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index b84fffc460..e480ff62a3 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -82,6 +82,9 @@ def track_forum_event(request, event_name, course, obj, data, id_map=None): def track_created_event(request, event_name, course, obj, data): + """ + Send analytics event for a newly created thread, response or comment. + """ if len(obj.body) > TRACKING_MAX_FORUM_BODY: data['truncated'] = True else: @@ -91,6 +94,9 @@ def track_created_event(request, event_name, course, obj, data): def track_thread_created_event(request, course, thread, followed): + """ + Send analytics event for a newly created thread. + """ event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='created') event_data = { 'commentable_id': thread.commentable_id, @@ -109,6 +115,9 @@ def track_thread_created_event(request, course, thread, followed): def track_comment_created_event(request, course, comment, commentable_id, followed): + """ + Send analytics event for a newly created 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='created') event_data = { @@ -123,6 +132,9 @@ def track_comment_created_event(request, course, comment, commentable_id, follow def track_voted_event(request, course, obj, vote_value, undo_vote=False): + """ + Send analytics event for a vote on a thread or response. + """ if isinstance(obj, cc.Thread): obj_type = 'thread' else: @@ -137,10 +149,19 @@ def track_voted_event(request, course, obj, vote_value, undo_vote=False): track_forum_event(request, event_name, course, obj, event_data) -def permitted(fn): - @functools.wraps(fn) +def permitted(func): + """ + View decorator to verify the user is authorized to access this endpoint. + """ + @functools.wraps(func) def wrapper(request, *args, **kwargs): + """ + Wrapper for the view that only calls the view if the user is authorized. + """ def fetch_content(): + """ + Extract the forum object from the keyword arguments to the view. + """ if "thread_id" in kwargs: content = cc.Thread.find(kwargs["thread_id"]).to_dict() elif "comment_id" in kwargs: @@ -152,13 +173,16 @@ def permitted(fn): return content course_key = SlashSeparatedCourseKey.from_deprecated_string(kwargs['course_id']) if check_permissions_by_view(request.user, course_key, fetch_content(), request.view_name): - return fn(request, *args, **kwargs) + return func(request, *args, **kwargs) else: return JsonError("unauthorized", status=401) return wrapper def ajax_content_response(request, course_key, content): + """ + Standard AJAX response returning the content hierarchy of the current thread. + """ user_info = cc.User.from_django_user(request.user).to_dict() annotated_content_info = get_annotated_content_info(course_key, content, request.user, user_info) return JsonResponse({ @@ -479,7 +503,7 @@ def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): @permitted def vote_for_comment(request, course_id, comment_id, value): """ - given a course_id and comment_id, + Given a course_id and comment_id, vote for this response. AJAX only. """ comment = cc.Comment.find(comment_id) result = _vote_or_unvote(request, course_id, comment, value)