From 4946b6b296be943b9830f8543a71ee2a21985f20 Mon Sep 17 00:00:00 2001 From: "J. Cliff Dyer" Date: Thu, 7 Dec 2017 14:26:35 -0500 Subject: [PATCH] Track viewing of individual blocks. * Implement a ViewedEvent handling system which calls handlers when a block has been viewed for 5 seconds (configurable). * Hook up Verticals to register their children blocks with this event, and submit completions once seen. OSPR-2093 OC-3358 --- .../public/js/vertical_student_view.js | 73 ++++--- .../xmodule/xmodule/library_content_module.py | 1 + .../xmodule/xmodule/tests/test_vertical.py | 11 ++ common/lib/xmodule/xmodule/vertical_block.py | 18 +- lms/envs/aws.py | 2 + lms/envs/common.py | 1 + lms/static/completion/js/.eslintrc.js | 7 + lms/static/completion/js/ViewedEvent.js | 182 ++++++++++++++++++ .../completion/js/spec/ViewedEvent_spec.js | 95 +++++++++ lms/static/karma_lms.conf.js | 4 +- lms/static/lms/js/spec/main.js | 1 + lms/templates/vert_module.html | 6 +- requirements/edx/base.txt | 2 +- webpack.common.config.js | 1 + 14 files changed, 368 insertions(+), 36 deletions(-) create mode 100644 lms/static/completion/js/.eslintrc.js create mode 100644 lms/static/completion/js/ViewedEvent.js create mode 100644 lms/static/completion/js/spec/ViewedEvent_spec.js diff --git a/common/lib/xmodule/xmodule/assets/vertical/public/js/vertical_student_view.js b/common/lib/xmodule/xmodule/assets/vertical/public/js/vertical_student_view.js index 66c37af33b..ca2dbc5240 100644 --- a/common/lib/xmodule/xmodule/assets/vertical/public/js/vertical_student_view.js +++ b/common/lib/xmodule/xmodule/assets/vertical/public/js/vertical_student_view.js @@ -1,6 +1,7 @@ /* JavaScript for Vertical Student View. */ -/* global Set:false */ // false means do not assign to Set +/* global Set:false */ // false means do not assign to Set +/* global ViewedEventTracker:false */ // The vertical marks blocks complete if they are completable by viewing. The // global variable SEEN_COMPLETABLES tracks blocks between separate loads of @@ -8,6 +9,7 @@ // navigates back within a given sequential) to protect against duplicate calls // to the server. + var SEEN_COMPLETABLES = new Set(); window.VerticalStudentView = function(runtime, element) { @@ -15,7 +17,6 @@ window.VerticalStudentView = function(runtime, element) { RequireJS.require(['course_bookmarks/js/views/bookmark_button'], function(BookmarkButton) { var $element = $(element); var $bookmarkButtonElement = $element.find('.bookmark-button'); - return new BookmarkButton({ el: $bookmarkButtonElement, bookmarkId: $bookmarkButtonElement.data('bookmarkId'), @@ -24,32 +25,46 @@ window.VerticalStudentView = function(runtime, element) { apiUrl: $bookmarkButtonElement.data('bookmarksApiUrl') }); }); - $(element).find('.vert').each( - function(idx, block) { - var blockKey = block.dataset.id; - - if (!block.dataset.completableByViewing) { - return; - } - // TODO: EDUCATOR-1778 - // * Check if blocks are in the browser's view window or in focus - // before marking complete. This will include a configurable - // delay so that blocks must be seen for a few seconds before - // being marked complete, to prevent completion via rapid - // scrolling. (OC-3358) - // * Limit network traffic by batching and throttling calls. - // (OC-3090) - if (blockKey && !SEEN_COMPLETABLES.has(blockKey)) { - $.ajax({ - type: 'POST', - url: runtime.handlerUrl(element, 'publish_completion'), - data: JSON.stringify({ - block_key: blockKey, - completion: 1.0 - }) - }); - SEEN_COMPLETABLES.add(blockKey); - } + RequireJS.require(['bundles/ViewedEvent'], function() { + var tracker, vertical, viewedAfter; + var completableBlocks = []; + var vertModDivs = element.getElementsByClassName('vert-mod'); + if (vertModDivs.length === 0) { + return; } - ); + vertical = vertModDivs[0]; + $(element).find('.vert').each(function(idx, block) { + if (block.dataset.completableByViewing !== undefined) { + completableBlocks.push(block); + } + }); + if (completableBlocks.length > 0) { + viewedAfter = parseInt(vertical.dataset.completionDelayMs, 10); + if (!(viewedAfter >= 0)) { + // parseInt will return NaN if it fails to parse, which is not >= 0. + viewedAfter = 5000; + } + tracker = new ViewedEventTracker(completableBlocks, viewedAfter); + tracker.addHandler(function(block, event) { + var blockKey = block.dataset.id; + + if (blockKey && !SEEN_COMPLETABLES.has(blockKey)) { + if (event.elementHasBeenViewed) { + $.ajax({ + type: 'POST', + url: runtime.handlerUrl(element, 'publish_completion'), + data: JSON.stringify({ + block_key: blockKey, + completion: 1.0 + }) + }).then( + function() { + SEEN_COMPLETABLES.add(blockKey); + } + ); + } + } + }); + } + }); }; diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index babd349a5f..5f23e442aa 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -339,6 +339,7 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): 'xblock_context': context, 'show_bookmark_button': False, 'watched_completable_blocks': set(), + 'completion_delay_ms': None, })) return fragment diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index 8995849162..aab72a565b 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -20,6 +20,8 @@ from .xml import factories as xml from ..x_module import STUDENT_VIEW, AUTHOR_VIEW +COMPLETION_DELAY = 9876 + JsonRequest = namedtuple('JsonRequest', ['method', 'body']) @@ -41,6 +43,7 @@ class StubCompletionService(object): def __init__(self, enabled, completion_value): self._enabled = enabled self._completion_value = completion_value + self.delay = COMPLETION_DELAY def completion_tracking_enabled(self): """ @@ -56,6 +59,12 @@ class StubCompletionService(object): """ return {candidate: self._completion_value for candidate in candidates} + def get_completion_by_viewing_delay_ms(self): + """ + Return the completion-by-viewing delay in milliseconds. + """ + return self.delay + class BaseVerticalBlockTest(XModuleXmlImportTest): """ @@ -117,6 +126,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): """ self.module_system._services['bookmarks'] = Mock() self.module_system._services['user'] = StubUserService() + self.module_system._services['completion'] = StubCompletionService(enabled=True, completion_value=0.0) html = self.module_system.render( self.vertical, STUDENT_VIEW, self.default_context if context is None else context @@ -124,6 +134,7 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): self.assertIn(self.test_html_1, html) self.assertIn(self.test_html_2, html) self.assert_bookmark_info_in(html) + self.assertIn(six.text_type(COMPLETION_DELAY), html) @staticmethod def _render_completable_blocks(template, context): # pylint: disable=unused-argument diff --git a/common/lib/xmodule/xmodule/vertical_block.py b/common/lib/xmodule/xmodule/vertical_block.py index 8625b4cff3..7a081c315b 100644 --- a/common/lib/xmodule/xmodule/vertical_block.py +++ b/common/lib/xmodule/xmodule/vertical_block.py @@ -60,14 +60,13 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse show_in_read_only_mode = True - def get_completable_by_viewing(self): + def get_completable_by_viewing(self, completion_service): """ Return a set of descendent blocks that this vertical still needs to mark complete upon viewing. Completed blocks are excluded to reduce network traffic from clients. """ - completion_service = self.runtime.service(self, 'completion') if completion_service is None: return set() if not completion_service.completion_tracking_enabled(): @@ -80,6 +79,15 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse completions = completion_service.get_completions(blocks) return {six.text_type(block_key) for block_key in blocks if completions[block_key] < 1.0} + def get_completion_delay_ms(self, completion_service): + """ + Do not mark blocks as complete until they have been visible to the user + for the returned amount of time (in milliseconds). + """ + if completion_service is None: + return 0 + return completion_service.get_completion_by_viewing_delay_ms() + def student_view(self, context): """ Renders the student view of the block in the LMS. @@ -99,8 +107,9 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse user_service = self.runtime.service(self, 'user') child_context['username'] = user_service.get_current_user().opt_attrs['edx-platform.username'] - child_context['child_of_vertical'] = True + completion_service = self.runtime.service(self, 'completion') + child_context['child_of_vertical'] = True is_child_of_vertical = context.get('child_of_vertical', False) # pylint: disable=no-member @@ -120,7 +129,8 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse 'show_bookmark_button': child_context.get('show_bookmark_button', not is_child_of_vertical), 'bookmarked': child_context['bookmarked'], 'bookmark_id': u"{},{}".format(child_context['username'], unicode(self.location)), # pylint: disable=no-member - 'watched_completable_blocks': self.get_completable_by_viewing(), + 'watched_completable_blocks': self.get_completable_by_viewing(completion_service), + 'completion_delay_ms': self.get_completion_delay_ms(completion_service), })) fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/vertical_student_view.js')) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 2d9c9a9427..564ccab362 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -1095,6 +1095,8 @@ COMPLETION_VIDEO_COMPLETE_PERCENTAGE = ENV_TOKENS.get( 'COMPLETION_VIDEO_COMPLETE_PERCENTAGE', COMPLETION_VIDEO_COMPLETE_PERCENTAGE, ) +# The time a block needs to be viewed to be considered complete, in milliseconds. +COMPLETION_BY_VIEWING_DELAY_MS = ENV_TOKENS.get('COMPLETION_BY_VIEWING_DELAY_MS', COMPLETION_BY_VIEWING_DELAY_MS) ############### Settings for django-fernet-fields ################## FERNET_KEYS = AUTH_TOKENS.get('FERNET_KEYS', FERNET_KEYS) diff --git a/lms/envs/common.py b/lms/envs/common.py index 0478c1fcad..bfc3191af7 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3447,6 +3447,7 @@ EDX_PLATFORM_REVISION = 'unknown' # Once a user has watched this percentage of a video, mark it as complete: # (0.0 = 0%, 1.0 = 100%) COMPLETION_VIDEO_COMPLETE_PERCENTAGE = 0.95 +COMPLETION_BY_VIEWING_DELAY_MS = 5000 ############### Settings for Django Rate limit ##################### RATELIMIT_ENABLE = True diff --git a/lms/static/completion/js/.eslintrc.js b/lms/static/completion/js/.eslintrc.js new file mode 100644 index 0000000000..12cb26eef9 --- /dev/null +++ b/lms/static/completion/js/.eslintrc.js @@ -0,0 +1,7 @@ +module.exports = { + extends: 'eslint-config-edx', + root: true, + settings: { + 'import/resolver': 'webpack', + }, +}; diff --git a/lms/static/completion/js/ViewedEvent.js b/lms/static/completion/js/ViewedEvent.js new file mode 100644 index 0000000000..d119abcb1b --- /dev/null +++ b/lms/static/completion/js/ViewedEvent.js @@ -0,0 +1,182 @@ +/** Ensure that a function is only run once every `wait` milliseconds */ +function throttle(fn, wait) { + let time = 0; + function delay() { + // Do not call the function until at least `wait` seconds after the + // last time the function was called. + const now = Date.now(); + if (time + wait < now) { + time = now; + fn(); + } + } + return delay; +} + + +export class ElementViewing { + /** + * A wrapper for an HTMLElement that tracks whether the element has been + * viewed or not. + */ + constructor(el, viewedAfterMs, callback) { + this.el = el; + this.viewedAfterMs = viewedAfterMs; + this.callback = callback; + + this.topSeen = false; + this.bottomSeen = false; + this.seenForMs = 0; + this.becameVisibleAt = undefined; + this.hasBeenViewed = false; + } + + getBoundingRect() { + return this.el.getBoundingClientRect(); + } + + /** This element has become visible on screen. + * + * (may be called even when already on screen though) + */ + handleVisible() { + if (!this.becameVisibleAt) { + this.becameVisibleAt = Date.now(); + // We're now visible; after viewedAfterMs, if the top and bottom have been + // seen, this block will count as viewed. + setTimeout( + () => { + this.checkIfViewed(); + }, + this.viewedAfterMs - this.seenForMs, + ); + } + } + + handleNotVisible() { + if (this.becameVisibleAt) { + this.seenForMs = Date.now() - this.becameVisibleAt; + } + this.becameVisibleAt = undefined; + } + + markTopSeen() { + // If this element has been seen for enough time, but the top wasn't visible, it may now be + // considered viewed. + this.topSeen = true; + this.checkIfViewed(); + } + + markBottomSeen() { + this.bottomSeen = true; + this.checkIfViewed(); + } + + getTotalTimeSeen() { + if (this.becameVisibleAt) { + return this.seenForMs + (Date.now() - this.becameVisibleAt); + } + return this.seenForMs; + } + + areViewedCriteriaMet() { + return this.topSeen && this.bottomSeen && (this.getTotalTimeSeen() >= this.viewedAfterMs); + } + + checkIfViewed() { + // User can provide a "now" value for testing purposes. + if (this.hasBeenViewed) { + return; + } + if (this.areViewedCriteriaMet()) { + this.hasBeenViewed = true; + // Report to the tracker that we have been viewed + this.callback(this.el, { elementHasBeenViewed: this.hasBeenViewed }); + } + } +} + + +export class ViewedEventTracker { + /** + * When the top or bottom of an element is first viewed, and the entire + * element is viewed for a specified amount of time, the callback is called, + * passing the element that was viewed, and an event object having the + * following field: + * + * * hasBeenViewed (bool): true if all the conditions for being + * considered "viewed" have been met. + */ + constructor(elements, viewedAfterMs) { + this.viewedAfterMs = viewedAfterMs; + this.elementViewings = new Set(); + this.handlers = []; + + this.interval = undefined; + elements.forEach((el) => { + this.elementViewings.add( + new ElementViewing( + el, + viewedAfterMs, + (element, event) => this.callHandlers(element, event), + ), + ); + }); + this.registerDomHandlers(); + } + + /** Register a new handler to be called when an element has been viewed. */ + addHandler(handler) { + this.handlers.push(handler); + } + + /** Mark which elements are currently visible. + * + * Also marks when an elements top or bottom has been seen. + * */ + updateVisible() { + this.elementViewings.forEach((elv) => { + if (elv.hasBeenViewed) { + return; + } + + const now = Date.now(); // Use the same "now" for all calculations + const rect = elv.getBoundingRect(); + let visible = false; + + if (rect.top > 0 && rect.top < window.innerHeight) { + elv.markTopSeen(now); + visible = true; + } + if (rect.bottom > 0 && rect.bottom < window.innerHeight) { + elv.markBottomSeen(now); + visible = true; + } + if (rect.top < 0 && rect.bottom > window.innerHeight) { + visible = true; + } + + if (visible) { + elv.handleVisible(now); + } else { + elv.handleNotVisible(now); + } + }); + } + + registerDomHandlers() { + window.onscroll = throttle(() => this.updateVisible(), 100); + window.onresize = throttle(() => this.updateVisible(), 100); + this.updateVisible(); + } + + /** Call the handlers for all newly-viewed elements and pause tracking + * for recently disappeared elements. + */ + callHandlers(el, event) { + this.handlers.forEach((handler) => { + handler(el, event); + }); + } +} + diff --git a/lms/static/completion/js/spec/ViewedEvent_spec.js b/lms/static/completion/js/spec/ViewedEvent_spec.js new file mode 100644 index 0000000000..30368e7cf9 --- /dev/null +++ b/lms/static/completion/js/spec/ViewedEvent_spec.js @@ -0,0 +1,95 @@ +import { ElementViewing, ViewedEventTracker } from '../ViewedEvent'; + + +describe('ViewedTracker', () => { + let existingHTML; + beforeEach(() => { + existingHTML = document.body.innerHTML; + }); + + afterEach(() => { + document.body.innerHTML = existingHTML; + }); + + it('calls the handlers when an element is viewed', () => { + document.body.innerHTML = '
'; + const tracker = new ViewedEventTracker(Array.from(document.getElementsByTagName('div')), 1000); + const handlerSpy = jasmine.createSpy('handlerSpy'); + tracker.addHandler(handlerSpy); + const elvIter = tracker.elementViewings.values(); + // Pick two elements, and mock them so that one has met the criteria to be viewed, + // and the other hasn't. + const viewed = elvIter.next().value; + spyOn(viewed, 'areViewedCriteriaMet').and.returnValue(true); + viewed.checkIfViewed(); + expect(handlerSpy).toHaveBeenCalledWith(viewed.el, { + elementHasBeenViewed: true, + }); + const unviewed = elvIter.next().value; + spyOn(unviewed, 'areViewedCriteriaMet').and.returnValue(false); + unviewed.checkIfViewed(); + expect(handlerSpy).not.toHaveBeenCalledWith(unviewed.el, jasmine.anything()); + }); +}); + +describe('ElementViewing', () => { + beforeEach(() => { + jasmine.clock().install(); + }); + + afterEach(() => { + jasmine.clock().uninstall(); + }); + + it('calls checkIfViewed when enough time has elapsed', () => { + const viewing = new ElementViewing({}, 500, () => {}); + spyOn(viewing, 'checkIfViewed').and.callThrough(); + viewing.seenForMs = 250; + viewing.handleVisible(); + jasmine.clock().tick(249); + expect(viewing.checkIfViewed).not.toHaveBeenCalled(); + jasmine.clock().tick(1); + expect(viewing.checkIfViewed).toHaveBeenCalled(); + }); + + it('has been viewed after the specified number of milliseconds', () => { + const viewing = new ElementViewing({}, 500, () => {}); + viewing.seenForMs = 250; + spyOn(Date, 'now').and.returnValue(750); + viewing.handleVisible(); + viewing.markTopSeen(); + viewing.markBottomSeen(); + Date.now.and.returnValue(999); + viewing.checkIfViewed(); + expect(viewing.hasBeenViewed).toBeFalsy(); + Date.now.and.returnValue(1000); + jasmine.clock().tick(250); + expect(viewing.hasBeenViewed).toBeTruthy(); + }); + + it('has not been viewed if the bottom has not been seen', () => { + const viewing = new ElementViewing(undefined, 500, () => {}); + viewing.markTopSeen(); + viewing.seenForMs = 500; + expect(viewing.areViewedCriteriaMet()).toBeFalsy(); + viewing.checkIfViewed(); + expect(viewing.hasBeenViewed).toBeFalsy(); + }); + + it('has not been viewed if the top has not been seen', () => { + const viewing = new ElementViewing(undefined, 500, () => {}); + viewing.markBottomSeen(); + viewing.seenForMs = 500; + expect(viewing.areViewedCriteriaMet()).toBeFalsy(); + viewing.checkIfViewed(); + expect(viewing.hasBeenViewed).toBeFalsy(); + }); + + it('does not update time seen if lastSeen is undefined', () => { + const viewing = new ElementViewing(undefined, 500, () => {}); + viewing.becameVisibleAt = undefined; + expect(viewing.becameVisibleAt).toBeUndefined(); + viewing.handleVisible(); + expect(viewing.becameVisibleAt).not.toBeUndefined(); + }); +}); diff --git a/lms/static/karma_lms.conf.js b/lms/static/karma_lms.conf.js index a3bdc64bb0..a2e1456591 100644 --- a/lms/static/karma_lms.conf.js +++ b/lms/static/karma_lms.conf.js @@ -34,12 +34,14 @@ var options = { {pattern: 'learner_profile/**/!(*spec).js'}, {pattern: 'lms/js/**/!(*spec).js'}, {pattern: 'support/js/**/!(*spec).js'}, - {pattern: 'teams/js/**/!(*spec).js'} + {pattern: 'teams/js/**/!(*spec).js'}, + {pattern: 'completion/js/**/!(*spec).js'} ], specFiles: [ // Define the Webpack-built spec files first {pattern: 'course_experience/js/**/*_spec.js', webpack: true}, + {pattern: 'completion/js/**/*_spec.js', webpack: true}, // Add all remaining spec files to be used without Webpack {pattern: '../**/*spec.js'} diff --git a/lms/static/lms/js/spec/main.js b/lms/static/lms/js/spec/main.js index d81b26dae4..9bc5ba3f0c 100644 --- a/lms/static/lms/js/spec/main.js +++ b/lms/static/lms/js/spec/main.js @@ -690,6 +690,7 @@ }); testFiles = [ + 'completion/js/spec/ViewedEvent_spec.js', 'course_bookmarks/js/spec/bookmark_button_view_spec.js', 'course_bookmarks/js/spec/bookmarks_list_view_spec.js', 'course_bookmarks/js/spec/course_bookmarks_factory_spec.js', diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index cc7cfd569a..cee2c635e0 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -8,7 +8,11 @@ <%include file='bookmark_button.html' args="bookmark_id=bookmark_id, is_bookmarked=bookmarked"/> % endif -
+
% for idx, item in enumerate(items):