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 ca2dbc5240..bdf33e7be3 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 @@ -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); }); }; diff --git a/common/lib/xmodule/xmodule/tests/test_vertical.py b/common/lib/xmodule/xmodule/tests/test_vertical.py index b6ac897fae..c272b95be3 100644 --- a/common/lib/xmodule/xmodule/tests/test_vertical.py +++ b/common/lib/xmodule/xmodule/tests/test_vertical.py @@ -59,12 +59,15 @@ class StubCompletionService(object): """ 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 @@ 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 - """ - 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 @@ class VerticalBlockTestCase(BaseVerticalBlockTest): 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}) diff --git a/common/lib/xmodule/xmodule/vertical_block.py b/common/lib/xmodule/xmodule/vertical_block.py index 7a081c315b..d667a4ed33 100644 --- a/common/lib/xmodule/xmodule/vertical_block.py +++ b/common/lib/xmodule/xmodule/vertical_block.py @@ -8,13 +8,9 @@ from copy import copy 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 @@ log = logging.getLogger(__name__) 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 @@ 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_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 @@ 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(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 @@ class VerticalBlock(SequenceFields, XModuleFields, StudioEditableBlock, XmlParse 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'} diff --git a/common/test/acceptance/pages/lms/completion.py b/common/test/acceptance/pages/lms/completion.py new file mode 100644 index 0000000000..0a09dc3648 --- /dev/null +++ b/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.') diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 2240808c60..6dabe9f2e5 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -8,11 +8,13 @@ from bok_choy.page_object import PageObject, unguarded 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 @@ class CourseNavPage(PageObject): # 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 diff --git a/common/test/acceptance/tests/lms/test_lms_courseware.py b/common/test/acceptance/tests/lms/test_lms_courseware.py index 4fcbe1739a..d2f65cd6cf 100644 --- a/common/test/acceptance/tests/lms/test_lms_courseware.py +++ b/common/test/acceptance/tests/lms/test_lms_courseware.py @@ -13,7 +13,7 @@ from ...fixtures.course import CourseFixture, XBlockFixtureDesc from ...pages.common.auto_auth import AutoAuthPage from ...pages.common.logout import LogoutPage from ...pages.lms.course_home import CourseHomePage -from ...pages.lms.courseware import CoursewarePage, CoursewareSequentialTabPage +from ...pages.lms.courseware import CoursewarePage, CoursewareSequentialTabPage, RenderXBlockPage from ...pages.lms.create_mode import ModeCreationPage from ...pages.lms.dashboard import DashboardPage from ...pages.lms.pay_and_verify import FakePaymentPage, FakeSoftwareSecureVerificationPage, PaymentAndVerificationFlow @@ -847,3 +847,129 @@ class SubsectionHiddenAfterDueDateTest(UniqueCourseTest): self.progress_page.visit() self.assertEqual(self.progress_page.scores('Test Section 1', 'Test Subsection 1'), [(0, 1)]) + + +@attr(shard=9) +class CompletionTestCase(UniqueCourseTest, EventsTestMixin): + """ + Test the completion on view functionality. + """ + USERNAME = "STUDENT_TESTER" + EMAIL = "student101@example.com" + COMPLETION_BY_VIEWING_DELAY_MS = '1000' + + def setUp(self): + super(CompletionTestCase, self).setUp() + + self.studio_course_outline = StudioCourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + # Install a course with sections/problems, tabs, updates, and handouts + course_fix = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) + + self.html_1_block = XBlockFixtureDesc('html', 'html 1', data="html 1 dummy body") + self.problem_1_block = XBlockFixtureDesc( + 'problem', 'Test Problem 1', data='problem 1 dummy body' + ) + + course_fix.add_children( + XBlockFixtureDesc('chapter', 'Test Section 1').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection 1,1').add_children( + XBlockFixtureDesc('vertical', 'Test Unit 1,1,1').add_children( + XBlockFixtureDesc('html', 'html 1', data="html 1 dummy body"), + XBlockFixtureDesc( + 'html', 'html 2', + data=("html 2 dummy body" * 100) + "End", + ), + XBlockFixtureDesc('problem', 'Test Problem 1', data='problem 1 dummy body'), + ), + XBlockFixtureDesc('vertical', 'Test Unit 1,1,2').add_children( + XBlockFixtureDesc('html', 'html 1', data="html 1 dummy body"), + XBlockFixtureDesc('problem', 'Test Problem 1', data='problem 1 dummy body'), + ), + XBlockFixtureDesc('vertical', 'Test Unit 1,1,2').add_children( + self.html_1_block, + self.problem_1_block, + ), + ), + ), + ).install() + + # Auto-auth register for the course. + AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL, + course_id=self.course_id, staff=False).visit() + + def test_courseware_publish_completion_is_sent_on_view(self): + """ + Test that when viewing courseware XBlocks are correctly marked as completed on view. + """ + courseware_page = CoursewarePage(self.browser, self.course_id) + courseware_page.visit() + courseware_page.wait_for_page() + + # Initially, the first two blocks in the first vertical should be marked as needing to be completed on view. + self.assertEqual( + courseware_page.xblock_components_mark_completed_on_view_value(), + [self.COMPLETION_BY_VIEWING_DELAY_MS, self.COMPLETION_BY_VIEWING_DELAY_MS, None], + ) + # Wait and verify that the first block which is completely visible is marked as completed. + courseware_page.wait_for_xblock_component_to_be_marked_completed_on_view(0) + self.assertEqual( + courseware_page.xblock_components_mark_completed_on_view_value(), + ['0', self.COMPLETION_BY_VIEWING_DELAY_MS, None], + ) + + # Scroll to the bottom of the second block. + courseware_page.scroll_to_element('#html2-end', 'Scroll to end of html 2 block') + # Wait and verify that the second block is also now marked as completed. + courseware_page.wait_for_xblock_component_to_be_marked_completed_on_view(1) + self.assertEqual(courseware_page.xblock_components_mark_completed_on_view_value(), ['0', '0', None]) + + # After page refresh, no blocks in the vertical should be marked as needing to be completed on view. + self.browser.refresh() + courseware_page.wait_for_page() + self.assertEqual(courseware_page.xblock_components_mark_completed_on_view_value(), [None, None, None]) + + courseware_page.go_to_sequential_position(2) + + # Initially, the first block in the second vertical should be marked as needing to be completed on view. + self.assertEqual( + courseware_page.xblock_components_mark_completed_on_view_value(), + [self.COMPLETION_BY_VIEWING_DELAY_MS, None], + ) + # Wait and verify that the first block which is completely visible is marked as completed. + courseware_page.wait_for_xblock_component_to_be_marked_completed_on_view(0) + self.assertEqual(courseware_page.xblock_components_mark_completed_on_view_value(), ['0', None]) + + # After page refresh, no blocks in the vertical should be marked as needing to be completed on view. + self.browser.refresh() + courseware_page.wait_for_page() + self.assertEqual(courseware_page.xblock_components_mark_completed_on_view_value(), [None, None]) + + def test_render_xblock_publish_completion_is_sent_on_view(self): + """ + Test that when viewing a XBlock in render_xblock, it is correctly marked as completed on view. + """ + block_page = RenderXBlockPage(self.browser, self.html_1_block.locator) + block_page.visit() + block_page.wait_for_page() + + # Initially the block should be marked as needing to be completed on view. + self.assertEqual( + block_page.xblock_components_mark_completed_on_view_value(), [self.COMPLETION_BY_VIEWING_DELAY_MS] + ) + # Wait and verify that the block is marked as completed on view. + block_page.wait_for_xblock_component_to_be_marked_completed_on_view(0) + self.assertEqual(block_page.xblock_components_mark_completed_on_view_value(), ['0']) + + # After page refresh, it should not be marked as needing to be completed on view. + self.browser.refresh() + block_page.wait_for_page() + self.assertEqual(block_page.xblock_components_mark_completed_on_view_value(), [None]) diff --git a/common/test/db_fixtures/waffle_flags.json b/common/test/db_fixtures/waffle_flags.json index 008df0f72e..d1f6c0e75f 100644 --- a/common/test/db_fixtures/waffle_flags.json +++ b/common/test/db_fixtures/waffle_flags.json @@ -23,6 +23,14 @@ "active": true } }, + { + "pk": 2, + "model": "waffle.switch", + "fields": { + "name": "completion.enable_completion_tracking", + "active": true + } + }, { "pk": 3, "model": "waffle.flag", diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index ec7ad6612d..63bf236bac 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -10,6 +10,7 @@ from HTMLParser import HTMLParser from urllib import quote, urlencode from uuid import uuid4 +from completion.test_utils import CompletionWaffleTestMixin import ddt from django.conf import settings from django.contrib.auth.models import AnonymousUser @@ -37,7 +38,7 @@ from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory from courseware.access_utils import check_course_open_for_learner from courseware.model_data import FieldDataCache, set_score -from courseware.module_render import get_module +from courseware.module_render import get_module, handle_xblock_callback from courseware.tests.factories import GlobalStaffFactory, StudentModuleFactory from courseware.testutils import RenderXBlockTestMixin from courseware.url_helpers import get_redirect_url @@ -61,6 +62,7 @@ from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag, WaffleFlagNam from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES, override_waffle_flag from openedx.core.djangolib.testing.utils import get_mock_request from openedx.core.lib.gating import api as gating_api +from openedx.core.lib.url_utils import quote_slashes from openedx.features.course_experience import COURSE_OUTLINE_PAGE_FLAG, UNIFIED_COURSE_TAB_FLAG from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired from student.models import CourseEnrollment @@ -2350,6 +2352,143 @@ class TestIndexView(ModuleStoreTestCase): assert response.status_code == 200 +@attr(shard=5) +@ddt.ddt +class TestIndexViewCompleteOnView(ModuleStoreTestCase, CompletionWaffleTestMixin): + """ + Tests CompleteOnView is set up correctly in CoursewareIndex. + """ + + def setup_course(self, default_store): + """ + Set up course content for modulestore. + """ + # pylint:disable=attribute-defined-outside-init + + self.request_factory = RequestFactory() + self.user = UserFactory() + + with modulestore().default_store(default_store): + self.course = CourseFactory.create() + + with self.store.bulk_operations(self.course.id): + + self.chapter = ItemFactory.create( + parent_location=self.course.location, category='chapter', display_name='Week 1' + ) + self.section_1 = ItemFactory.create( + parent_location=self.chapter.location, category='sequential', display_name='Lesson 1' + ) + self.vertical_1 = ItemFactory.create( + parent_location=self.section_1.location, category='vertical', display_name='Subsection 1' + ) + self.html_1_1 = ItemFactory.create( + parent_location=self.vertical_1.location, category='html', display_name="HTML 1_1" + ) + self.problem_1 = ItemFactory.create( + parent_location=self.vertical_1.location, category='problem', display_name="Problem 1" + ) + self.html_1_2 = ItemFactory.create( + parent_location=self.vertical_1.location, category='html', display_name="HTML 1_2" + ) + + self.section_2 = ItemFactory.create( + parent_location=self.chapter.location, category='sequential', display_name='Lesson 2' + ) + self.vertical_2 = ItemFactory.create( + parent_location=self.section_2.location, category='vertical', display_name='Subsection 2' + ) + self.video_2 = ItemFactory.create( + parent_location=self.vertical_2.location, category='video', display_name="Video 2" + ) + self.problem_2 = ItemFactory.create( + parent_location=self.vertical_2.location, category='problem', display_name="Problem 2" + ) + + self.section_1_url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course.id), + 'chapter': self.chapter.url_name, + 'section': self.section_1.url_name, + } + ) + + self.section_2_url = reverse( + 'courseware_section', + kwargs={ + 'course_id': unicode(self.course.id), + 'chapter': self.chapter.url_name, + 'section': self.section_2.url_name, + } + ) + + CourseOverview.load_from_module_store(self.course.id) + CourseEnrollmentFactory(user=self.user, course_id=self.course.id) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_completion_service_disabled(self, default_store): + + self.setup_course(default_store) + self.assertTrue(self.client.login(username=self.user.username, password='test')) + + response = self.client.get(self.section_1_url) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + response = self.client.get(self.section_2_url) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_completion_service_enabled(self, default_store): + + self.override_waffle_switch(True) + + self.setup_course(default_store) + self.assertTrue(self.client.login(username=self.user.username, password='test')) + + response = self.client.get(self.section_1_url) + self.assertIn('data-mark-completed-on-view-after-delay', response.content) + self.assertEquals(response.content.count("data-mark-completed-on-view-after-delay"), 2) + + request = self.request_factory.post( + '/', + data=json.dumps({"completion": 1}), + content_type='application/json', + ) + request.user = self.user + response = handle_xblock_callback( + request, + unicode(self.course.id), + quote_slashes(unicode(self.html_1_1.scope_ids.usage_id)), + 'publish_completion', + ) + self.assertEqual(json.loads(response.content), {'result': "ok"}) + + response = self.client.get(self.section_1_url) + self.assertIn('data-mark-completed-on-view-after-delay', response.content) + self.assertEquals(response.content.count("data-mark-completed-on-view-after-delay"), 1) + + request = self.request_factory.post( + '/', + data=json.dumps({"completion": 1}), + content_type='application/json', + ) + request.user = self.user + response = handle_xblock_callback( + request, + unicode(self.course.id), + quote_slashes(unicode(self.html_1_2.scope_ids.usage_id)), + 'publish_completion', + ) + self.assertEqual(json.loads(response.content), {'result': "ok"}) + + response = self.client.get(self.section_1_url) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + response = self.client.get(self.section_2_url) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + @attr(shard=5) @ddt.ddt class TestIndexViewWithVerticalPositions(ModuleStoreTestCase): @@ -2471,7 +2610,7 @@ class TestIndexViewWithGating(ModuleStoreTestCase, MilestonesTestCaseMixin): @attr(shard=5) -class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): +class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase, CompletionWaffleTestMixin): """ Tests for the courseware.render_xblock endpoint. This class overrides the get_response method, which is used by @@ -2498,6 +2637,57 @@ class TestRenderXBlock(RenderXBlockTestMixin, ModuleStoreTestCase): url += '?' + url_encoded_params return self.client.get(url) + def test_render_xblock_with_completion_service_disabled(self): + """ + Test that render_xblock does not set up the CompletionOnViewService. + """ + self.setup_course(ModuleStoreEnum.Type.split) + self.setup_user(admin=True, enroll=True, login=True) + + response = self.get_response(usage_key=self.html_block.location) + self.assertEqual(response.status_code, 200) + self.assertIn('data-enable-completion-on-view-service="false"', response.content) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + def test_render_xblock_with_completion_service_enabled(self): + """ + Test that render_xblock sets up the CompletionOnViewService for relevant xblocks. + """ + self.override_waffle_switch(True) + + self.setup_course(ModuleStoreEnum.Type.split) + self.setup_user(admin=False, enroll=True, login=True) + + response = self.get_response(usage_key=self.html_block.location) + self.assertEqual(response.status_code, 200) + self.assertIn('data-enable-completion-on-view-service="true"', response.content) + self.assertIn('data-mark-completed-on-view-after-delay', response.content) + + request = RequestFactory().post( + '/', + data=json.dumps({"completion": 1}), + content_type='application/json', + ) + request.user = self.user + response = handle_xblock_callback( + request, + unicode(self.course.id), + quote_slashes(unicode(self.html_block.location)), + 'publish_completion', + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(json.loads(response.content), {'result': "ok"}) + + response = self.get_response(usage_key=self.html_block.location) + self.assertEqual(response.status_code, 200) + self.assertIn('data-enable-completion-on-view-service="false"', response.content) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + + response = self.get_response(usage_key=self.problem_block.location) + self.assertEqual(response.status_code, 200) + self.assertIn('data-enable-completion-on-view-service="false"', response.content) + self.assertNotIn('data-mark-completed-on-view-after-delay', response.content) + class TestRenderXBlockSelfPaced(TestRenderXBlock): """ diff --git a/lms/djangoapps/courseware/testutils.py b/lms/djangoapps/courseware/testutils.py index f90188c3b3..74151717a9 100644 --- a/lms/djangoapps/courseware/testutils.py +++ b/lms/djangoapps/courseware/testutils.py @@ -105,6 +105,11 @@ class RenderXBlockTestMixin(object): category='html', data="

Test HTML Content

" ) + self.problem_block = ItemFactory.create( + parent=self.vertical_block, + category='problem', + display_name='Problem' + ) CourseOverview.load_from_module_store(self.course.id) # block_name_to_be_tested can be `html_block` or `vertical_block`. @@ -150,9 +155,9 @@ class RenderXBlockTestMixin(object): return response @ddt.data( - ('vertical_block', ModuleStoreEnum.Type.mongo, 10), + ('vertical_block', ModuleStoreEnum.Type.mongo, 11), ('vertical_block', ModuleStoreEnum.Type.split, 6), - ('html_block', ModuleStoreEnum.Type.mongo, 11), + ('html_block', ModuleStoreEnum.Type.mongo, 12), ('html_block', ModuleStoreEnum.Type.split, 6), ) @ddt.unpack diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index d4c98892d8..f795034a78 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -1472,6 +1472,15 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): student_view_context = request.GET.dict() student_view_context['show_bookmark_button'] = False + enable_completion_on_view_service = False + completion_service = block.runtime.service(block, 'completion') + if completion_service and completion_service.completion_tracking_enabled(): + if completion_service.blocks_to_mark_complete_on_view({block}): + enable_completion_on_view_service = True + student_view_context['wrap_xblock_data'] = { + 'mark-completed-on-view-after-delay': completion_service.get_complete_on_view_delay_ms() + } + context = { 'fragment': block.render('student_view', context=student_view_context), 'course': course, @@ -1480,6 +1489,7 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): 'disable_header': True, 'disable_footer': True, 'disable_window_wrap': True, + 'enable_completion_on_view_service': enable_completion_on_view_service, 'staff_access': bool(has_access(request.user, 'staff', course)), 'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://your_xqa_server.com'), } diff --git a/lms/djangoapps/lms_xblock/mixin.py b/lms/djangoapps/lms_xblock/mixin.py index ed25c5c641..9eb4942376 100644 --- a/lms/djangoapps/lms_xblock/mixin.py +++ b/lms/djangoapps/lms_xblock/mixin.py @@ -5,6 +5,7 @@ Namespace that defines fields common to all blocks used in the LMS #from django.utils.translation import ugettext_noop as _ from lazy import lazy from xblock.core import XBlock, XBlockMixin +from xblock.exceptions import JsonHandlerError from xblock.fields import Boolean, Dict, Scope, String from xblock.validation import ValidationMessage @@ -44,6 +45,7 @@ class GroupAccessDict(Dict): @XBlock.needs('partitions') @XBlock.needs('i18n') +@XBlock.wants('completion') class LmsBlockMixin(XBlockMixin): """ Mixin that defines fields common to all blocks used in the LMS @@ -241,3 +243,18 @@ class LmsBlockMixin(XBlockMixin): ) return validation + + @XBlock.json_handler + def publish_completion(self, data, suffix=''): # pylint: disable=unused-argument + """ + Publish completion data from the front end. + """ + completion_service = self.runtime.service(self, 'completion') + if completion_service is None: + raise JsonHandlerError(500, u"No completion service found") + elif not completion_service.completion_tracking_enabled(): + raise JsonHandlerError(404, u"Completion tracking is not enabled and API calls are unexpected") + if not completion_service.can_mark_block_complete_on_view(self): + raise JsonHandlerError(400, u"Block not configured for completion on view.") + self.runtime.publish(self, "completion", data) + return {'result': 'ok'} diff --git a/lms/envs/bok_choy.py b/lms/envs/bok_choy.py index f12920e27f..9e26fa9fbb 100644 --- a/lms/envs/bok_choy.py +++ b/lms/envs/bok_choy.py @@ -239,6 +239,10 @@ if RELEASE_LINE == "master": WAFFLE_OVERRIDE = True +############## Settings for Completion API ######################### + +COMPLETION_BY_VIEWING_DELAY_MS = 1000 + ##################################################################### # Lastly, see if the developer has any local overrides. try: diff --git a/lms/static/completion/js/CompletionOnViewService.js b/lms/static/completion/js/CompletionOnViewService.js new file mode 100644 index 0000000000..b5c496e703 --- /dev/null +++ b/lms/static/completion/js/CompletionOnViewService.js @@ -0,0 +1,42 @@ +import { ViewedEventTracker } from './ViewedEvent'; + +const completedBlocksKeys = new Set(); + +export function markBlocksCompletedOnViewIfNeeded(runtime, containerElement) { + const blockElements = $(containerElement).find( + '.xblock-student_view[data-mark-completed-on-view-after-delay]', + ).get(); + + if (blockElements.length > 0) { + const tracker = new ViewedEventTracker(); + + blockElements.forEach((blockElement) => { + const markCompletedOnViewAfterDelay = parseInt( + blockElement.dataset.markCompletedOnViewAfterDelay, 10, + ); + if (markCompletedOnViewAfterDelay >= 0) { + tracker.addElement(blockElement, markCompletedOnViewAfterDelay); + } + }); + + tracker.addHandler((blockElement, event) => { + const blockKey = blockElement.dataset.usageId; + if (blockKey && !completedBlocksKeys.has(blockKey)) { + if (event.elementHasBeenViewed) { + $.ajax({ + type: 'POST', + url: runtime.handlerUrl(blockElement, 'publish_completion'), + data: JSON.stringify({ + completion: 1.0, + }), + }).then( + () => { + completedBlocksKeys.add(blockKey); + blockElement.dataset.markCompletedOnViewAfterDelay = 0; + }, + ); + } + } + }); + } +} diff --git a/lms/static/completion/js/ViewedEvent.js b/lms/static/completion/js/ViewedEvent.js index d119abcb1b..fe2d8f3fea 100644 --- a/lms/static/completion/js/ViewedEvent.js +++ b/lms/static/completion/js/ViewedEvent.js @@ -107,24 +107,24 @@ export class ViewedEventTracker { * * hasBeenViewed (bool): true if all the conditions for being * considered "viewed" have been met. */ - constructor(elements, viewedAfterMs) { - this.viewedAfterMs = viewedAfterMs; + constructor() { 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(); } + /** Add an element to track. */ + addElement(element, viewedAfterMs) { + this.elementViewings.add( + new ElementViewing( + element, + viewedAfterMs, + (el, event) => this.callHandlers(el, event), + ), + ); + this.updateVisible(); + } + /** Register a new handler to be called when an element has been viewed. */ addHandler(handler) { this.handlers.push(handler); diff --git a/lms/static/completion/js/spec/ViewedEvent_spec.js b/lms/static/completion/js/spec/ViewedEvent_spec.js index 30368e7cf9..1a8839e9e9 100644 --- a/lms/static/completion/js/spec/ViewedEvent_spec.js +++ b/lms/static/completion/js/spec/ViewedEvent_spec.js @@ -13,7 +13,10 @@ describe('ViewedTracker', () => { it('calls the handlers when an element is viewed', () => { document.body.innerHTML = '

