diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index ed841fd031..96fe986e2a 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -720,8 +720,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, submit_completions_for_testing(self.user, course_key, block_keys) - with patch('completion.utilities.visual_progress_enabled', return_value=True): - response = self.client.get(reverse('dashboard')) + response = self.client.get(reverse('dashboard')) course_key_string = str(course_key) resume_block_key_string = str(block_keys[-1]) @@ -808,8 +807,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin, ) ) - with patch('completion.utilities.visual_progress_enabled', return_value=True): - response = self.client.get(reverse('dashboard')) + response = self.client.get(reverse('dashboard')) html_for_view_buttons = [ self._remove_whitespace_from_html_string(button) diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 751a895fae..8f7320381c 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -210,8 +210,6 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): 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)) @@ -472,8 +470,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) + if is_user_authenticated: + if item.location.block_type == 'vertical': + iteminfo['complete'] = completion_service.vertical_is_complete(item) contents.append(iteminfo) diff --git a/common/lib/xmodule/xmodule/tests/test_sequence.py b/common/lib/xmodule/xmodule/tests/test_sequence.py index fbc6281000..67d46a3ae3 100644 --- a/common/lib/xmodule/xmodule/tests/test_sequence.py +++ b/common/lib/xmodule/xmodule/tests/test_sequence.py @@ -280,22 +280,6 @@ 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 diff --git a/common/test/acceptance/tests/lms/test_lms_course_home.py b/common/test/acceptance/tests/lms/test_lms_course_home.py index 82545527d7..65b2cbec7b 100644 --- a/common/test/acceptance/tests/lms/test_lms_course_home.py +++ b/common/test/acceptance/tests/lms/test_lms_course_home.py @@ -124,16 +124,6 @@ class CourseHomeTest(CourseHomeBaseTest): bookmarks_page = BookmarksPage(self.browser, self.course_id) self.assertTrue(bookmarks_page.is_browser_on_page()) - # Test "Resume Course" button from header - self.course_home_page.visit() - self.course_home_page.resume_course_from_header() - self.assertTrue(self.courseware_page.nav.is_on_section('Test Section 2', 'Test Subsection 3')) - - # Test "Resume Course" button from within outline - self.course_home_page.visit() - self.course_home_page.outline.resume_course_from_outline() - self.assertTrue(self.courseware_page.nav.is_on_section('Test Section 2', 'Test Subsection 3')) - @attr('a11y') class CourseHomeA11yTest(CourseHomeBaseTest): diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py index 94e39bd7bc..ec8cf1f1e9 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_utils.py @@ -113,9 +113,8 @@ class CompletionUtilsTestCase(SharedModuleStoreTestCase, CompletionWaffleTestMix ) @override_settings(LMS_ROOT_URL='test_url:9999') - @patch('completion.waffle.get_current_site') @ddt.data(True, False) - def test_retrieve_last_sitewide_block_completed(self, use_username, get_patched_current_site): # pylint: disable=unused-argument + def test_retrieve_last_sitewide_block_completed(self, use_username): """ Test that the method returns a URL for the "last completed" block when sending a user object diff --git a/openedx/features/course_experience/templates/course_experience/course-outline-fragment-new.html b/openedx/features/course_experience/templates/course_experience/course-outline-fragment-new.html index 23ec168a42..3d596f174a 100644 --- a/openedx/features/course_experience/templates/course_experience/course-outline-fragment-new.html +++ b/openedx/features/course_experience/templates/course_experience/course-outline-fragment-new.html @@ -37,7 +37,7 @@ from openedx.core.djangolib.markup import HTML, Text id="${ section['id'] }">

${ section['display_name'] }

- % if show_visual_progress and section.get('complete'): + % if section.get('complete'): % endif @@ -83,7 +83,7 @@ from openedx.core.djangolib.markup import HTML, Text ${ subsection['display_name'] } - % if subsection.get('complete') and show_visual_progress: + % if subsection.get('complete'): % endif % endif @@ -168,7 +168,7 @@ from openedx.core.djangolib.markup import HTML, Text ${ vertical['display_name'] } - % if vertical.get('complete') and show_visual_progress: + % if vertical.get('complete'): % endif diff --git a/openedx/features/course_experience/tests/views/test_course_outline.py b/openedx/features/course_experience/tests/views/test_course_outline.py index ac734e42f8..28d43c2044 100644 --- a/openedx/features/course_experience/tests/views/test_course_outline.py +++ b/openedx/features/course_experience/tests/views/test_course_outline.py @@ -307,11 +307,6 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT cls.user = UserFactory(password=TEST_PASSWORD) CourseEnrollment.enroll(cls.user, cls.course.id) cls.site = Site.objects.get_current() - SiteConfiguration.objects.get_or_create( - site=cls.site, - enabled=True, - values={waffle.ENABLE_SITE_VISUAL_PROGRESS: True} - ) @classmethod def create_test_course(cls): @@ -354,6 +349,27 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT ) self.assertEqual(200, self.client.get(last_accessed_url).status_code) + @override_switch( + '{}.{}'.format( + waffle.WAFFLE_NAMESPACE, waffle.ENABLE_COMPLETION_TRACKING + ), + active=True + ) + def complete_sequential(self, course, sequential): + """ + Completes provided sequential. + """ + course_key = CourseKey.from_string(str(course.id)) + # Fake a visit to sequence2/vertical2 + block_key = UsageKey.from_string(unicode(sequential.location)) + completion = 1.0 + BlockCompletion.objects.submit_completion( + user=self.user, + course_key=course_key, + block_key=block_key, + completion=completion + ) + def visit_course_home(self, course, start_count=0, resume_count=0): """ Helper function to navigates to course home page, test for resume buttons @@ -384,37 +400,12 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT self.assertTrue(content('.action-resume-course').attr('href').endswith('/course/' + course.url_name)) - def test_resume_course(self): - """ - Tests that two resume course buttons appear when the course has been accessed. - """ - course = self.course - - # first navigate to a sequential to make it the last accessed - chapter = course.children[0] - sequential = chapter.children[0] - vertical = sequential.children[0] - self.visit_sequential(course, chapter, sequential) - - # check resume course buttons - response = self.visit_course_home(course, resume_count=2) - content = pq(response.content) - self.assertTrue(content('.action-resume-course').attr('href').endswith('/vertical/' + vertical.url_name)) - - @override_switch( - '{}.{}'.format( - waffle.WAFFLE_NAMESPACE, waffle.ENABLE_VISUAL_PROGRESS - ), - active=True - ) @override_settings(LMS_BASE='test_url:9999') - @patch('completion.waffle.get_current_site') - def test_resume_course_with_completion_api(self, get_patched_current_site): + def test_resume_course_with_completion_api(self): """ Tests completion API resume button functionality """ self.override_waffle_switch(True) - get_patched_current_site.return_value = self.site # Course tree course = self.course @@ -422,16 +413,7 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT vertical1 = course.children[0].children[0].children[0] vertical2 = course.children[0].children[1].children[0] - # Fake a visit to sequence1/vertical1 - block_key = UsageKey.from_string(unicode(vertical1.location)) - completion = 1.0 - BlockCompletion.objects.submit_completion( - user=self.user, - course_key=course_key, - block_key=block_key, - completion=completion - ) - + self.complete_sequential(self.course, vertical1) # Test for 'resume' link response = self.visit_course_home(course, resume_count=2) @@ -439,15 +421,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT content = pq(response.content) self.assertTrue(content('.action-resume-course').attr('href').endswith('/vertical/' + vertical1.url_name)) - # Fake a visit to sequence2/vertical2 - block_key = UsageKey.from_string(unicode(vertical2.location)) - completion = 1.0 - BlockCompletion.objects.submit_completion( - user=self.user, - course_key=course_key, - block_key=block_key, - completion=completion - ) + self.complete_sequential(self.course, vertical2) + # Test for 'resume' link response = self.visit_course_home(course, resume_count=2) # Test for 'resume' link URL - should be vertical 2 @@ -465,7 +440,7 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT def test_resume_course_deleted_sequential(self): """ - Tests resume course when the last accessed sequential is deleted and + Tests resume course when the last completed sequential is deleted and there is another sequential in the vertical. """ @@ -476,7 +451,8 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT self.assertGreaterEqual(len(chapter.children), 2) sequential = chapter.children[0] sequential2 = chapter.children[1] - self.visit_sequential(course, chapter, sequential) + self.complete_sequential(course, sequential) + self.complete_sequential(course, sequential2) # remove one of the sequentials from the chapter with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id): @@ -490,7 +466,7 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT def test_resume_course_deleted_sequentials(self): """ - Tests resume course when the last accessed sequential is deleted and + Tests resume course when the last completed sequential is deleted and there are no sequentials left in the vertical. """ @@ -500,7 +476,7 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT chapter = course.children[0] self.assertEqual(len(chapter.children), 2) sequential = chapter.children[0] - self.visit_sequential(course, chapter, sequential) + self.complete_sequential(course, sequential) # remove all sequentials from chapter with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id): @@ -508,22 +484,14 @@ class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase, CompletionWaffleT self.store.delete_item(sequential.location, self.user.id) # check resume course buttons - self.visit_course_home(course, resume_count=1) + self.visit_course_home(course, start_count=1, resume_count=0) - @override_switch( - '{}.{}'.format( - waffle.WAFFLE_NAMESPACE, waffle.ENABLE_VISUAL_PROGRESS - ), - active=True - ) - @patch('completion.waffle.get_current_site') - def test_course_home_for_global_staff(self, get_patched_current_site): + def test_course_home_for_global_staff(self): """ Tests that staff user can access the course home without being enrolled in the course. """ course = self.course - get_patched_current_site.return_value = self.site self.user.is_staff = True self.user.save() diff --git a/openedx/features/course_experience/utils.py b/openedx/features/course_experience/utils.py index 50467714e8..3870588800 100644 --- a/openedx/features/course_experience/utils.py +++ b/openedx/features/course_experience/utils.py @@ -2,7 +2,6 @@ Common utilities for the course experience, including course outline. """ from completion.models import BlockCompletion -from completion.waffle import visual_progress_enabled from lms.djangoapps.course_api.blocks.api import get_blocks from lms.djangoapps.course_blocks.utils import get_student_module_as_dict @@ -154,14 +153,11 @@ def get_course_outline_block_tree(request, course_id): populate_children(course_outline_root_block, all_blocks['blocks']) set_last_accessed_default(course_outline_root_block) - if visual_progress_enabled(course_key=course_key): - mark_blocks_completed( - block=course_outline_root_block, - user=request.user, - course_key=course_key - ) - else: - mark_last_accessed(request.user, course_key, course_outline_root_block) + mark_blocks_completed( + block=course_outline_root_block, + user=request.user, + course_key=course_key + ) return course_outline_root_block diff --git a/openedx/features/course_experience/views/course_outline.py b/openedx/features/course_experience/views/course_outline.py index cd12fd97f0..d404813659 100644 --- a/openedx/features/course_experience/views/course_outline.py +++ b/openedx/features/course_experience/views/course_outline.py @@ -43,18 +43,11 @@ class CourseOutlineFragmentView(EdxFragmentView): if not course_block_tree: return None - # TODO: EDUCATOR-2283 Remove 'show_visual_progress' from context - # and remove the check for it in the HTML file - show_visual_progress = ( - completion_waffle.visual_progress_enabled(course_key) and - self.user_enrolled_after_completion_collection(request.user, course_key) - ) context = { 'csrf': csrf(request)['csrf_token'], 'course': course_overview, - 'blocks': course_block_tree, - 'show_visual_progress': show_visual_progress, 'due_date_display_format': course.due_date_display_format, + 'blocks': course_block_tree } # TODO: EDUCATOR-2283 Remove this check when the waffle flag is turned on in production diff --git a/openedx/tests/completion_integration/test_waffle.py b/openedx/tests/completion_integration/test_waffle.py deleted file mode 100644 index 963009896a..0000000000 --- a/openedx/tests/completion_integration/test_waffle.py +++ /dev/null @@ -1,79 +0,0 @@ -""" -Tests waffle mechanics and feature-gating for block completion features. -""" - -from __future__ import absolute_import, unicode_literals - -from completion import waffle -import ddt -from django.test import TestCase -import mock -from opaque_keys.edx.keys import CourseKey - -from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory, SiteConfigurationFactory -from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag -from openedx.core.djangolib.testing.utils import skip_unless_lms - - -@skip_unless_lms -@ddt.ddt -class FeatureGatingTests(TestCase): - """ - Tests the logic of the completion feature-gating functions in the LMS. - """ - def setUp(self): - self.site = SiteFactory.create() - self.site_patcher = mock.patch('completion.waffle.get_current_site') - mocked_current_site = self.site_patcher.start() - mocked_current_site.return_value = self.site - - self.course_key = CourseKey.from_string('course-v1:edX+DemoX+Demo_Course') - - def tearDown(self): - self.site_patcher.stop() - - def _make_site_config(self, enable_feature): - site_config = SiteConfigurationFactory.create(site=self.site) - site_config.values[waffle.ENABLE_SITE_VISUAL_PROGRESS] = enable_feature - site_config.save() - - def test_site_disables_visual_progress_no_config(self): - assert waffle.site_disables_visual_progress() is False - - @ddt.data(True, False) - def test_site_disables_visual_progress_with_config(self, config_value): - self._make_site_config(config_value) - assert (not config_value) == waffle.site_disables_visual_progress() - - def test_visual_progress_gating_tracking_disabled(self): - with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, False): - assert waffle.visual_progress_enabled(self.course_key) is False - - def test_visual_progress_gating_site_disabled(self): - self._make_site_config(False) - assert waffle.visual_progress_enabled(self.course_key) is False - - def test_visual_progress_gating_course_disabled(self): - self._make_site_config(True) - with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True): - with waffle.waffle().override(waffle.ENABLE_VISUAL_PROGRESS, False): - with override_waffle_flag(waffle.waffle_flag(), active=False): - assert waffle.visual_progress_enabled(self.course_key) is False - - def test_visual_progress_happy_path_no_site_config(self): - with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True): - with waffle.waffle().override(waffle.ENABLE_VISUAL_PROGRESS, True): - assert waffle.visual_progress_enabled(self.course_key) is True - - def test_visual_progress_happy_path_with_site_config(self): - self._make_site_config(True) - with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True): - with waffle.waffle().override(waffle.ENABLE_VISUAL_PROGRESS, True): - assert waffle.visual_progress_enabled(self.course_key) is True - - def test_visual_progress_happy_path_visual_switch_disabled(self): - self._make_site_config(True) - with waffle.waffle().override(waffle.ENABLE_COMPLETION_TRACKING, True): - with waffle.waffle().override(waffle.ENABLE_VISUAL_PROGRESS, False): - with override_waffle_flag(waffle.waffle_flag(), active=True): - assert waffle.visual_progress_enabled(self.course_key) is True diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index d143013ffb..c61145cb34 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -54,7 +54,7 @@ edx-lint==0.5.4 git+https://github.com/cpennington/pylint-django@fix-field-inference-during-monkey-patching#egg=pylint-django==0.0 enum34==1.1.6 -edx-completion==0.1.4 +edx-completion==0.1.5 edx-django-oauth2-provider==1.2.5 edx-django-sites-extensions==2.3.1 edx-enterprise==0.67.3