Update seq_* to edx.ui.lms.sequence.* format

seq_next, seq_prev, and seq_goto events are all renamed, and maintain
legacy compatibility.

This PR also introduces new EventTransformer framework to shim events
based on name prefix.

MA-2221

cms/envs/common.py

@@ -685,8 +685,11 @@ REQUIRE_DEBUG = False
 REQUIRE_EXCLUDE = ("build.txt",)
 
 # The execution environment in which to run r.js: auto, node or rhino.
-# auto will autodetect the environment and make use of node if available and rhino if not.
-# It can also be a path to a custom class that subclasses require.environments.Environment and defines some "args" function that returns a list with the command arguments to execute.
+# auto will autodetect the environment and make use of node if available and
+# rhino if not.
+# It can also be a path to a custom class that subclasses
+# require.environments.Environment and defines some "args" function that
+# returns a list with the command arguments to execute.
 REQUIRE_ENVIRONMENT = "node"
 
 
@@ -957,7 +960,7 @@ EVENT_TRACKING_BACKENDS = {
             },
             'processors': [
                 {'ENGINE': 'track.shim.LegacyFieldMappingProcessor'},
-                {'ENGINE': 'track.shim.VideoEventProcessor'}
+                {'ENGINE': 'track.shim.PrefixedEventProcessor'}
             ]
         }
     },

common/djangoapps/track/shim.py

 """Map new event context values to old top-level field values. Ensures events can be parsed by legacy parsers."""
 
 import json
-import logging
 
