Implement event for forum thread views

EDUCATOR-341

common/djangoapps/track/views/segmentio.py

@@ -28,6 +28,10 @@ ERROR_MISSING_TIMESTAMP = 'Required timestamp field not found'
 ERROR_MISSING_RECEIVED_AT = 'Required receivedAt field not found'
 
 
+FORUM_THREAD_VIEWED_EVENT_LABEL = 'Forum: View Thread'
+BI_SCREEN_VIEWED_EVENT_NAME = u'edx.bi.app.navigation.screen'
+
+
 @require_POST
 @expect_json
 @csrf_exempt
@@ -141,11 +145,8 @@ def track_segmentio_event(request):  # pylint: disable=too-many-statements
     if not segment_event_type or (segment_event_type.lower() not in allowed_types):
         return
 
-    if 'name' not in segment_properties:
-        raise EventValidationError(ERROR_MISSING_NAME)
-
     # Ignore event names that are unsupported
-    segment_event_name = segment_properties['name']
+    segment_event_name = _get_segmentio_event_name(segment_properties)
     disallowed_substring_names = [
         a.lower() for a in getattr(settings, 'TRACKING_SEGMENTIO_DISALLOWED_SUBSTRING_NAMES', [])
     ]
@@ -220,10 +221,49 @@ def track_segmentio_event(request):  # pylint: disable=too-many-statements
 
     context['ip'] = segment_properties.get('context', {}).get('ip', '')
 
+    # For Business Intelligence events: Add label to context
+    if 'label' in segment_properties:
+        context['label'] = segment_properties['label']
+
+    # For Android-sourced Business Intelligence events: add course ID to context
+    if 'course_id' in segment_properties:
+        context['course_id'] = segment_properties['course_id']
+
     with tracker.get_tracker().context('edx.segmentio', context):
         tracker.emit(segment_event_name, segment_properties.get('data', {}))
 
 
+def _get_segmentio_event_name(event_properties):
+    """
+    Get the name of a SegmentIO event.
+
+    Args:
+        event_properties: dict
+            The properties of the event, which should contain the event's
+            name or, in the case of an old Android screen event, its screen
+            label.
+
+    Returns: str
+        The name (or effective name) of the event.
+
+    Note:
+        In older versions of the Android app, screen-view tracking events
+        did not have a name. So, in order to capture forum-thread-viewed events
+        from those old-versioned apps, we have to accept the event based on
+        its screen label. We return an event name that matches screen-view
+        events in the iOS app and newer versions of the Android app.
+
+    Raises:
+        EventValidationError if name is missing
+    """
+    if 'name' in event_properties:
+        return event_properties['name']
+    elif event_properties.get('label') == FORUM_THREAD_VIEWED_EVENT_LABEL:
+        return BI_SCREEN_VIEWED_EVENT_NAME
+    else:
+        raise EventValidationError(ERROR_MISSING_NAME)
+
+
 def parse_iso8601_timestamp(timestamp):
     """Parse a particular type of ISO8601 formatted timestamp"""
     return datetime.datetime.strptime(timestamp, "%Y-%m-%dT%H:%M:%S.%fZ")

common/djangoapps/track/views/tests/base.py

+"""
+Base class for tests related to emitted events to one of the tracking 'views'
+(e.g. SegmentIO).
+"""
+import json
+from mock import sentinel
+
+from django.test.client import RequestFactory
+from django.test.utils import override_settings
+
+from track.views import segmentio
+from track.tests import EventTrackingTestCase
+
+
+SEGMENTIO_TEST_SECRET = 'anything'
+SEGMENTIO_TEST_ENDPOINT = '/segmentio/test/event'
+SEGMENTIO_TEST_USER_ID = 10
+
+_MOBILE_SHIM_PROCESSOR = [
+    {'ENGINE': 'track.shim.LegacyFieldMappingProcessor'},
+    {'ENGINE': 'track.shim.PrefixedEventProcessor'},
+]
+
+
+@override_settings(
+    TRACKING_SEGMENTIO_WEBHOOK_SECRET=SEGMENTIO_TEST_SECRET,
+    TRACKING_IGNORE_URL_PATTERNS=[SEGMENTIO_TEST_ENDPOINT],
+    TRACKING_SEGMENTIO_ALLOWED_TYPES=['track'],
+    TRACKING_SEGMENTIO_DISALLOWED_SUBSTRING_NAMES=[],
+    TRACKING_SEGMENTIO_SOURCE_MAP={'test-app': 'mobile'},
+    EVENT_TRACKING_PROCESSORS=_MOBILE_SHIM_PROCESSOR,
+)
+class SegmentIOTrackingTestCaseBase(EventTrackingTestCase):
+    """
+    Base class for tests that test the processing of Segment events.
+    """
+
+    def setUp(self):
+        super(SegmentIOTrackingTestCaseBase, self).setUp()
+        self.maxDiff = None  # pylint: disable=invalid-name
+        self.request_factory = RequestFactory()
+
+    def create_request(self, key=None, **kwargs):
+        """Create a fake request that emulates a request from the Segment servers to ours"""
+        if key is None:
+            key = SEGMENTIO_TEST_SECRET
+
+        request = self.request_factory.post(SEGMENTIO_TEST_ENDPOINT + "?key=" + key, **kwargs)
+        if 'data' in kwargs:
+            request.json = json.loads(kwargs['data'])
+
+        return request
+
+    def post_segmentio_event(self, **kwargs):
+        """Post a fake Segment event to the view that processes it"""
+        request = self.create_request(
+            data=self.create_segmentio_event_json(**kwargs),
+            content_type='application/json'
+        )
+        segmentio.track_segmentio_event(request)
+
+    def create_segmentio_event(self, **kwargs):
+        """Populate a fake Segment event with data of interest"""
+        action = kwargs.get('action', 'Track')
+        sample_event = {
+            "userId": kwargs.get('user_id', SEGMENTIO_TEST_USER_ID),
+            "event": "Did something",
+            "properties": {
+                'name': kwargs.get('name', str(sentinel.name)),
+                'data': kwargs.get('data', {}),
+                'context': {
+                    'course_id': kwargs.get('course_id') or '',
+                    'app_name': 'edx.mobile.android',
+                }
+            },
+            "channel": 'server',
+            "context": {
+                "library": {
+                    "name": kwargs.get('library_name', 'test-app'),
+                    "version": "unknown"
+                },
+                "app": {
+                    "version": "1.0.1",
+                },
+                'userAgent': str(sentinel.user_agent),
+            },
+            "receivedAt": "2014-08-27T16:33:39.100Z",
+            "timestamp": "2014-08-27T16:33:39.215Z",
+            "type": action.lower(),
+            "projectId": "u0j33yjkr8",
+            "messageId": "qy52hwp4",
+            "version": 2,
+            "integrations": {},
+            "options": {
+                "library": "unknown",
+                "providers": {}
+            },
+            "action": action
+        }
+
+        if 'context' in kwargs:
+            sample_event['properties']['context'].update(kwargs['context'])
+        if 'label' in kwargs:
+            sample_event['properties']['label'] = kwargs['label']
+        if kwargs.get('exclude_name') is True:
+            del sample_event['properties']['name']
+
+        return sample_event
+
+    def create_segmentio_event_json(self, **kwargs):
+        """Return a json string containing a fake Segment event"""
+        return json.dumps(self.create_segmentio_event(**kwargs))

common/djangoapps/util/testing.py

@@ -96,12 +96,29 @@ class EventTestMixin(object):
             kwargs
         )
 
