diff --git a/cms/envs/common.py b/cms/envs/common.py
index 4fc14fdcb6852408035479a6e07d1584fcc396b1..58f831ae77d8b6e2e8684e5cadb9be20feb64ec4 100644
--- a/cms/envs/common.py
+++ b/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'}
]
}
},
diff --git a/common/djangoapps/track/shim.py b/common/djangoapps/track/shim.py
index 311b2cacc1449c6808cd99fd2bad9af726c71aab..76e4a40a9312169fe2836f8742be4f871f633b86 100644
--- a/common/djangoapps/track/shim.py
+++ b/common/djangoapps/track/shim.py
@@ -1,14 +1,10 @@
"""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
diff --git a/common/djangoapps/track/tests/test_shim.py b/common/djangoapps/track/tests/test_shim.py
index f9e0eada98df71509d9134d5edc811a7ee7c53a6..039a0d6cbd18c44289960107931927fd15de7aa1 100644
--- a/common/djangoapps/track/tests/test_shim.py
+++ b/common/djangoapps/track/tests/test_shim.py
@@ -1,10 +1,16 @@
"""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)
diff --git a/common/djangoapps/track/transformers.py b/common/djangoapps/track/transformers.py
new file mode 100644
index 0000000000000000000000000000000000000000..471defe9ce13cd0c9b0a18d1002fd33ff81caaf9
--- /dev/null
+++ b/common/djangoapps/track/transformers.py
@@ -0,0 +1,502 @@
+"""
+EventTransformers are data structures that represents events, and modify those
+events to match the format desired for the tracking logs. They are registered
+by name (or name prefix) in the EventTransformerRegistry, which is used to
+apply them to the appropriate events.
+"""
+
+import json
+import logging
+
+from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import UsageKey
+
+log = logging.getLogger(__name__)
+
+
+class DottedPathMapping(object):
+ """
+ Dictionary-like object for creating keys of dotted paths.
+
+ If a key is created that ends with a dot, it will be treated as a path
+ prefix. Any value whose prefix matches the dotted path can be used
+ as a key for that value, but only the most specific match will
+ be used.
+ """
+
+ # TODO: The current implementation of the prefix registry requires
+ # O(number of prefix event transformers) to access an event. If we get a
+ # large number of EventTransformers, it may be worth writing a tree-based
+ # map structure where each node is a segment of the match key, which would
+ # reduce access time to O(len(match.key.split('.'))), or essentially constant
+ # time.
+
+ def __init__(self, registry=None):
+ self._match_registry = {}
+ self._prefix_registry = {}
+ self.update(registry or {})
+
+ def __contains__(self, key):
+ try:
+ _ = self[key]
+ return True
+ except KeyError:
+ return False
+
+ def __getitem__(self, key):
+ if key in self._match_registry:
+ return self._match_registry[key]
+ if isinstance(key, basestring):
+ # Reverse-sort the keys to find the longest matching prefix.
+ for prefix in sorted(self._prefix_registry, reverse=True):
+ if key.startswith(prefix):
+ return self._prefix_registry[prefix]
+ raise KeyError('Key {} not found in {}'.format(key, type(self)))
+
+ def __setitem__(self, key, value):
+ if key.endswith('.'):
+ self._prefix_registry[key] = value
+ else:
+ self._match_registry[key] = value
+
+ def __delitem__(self, key):
+ if key.endswith('.'):
+ del self._prefix_registry[key]
+ else:
+ del self._match_registry[key]
+
+ def get(self, key, default=None):
+ """
+ Return `self[key]` if it exists, otherwise, return `None` or `default`
+ if it is specified.
+ """
+ try:
+ self[key]
+ except KeyError:
+ return default
+
+ def update(self, dict_):
+ """
+ Update the mapping with the values in the supplied `dict`.
+ """
+ for key, value in dict_:
+ self[key] = value
+
+ def keys(self):
+ """
+ Return the keys of the mapping, including both exact matches and
+ prefix matches.
+ """
+ return self._match_registry.keys() + self._prefix_registry.keys()
+
+
+class EventTransformerRegistry(object):
+ """
+ Registry to track which EventTransformers handle which events. The
+ EventTransformer must define a `match_key` attribute which contains the
+ name or prefix of the event names it tracks. Any `match_key` that ends
+ with a `.` will match all events that share its prefix. A transformer name
+ without a trailing dot only processes exact matches.
+ """
+ mapping = DottedPathMapping()
+
+ @classmethod
+ def register(cls, transformer):
+ """
+ Decorator to register an EventTransformer. It must have a `match_key`
+ class attribute defined.
+ """
+ cls.mapping[transformer.match_key] = transformer
+ return transformer
+
+ @classmethod
+ def create_transformer(cls, event):
+ """
+ Create an EventTransformer of the given event.
+
+ If no transformer is registered to handle the event, this raises a
+ KeyError.
+ """
+ name = event.get(u'name')
+ return cls.mapping[name](event)
+
+
+class EventTransformer(dict):
+ """
+ Creates a transformer to modify analytics events based on event type.
+
+ To use the transformer, instantiate it using the
+ `EventTransformer.create_transformer()` classmethod with the event
+ dictionary as the sole argument, and then call `transformer.transform()` on
+ the created object to modify the event to the format required for output.
+
+ Custom transformers will want to define some or all of the following values
+
+ Attributes:
+
+ match_key:
+ This is the name of the event you want to transform. If the name
+ ends with a `'.'`, it will be treated as a *prefix transformer*.
+ All other names denote *exact transformers*.
+
+ A *prefix transformer* will handle any event whose name begins with
+ the name of the prefix transformer. Only the most specific match
+ will be used, so if a transformer exists with a name of
+ `'edx.ui.lms.'` and another transformer has the name
+ `'edx.ui.lms.sequence.'` then an event called
+ `'edx.ui.lms.sequence.tab_selected'` will be handled by the
+ `'edx.ui.lms.sequence.'` transformer.
+
+ An *exact transformer* will only handle events whose name matches
+ name of the transformer exactly.
+
+ Exact transformers always take precedence over prefix transformers.
+
+ Transformers without a name will not be added to the registry, and
+ cannot be accessed via the `EventTransformer.create_transformer()`
+ classmethod.
+
+ is_legacy_event:
+ If an event is a legacy event, it needs to set event_type to the
+ legacy name for the event, and may need to set certain event fields
+ to maintain backward compatiblity. If an event needs to provide
+ legacy support in some contexts, `is_legacy_event` can be defined
+ as a property to add dynamic behavior.
+
+ Default: False
+
+ legacy_event_type:
+ If the event is or can be a legacy event, it should define
+ the legacy value for the event_type field here.
+
+ Processing methods. Override these to provide the behavior needed for your
+ particular EventTransformer:
+
+ self.process_legacy_fields():
+ This method should modify the event payload in any way necessary to
+ support legacy event types. It will only be run if
+ `is_legacy_event` returns a True value.
+
+ self.process_event()
+ This method modifies the event payload unconditionally. It will
+ always be run.
+ """
+ def __init__(self, *args, **kwargs):
+ super(EventTransformer, self).__init__(*args, **kwargs)
+ self.load_payload()
+
+ # Properties to be overridden
+
+ is_legacy_event = False
+
+ @property
+ def legacy_event_type(self):
+ """
+ Override this as an attribute or property to provide the value for
+ the event's `event_type`, if it does not match the event's `name`.
+ """
+ raise NotImplementedError
+
+ # Convenience properties
+
+ @property
+ def name(self):
+ """
+ Returns the event's name.
+ """
+ return self[u'name']
+
+ @property
+ def context(self):
+ """
+ Returns the event's context dict.
+ """
+ return self.get(u'context', {})
+
+ # Transform methods
+
+ def load_payload(self):
+ """
+ Create a data version of self[u'event'] at self.event
+ """
+ if u'event' in self:
+ if isinstance(self[u'event'], basestring):
+ self.event = json.loads(self[u'event'])
+ else:
+ self.event = self[u'event']
+
+ def dump_payload(self):
+ """
+ Write self.event back to self[u'event'].
+
+ Keep the same format we were originally given.
+ """
+ if isinstance(self.get(u'event'), basestring):
+ self[u'event'] = json.dumps(self.event)
+ else:
+ self[u'event'] = self.event
+
+ def transform(self):
+ """
+ Transform the event with legacy fields and other necessary
+ modifications.
+ """
+ if self.is_legacy_event:
+ self._set_legacy_event_type()
+ self.process_legacy_fields()
+ self.process_event()
+ self.dump_payload()
+
+ def _set_legacy_event_type(self):
+ """
+ Update the event's `event_type` to the value specified by
+ `self.legacy_event_type`.
+ """
+ self['event_type'] = self.legacy_event_type
+
+ def process_legacy_fields(self):
+ """
+ Override this method to specify how to update event fields to maintain
+ compatibility with legacy events.
+ """
+ pass
+
+ def process_event(self):
+ """
+ Override this method to make unconditional modifications to event
+ fields.
+ """
+ pass
+
+
+@EventTransformerRegistry.register
+class SequenceTabSelectedEventTransformer(EventTransformer):
+ """
+ Transformer to maintain backward compatiblity with seq_goto events.
+ """
+
+ match_key = u'edx.ui.lms.sequence.tab_selected'
+ is_legacy_event = True
+ legacy_event_type = u'seq_goto'
+
+ def process_legacy_fields(self):
+ self.event[u'old'] = self.event[u'current_tab']
+ self.event[u'new'] = self.event[u'target_tab']
+
+
+class _BaseLinearSequenceEventTransformer(EventTransformer):
+ """
+ Common functionality for transforming
+ `edx.ui.lms.sequence.{next,previous}_selected` events.
+ """
+
+ offset = None
+
+ @property
+ def is_legacy_event(self):
+ """
+ Linear sequence events are legacy events if the origin and target lie
+ within the same sequence.
+ """
+ return not self.crosses_boundary()
+
+ def process_legacy_fields(self):
+ """
+ Set legacy payload fields:
+ old: equal to the new current_tab field
+ new: the tab to which the user is navigating
+ """
+ self.event[u'old'] = self.event[u'current_tab']
+ self.event[u'new'] = self.event[u'current_tab'] + self.offset
+
+ def crosses_boundary(self):
+ """
+ Returns true if the navigation takes the focus out of the current
+ sequence.
+ """
+ raise NotImplementedError
+
+
+@EventTransformerRegistry.register
+class NextSelectedEventTransformer(_BaseLinearSequenceEventTransformer):
+ """
+ Transformer to maintain backward compatiblity with seq_next events.
+ """
+
+ match_key = u'edx.ui.lms.sequence.next_selected'
+ offset = 1
+ legacy_event_type = u'seq_next'
+
+ def crosses_boundary(self):
+ """
+ Returns true if the navigation moves the focus to the next sequence.
+ """
+ return self.event[u'current_tab'] == self.event[u'tab_count']
+
+
+@EventTransformerRegistry.register
+class PreviousSelectedEventTransformer(_BaseLinearSequenceEventTransformer):
+ """
+ Transformer to maintain backward compatiblity with seq_prev events.
+ """
+
+ match_key = u'edx.ui.lms.sequence.previous_selected'
+ offset = -1
+ legacy_event_type = u'seq_prev'
+
+ def crosses_boundary(self):
+ """
+ Returns true if the navigation moves the focus to the previous
+ sequence.
+ """
+ return self.event[u'current_tab'] == 1
+
+
+@EventTransformerRegistry.register
+class VideoEventTransformer(EventTransformer):
+ """
+ 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 transformer here
+ that converts the events they *can* easily emit and converts them into the
+ legacy format.
+ """
+ match_key = u'edx.video.'
+
+ name_to_event_type_map = {
+ u'edx.video.played': u'play_video',
+ u'edx.video.paused': u'pause_video',
+ u'edx.video.stopped': u'stop_video',
+ u'edx.video.loaded': u'load_video',
+ u'edx.video.position.changed': u'seek_video',
+ u'edx.video.seeked': u'seek_video',
+ u'edx.video.transcript.shown': u'show_transcript',
+ u'edx.video.transcript.hidden': u'hide_transcript',
+ }
+
+ is_legacy_event = True
+
+ @property
+ def legacy_event_type(self):
+ """
+ Return the legacy event_type of the current event
+ """
+ return self.name_to_event_type_map[self.name]
+
+ def transform(self):
+ """
+ Transform the event with necessary modifications if it is one of the
+ expected types of events.
+ """
+ if self.name in self.name_to_event_type_map:
+ super(VideoEventTransformer, self).transform()
+
+ def process_event(self):
+ """
+ Modify event fields.
+ """
+
+ # Convert edx.video.seeked to edx.video.position.changed because edx.video.seeked was not intended to actually
+ # ever be emitted.
+ if self.name == "edx.video.seeked":
+ self['name'] = "edx.video.position.changed"
+
+ if not self.event:
+ return
+
+ self.set_id_to_usage_key()
+ self.capcase_current_time()
+
+ self.convert_seek_type()
+ self.disambiguate_skip_and_seek()
+ self.set_page_to_browser_url()
+ self.handle_ios_seek_bug()
+
+ def set_id_to_usage_key(self):
+ """
+ Validate that the module_id is a valid usage key, and set the id field
+ accordingly.
+ """
+ if 'module_id' in self.event:
+ module_id = self.event['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:
+ self.event['id'] = usage_key.html_id()
+
+ del self.event['module_id']
+
+ def capcase_current_time(self):
+ """
+ Convert the current_time field to currentTime.
+ """
+ if 'current_time' in self.event:
+ self.event['currentTime'] = self.event.pop('current_time')
+
+ def convert_seek_type(self):
+ """
+ Converts seek_type to seek and converts skip|slide to
+ onSlideSeek|onSkipSeek.
+ """
+ if 'seek_type' in self.event:
+ seek_type = self.event['seek_type']
+ if seek_type == 'slide':
+ self.event['type'] = "onSlideSeek"
+ elif seek_type == 'skip':
+ self.event['type'] = "onSkipSeek"
+ del self.event['seek_type']
+
+ def disambiguate_skip_and_seek(self):
+ """
+ For the Android build that isn't distinguishing between skip/seek.
+ """
+ if 'requested_skip_interval' in self.event:
+ if abs(self.event['requested_skip_interval']) != 30:
+ if 'type' in self.event:
+ self.event['type'] = 'onSlideSeek'
+
+ def set_page_to_browser_url(self):
+ """
+ If `open_in_browser_url` is specified, set the page to the base of the
+ specified url.
+ """
+ if 'open_in_browser_url' in self.context:
+ self['page'] = self.context.pop('open_in_browser_url').rpartition('/')[0]
+
+ def handle_ios_seek_bug(self):
+ """
+ Handle seek bug in iOS.
+
+ iOS build 1.0.02 has a bug where it returns a +30 second skip when
+ it should be returning -30.
+ """
+ if self._build_requests_plus_30_for_minus_30():
+ if self._user_requested_plus_30_skip():
+ self.event[u'requested_skip_interval'] = -30
+
+ def _build_requests_plus_30_for_minus_30(self):
+ """
+ Returns True if this build contains the seek bug
+ """
+ if u'application' in self.context:
+ if all(key in self.context[u'application'] for key in (u'version', u'name')):
+ app_version = self.context[u'application'][u'version']
+ app_name = self.context[u'application'][u'name']
+ return app_version == u'1.0.02' and app_name == u'edx.mobileapp.iOS'
+ return False
+
+ def _user_requested_plus_30_skip(self):
+ """
+ If the user requested a +30 second skip, return True.
+ """
+
+ if u'requested_skip_interval' in self.event and u'type' in self.event:
+ interval = self.event[u'requested_skip_interval']
+ action = self.event[u'type']
+ return interval == 30 and action == u'onSkipSeek'
+ else:
+ return False
diff --git a/common/djangoapps/track/views/tests/test_segmentio.py b/common/djangoapps/track/views/tests/test_segmentio.py
index 2ee9641e80f51e36a9314689f77436179e160c5f..dfb009282a9a1c182ab027cd2851fcde865e5e9c 100644
--- a/common/djangoapps/track/views/tests/test_segmentio.py
+++ b/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')
diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee
index 574b67d7911e32e534f5253cea803529c0ef34f0..fd976a4b84f588e5914c6373d99cd165abb9399c 100644
--- a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee
+++ b/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) ->
diff --git a/common/test/acceptance/tests/lms/test_lms_courseware.py b/common/test/acceptance/tests/lms/test_lms_courseware.py
index 4f551f378f7456f77887ec52c33f3671d7814e98..91d23f7d28033c0193167f7a6f8d70851623c080 100644
--- a/common/test/acceptance/tests/lms/test_lms_courseware.py
+++ b/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
):
diff --git a/lms/envs/common.py b/lms/envs/common.py
index 8da0003274e6b88cc8fb8ca4410321dc4f533803..e6a1d7889f06a975de9a700afe06a82ad3f78286 100644
--- a/lms/envs/common.py
+++ b/lms/envs/common.py
@@ -625,7 +625,7 @@ EVENT_TRACKING_BACKENDS = {
},
'processors': [
{'ENGINE': 'track.shim.LegacyFieldMappingProcessor'},
- {'ENGINE': 'track.shim.VideoEventProcessor'}
+ {'ENGINE': 'track.shim.PrefixedEventProcessor'}
]
}
},