-from opaque_keys import InvalidKeyError
-from opaque_keys.edx.keys import UsageKey
+from .transformers import EventTransformerRegistry
 
 
-log = logging.getLogger(__name__)
-
 CONTEXT_FIELDS_TO_INCLUDE = [
     'username',
     'session',
@@ -63,6 +59,9 @@ class LegacyFieldMappingProcessor(object):
 
 
 def remove_shim_context(event):
+    """
+    Remove obsolete fields from event context.
+    """
     if 'context' in event:
         context = event['context']
         # These fields are present elsewhere in the event at this point
@@ -74,100 +73,6 @@ def remove_shim_context(event):
                 del context[field]
 
 
-NAME_TO_EVENT_TYPE_MAP = {
-    'edx.video.played': 'play_video',
-    'edx.video.paused': 'pause_video',
-    'edx.video.stopped': 'stop_video',
-    'edx.video.loaded': 'load_video',
-    'edx.video.position.changed': 'seek_video',
-    'edx.video.seeked': 'seek_video',
-    'edx.video.transcript.shown': 'show_transcript',
-    'edx.video.transcript.hidden': 'hide_transcript',
-}
-
-
-class VideoEventProcessor(object):
-    """
-    Converts new format video events into the legacy video event format.
-
-    Mobile devices cannot actually emit events that exactly match their counterparts emitted by the LMS javascript
-    video player. Instead of attempting to get them to do that, we instead insert a shim here that converts the events
-    they *can* easily emit and converts them into the legacy format.
-
-    TODO: Remove this shim and perform the conversion as part of some batch canonicalization process.
-
-    """
-
-    def __call__(self, event):
-        name = event.get('name')
-        if not name:
-            return
-
-        if name not in NAME_TO_EVENT_TYPE_MAP:
-            return
-
-        # Convert edx.video.seeked to edx.video.position.changed because edx.video.seeked was not intended to actually
-        # ever be emitted.
-        if name == "edx.video.seeked":
-            event['name'] = "edx.video.position.changed"
-
-        event['event_type'] = NAME_TO_EVENT_TYPE_MAP[name]
-
-        if 'event' not in event:
-            return
-        payload = event['event']
-
-        if 'module_id' in payload:
-            module_id = payload['module_id']
-            try:
-                usage_key = UsageKey.from_string(module_id)
-            except InvalidKeyError:
-                log.warning('Unable to parse module_id "%s"', module_id, exc_info=True)
-            else:
-                payload['id'] = usage_key.html_id()
-
-            del payload['module_id']
-
-        if 'current_time' in payload:
-            payload['currentTime'] = payload.pop('current_time')
-
-        if 'context' in event:
-            context = event['context']
-
-            # Converts seek_type to seek and skip|slide to onSlideSeek|onSkipSeek
-            if 'seek_type' in payload:
-                seek_type = payload['seek_type']
-                if seek_type == 'slide':
-                    payload['type'] = "onSlideSeek"
-                elif seek_type == 'skip':
-                    payload['type'] = "onSkipSeek"
-                del payload['seek_type']
-
-            # For the iOS build that is returning a +30 for back skip 30
-            if (
-                context['application']['version'] == "1.0.02" and
-                context['application']['name'] == "edx.mobileapp.iOS"
-            ):
-                if 'requested_skip_interval' in payload and 'type' in payload:
-                    if (
-                        payload['requested_skip_interval'] == 30 and
-                        payload['type'] == "onSkipSeek"
-                    ):
-                        payload['requested_skip_interval'] = -30
-
-            # For the Android build that isn't distinguishing between skip/seek
-            if 'requested_skip_interval' in payload:
-                if abs(payload['requested_skip_interval']) != 30:
-                    if 'type' in payload:
-                        payload['type'] = 'onSlideSeek'
-
-            if 'open_in_browser_url' in context:
-                page, _sep, _tail = context.pop('open_in_browser_url').rpartition('/')
-                event['page'] = page
-
-        event['event'] = json.dumps(payload)
-
-
 class GoogleAnalyticsProcessor(object):
     """Adds course_id as label, and sets nonInteraction property"""
 
@@ -184,3 +89,22 @@ class GoogleAnalyticsProcessor(object):
         copied_event['nonInteraction'] = 1
 
         return copied_event
+
+
+class PrefixedEventProcessor(object):
+    """
+    Process any events whose name or prefix (ending with a '.') is registered
+    as an EventTransformer.
+    """
+
+    def __call__(self, event):
+        """
+        If the event is registered with the EventTransformerRegistry, transform
+        it.  Otherwise do nothing to it, and continue processing.
+        """
+        try:
+            event = EventTransformerRegistry.create_transformer(event)
+        except KeyError:
+            return
+        event.transform()
+        return event

common/djangoapps/track/tests/test_shim.py

 """Ensure emitted events contain the fields legacy processors expect to find."""
 
+from collections import namedtuple
+
+import ddt
 from mock import sentinel
 from django.test.utils import override_settings
 
 from openedx.core.lib.tests.assertions.events import assert_events_equal
-from track.tests import EventTrackingTestCase, FROZEN_TIME
+
+from . import EventTrackingTestCase, FROZEN_TIME
+from ..shim import PrefixedEventProcessor
+from .. import transformers
 
 
 LEGACY_SHIM_PROCESSOR = [
@@ -216,3 +222,100 @@ class MultipleShimGoogleAnalyticsProcessorTestCase(EventTrackingTestCase):
             'timestamp': FROZEN_TIME,
         }
         assert_events_equal(expected_event, log_emitted_event)
+
+
+SequenceDDT = namedtuple('SequenceDDT', ['action', 'tab_count', 'current_tab', 'legacy_event_type'])
+
+
+@ddt.ddt
+class EventTransformerRegistryTestCase(EventTrackingTestCase):
+    """
+    Test the behavior of the event registry
+    """
+
+    def setUp(self):
+        super(EventTransformerRegistryTestCase, self).setUp()
+        self.registry = transformers.EventTransformerRegistry()
+
+    @ddt.data(
+        ('edx.ui.lms.sequence.next_selected', transformers.NextSelectedEventTransformer),
+        ('edx.ui.lms.sequence.previous_selected', transformers.PreviousSelectedEventTransformer),
+        ('edx.ui.lms.sequence.tab_selected', transformers.SequenceTabSelectedEventTransformer),
+        ('edx.video.foo.bar', transformers.VideoEventTransformer),
+    )
+    @ddt.unpack
+    def test_event_registry_dispatch(self, event_name, expected_transformer):
+        event = {'name': event_name}
+        transformer = self.registry.create_transformer(event)
+        self.assertIsInstance(transformer, expected_transformer)
+
+    @ddt.data(
+        'edx.ui.lms.sequence.next_selected.what',
+        'edx',
+        'unregistered_event',
+    )
+    def test_dispatch_to_nonexistent_events(self, event_name):
+        event = {'name': event_name}
+        with self.assertRaises(KeyError):
+            self.registry.create_transformer(event)
+
+
+@ddt.ddt
+class PrefixedEventProcessorTestCase(EventTrackingTestCase):
+    """
+    Test PrefixedEventProcessor
+    """
+
+    @ddt.data(
+        SequenceDDT(action=u'next', tab_count=5, current_tab=3, legacy_event_type=u'seq_next'),
+        SequenceDDT(action=u'next', tab_count=5, current_tab=5, legacy_event_type=None),
+        SequenceDDT(action=u'previous', tab_count=5, current_tab=3, legacy_event_type=u'seq_prev'),
+        SequenceDDT(action=u'previous', tab_count=5, current_tab=1, legacy_event_type=None),
+    )
+    def test_sequence_linear_navigation(self, sequence_ddt):
+        event_name = u'edx.ui.lms.sequence.{}_selected'.format(sequence_ddt.action)
+
+        event = {
+            u'name': event_name,
+            u'event': {
+                u'current_tab': sequence_ddt.current_tab,
+                u'tab_count': sequence_ddt.tab_count,
+                u'id': u'ABCDEFG',
+            }
+        }
+
+        process_event_shim = PrefixedEventProcessor()
+        result = process_event_shim(event)
+
+        # Legacy fields get added when needed
+        if sequence_ddt.action == u'next':
+            offset = 1
+        else:
+            offset = -1
+        if sequence_ddt.legacy_event_type:
+            self.assertEqual(result[u'event_type'], sequence_ddt.legacy_event_type)
+            self.assertEqual(result[u'event'][u'old'], sequence_ddt.current_tab)
+            self.assertEqual(result[u'event'][u'new'], sequence_ddt.current_tab + offset)
+        else:
+            self.assertNotIn(u'event_type', result)
+            self.assertNotIn(u'old', result[u'event'])
+            self.assertNotIn(u'new', result[u'event'])
+
+    def test_sequence_tab_navigation(self):
+        event_name = u'edx.ui.lms.sequence.tab_selected'
+        event = {
+            u'name': event_name,
+            u'event': {
+                u'current_tab': 2,
+                u'target_tab': 5,
+                u'tab_count': 9,
+                u'id': u'block-v1:abc',
+                u'widget_placement': u'top',
+            }
+        }
+
+        process_event_shim = PrefixedEventProcessor()
+        result = process_event_shim(event)
+        self.assertEqual(result[u'event_type'], u'seq_goto')
+        self.assertEqual(result[u'event'][u'old'], 2)
+        self.assertEqual(result[u'event'][u'new'], 5)

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

@@ -21,12 +21,8 @@ ENDPOINT = '/segmentio/test/event'
 USER_ID = 10
 
 MOBILE_SHIM_PROCESSOR = [
-    {
-        'ENGINE': 'track.shim.LegacyFieldMappingProcessor'
-    },
-    {
-        'ENGINE': 'track.shim.VideoEventProcessor'
-    }
+    {'ENGINE': 'track.shim.LegacyFieldMappingProcessor'},
+    {'ENGINE': 'track.shim.PrefixedEventProcessor'},
 ]
 
 
@@ -411,19 +407,29 @@ class SegmentIOTrackingTestCase(EventTrackingTestCase):
         assert_event_matches(expected_event, actual_event)
 
     @data(
-        # Verify positive slide case. Verify slide to onSlideSeek. Verify edx.video.seeked emitted from iOS v1.0.02 is changed to edx.video.position.changed.
+        # Verify positive slide case. Verify slide to onSlideSeek. Verify
+        # edx.video.seeked emitted from iOS v1.0.02 is changed to
+        # edx.video.position.changed.
         (1, 1, "seek_type", "slide", "onSlideSeek", "edx.video.seeked", "edx.video.position.changed", 'edx.mobileapp.iOS', '1.0.02'),
-        # Verify negative slide case. Verify slide to onSlideSeek. Verify edx.video.seeked to edx.video.position.changed.
+        # Verify negative slide case. Verify slide to onSlideSeek. Verify
+        # edx.video.seeked to edx.video.position.changed.
         (-2, -2, "seek_type", "slide", "onSlideSeek", "edx.video.seeked", "edx.video.position.changed", 'edx.mobileapp.iOS', '1.0.02'),
-        # Verify +30 is changed to -30 which is incorrectly emitted in iOS v1.0.02. Verify skip to onSkipSeek
+        # Verify +30 is changed to -30 which is incorrectly emitted in iOS
+        # v1.0.02. Verify skip to onSkipSeek
         (30, -30, "seek_type", "skip", "onSkipSeek", "edx.video.position.changed", "edx.video.position.changed", 'edx.mobileapp.iOS', '1.0.02'),
-        # Verify the correct case of -30 is also handled as well. Verify skip to onSkipSeek
+        # Verify the correct case of -30 is also handled as well. Verify skip
+        # to onSkipSeek
         (-30, -30, "seek_type", "skip", "onSkipSeek", "edx.video.position.changed", "edx.video.position.changed", 'edx.mobileapp.iOS', '1.0.02'),
-        # Verify positive slide case where onSkipSeek is changed to onSlideSkip. Verify edx.video.seeked emitted from Android v1.0.02 is changed to edx.video.position.changed.
+        # Verify positive slide case where onSkipSeek is changed to
+        # onSlideSkip. Verify edx.video.seeked emitted from Android v1.0.02 is
+        # changed to edx.video.position.changed.
         (1, 1, "type", "onSkipSeek", "onSlideSeek", "edx.video.seeked", "edx.video.position.changed", 'edx.mobileapp.android', '1.0.02'),
-        # Verify positive slide case where onSkipSeek is changed to onSlideSkip. Verify edx.video.seeked emitted from Android v1.0.02 is changed to edx.video.position.changed.
+        # Verify positive slide case where onSkipSeek is changed to
+        # onSlideSkip. Verify edx.video.seeked emitted from Android v1.0.02 is
+        # changed to edx.video.position.changed.
         (-2, -2, "type", "onSkipSeek", "onSlideSeek", "edx.video.seeked", "edx.video.position.changed", 'edx.mobileapp.android', '1.0.02'),
-        # Verify positive skip case where onSkipSeek is not changed and does not become negative.
+        # Verify positive skip case where onSkipSeek is not changed and does
+        # not become negative.
         (30, 30, "type", "onSkipSeek", "onSkipSeek", "edx.video.position.changed", "edx.video.position.changed", 'edx.mobileapp.android', '1.0.02'),
         # Verify positive skip case where onSkipSeek is not changed.
         (-30, -30, "type", "onSkipSeek", "onSkipSeek", "edx.video.position.changed", "edx.video.position.changed", 'edx.mobileapp.android', '1.0.02')

common/lib/xmodule/xmodule/js/src/sequence/display.coffee

@@ -132,14 +132,26 @@ class @Sequence
     @$('.sequence-nav-button').unbind('click')
 
     # previous button
-    first_tab = @position == 1
+    is_first_tab = @position == 1
     previous_button_class = '.sequence-nav-button.button-previous'
-    @updateButtonState(previous_button_class, @previous, 'Previous', first_tab, @prevUrl)
+    @updateButtonState(
+      previous_button_class,  # bound element
+      @selectPrevious,  # action
+      'Previous',  # label prefix
+      is_first_tab,  # is boundary?
+      @prevUrl  # boundary_url
+    )
 
     # next button
-    last_tab = @position >= @contents.length  # use inequality in case contents.length is 0 and position is 1.
+    is_last_tab = @position >= @contents.length  # use inequality in case contents.length is 0 and position is 1.
     next_button_class = '.sequence-nav-button.button-next'
-    @updateButtonState(next_button_class, @next, 'Next', last_tab, @nextUrl)
+    @updateButtonState(
+      next_button_class,  # bound element
+      @selectNext,  # action
+      'Next',  # label prefix
+      is_last_tab,  # is boundary?
+      @nextUrl  # boundary_url
+    )
 
   render: (new_position) ->
     if @position != new_position
@@ -180,7 +192,7 @@ class @Sequence
 
       @el.find('.path').text(@el.find('.nav-item.active').data('path'))
 
-      @sr_container.focus();
+      @sr_container.focus()
 
   goto: (event) =>
     event.preventDefault()
@@ -190,7 +202,17 @@ class @Sequence
       new_position = $(event.currentTarget).data('element')
 
     if (1 <= new_position) and (new_position <= @num_contents)
-      Logger.log "seq_goto", old: @position, new: new_position, id: @id
+      is_bottom_nav = $(event.target).closest('nav[class="sequence-bottom"]').length > 0
+      if is_bottom_nav
+        widget_placement = 'bottom'
+      else
+        widget_placement = 'top'
+      Logger.log "edx.ui.lms.sequence.tab_selected",  # Formerly known as seq_goto
+        current_tab: @position
+        target_tab: new_position
+        tab_count: @num_contents
+        id: @id
+        widget_placement: widget_placement
 
       # On Sequence change, destroy any existing polling thread
       # for queued submissions, see ../capa/display.coffee
@@ -204,32 +226,43 @@ class @Sequence
       alert_text = interpolate(alert_template, {tab_name: new_position}, true)
       alert alert_text
 
-  next: (event) => @_change_sequential 'seq_next', event
-  previous: (event) => @_change_sequential 'seq_prev', event
+  selectNext: (event) => @_change_sequential 'next', event
 
-  # `direction` can be 'seq_prev' or 'seq_next'
+  selectPrevious: (event) => @_change_sequential 'previous', event
+
+  # `direction` can be 'previous' or 'next'
   _change_sequential: (direction, event) =>
     # silently abort if direction is invalid.
-    return unless direction in ['seq_prev', 'seq_next']
+    return unless direction in ['previous', 'next']
 
     event.preventDefault()
-    offset =
-      seq_next: 1
-      seq_prev: -1
-    new_position = @position + offset[direction]
-    Logger.log direction,
-      old: @position
-      new: new_position
+
+    analytics_event_name = "edx.ui.lms.sequence.#{direction}_selected"
+    is_bottom_nav = $(event.target).closest('nav[class="sequence-bottom"]').length > 0
+
+    if is_bottom_nav
+      widget_placement = 'bottom'
+    else
+      widget_placement = 'top'
+
+    Logger.log analytics_event_name,  # Formerly known as seq_next and seq_prev
       id: @id
+      current_tab: @position
+      tab_count: @num_contents
+      widget_placement: widget_placement
 
-    if (direction == "seq_next") and (@position == @contents.length)
+    if (direction == 'next') and (@position == @contents.length)
       window.location.href = @nextUrl
-    else if (direction == "seq_prev") and (@position == 1)
+    else if (direction == 'previous') and (@position == 1)
       window.location.href = @prevUrl
     else
       # If the bottom nav is used, scroll to the top of the page on change.
-      if $(event.target).closest('nav[class="sequence-bottom"]').length > 0
+      if is_bottom_nav 
         $.scrollTo 0, 150
+      offset =
+        next: 1
+        previous: -1
+      new_position = @position + offset[direction]
       @render new_position
 
   link_for: (position) ->

common/test/acceptance/tests/lms/test_lms_courseware.py

@@ -2,10 +2,12 @@
 """
 End-to-end tests for the LMS.
 """
+
+import json
 from nose.plugins.attrib import attr
 
-from ..helpers import UniqueCourseTest
 from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
+from ..helpers import UniqueCourseTest, EventsTestMixin
 from ...pages.studio.auto_auth import AutoAuthPage
 from ...pages.lms.create_mode import ModeCreationPage
 from ...pages.studio.overview import CourseOutlinePage
@@ -66,7 +68,7 @@ class CoursewareTest(UniqueCourseTest):
         Open problem page with assertion.
         """
         self.courseware_page.visit()
-        self.problem_page = ProblemPage(self.browser)
+        self.problem_page = ProblemPage(self.browser)  # pylint: disable=attribute-defined-outside-init
         self.assertEqual(self.problem_page.problem_name, 'Test Problem 1')
 
     def _create_breadcrumb(self, index):
@@ -394,7 +396,7 @@ class ProctoredExamTest(UniqueCourseTest):
         self.assertTrue(self.course_outline.time_allotted_field_visible())
 
 
-class CoursewareMultipleVerticalsTest(UniqueCourseTest):
+class CoursewareMultipleVerticalsTest(UniqueCourseTest, EventsTestMixin):
     """
     Test courseware with multiple verticals
     """
@@ -476,6 +478,87 @@ class CoursewareMultipleVerticalsTest(UniqueCourseTest):
         self.courseware_page.click_previous_button_on_bottom()
         self.assert_navigation_state('Test Section 1', 'Test Subsection 1,1', 2, next_enabled=True, prev_enabled=True)
 
+        # test UI events emitted by navigation
+        filter_sequence_ui_event = lambda event: event.get('name', '').startswith('edx.ui.lms.sequence.')
+
+        sequence_ui_events = self.wait_for_events(event_filter=filter_sequence_ui_event, timeout=2)
+        legacy_events = [ev for ev in sequence_ui_events if ev['event_type'] in {'seq_next', 'seq_prev', 'seq_goto'}]
+        nonlegacy_events = [ev for ev in sequence_ui_events if ev not in legacy_events]
+
+        self.assertTrue(all('old' in json.loads(ev['event']) for ev in legacy_events))
+        self.assertTrue(all('new' in json.loads(ev['event']) for ev in legacy_events))
+        self.assertFalse(any('old' in json.loads(ev['event']) for ev in nonlegacy_events))
+        self.assertFalse(any('new' in json.loads(ev['event']) for ev in nonlegacy_events))
+
+        self.assert_events_match(
+            [
+                {
+                    'event_type': 'seq_next',
+                    'event': {
+                        'old': 1,
+                        'new': 2,
+                        'current_tab': 1,
+                        'tab_count': 4,
+                        'widget_placement': 'top',
+                    }
+                },
+                {
+                    'event_type': 'seq_goto',
+                    'event': {
+                        'old': 2,
+                        'new': 4,
+                        'current_tab': 2,
+                        'target_tab': 4,
+                        'tab_count': 4,
+                        'widget_placement': 'top',
+                    }
+                },
+                {
+                    'event_type': 'edx.ui.lms.sequence.next_selected',
+                    'event': {
+                        'current_tab': 4,
+                        'tab_count': 4,
+                        'widget_placement': 'bottom',
+                    }
+                },
+                {
+                    'event_type': 'edx.ui.lms.sequence.next_selected',
+                    'event': {
+                        'current_tab': 1,
+                        'tab_count': 1,
+                        'widget_placement': 'top',
+                    }
+                },
+                {
+                    'event_type': 'edx.ui.lms.sequence.previous_selected',
+                    'event': {
+                        'current_tab': 1,
+                        'tab_count': 1,
+                        'widget_placement': 'top',
+                    }
+                },
+                {
+                    'event_type': 'edx.ui.lms.sequence.previous_selected',
+                    'event': {
+                        'current_tab': 1,
+                        'tab_count': 1,
+                        'widget_placement': 'bottom',
+                    }
+                },
+                {
+                    'event_type': 'seq_prev',
+                    'event': {
+                        'old': 4,
+                        'new': 3,
+                        'current_tab': 4,
+                        'tab_count': 4,
+                        'widget_placement': 'bottom',
+                    }
+                },
+            ],
+            sequence_ui_events
+        )
+
     def assert_navigation_state(
             self, section_title, subsection_title, subsection_position, next_enabled, prev_enabled
     ):

lms/envs/common.py

@@ -625,7 +625,7 @@ EVENT_TRACKING_BACKENDS = {
             },
             'processors': [
                 {'ENGINE': 'track.shim.LegacyFieldMappingProcessor'},
-                {'ENGINE': 'track.shim.VideoEventProcessor'}
+                {'ENGINE': 'track.shim.PrefixedEventProcessor'}
             ]
         }
     },