+    def assert_event_emission_count(self, event_name, expected_count):
+        """
+        Verify that the event with the given name was emitted
+        a specific number of times.
+        """
+        actual_count = 0
+        for call_args in self.mock_tracker.emit.call_args_list:
+            if call_args[0][0] == event_name:
+                actual_count += 1
+        self.assertEqual(actual_count, expected_count)
+
     def reset_tracker(self):
         """
         Reset the mock tracker in order to forget about old events.
         """
         self.mock_tracker.reset_mock()
 
+    def get_latest_call_args(self):
+        """
+        Return the arguments of the latest call to emit.
+        """
+        return self.mock_tracker.emit.call_args[0]
+
 
 class PatchMediaTypeMixin(object):
     """

lms/djangoapps/discussion/views.py

@@ -28,6 +28,7 @@ import lms.lib.comment_client as cc
 from courseware.access import has_access
 from courseware.courses import get_course_with_access
 from courseware.views.views import CourseTabView
+from django_comment_client.base.views import track_thread_viewed_event
 from django_comment_client.constants import TYPE_ENTRY
 from django_comment_client.permissions import get_team, has_permission
 from django_comment_client.utils import (
@@ -291,10 +292,13 @@ def single_thread(request, course_key, discussion_id, thread_id):
         cc_user = cc.User.from_django_user(request.user)
         user_info = cc_user.to_dict()
         is_staff = has_permission(request.user, 'openclose_thread', course.id)
-
-        thread = _find_thread(request, course, discussion_id=discussion_id, thread_id=thread_id)
-        if not thread:
-            raise Http404
+        thread = _load_thread_for_viewing(
+            request,
+            course,
+            discussion_id=discussion_id,
+            thread_id=thread_id,
+            raise_event=True,
+        )
 
         with newrelic_function_trace("get_annotated_content_infos"):
             annotated_content_info = utils.get_annotated_content_infos(
@@ -358,6 +362,34 @@ def _find_thread(request, course, discussion_id, thread_id):
     return thread
 
 
+def _load_thread_for_viewing(request, course, discussion_id, thread_id, raise_event):
+    """
+    Loads the discussion thread with the specified ID and fires an
+    edx.forum.thread.viewed event.
+
+    Args:
+        request: The Django request.
+        course_id: The ID of the owning course.
+        discussion_id: The ID of the owning discussion.
+        thread_id: The ID of the thread.
+        raise_event: Whether an edx.forum.thread.viewed tracking event should
+                     be raised
+
+    Returns:
+        The thread in question if the user can see it.
+
+    Raises:
+        Http404 if the thread does not exist or the user cannot
+        see it.
+    """
+    thread = _find_thread(request, course, discussion_id=discussion_id, thread_id=thread_id)
+    if not thread:
+        raise Http404
+    if raise_event:
+        track_thread_viewed_event(request, course, thread)
+    return thread
+
+
 def _create_base_discussion_view_context(request, course_key):
     """
     Returns the default template context for rendering any discussion view.
@@ -393,20 +425,20 @@ def _get_discussion_default_topic_id(course):
             return entry['id']
 
 
-def _create_discussion_board_context(request, course_key, discussion_id=None, thread_id=None):
+def _create_discussion_board_context(request, base_context, thread=None):
     """
     Returns the template context for rendering the discussion board.
     """
-    context = _create_base_discussion_view_context(request, course_key)
+    context = base_context.copy()
     course = context['course']
+    course_key = course.id
+    thread_id = thread.id if thread else None
+    discussion_id = thread.commentable_id if thread else None
     course_settings = context['course_settings']
     user = context['user']
     cc_user = cc.User.from_django_user(user)
     user_info = context['user_info']
-    if thread_id:
-        thread = _find_thread(request, course, discussion_id=discussion_id, thread_id=thread_id)
-        if not thread:
-            raise Http404
+    if thread:
 
         # Since we're in page render mode, and the discussions UI will request the thread list itself,
         # we need only return the thread information for this one.
@@ -637,12 +669,25 @@ class DiscussionBoardFragmentView(EdxFragmentView):
         """
         course_key = CourseKey.from_string(course_id)
         try:
-            context = _create_discussion_board_context(
-                request,
-                course_key,
-                discussion_id=discussion_id,
-                thread_id=thread_id,
+            base_context = _create_base_discussion_view_context(request, course_key)
+            # Note:
+            #   After the thread is rendered in this fragment, an AJAX
+            #   request is made and the thread is completely loaded again
+            #   (yes, this is something to fix). Because of this, we pass in
+            #   raise_event=False to _load_thread_for_viewing avoid duplicate
+            #   tracking events.
+            thread = (
+                _load_thread_for_viewing(
+                    request,
+                    base_context['course'],
+                    discussion_id=discussion_id,
+                    thread_id=thread_id,
+                    raise_event=False,
+                )
+                if thread_id
+                else None
             )
+            context = _create_discussion_board_context(request, base_context, thread=thread)
             html = render_to_string('discussion/discussion_board_fragment.html', context)
             inline_js = render_to_string('discussion/discussion_board_js.template', context)
 

lms/djangoapps/django_comment_client/base/__init__.py

+# This import registers the ForumThreadViewedEventTransformer
+import event_transformers  # pylint: disable=unused-import

lms/djangoapps/django_comment_client/base/event_transformers.py

+"""
+Transformers for Discussion-related events.
+"""
+from django.contrib.auth.models import User
+from django.core.urlresolvers import reverse, NoReverseMatch
+
+from eventtracking.processors.exceptions import EventEmissionExit
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.locator import CourseLocator
+
+from django_comment_client.base.views import add_truncated_title_to_event_data
+from django_comment_client.permissions import get_team
+from django_comment_client.utils import get_cached_discussion_id_map_by_course_id
+from track.transformers import EventTransformer, EventTransformerRegistry
+from track.views.segmentio import (
+    BI_SCREEN_VIEWED_EVENT_NAME,
+    FORUM_THREAD_VIEWED_EVENT_LABEL
+)
+
+
+def _get_string(dictionary, key, del_if_bad=True):
+    """
+    Get a string from a dictionary by key.
+
+    If the key is not in the dictionary or does not refer to a string:
+        - Return None
+        - Optionally delete the key (del_if_bad)
+    """
+    if key in dictionary:
+        value = dictionary[key]
+        if isinstance(value, basestring):
+            return value
+        else:
+            if del_if_bad:
+                del dictionary[key]
+            return None
+    else:
+        return None
+
+
+@EventTransformerRegistry.register
+class ForumThreadViewedEventTransformer(EventTransformer):
+    """
+    Transformer for forum-thread-viewed mobile navigation events.
+    """
+
+    match_key = BI_SCREEN_VIEWED_EVENT_NAME
+
+    def process_event(self):
+        """
+        Process incoming mobile navigation events.
+
+        For forum-thread-viewed events, change their names to
+        edx.forum.thread.viewed and manipulate their data to conform with
+        edx.forum.thread.viewed event design.
+
+        Throw out other events.
+        """
+
+        # Get event context dict
+        # Throw out event if context nonexistent or wrong type
+        context = self.get('context')
+        if not isinstance(context, dict):
+            raise EventEmissionExit()
+
+        # Throw out event if it's not a forum thread view
+        if _get_string(context, 'label', del_if_bad=False) != FORUM_THREAD_VIEWED_EVENT_LABEL:
+            raise EventEmissionExit()
+
+        # Change name and event type
+        self['name'] = 'edx.forum.thread.viewed'
+        self['event_type'] = self['name']
+
+        # If no event data, set it to an empty dict
+        if 'event' not in self:
+            self['event'] = {}
+            self.event = {}
+
+        # Throw out the context dict within the event data
+        # (different from the context dict extracted above)
+        if 'context' in self.event:
+            del self.event['context']
+
+        # Parse out course key
+        course_id_string = _get_string(context, 'course_id') if context else None
+        course_id = None
+        if course_id_string:
+            try:
+                course_id = CourseLocator.from_string(course_id_string)
+            except InvalidKeyError:
+                pass
+
+        # Change 'thread_id' field to 'id'
+        thread_id = _get_string(self.event, 'thread_id')
+        if thread_id:
+            del self.event['thread_id']
+            self.event['id'] = thread_id
+
+        # Change 'topic_id' to 'commentable_id'
+        commentable_id = _get_string(self.event, 'topic_id')
+        if commentable_id:
+            del self.event['topic_id']
+            self.event['commentable_id'] = commentable_id
+
+        # Change 'action' to 'title' and truncate
+        title = _get_string(self.event, 'action')
+        if title is not None:
+            del self.event['action']
+            add_truncated_title_to_event_data(self.event, title)
+
+        # Change 'author' to 'target_username'
+        author = _get_string(self.event, 'author')
+        if author is not None:
+            del self.event['author']
+            self.event['target_username'] = author
+
+        # Load user
+        username = _get_string(self, 'username')
+        user = None
+        if username:
+            try:
+                user = User.objects.get(username=username)
+            except User.DoesNotExist:
+                pass
+
+        # If in a category, add category name and ID
+        if course_id and commentable_id and user:
+            id_map = get_cached_discussion_id_map_by_course_id(course_id, [commentable_id], user)
+            if commentable_id in id_map:
+                self.event['category_name'] = id_map[commentable_id]['title']
+                self.event['category_id'] = commentable_id
+
+        # Add thread URL
+        if course_id and commentable_id and thread_id:
+            url_kwargs = {
+                'course_id': course_id_string,
+                'discussion_id': commentable_id,
+                'thread_id': thread_id
+            }
+            try:
+                self.event['url'] = reverse('single_thread', kwargs=url_kwargs)
+            except NoReverseMatch:
+                pass
+
+        # Add user's forum and course roles
+        if course_id and user:
+            self.event['user_forums_roles'] = [
+                role.name for role in user.roles.filter(course_id=course_id)
+            ]
+            self.event['user_course_roles'] = [
+                role.role for role in user.courseaccessrole_set.filter(course_id=course_id)
+            ]
+
+        # Add team ID
+        if commentable_id:
+            team = get_team(commentable_id)
+            if team:
+                self.event['team_id'] = team.team_id

lms/djangoapps/django_comment_client/base/views.py

@@ -45,7 +45,7 @@ from django_comment_common.signals import (
     thread_voted
 )
 from django_comment_common.utils import ThreadContext
-from eventtracking import tracker
+import eventtracking
 from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from util.file import store_uploaded_file
 
@@ -82,7 +82,7 @@ def track_forum_event(request, event_name, course, obj, data, id_map=None):
         role.role for role in user.courseaccessrole_set.filter(course_id=course.id)
     ]
 
-    tracker.emit(event_name, data)
+    eventtracking.tracker.emit(event_name, data)
 
 
 def track_created_event(request, event_name, course, obj, data):
@@ -97,7 +97,7 @@ def track_created_event(request, event_name, course, obj, data):
     track_forum_event(request, event_name, course, obj, data)
 
 
-def add_truncated_title_to_event_data(event_data, full_title):
+def add_truncated_title_to_event_data(event_data, full_title):  # pylint: disable=invalid-name
     event_data['title_truncated'] = (len(full_title) > TRACKING_MAX_FORUM_TITLE)
     event_data['title'] = full_title[:TRACKING_MAX_FORUM_TITLE]
 
@@ -158,6 +158,19 @@ def track_voted_event(request, course, obj, vote_value, undo_vote=False):
     track_forum_event(request, event_name, course, obj, event_data)
 
 
+def track_thread_viewed_event(request, course, thread):
+    """
+    Send analytics event for a viewed thread.
+    """
+    event_name = _EVENT_NAME_TEMPLATE.format(obj_type='thread', action_name='viewed')
+    event_data = {}
+    event_data['commentable_id'] = thread.commentable_id
+    if hasattr(thread, 'username'):
+        event_data['target_username'] = thread.username
+    add_truncated_title_to_event_data(event_data, thread.title)
+    track_forum_event(request, event_name, course, thread, event_data)
+
+
 def permitted(func):
     """
     View decorator to verify the user is authorized to access this endpoint.

