From be02613aacc7d03ae0f07c628c450ae033e2f895 Mon Sep 17 00:00:00 2001 From: Jonathan Piacenti Date: Tue, 30 Sep 2014 18:41:41 +0000 Subject: [PATCH] Do event tracking for major forum events. --- common/djangoapps/student/tests/factories.py | 10 +- common/test/acceptance/fixtures/discussion.py | 2 +- .../django_comment_client/base/tests.py | 151 ++++++++++++++++-- .../django_comment_client/base/views.py | 87 +++++++++- lms/djangoapps/django_comment_client/utils.py | 5 +- 5 files changed, 232 insertions(+), 23 deletions(-) diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index 880beac7da..23d3750202 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -2,7 +2,7 @@ from student.models import (User, UserProfile, Registration, CourseEnrollmentAllowed, CourseEnrollment, PendingEmailChange, UserStanding, - ) + CourseAccessRole) from course_modes.models import CourseMode from django.contrib.auth.models import Group, AnonymousUser from datetime import datetime @@ -113,6 +113,14 @@ class CourseEnrollmentFactory(DjangoModelFactory): course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') +class CourseAccessRoleFactory(DjangoModelFactory): + FACTORY_FOR = CourseAccessRole + + user = factory.SubFactory(UserFactory) + course_id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') + role = 'TestRole' + + class CourseEnrollmentAllowedFactory(DjangoModelFactory): FACTORY_FOR = CourseEnrollmentAllowed diff --git a/common/test/acceptance/fixtures/discussion.py b/common/test/acceptance/fixtures/discussion.py index c14b2cf56e..71cc2899c0 100644 --- a/common/test/acceptance/fixtures/discussion.py +++ b/common/test/acceptance/fixtures/discussion.py @@ -44,7 +44,7 @@ class Thread(ContentFactory): class Comment(ContentFactory): - thread_id = None + thread_id = "dummy thread" depth = 0 type = "comment" body = "dummy comment body" diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 28219bcf76..f949f434f9 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -9,6 +9,7 @@ from django.core.urlresolvers import reverse from mock import patch, ANY, Mock from nose.tools import assert_true, assert_equal # pylint: disable=no-name-in-module from opaque_keys.edx.locations import SlashSeparatedCourseKey +from lms.lib.comment_client import Thread from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from django_comment_client.base import views @@ -17,7 +18,7 @@ from django_comment_client.tests.utils import CohortedTestCase from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_common.models import Role from django_comment_common.utils import seed_permissions_roles -from student.tests.factories import CourseEnrollmentFactory, UserFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory, CourseAccessRoleFactory from util.testing import UrlResetMixin from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -852,7 +853,10 @@ class CreateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockReq CourseEnrollmentFactory(user=self.student, course_id=self.course.id) @patch('lms.lib.comment_client.utils.requests.request') - def _test_unicode_data(self, text, mock_request): + def _test_unicode_data(self, text, mock_request,): + """ + Test to make sure unicode data in a thread doesn't break it. + """ self._set_mock_request_data(mock_request, {}) request = RequestFactory().post("dummy_url", {"thread_type": "discussion", "body": text, "title": text}) request.user = self.student @@ -908,14 +912,22 @@ class CreateCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRe self._set_mock_request_data(mock_request, { "closed": False, }) - request = RequestFactory().post("dummy_url", {"body": text}) - request.user = self.student - request.view_name = "create_comment" - response = views.create_comment(request, course_id=self.course.id.to_deprecated_string(), thread_id="dummy_thread_id") + # We have to get clever here due to Thread's setters and getters. + # Patch won't work with it. + try: + Thread.commentable_id = Mock() + request = RequestFactory().post("dummy_url", {"body": text}) + request.user = self.student + request.view_name = "create_comment" + response = views.create_comment( + request, course_id=unicode(self.course.id), thread_id="dummy_thread_id" + ) - self.assertEqual(response.status_code, 200) - self.assertTrue(mock_request.called) - self.assertEqual(mock_request.call_args[1]["data"]["body"], text) + self.assertEqual(response.status_code, 200) + self.assertTrue(mock_request.called) + self.assertEqual(mock_request.call_args[1]["data"]["body"], text) + finally: + del Thread.commentable_id class UpdateCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRequestSetupMixin): @@ -944,6 +956,9 @@ class UpdateCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRe class CreateSubCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRequestSetupMixin): + """ + Make sure comments under a response can handle unicode. + """ def setUp(self): super(CreateSubCommentUnicodeTestCase, self).setUp() @@ -954,20 +969,130 @@ class CreateSubCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, Moc @patch('lms.lib.comment_client.utils.requests.request') def _test_unicode_data(self, text, mock_request): + """ + Create a comment with unicode in it. + """ self._set_mock_request_data(mock_request, { "closed": False, "depth": 1, + "thread_id": "test_thread" }) request = RequestFactory().post("dummy_url", {"body": text}) request.user = self.student request.view_name = "create_sub_comment" - response = views.create_sub_comment(request, course_id=self.course.id.to_deprecated_string(), comment_id="dummy_comment_id") + Thread.commentable_id = Mock() + try: + response = views.create_sub_comment( + request, course_id=self.course.id.to_deprecated_string(), comment_id="dummy_comment_id" + ) - self.assertEqual(response.status_code, 200) - self.assertTrue(mock_request.called) - self.assertEqual(mock_request.call_args[1]["data"]["body"], text) + self.assertEqual(response.status_code, 200) + self.assertTrue(mock_request.called) + self.assertEqual(mock_request.call_args[1]["data"]["body"], text) + finally: + del Thread.commentable_id +@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin): + """ + Forum actions are expected to launch analytics events. Test these here. + """ + def setUp(self): + super(ForumEventTestCase, self).setUp() + self.course = CourseFactory.create() + seed_permissions_roles(self.course.id) + self.student = UserFactory.create() + CourseEnrollmentFactory(user=self.student, course_id=self.course.id) + self.student.roles.add(Role.objects.get(name="Student", course_id=self.course.id)) + CourseAccessRoleFactory(course_id=self.course.id, user=self.student, role='Wizard') + + @patch('eventtracking.tracker.emit') + @patch('lms.lib.comment_client.utils.requests.request') + def test_thread_event(self, __, mock_emit): + request = RequestFactory().post( + "dummy_url", { + "thread_type": "discussion", + "body": "Test text", + "title": "Test", + "auto_subscribe": True + } + ) + request.user = self.student + request.view_name = "create_thread" + + views.create_thread(request, course_id=self.course.id.to_deprecated_string(), commentable_id="test_commentable") + + event_name, event = mock_emit.call_args[0] + self.assertEqual(event_name, 'edx.forum.thread.created') + self.assertEqual(event['body'], 'Test text') + self.assertEqual(event['title'], 'Test') + self.assertEqual(event['commentable_id'], 'test_commentable') + self.assertEqual(event['user_forums_roles'], ['Student']) + self.assertEqual(event['options']['followed'], True) + self.assertEqual(event['user_course_roles'], ['Wizard']) + self.assertEqual(event['anonymous'], False) + self.assertEqual(event['group_id'], None) + self.assertEqual(event['thread_type'], 'discussion') + self.assertEquals(event['anonymous_to_peers'], False) + + @patch('eventtracking.tracker.emit') + @patch('lms.lib.comment_client.utils.requests.request') + def test_response_event(self, mock_request, mock_emit): + """ + Check to make sure an event is fired when a user responds to a thread. + """ + mock_request.return_value.status_code = 200 + self._set_mock_request_data(mock_request, { + "closed": False, + "commentable_id": 'test_commentable_id', + 'thread_id': 'test_thread_id', + }) + request = RequestFactory().post("dummy_url", {"body": "Test comment", 'auto_subscribe': True}) + request.user = self.student + request.view_name = "create_comment" + views.create_comment(request, course_id=self.course.id.to_deprecated_string(), thread_id='test_thread_id') + + event_name, event = mock_emit.call_args[0] + self.assertEqual(event_name, 'edx.forum.response.created') + self.assertEqual(event['body'], "Test comment") + self.assertEqual(event['commentable_id'], 'test_commentable_id') + self.assertEqual(event['user_forums_roles'], ['Student']) + self.assertEqual(event['user_course_roles'], ['Wizard']) + self.assertEqual(event['discussion']['id'], 'test_thread_id') + self.assertEqual(event['options']['followed'], True) + + @patch('eventtracking.tracker.emit') + @patch('lms.lib.comment_client.utils.requests.request') + def test_comment_event(self, mock_request, mock_emit): + """ + Ensure an event is fired when someone comments on a response. + """ + self._set_mock_request_data(mock_request, { + "closed": False, + "depth": 1, + "thread_id": "test_thread_id", + "commentable_id": "test_commentable_id", + "parent_id": "test_response_id" + }) + request = RequestFactory().post("dummy_url", {"body": "Another comment"}) + request.user = self.student + request.view_name = "create_sub_comment" + views.create_sub_comment( + request, course_id=self.course.id.to_deprecated_string(), comment_id="dummy_comment_id" + ) + + event_name, event = mock_emit.call_args[0] + self.assertEqual(event_name, "edx.forum.comment.created") + self.assertEqual(event['body'], 'Another comment') + self.assertEqual(event['discussion']['id'], 'test_thread_id') + self.assertEqual(event['response']['id'], 'test_response_id') + self.assertEqual(event['user_forums_roles'], ['Student']) + self.assertEqual(event['user_course_roles'], ['Wizard']) + self.assertEqual(event['options']['followed'], False) + + +@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) class UsersEndpointTestCase(ModuleStoreTestCase, MockRequestSetupMixin): def set_post_counts(self, mock_request, threads_count=1, comments_count=1): diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 2c5af9bab5..dc41404e28 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -1,6 +1,5 @@ import functools import logging -import os.path import random import time import urlparse @@ -27,13 +26,17 @@ from django_comment_client.utils import ( JsonResponse, prepare_content, get_group_id_for_comments_service, - get_discussion_categories_ids + get_discussion_categories_ids, + get_discussion_id_map, ) from django_comment_client.permissions import check_permissions_by_view, cached_has_permission +from eventtracking import tracker import lms.lib.comment_client as cc log = logging.getLogger(__name__) +TRACKING_MAX_FORUM_BODY = 2000 + def permitted(fn): @functools.wraps(fn) @@ -63,6 +66,37 @@ 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 + if id_map is None: + id_map = get_discussion_id_map(course, user) + + commentable_id = data['commentable_id'] + 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) + + @require_POST @login_required @permitted @@ -116,11 +150,36 @@ def create_thread(request, course_id, commentable_id): if 'pinned' not in thread.attributes: thread['pinned'] = False - if post.get('auto_subscribe', 'false').lower() == 'true': + follow = post.get('auto_subscribe', 'false').lower() == 'true' + + if follow: user = cc.User.from_django_user(request.user) user.follow(thread) + + event_data = { + 'title': thread.title, + 'commentable_id': commentable_id, + 'options': {'followed': follow}, + 'anonymous': anonymous, + 'thread_type': thread.thread_type, + 'group_id': group_id, + 'anonymous_to_peers': anonymous_to_peers, + # 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. + } data = thread.to_dict() - add_courseware_context([data], course, request.user) + + # Calls to id map are expensive, but we need this more than once. + # Prefetch it. + id_map = get_discussion_id_map(course, request.user) + + add_courseware_context([data], course, request.user, id_map=id_map) + + track_forum_event(request, 'edx.forum.thread.created', + course, thread, event_data, id_map=id_map) + if request.is_ajax(): return ajax_content_response(request, course_key, data) else: @@ -194,9 +253,25 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None): body=post["body"] ) comment.save() - if post.get('auto_subscribe', 'false').lower() == 'true': + + followed = post.get('auto_subscribe', 'false').lower() == 'true' + + if followed: user = cc.User.from_django_user(request.user) user.follow(comment.thread) + + event_data = {'discussion': {'id': comment.thread_id}, 'options': {'followed': followed}} + + if parent_id: + event_data['response'] = {'id': comment.parent_id} + event_name = 'edx.forum.comment.created' + else: + event_name = 'edx.forum.response.created' + + event_data['commentable_id'] = comment.thread.commentable_id + + track_forum_event(request, event_name, course, comment, event_data) + if request.is_ajax(): return ajax_content_response(request, course_key, comment.to_dict()) else: @@ -220,7 +295,7 @@ def create_comment(request, course_id, thread_id): @require_POST @login_required @permitted -def delete_thread(request, course_id, thread_id): +def delete_thread(request, course_id, thread_id): # pylint: disable=unused-argument """ given a course_id and thread_id, delete this thread this is ajax only diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 6b42eb4bac..ef2977f345 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -383,11 +383,12 @@ def extend_content(content): return merge_dict(content, content_info) -def add_courseware_context(content_list, course, user): +def add_courseware_context(content_list, course, user, id_map=None): """ Decorates `content_list` with courseware metadata. """ - id_map = get_discussion_id_map(course, user) + if id_map is None: + id_map = get_discussion_id_map(course, user) for content in content_list: commentable_id = content['commentable_id']