'; - const tracker = new ViewedEventTracker(Array.from(document.getElementsByTagName('div')), 1000); + const tracker = new ViewedEventTracker(); + for (const element of document.getElementsByTagName('div')) { + tracker.addElement(element, 1000); + } const handlerSpy = jasmine.createSpy('handlerSpy'); tracker.addHandler(handlerSpy); const elvIter = tracker.elementViewings.values(); diff --git a/lms/static/js/courseware.js b/lms/static/js/courseware.js index 1a593cf202..2e5dbe0d54 100644 --- a/lms/static/js/courseware.js +++ b/lms/static/js/courseware.js @@ -14,7 +14,15 @@ }; Courseware.prototype.render = function() { - XBlock.initializeBlocks($('.course-content')); + var courseContentElement = $('.course-content')[0]; + var blocks = XBlock.initializeBlocks(courseContentElement); + + if (courseContentElement.dataset.enableCompletionOnViewService === 'true') { + RequireJS.require(['bundles/CompletionOnViewService'], function() { + markBlocksCompletedOnViewIfNeeded(blocks[0].runtime, courseContentElement); + }); + } + return $('.course-content .histogram').each(function() { var error, histg, id; id = $(this).attr('id').replace(/histogram_/, ''); diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html index 845af68d8e..d11861f811 100644 --- a/lms/templates/courseware/courseware-chromeless.html +++ b/lms/templates/courseware/courseware-chromeless.html @@ -70,7 +70,13 @@ ${HTML(fragment.foot_html())}
-
+
${HTML(fragment.body_html())}
diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index cee2c635e0..1ac7a09736 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -8,17 +8,9 @@ <%include file='bookmark_button.html' args="bookmark_id=bookmark_id, is_bookmarked=bookmarked"/> % endif -
+
% for idx, item in enumerate(items): -
+
${HTML(item['content'])}
% endfor diff --git a/openedx/core/lib/tests/test_xblock_utils.py b/openedx/core/lib/tests/test_xblock_utils.py index 0e122610f2..050a5fde17 100644 --- a/openedx/core/lib/tests/test_xblock_utils.py +++ b/openedx/core/lib/tests/test_xblock_utils.py @@ -100,7 +100,7 @@ class TestXblockUtils(SharedModuleStoreTestCase): block=course, view='baseview', frag=fragment, - context=None, + context={"wrap_xblock_data": {"custom-attribute": "custom-value"}}, usage_id_serializer=lambda usage_id: quote_slashes(unicode(usage_id)), request_token=uuid.uuid1().get_hex() ) @@ -109,6 +109,7 @@ class TestXblockUtils(SharedModuleStoreTestCase): self.assertIn('data-runtime-class="TestRuntime"', test_wrap_output.content) self.assertIn(data_usage_id, test_wrap_output.content) self.assertIn('

Test!

', test_wrap_output.content) + self.assertIn('data-custom-attribute="custom-value"', test_wrap_output.content) self.assertEqual(test_wrap_output.resources[0].data, u'body {background-color:red;}') self.assertEqual(test_wrap_output.resources[1].data, 'alert("Hi!");') diff --git a/openedx/core/lib/xblock_utils/__init__.py b/openedx/core/lib/xblock_utils/__init__.py index c46739976e..d506c091eb 100644 --- a/openedx/core/lib/xblock_utils/__init__.py +++ b/openedx/core/lib/xblock_utils/__init__.py @@ -92,6 +92,9 @@ def wrap_xblock( data = {} data.update(extra_data) + if context: + data.update(context.get('wrap_xblock_data', {})) + css_classes = [ 'xblock', 'xblock-{}'.format(markupsafe.escape(view)), diff --git a/openedx/tests/completion_integration/test_services.py b/openedx/tests/completion_integration/test_services.py index 2027dd0d15..3d41cf0930 100644 --- a/openedx/tests/completion_integration/test_services.py +++ b/openedx/tests/completion_integration/test_services.py @@ -36,6 +36,10 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest parent=cls.sequence, category='vertical', ) + cls.html = ItemFactory.create( + parent=cls.vertical, + category='html', + ) cls.problem = ItemFactory.create( parent=cls.vertical, category="problem", @@ -70,6 +74,13 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest self.completion_service = CompletionService(self.user, self.course_key) # Proper completions for the given runtime + BlockCompletion.objects.submit_completion( + user=self.user, + course_key=self.course_key, + block_key=self.html.location, + completion=1.0, + ) + for idx, block_key in enumerate(self.block_keys[0:3]): BlockCompletion.objects.submit_completion( user=self.user, @@ -148,3 +159,12 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTest self.completion_service.vertical_is_complete(self.vertical), False, ) + + def test_can_mark_block_complete_on_view(self): + + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.course), False) + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.chapter), False) + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.sequence), False) + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.vertical), False) + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.html), True) + self.assertEqual(self.completion_service.can_mark_block_complete_on_view(self.problem), False) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f4463cf502..056401c72a 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -111,7 +111,7 @@ edx-ace==0.1.8 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 -edx-completion==0.1.6 +edx-completion==0.1.7 edx-django-oauth2-provider==1.2.5 edx-django-release-util==0.3.1 edx-django-sites-extensions==2.3.1 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 047277c86b..60d96a7714 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -131,7 +131,7 @@ edx-ace==0.1.8 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 -edx-completion==0.1.6 +edx-completion==0.1.7 edx-django-oauth2-provider==1.2.5 edx-django-release-util==0.3.1 edx-django-sites-extensions==2.3.1 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 05c0b2da40..74c060e015 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -126,7 +126,7 @@ edx-ace==0.1.8 edx-analytics-data-api-client==0.14.4 edx-ccx-keys==0.2.1 edx-celeryutils==0.2.7 -edx-completion==0.1.6 +edx-completion==0.1.7 edx-django-oauth2-provider==1.2.5 edx-django-release-util==0.3.1 edx-django-sites-extensions==2.3.1 diff --git a/webpack.common.config.js b/webpack.common.config.js index 4e2f98921f..044dc849a1 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -40,7 +40,7 @@ module.exports = { ProgramDetailsFactory: './lms/static/js/learner_dashboard/program_details_factory.js', ProgramListFactory: './lms/static/js/learner_dashboard/program_list_factory.js', UnenrollmentFactory: './lms/static/js/learner_dashboard/unenrollment_factory.js', - ViewedEvent: './lms/static/completion/js/ViewedEvent.js', + CompletionOnViewService: './lms/static/completion/js/CompletionOnViewService.js', // Features CourseGoals: './openedx/features/course_experience/static/course_experience/js/CourseGoals.js',