Skip to content

E
edx-platform
Commit 77551089 by Cliff Dyer
Options

Merge pull request #12127 from edx/cdyer/ui-events 

Sequence navigation UI events.
parents 974233f5 001874c4
Showing with 292 additions and 140 deletions
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'}
]
}
},
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment