Mark blocks completed on view in render_xblock view.

common/lib/xmodule/xmodule/assets/vertical/public/js/vertical_student_view.js

@@ -9,7 +9,6 @@
 // navigates back within a given sequential) to protect against duplicate calls
 // to the server.
 
-
 var SEEN_COMPLETABLES = new Set();
 
 window.VerticalStudentView = function(runtime, element) {
@@ -25,46 +24,7 @@ window.VerticalStudentView = function(runtime, element) {
             apiUrl: $bookmarkButtonElement.data('bookmarksApiUrl')
         });
     });
-    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);
-                            }
-                        );
-                    }
-                }
-            });
-        }
+    RequireJS.require(['bundles/CompletionOnViewService'], function() {
+        markBlocksCompletedOnViewIfNeeded(runtime, element);
     });
 };

common/lib/xmodule/xmodule/tests/test_vertical.py

@@ -59,12 +59,15 @@ def get_completions(self, candidates):
         """
         return {candidate: self._completion_value for candidate in candidates}
 
-    def get_completion_by_viewing_delay_ms(self):
+    def get_complete_on_view_delay_ms(self):
         """
         Return the completion-by-viewing delay in milliseconds.
         """
         return self.delay
 
+    def blocks_to_mark_complete_on_view(self, blocks):
+        return {} if self._completion_value == 1.0 else blocks
+
 
 class BaseVerticalBlockTest(XModuleXmlImportTest):
     """
@@ -136,39 +139,31 @@ def test_render_student_view(self, context):
         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
-        """
-        A custom template rendering function that displays the
-        watched_completable_blocks of the template.
-
-        This is used because the default test renderer is haphazardly
-        formatted, and is difficult to make assertions about.
-        """
-        return u'|'.join(context['watched_completable_blocks'])
 
     @ddt.unpack
     @ddt.data(
-        (True, 0.9, 'assertIn'),
-        (False, 0.9, 'assertNotIn'),
-        (True, 1.0, 'assertNotIn'),
+        (True, 0.9, True),
+        (False, 0.9, False),
+        (True, 1.0, False),
     )
-    def test_completion_data_attrs(self, completion_enabled, completion_value, assertion_method):
+    def test_mark_completed_on_view_after_delay_in_context(
+            self, completion_enabled, completion_value, mark_completed_enabled
+    ):
         """
-        Test that data-completable-by-viewing attributes are included only when
-        the completion service is enabled, and only for blocks with a
-        completion value less than 1.0.
+        Test that mark-completed-on-view-after-delay is only set for relevant child Xblocks.
         """
-        with patch.object(self.module_system, 'render_template', new=self._render_completable_blocks):
+        with patch.object(self.html1block, 'render') as mock_student_view:
             self.module_system._services['completion'] = StubCompletionService(
                 enabled=completion_enabled,
                 completion_value=completion_value,
             )
-            response = self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context)
-        getattr(self, assertion_method)(six.text_type(self.html1block.location), response.content)
-        getattr(self, assertion_method)(six.text_type(self.html2block.location), response.content)
+            self.module_system.render(self.vertical, STUDENT_VIEW, self.default_context)
+            if (mark_completed_enabled):
+                self.assertEqual(
+                    mock_student_view.call_args[0][1]['wrap_xblock_data']['mark-completed-on-view-after-delay'], 9876
+                )
+            else:
+                self.assertNotIn('wrap_xblock_data', mock_student_view.call_args[0][1])
 
     def test_render_studio_view(self):
         """
@@ -191,14 +186,3 @@ def test_render_studio_view(self):
         html = self.module_system.render(self.vertical, AUTHOR_VIEW, context).content
         self.assertIn(self.test_html_1, html)
         self.assertIn(self.test_html_2, html)
-
-    def test_publish_completion(self):
-        request = get_json_request({"block_key": six.text_type(self.html1block.location), "completion": 1.0})
-        with patch.object(self.vertical.runtime, 'publish') as mock_publisher:
-            response = self.vertical.publish_completion(request)
-            self.assertEqual(
-                response.status_code,
-                200,
-                "Expected 200, got {}: {}".format(response.status_code, response.body),
-            )
-            mock_publisher.assert_called_with(self.html1block, "completion", {"completion": 1.0})

common/lib/xmodule/xmodule/vertical_block.py

@@ -8,13 +8,9 @@
 import logging
 
 from lxml import etree
-from opaque_keys.edx.keys import UsageKey
 import six
 from web_fragments.fragment import Fragment
-from xblock.completable import XBlockCompletionMode
 from xblock.core import XBlock
-from xblock.exceptions import JsonHandlerError
-
 
 from xmodule.mako_module import MakoTemplateBlockBase
 from xmodule.progress import Progress
@@ -31,19 +27,6 @@
 CLASS_PRIORITY = ['video', 'problem']
 
 
-def is_completable_by_viewing(block):
-    """
-    Returns True if the block can by completed by viewing it.
-
-    This is true of any non-customized, non-scorable, completable block.
-    """
-    return (
-        getattr(block, 'completion_mode', XBlockCompletionMode.COMPLETABLE) == XBlockCompletionMode.COMPLETABLE
-        and not getattr(block, 'has_custom_completion', False)
-        and not block.has_score
-    )
-
-
 @XBlock.needs('user', 'bookmarks')
 @XBlock.wants('completion')
 class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParserMixin, MakoTemplateBlockBase, XBlock):
@@ -60,34 +43,6 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse
 
     show_in_read_only_mode = True
 
-    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.
-        """
-        if completion_service is None:
-            return set()
-        if not completion_service.completion_tracking_enabled():
-            return set()
-        # pylint: disable=no-member
-        blocks = {block.location for block in self.get_display_items() if is_completable_by_viewing(block)}
-        # pylint: enable=no-member
-
-        # Exclude completed blocks to reduce traffic from client.
-        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.
@@ -107,14 +62,25 @@ def student_view(self, context):
             user_service = self.runtime.service(self, 'user')
             child_context['username'] = user_service.get_current_user().opt_attrs['edx-platform.username']
 
+        child_blocks = self.get_display_items()
+
+        child_blocks_to_complete_on_view = set()
         completion_service = self.runtime.service(self, 'completion')
+        if completion_service and completion_service.completion_tracking_enabled():
+            child_blocks_to_complete_on_view = completion_service.blocks_to_mark_complete_on_view(child_blocks)
+            complete_on_view_delay = completion_service.get_complete_on_view_delay_ms()
 
         child_context['child_of_vertical'] = True
         is_child_of_vertical = context.get('child_of_vertical', False)
 
         # pylint: disable=no-member
-        for child in self.get_display_items():
-            rendered_child = child.render(STUDENT_VIEW, child_context)
+        for child in child_blocks:
+            child_block_context = copy(child_context)
+            if child in child_blocks_to_complete_on_view:
+                child_block_context['wrap_xblock_data'] = {
+                    'mark-completed-on-view-after-delay': complete_on_view_delay
+                }
+            rendered_child = child.render(STUDENT_VIEW, child_block_context)
             fragment.add_fragment_resources(rendered_child)
 
             contents.append({
@@ -129,8 +95,6 @@ def student_view(self, context):
             '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(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'))
@@ -231,29 +195,3 @@ def index_dictionary(self):
         xblock_body["content_type"] = "Sequence"
 
         return xblock_body
-
-    def find_descendent(self, block_key):
-        """
-        Return the descendent block with the given block key if it exists.
-
-        Otherwise return None.
-        """
-        for block in self.get_display_items():  # pylint: disable=no-member
-            if block.location == block_key:
-                return block
-
-    @XBlock.json_handler
-    def publish_completion(self, data, suffix=''):  # pylint: disable=unused-argument
-        """
-        Publish data from the front end.
-        """
-        block_key = UsageKey.from_string(data.pop('block_key')).map_into_course(self.course_id)
-        block = self.find_descendent(block_key)
-        if block is None:
-            message = "Invalid block: {} not found in {}"
-            raise JsonHandlerError(400, message.format(block_key, self.location))  # pylint: disable=no-member
-        elif not is_completable_by_viewing(block):
-            message = "Invalid block type: {} in block {} not configured for completion by viewing"
-            raise JsonHandlerError(400, message.format(type(block), block_key))
-        self.runtime.publish(block, "completion", data)
-        return {'result': 'ok'}

common/test/acceptance/pages/lms/completion.py

@@ -0,0 +1,26 @@
+# -*- coding: utf-8 -*-
+"""
+Mixins for completion.
+"""
+
+
+class CompletionOnViewMixin(object):
+    """
+    Methods for testing completion on view.
+    """
+
+    def xblock_components_mark_completed_on_view_value(self):
+        """
+        Return the xblock components data-mark-completed-on-view-after-delay value.
+        """
+        return self.q(css=self.xblock_component_selector).attrs('data-mark-completed-on-view-after-delay')
+
+    def wait_for_xblock_component_to_be_marked_completed_on_view(self, index=0):
+        """
+        Wait for xblock component to be marked completed on view.
+
+        Arguments
+            index (int): index of block to wait on. (default is 0)
+        """
+        self.wait_for(lambda: (self.xblock_components_mark_completed_on_view_value()[index] == '0'),
+                      'Waiting for xblock to be marked completed on view.')

common/test/acceptance/pages/lms/courseware.py

@@ -8,11 +8,13 @@
 from bok_choy.promise import EmptyPromise
 from selenium.webdriver.common.action_chains import ActionChains
 
+from common.test.acceptance.pages.lms import BASE_URL
 from common.test.acceptance.pages.lms.bookmarks import BookmarksPage
+from common.test.acceptance.pages.lms.completion import CompletionOnViewMixin
 from common.test.acceptance.pages.lms.course_page import CoursePage
 
 
-class CoursewarePage(CoursePage):
+class CoursewarePage(CoursePage, CompletionOnViewMixin):
     """
     Course info.
     """
@@ -587,3 +589,25 @@ def visit_course_outline_page(self):
         # reload the same page with the course_outline_page flag
         self.browser.get(self.browser.current_url + "&course_experience.course_outline_page=1")
         self.wait_for_page()
+
+
+class RenderXBlockPage(PageObject, CompletionOnViewMixin):
+    """
+    render_xblock page.
+    """
+
+    xblock_component_selector = '.xblock'
+
+    def __init__(self, browser, block_id):
+        super(RenderXBlockPage, self).__init__(browser)
+        self.block_id = block_id
+
+    @property
+    def url(self):
+        """
+        Construct a URL to the page within the course.
+        """
+        return BASE_URL + "/xblock/" + self.block_id
+
+    def is_browser_on_page(self):
+        return self.q(css='.course-content').present