lms/djangoapps/django_comment_client/utils.py

@@ -130,11 +130,19 @@ def get_accessible_discussion_xblocks(course, user, include_all=False):  # pylin
     Return a list of all valid discussion xblocks in this course that
     are accessible to the given user.
     """
-    all_xblocks = modulestore().get_items(course.id, qualifiers={'category': 'discussion'}, include_orphans=False)
+    return get_accessible_discussion_xblocks_by_course_id(course.id, user, include_all=include_all)
+
+
+def get_accessible_discussion_xblocks_by_course_id(course_id, user, include_all=False):  # pylint: disable=invalid-name
+    """
+    Return a list of all valid discussion xblocks in this course that
+    are accessible to the given user.
+    """
+    all_xblocks = modulestore().get_items(course_id, qualifiers={'category': 'discussion'}, include_orphans=False)
 
     return [
         xblock for xblock in all_xblocks
-        if has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course.id))
+        if has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course_id))
     ]
 
 
@@ -178,19 +186,27 @@ def get_cached_discussion_id_map(course, discussion_ids, user):
     Returns a dict mapping discussion_ids to respective discussion xblock metadata if it is cached and visible to the
     user. If not, returns the result of get_discussion_id_map
     """
+    return get_cached_discussion_id_map_by_course_id(course.id, discussion_ids, user)
+
+
+def get_cached_discussion_id_map_by_course_id(course_id, discussion_ids, user):  # pylint: disable=invalid-name
+    """
+    Returns a dict mapping discussion_ids to respective discussion xblock metadata if it is cached and visible to the
+    user. If not, returns the result of get_discussion_id_map
+    """
     try:
         entries = []
         for discussion_id in discussion_ids:
-            key = get_cached_discussion_key(course.id, discussion_id)
+            key = get_cached_discussion_key(course_id, discussion_id)
             if not key:
                 continue
             xblock = modulestore().get_item(key)
-            if not (has_required_keys(xblock) and has_access(user, 'load', xblock, course.id)):
+            if not (has_required_keys(xblock) and has_access(user, 'load', xblock, course_id)):
                 continue
             entries.append(get_discussion_id_map_entry(xblock))
         return dict(entries)
     except DiscussionIdMapIsNotCached:
-        return get_discussion_id_map(course, user)
+        return get_discussion_id_map_by_course_id(course_id, user)
 
 
 def get_discussion_id_map(course, user):
@@ -198,7 +214,16 @@ def get_discussion_id_map(course, user):
     Transform the list of this course's discussion xblocks (visible to a given user) into a dictionary of metadata keyed
     by discussion_id.
     """
-    return dict(map(get_discussion_id_map_entry, get_accessible_discussion_xblocks(course, user)))
+    return get_discussion_id_map_by_course_id(course.id, user)
+
+
+def get_discussion_id_map_by_course_id(course_id, user):  # pylint: disable=invalid-name
+    """
+    Transform the list of this course's discussion xblocks (visible to a given user) into a dictionary of metadata keyed
+    by discussion_id.
+    """
+    xblocks = get_accessible_discussion_xblocks_by_course_id(course_id, user)
+    return dict(map(get_discussion_id_map_entry, xblocks))
 
 
 def _filter_unstarted_categories(category_map, course):

lms/envs/common.py

@@ -739,7 +739,7 @@ if FEATURES.get('ENABLE_SQL_TRACKING_LOGS'):
 
 TRACKING_SEGMENTIO_WEBHOOK_SECRET = None
 TRACKING_SEGMENTIO_ALLOWED_TYPES = ['track']
-TRACKING_SEGMENTIO_DISALLOWED_SUBSTRING_NAMES = ['.bi.']
+TRACKING_SEGMENTIO_DISALLOWED_SUBSTRING_NAMES = []
 TRACKING_SEGMENTIO_SOURCE_MAP = {
     'analytics-android': 'mobile',
     'analytics-ios': 'mobile',

lms/lib/comment_client/thread.py

@@ -5,15 +5,7 @@ import settings
 import models
 from eventtracking import tracker
 
-from .utils import (
-    CommentClientPaginatedResult,
-    CommentClientRequestError,
-    extract,
-    merge_dict,
-    perform_request,
-    strip_blank,
-    strip_none
-)
+import utils
 
 log = logging.getLogger(__name__)
 
@@ -61,15 +53,15 @@ class Thread(models.Model):
         default_params = {'page': 1,
                           'per_page': 20,
                           'course_id': query_params['course_id']}
-        params = merge_dict(default_params, strip_blank(strip_none(query_params)))
+        params = utils.merge_dict(default_params, utils.strip_blank(utils.strip_none(query_params)))
 
         if query_params.get('text'):
             url = cls.url(action='search')
         else:
-            url = cls.url(action='get_all', params=extract(params, 'commentable_id'))
+            url = cls.url(action='get_all', params=utils.extract(params, 'commentable_id'))
             if params.get('commentable_id'):
                 del params['commentable_id']
-        response = perform_request(
+        response = utils.perform_request(
             'get',
             url,
             params,
@@ -107,7 +99,7 @@ class Thread(models.Model):
                 )
             )
 
-        return CommentClientPaginatedResult(
+        return utils.CommentClientPaginatedResult(
             collection=response.get('collection', []),
             page=response.get('page', 1),
             num_pages=response.get('num_pages', 1),
@@ -149,9 +141,9 @@ class Thread(models.Model):
             'resp_skip': kwargs.get('response_skip'),
             'resp_limit': kwargs.get('response_limit'),
         }
-        request_params = strip_none(request_params)
+        request_params = utils.strip_none(request_params)
 
-        response = perform_request(
+        response = utils.perform_request(
             'get',
             url,
             request_params,
@@ -166,9 +158,9 @@ class Thread(models.Model):
         elif voteable.type == 'comment':
             url = _url_for_flag_comment(voteable.id)
         else:
-            raise CommentClientRequestError("Can only flag/unflag threads or comments")
+            raise utils.CommentClientRequestError("Can only flag/unflag threads or comments")
         params = {'user_id': user.id}
-        response = perform_request(
+        response = utils.perform_request(
             'put',
             url,
             params,
@@ -183,13 +175,13 @@ class Thread(models.Model):
         elif voteable.type == 'comment':
             url = _url_for_unflag_comment(voteable.id)
         else:
-            raise CommentClientRequestError("Can only flag/unflag for threads or comments")
+            raise utils.CommentClientRequestError("Can only flag/unflag for threads or comments")
         params = {'user_id': user.id}
         #if you're an admin, when you unflag, remove ALL flags
         if removeAll:
             params['all'] = True
 
-        response = perform_request(
+        response = utils.perform_request(
             'put',
             url,
             params,
@@ -201,7 +193,7 @@ class Thread(models.Model):
     def pin(self, user, thread_id):
         url = _url_for_pin_thread(thread_id)
         params = {'user_id': user.id}
-        response = perform_request(
+        response = utils.perform_request(
             'put',
             url,
             params,
@@ -213,7 +205,7 @@ class Thread(models.Model):
     def un_pin(self, user, thread_id):
         url = _url_for_un_pin_thread(thread_id)
         params = {'user_id': user.id}
-        response = perform_request(
+        response = utils.perform_request(
             'put',
             url,
             params,

lms/lib/comment_client/user.py

@@ -3,7 +3,7 @@ import settings
 
 import models
 
-from .utils import CommentClientPaginatedResult, CommentClientRequestError, merge_dict, perform_request
+import utils
 
 
 class User(models.Model):
@@ -36,7 +36,7 @@ class User(models.Model):
         Calls cs_comments_service to mark thread as read for the user
         """
         params = {'source_type': source.type, 'source_id': source.id}
-        perform_request(
+        utils.perform_request(
             'post',
             _url_for_read(self.id),
             params,
@@ -46,7 +46,7 @@ class User(models.Model):
 
     def follow(self, source):
         params = {'source_type': source.type, 'source_id': source.id}
-        response = perform_request(
+        response = utils.perform_request(
             'post',
             _url_for_subscription(self.id),
             params,
@@ -56,7 +56,7 @@ class User(models.Model):
 
     def unfollow(self, source):
         params = {'source_type': source.type, 'source_id': source.id}
-        response = perform_request(
+        response = utils.perform_request(
             'delete',
             _url_for_subscription(self.id),
             params,
@@ -70,9 +70,9 @@ class User(models.Model):
         elif voteable.type == 'comment':
             url = _url_for_vote_comment(voteable.id)
         else:
-            raise CommentClientRequestError("Can only vote / unvote for threads or comments")
+            raise utils.CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id, 'value': value}
-        response = perform_request(
+        response = utils.perform_request(
             'put',
             url,
             params,
@@ -87,9 +87,9 @@ class User(models.Model):
         elif voteable.type == 'comment':
             url = _url_for_vote_comment(voteable.id)
         else:
-            raise CommentClientRequestError("Can only vote / unvote for threads or comments")
+            raise utils.CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id}
-        response = perform_request(
+        response = utils.perform_request(
             'delete',
             url,
             params,
@@ -100,11 +100,11 @@ class User(models.Model):
 
     def active_threads(self, query_params={}):
         if not self.course_id:
-            raise CommentClientRequestError("Must provide course_id when retrieving active threads for the user")
+            raise utils.CommentClientRequestError("Must provide course_id when retrieving active threads for the user")
         url = _url_for_user_active_threads(self.id)
         params = {'course_id': self.course_id.to_deprecated_string()}
-        params = merge_dict(params, query_params)
-        response = perform_request(
+        params = utils.merge_dict(params, query_params)
+        response = utils.perform_request(
             'get',
             url,
             params,
@@ -116,11 +116,11 @@ class User(models.Model):
 
     def subscribed_threads(self, query_params={}):
         if not self.course_id:
-            raise CommentClientRequestError("Must provide course_id when retrieving subscribed threads for the user")
+            raise utils.CommentClientRequestError("Must provide course_id when retrieving subscribed threads for the user")
         url = _url_for_user_subscribed_threads(self.id)
         params = {'course_id': self.course_id.to_deprecated_string()}
-        params = merge_dict(params, query_params)
-        response = perform_request(
+        params = utils.merge_dict(params, query_params)
+        response = utils.perform_request(
             'get',
             url,
             params,
@@ -128,7 +128,7 @@ class User(models.Model):
             metric_tags=self._metric_tags,
             paged_results=True
         )
-        return CommentClientPaginatedResult(
+        return utils.CommentClientPaginatedResult(
             collection=response.get('collection', []),
             page=response.get('page', 1),
             num_pages=response.get('num_pages', 1),
@@ -144,19 +144,19 @@ class User(models.Model):
         if self.attributes.get('group_id'):
             retrieve_params['group_id'] = self.group_id
         try:
-            response = perform_request(
+            response = utils.perform_request(
                 'get',
                 url,
                 retrieve_params,
                 metric_action='model.retrieve',
                 metric_tags=self._metric_tags,
             )
-        except CommentClientRequestError as e:
+        except utils.CommentClientRequestError as e:
             if e.status_code == 404:
                 # attempt to gracefully recover from a previous failure
                 # to sync this user to the comments service.
                 self.save()
-                response = perform_request(
+                response = utils.perform_request(
                     'get',
                     url,
                     retrieve_params,