diff --git a/common/lib/xmodule/xmodule/css/sequence/display.scss b/common/lib/xmodule/xmodule/css/sequence/display.scss index 620fdae718..6b9fa37e85 100644 --- a/common/lib/xmodule/xmodule/css/sequence/display.scss +++ b/common/lib/xmodule/xmodule/css/sequence/display.scss @@ -120,7 +120,7 @@ $seq-nav-height: 50px; overflow: visible; // for tooltip - IE11 uses 'hidden' by default if width/height is specified .icon { - display: block; + display: inline-block; line-height: 100%; // This matches the height of the its within (the parent) to get vertical centering. font-size: 110%; color: $seq-nav-icon-color-muted; diff --git a/common/lib/xmodule/xmodule/js/fixtures/sequence.html b/common/lib/xmodule/xmodule/js/fixtures/sequence.html index ab34e566fa..a92fccaa5f 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/sequence.html +++ b/common/lib/xmodule/xmodule/js/fixtures/sequence.html @@ -15,6 +15,7 @@ @@ -22,6 +23,7 @@ diff --git a/common/lib/xmodule/xmodule/js/spec/sequence/display_spec.js b/common/lib/xmodule/xmodule/js/spec/sequence/display_spec.js index 2ae0381e71..7413547b8e 100644 --- a/common/lib/xmodule/xmodule/js/spec/sequence/display_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/sequence/display_spec.js @@ -14,6 +14,7 @@ beforeEach(function() { loadFixtures('sequence.html'); local.XBlock = window.XBlock = jasmine.createSpyObj('XBlock', ['initializeBlocks']); + this.sequence = new Sequence($('.xblock-student_view-sequential')); }); afterEach(function() { @@ -29,7 +30,6 @@ describe('Navbar', function() { it('works with keyboard navigation LEFT and ENTER', function() { - this.sequence = new Sequence($('.xblock-student_view-sequential')); this.sequence.$('.nav-item[data-index=0]').focus(); keydownHandler(keys.LEFT); keydownHandler(keys.ENTER); @@ -47,7 +47,6 @@ }); it('works with keyboard navigation RIGHT and ENTER', function() { - this.sequence = new Sequence($('.xblock-student_view-sequential')); this.sequence.$('.nav-item[data-index=0]').focus(); keydownHandler(keys.RIGHT); keydownHandler(keys.ENTER); @@ -63,6 +62,64 @@ tabindex: '0' }); }); + + it('Completion Indicator missing', function() { + this.sequence.$('.nav-item[data-index=0]').children('.check-circle').remove(); + spyOn($, 'postWithPrefix').and.callFake(function(url, data, callback) { + callback({ + complete: true + }); + }); + this.sequence.update_completion(1); + expect($.postWithPrefix).not.toHaveBeenCalled(); + }); + + describe('Completion', function() { + beforeEach(function() { + expect( + this.sequence.$('.nav-item[data-index=0]').children('.check-circle').first() + .hasClass('is-hidden') + ).toBe(true); + expect( + this.sequence.$('.nav-item[data-index=1]').children('.check-circle').first() + .hasClass('is-hidden') + ).toBe(true); + }); + + afterEach(function() { + expect($.postWithPrefix).toHaveBeenCalled(); + expect( + this.sequence.$('.nav-item[data-index=1]').children('.check-circle').first() + .hasClass('is-hidden') + ).toBe(true); + }); + + it('API check returned true', function() { + spyOn($, 'postWithPrefix').and.callFake(function(url, data, callback) { + callback({ + complete: true + }); + }); + this.sequence.update_completion(1); + expect( + this.sequence.$('.nav-item[data-index=0]').children('.check-circle').first() + .hasClass('is-hidden') + ).toBe(false); + }); + + it('API check returned false', function() { + spyOn($, 'postWithPrefix').and.callFake(function(url, data, callback) { + callback({ + complete: false + }); + }); + this.sequence.update_completion(1); + expect( + this.sequence.$('.nav-item[data-index=0]').children('.check-circle').first() + .hasClass('is-hidden') + ).toBe(true); + }); + }); }); }); }).call(this); diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.js b/common/lib/xmodule/xmodule/js/src/sequence/display.js index 65588aaaa1..ff2d01eab1 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/display.js +++ b/common/lib/xmodule/xmodule/js/src/sequence/display.js @@ -230,6 +230,7 @@ if (this.position !== newPosition) { if (this.position) { this.mark_visited(this.position); + this.update_completion(this.position); modxFullUrl = '' + this.ajaxUrl + '/goto_position'; $.postWithPrefix(modxFullUrl, { position: newPosition @@ -400,6 +401,22 @@ .addClass('visited'); }; + Sequence.prototype.update_completion = function(position) { + var element = this.link_for(position); + var completionUrl = this.ajaxUrl + '/get_completion'; + var usageKey = element[0].attributes['data-id'].value; + var completionIndicators = element.find('.check-circle'); + if (completionIndicators.length) { + $.postWithPrefix(completionUrl, { + usage_key: usageKey + }, function(data) { + if (data.complete === true) { + completionIndicators.removeClass('is-hidden'); + } + }); + } + }; + Sequence.prototype.mark_active = function(position) { // Don't overwrite class attribute to avoid changing Progress class var element = this.link_for(position); diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 06002b001d..751a895fae 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -9,6 +9,7 @@ import logging from datetime import datetime from lxml import etree +from opaque_keys.edx.keys import UsageKey from pkg_resources import resource_string from pytz import UTC from six import text_type @@ -161,6 +162,7 @@ class ProctoringFields(object): @XBlock.wants('verification') @XBlock.wants('gating') @XBlock.wants('credit') +@XBlock.wants('completion') @XBlock.needs('user') @XBlock.needs('bookmarks') class SequenceModule(SequenceFields, ProctoringFields, XModule): @@ -206,6 +208,20 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): self.position = 1 return json.dumps({'success': True}) + if dispatch == 'get_completion': + completion_service = self.runtime.service(self, 'completion') + if not completion_service.visual_progress_enabled(): + return None + + usage_key = data.get('usage_key', None) + item = self.get_child(UsageKey.from_string(usage_key)) + if not item: + return None + + complete = completion_service.vertical_is_complete(item) + return json.dumps({ + 'complete': complete + }) raise NotFoundError('Unexpected dispatch type') @classmethod @@ -414,6 +430,7 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): """ is_user_authenticated = self.is_user_authenticated(context) bookmarks_service = self.runtime.service(self, 'bookmarks') + completion_service = self.runtime.service(self, 'completion') context['username'] = self.runtime.service(self, 'user').get_current_user().opt_attrs.get( 'edx-platform.username') display_names = [ @@ -455,6 +472,9 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): 'path': " > ".join(display_names + [item.display_name_with_default]), } + if is_user_authenticated and completion_service.visual_progress_enabled(): + iteminfo['complete'] = completion_service.vertical_is_complete(item) + contents.append(iteminfo) return contents diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index b953dc436c..fbc6281000 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -2,6 +2,8 @@ Tests for sequence module. """ # pylint: disable=no-member +import json + from datetime import timedelta from django.utils.timezone import now from freezegun import freeze_time @@ -72,6 +74,9 @@ class SequenceBlockTestCase(XModuleXmlImportTest): self._set_up_module_system(block) block.xmodule_runtime._services['bookmarks'] = Mock() # pylint: disable=protected-access + block.xmodule_runtime._services['completion'] = Mock( # pylint: disable=protected-access + return_value=Mock(vertical_is_complete=Mock(return_value=True)) + ) block.xmodule_runtime._services['user'] = StubUserService() # pylint: disable=protected-access block.xmodule_runtime.xmodule_instance = getattr(block, '_xmodule', None) # pylint: disable=protected-access block.parent = parent.location @@ -121,6 +126,7 @@ class SequenceBlockTestCase(XModuleXmlImportTest): self.assertIn("'gated': False", html) self.assertIn("'next_url': 'NextSequential'", html) self.assertIn("'prev_url': 'PrevSequential'", html) + self.assertNotIn("fa fa-check-circle check-circle is-hidden", html) def test_student_view_first_child(self): html = self._get_rendered_student_view(self.sequence_3_1, requested_child='first') @@ -273,3 +279,34 @@ class SequenceBlockTestCase(XModuleXmlImportTest): # assert content shown as normal self._assert_ungated(html, self.sequence_1_2) + + def test_handle_ajax_get_completion_disabled(self): + """ + Test when completion service is turned off by waffle, the ajax call returns correct + None value + """ + completion_waffle_mock = Mock() + completion_waffle_mock.return_value.visual_progress_enabled.return_value = False + self.sequence_3_1.xmodule_runtime._services['completion'] = completion_waffle_mock # pylint: disable=protected-access + for child in self.sequence_3_1.get_children(): + usage_key = unicode(child.location) + completion_return = self.sequence_3_1.handle_ajax( + 'get_completion', + {'usage_key': usage_key} + ) + self.assertIs(completion_return, None) + + def test_handle_ajax_get_completion_success(self): + """ + Test that the completion data is returned successfully on + targeted vertical through ajax call + """ + for child in self.sequence_3_1.get_children(): + usage_key = unicode(child.location) + completion_return = json.loads(self.sequence_3_1.handle_ajax( + 'get_completion', + {'usage_key': usage_key} + )) + self.assertIsNot(completion_return, None) + self.assertTrue('complete' in completion_return) + self.assertEqual(completion_return['complete'], True) diff --git a/lms/djangoapps/completion/models.py b/lms/djangoapps/completion/models.py index ebc1ad6391..e6d7716f85 100644 --- a/lms/djangoapps/completion/models.py +++ b/lms/djangoapps/completion/models.py @@ -6,7 +6,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera from django.contrib.auth.models import User from django.core.exceptions import ValidationError -from django.db import models, transaction, connection +from django.db import models, transaction from django.utils.translation import ugettext as _ from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField @@ -161,12 +161,26 @@ class BlockCompletion(TimeStampedModel, models.Model): id = BigAutoField(primary_key=True) # pylint: disable=invalid-name user = models.ForeignKey(User) course_key = CourseKeyField(max_length=255) + + # note: this usage key may not have the run filled in for + # old mongo courses. Use the full_block_key property + # instead when you want to use/compare the usage_key. block_key = UsageKeyField(max_length=255) block_type = models.CharField(max_length=64) completion = models.FloatField(validators=[validate_percent]) objects = BlockCompletionManager() + @property + def full_block_key(self): + """ + Returns the "correct" usage key value with the run filled in. + """ + if self.block_key.run is None: + return self.block_key.replace(course_key=self.course_key) # pylint: disable=unexpected-keyword-arg, no-value-for-parameter + else: + return self.block_key + @classmethod def get_course_completions(cls, user, course_key): """ diff --git a/lms/djangoapps/completion/services.py b/lms/djangoapps/completion/services.py index 862a2af338..d92db3f351 100644 --- a/lms/djangoapps/completion/services.py +++ b/lms/djangoapps/completion/services.py @@ -13,7 +13,9 @@ class CompletionService(object): Exposes * self.completion_tracking_enabled() -> bool + * self.visual_progress_enabled() -> bool * self.get_completions(candidates) + * self.vertical_is_complete(vertical_item) Constructor takes a user object and course_key as arguments. """ @@ -31,6 +33,16 @@ class CompletionService(object): """ return waffle.waffle().is_enabled(waffle.ENABLE_COMPLETION_TRACKING) + def visual_progress_enabled(self): + """ + Exposes VISUAL_PROGRESS_ENABLED waffle switch to XModule runtime + + Return value: + + bool -> True if VISUAL_PROGRESS flag is enabled. + """ + return waffle.visual_progress_enabled(self._course_key) + def get_completions(self, candidates): """ Given an iterable collection of block_keys in the course, returns a @@ -54,8 +66,34 @@ class CompletionService(object): course_key=self._course_key, block_key__in=candidates, ) - completions = {block.block_key: block.completion for block in completion_queryset} + completions = { + block.full_block_key: block.completion for block in completion_queryset # pylint: disable=not-an-iterable + } for candidate in candidates: if candidate not in completions: completions[candidate] = 0.0 return completions + + def vertical_is_complete(self, item): + """ + Calculates and returns whether a particular vertical is complete. + The logic in this method is temporary, and will go away once the + completion API is able to store a first-order notion of completeness + for parent blocks (right now it just stores completion for leaves- + problems, HTML, video, etc.). + """ + if item.location.block_type != 'vertical': + raise ValueError('The passed in xblock is not a vertical type!') + + if not self.completion_tracking_enabled(): + return None + + # this is temporary local logic and will be removed when the whole course tree is included in completion + child_locations = [ + child.location for child in item.get_children() if child.location.block_type != 'discussion' + ] + completions = self.get_completions(child_locations) + for child_location in child_locations: + if completions[child_location] < 1.0: + return False + return True diff --git a/lms/djangoapps/completion/tests/test_services.py b/lms/djangoapps/completion/tests/test_services.py index 517f502e68..24563ba4b0 100644 --- a/lms/djangoapps/completion/tests/test_services.py +++ b/lms/djangoapps/completion/tests/test_services.py @@ -2,9 +2,10 @@ Tests of completion xblock runtime services """ import ddt -from django.test import TestCase -from opaque_keys.edx.keys import CourseKey, UsageKey +from opaque_keys.edx.keys import CourseKey from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from ..models import BlockCompletion from ..services import CompletionService @@ -12,20 +13,58 @@ from ..test_utils import CompletionWaffleTestMixin @ddt.ddt -class CompletionServiceTestCase(CompletionWaffleTestMixin, TestCase): +class CompletionServiceTestCase(CompletionWaffleTestMixin, SharedModuleStoreTestCase): """ Test the data returned by the CompletionService. """ + @classmethod + def setUpClass(cls): + super(CompletionServiceTestCase, cls).setUpClass() + cls.course = CourseFactory.create() + with cls.store.bulk_operations(cls.course.id): + cls.chapter = ItemFactory.create( + parent=cls.course, + category="chapter", + ) + cls.sequence = ItemFactory.create( + parent=cls.chapter, + category='sequential', + ) + cls.vertical = ItemFactory.create( + parent=cls.sequence, + category='vertical', + ) + cls.problem = ItemFactory.create( + parent=cls.vertical, + category="problem", + ) + cls.problem2 = ItemFactory.create( + parent=cls.vertical, + category="problem", + ) + cls.problem3 = ItemFactory.create( + parent=cls.vertical, + category="problem", + ) + cls.problem4 = ItemFactory.create( + parent=cls.vertical, + category="problem", + ) + cls.problem5 = ItemFactory.create( + parent=cls.vertical, + category="problem", + ) + cls.store.update_item(cls.course, UserFactory().id) + cls.problems = [cls.problem, cls.problem2, cls.problem3, cls.problem4, cls.problem5] def setUp(self): super(CompletionServiceTestCase, self).setUp() self.override_waffle_switch(True) self.user = UserFactory.create() self.other_user = UserFactory.create() - self.course_key = CourseKey.from_string("edX/MOOC101/2049_T2") + self.course_key = self.course.id self.other_course_key = CourseKey.from_string("course-v1:ReedX+Hum110+1904") - self.block_keys = [UsageKey.from_string("i4x://edX/MOOC101/video/{}".format(number)) for number in xrange(5)] - + self.block_keys = [problem.location for problem in self.problems] self.completion_service = CompletionService(self.user, self.course_key) # Proper completions for the given runtime @@ -72,3 +111,38 @@ class CompletionServiceTestCase(CompletionWaffleTestMixin, TestCase): def test_enabled_honors_waffle_switch(self, enabled): self.override_waffle_switch(enabled) self.assertEqual(self.completion_service.completion_tracking_enabled(), enabled) + + def test_vertical_completion(self): + self.assertEqual( + self.completion_service.vertical_is_complete(self.vertical), + False, + ) + + for block_key in self.block_keys: + BlockCompletion.objects.submit_completion( + user=self.user, + course_key=self.course_key, + block_key=block_key, + completion=1.0 + ) + + self.assertEqual( + self.completion_service.vertical_is_complete(self.vertical), + True, + ) + + def test_vertical_partial_completion(self): + block_keys_count = len(self.block_keys) + for i in range(block_keys_count - 1): + # Mark all the child blocks completed except the last one + BlockCompletion.objects.submit_completion( + user=self.user, + course_key=self.course_key, + block_key=self.block_keys[i], + completion=1.0 + ) + + self.assertEqual( + self.completion_service.vertical_is_complete(self.vertical), + False, + ) diff --git a/lms/djangoapps/course_api/blocks/transformers/block_completion.py b/lms/djangoapps/course_api/blocks/transformers/block_completion.py index 99404b6f22..09c2f8d0f7 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_completion.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_completion.py @@ -16,9 +16,6 @@ class BlockCompletionTransformer(BlockStructureTransformer): WRITE_VERSION = 1 COMPLETION = 'completion' - def __init__(self): - super(BlockCompletionTransformer, self).__init__() - @classmethod def name(cls): return "blocks_api:completion" diff --git a/lms/templates/seq_module.html b/lms/templates/seq_module.html index bf70279ded..a6bc969de6 100644 --- a/lms/templates/seq_module.html +++ b/lms/templates/seq_module.html @@ -13,6 +13,7 @@ % endif +
- + % else: % for idx, item in enumerate(items):
  • @@ -47,6 +48,13 @@ id="tab_${idx}" ${"disabled=disabled" if disable_navigation else ""}> + % if 'complete' in item: + + % endif
    ${item['type']} ${item['page_title']} ${_("Bookmarked") if item['bookmarked'] else ""}
    @@ -56,7 +64,6 @@
    • - % if gated_content['gated']: <%include file="_gated_content.html" args="prereq_url=gated_content['prereq_url'], prereq_section_name=gated_content['prereq_section_name'], gated_section_name=gated_content['gated_section_name']"/> % else: