From f9c37595715f07cd9de9d1d1e86afc1f0a16dd26 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Thu, 4 Feb 2016 17:59:13 +0500 Subject: [PATCH 1/5] Fix self-paced course displaying "published, not yet released" issue in Studio ECOM-3615 --- cms/djangoapps/contentstore/utils.py | 15 +++++++++++ cms/djangoapps/contentstore/views/item.py | 14 ++++++---- .../contentstore/views/tests/test_item.py | 27 +++++++++++++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 711b3947f89b..d17ab74393e8 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -13,6 +13,8 @@ from django_comment_common.models import assign_default_role from django_comment_common.utils import seed_permissions_roles +from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration + from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError @@ -455,3 +457,16 @@ def get_visibility_partition_info(xblock): "has_selected_groups": has_selected_groups, "selected_verified_partition_id": selected_verified_partition_id, } + + +def is_self_paced(course): + """ + Returns True if course is self-paced, False otherwise. + """ + if course: + try: + return course.self_paced and SelfPacedConfiguration.current().enabled + except AttributeError: + # if course object has no self_paced attribute + pass + return False diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 6b305b075feb..b5348df581db 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -31,6 +31,8 @@ from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ xblock_type_display_name, get_parent_xblock, create_xblock, usage_key_with_run from contentstore.views.preview import get_preview_fragment +from contentstore.utils import is_self_paced + from openedx.core.lib.gating import api as gating_api from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel @@ -855,7 +857,9 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F release_date = _get_release_date(xblock, user) if xblock.category != 'course': - visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes) + visibility_state = _compute_visibility_state( + xblock, child_info, is_xblock_unit and has_changes, is_self_paced(course) + ) else: visibility_state = None published = modulestore().has_published_version(xblock) if not is_library_block else None @@ -1017,7 +1021,7 @@ class VisibilityState(object): gated = 'gated' -def _compute_visibility_state(xblock, child_info, is_unit_with_changes): +def _compute_visibility_state(xblock, child_info, is_unit_with_changes, is_course_self_paced): """ Returns the current publish state for the specified xblock and its children """ @@ -1027,10 +1031,10 @@ def _compute_visibility_state(xblock, child_info, is_unit_with_changes): # Note that a unit that has never been published will fall into this category, # as well as previously published units with draft content. return VisibilityState.needs_attention + is_unscheduled = xblock.start == DEFAULT_START_DATE - is_live = datetime.now(UTC) > xblock.start - children = child_info and child_info.get('children', []) - if children and len(children) > 0: + is_live = is_course_self_paced or datetime.now(UTC) > xblock.start + if child_info and child_info.get('children', []): all_staff_only = True all_unscheduled = True all_live = True diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 558bdeeae27b..4711be650ee8 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -14,6 +14,7 @@ from django.core.urlresolvers import reverse from contentstore.utils import reverse_usage_url, reverse_course_url +from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration from contentstore.views.component import ( component_handler, get_component_templates ) @@ -2169,3 +2170,29 @@ def test_locked_unit_staff_only_message(self): self._verify_has_staff_only_message(xblock_info, True) self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_SUBSECTION_PATH) self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_UNIT_PATH) + + def test_self_paced_item_visibility_state(self): + """ + Test that in self-paced course, item has `live` visibility state. + Test that when item was initially in `scheduled` state in instructor mode, change course pacing to self-paced, + now in self-paced course, item should have `live` visibility state. + """ + SelfPacedConfiguration(enabled=True).save() + + # Create chapter and setup future release date to make chapter in scheduled state + chapter = self._create_child(self.course, 'chapter', "Test Chapter") + self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) + + # Check that has scheduled state + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.ready) + self.assertFalse(self.course.self_paced) + + # Change course pacing to self paced + self.course.self_paced = True + self.store.update_item(self.course, self.user.id) + self.assertTrue(self.course.self_paced) + + # Check that in self paced course content has live state now + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.live) From df0b983ea3cac91e5cfd36065d7da5f93822f2fc Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Fri, 12 Feb 2016 17:31:00 +0500 Subject: [PATCH 2/5] Improve is_self_paced method --- cms/djangoapps/contentstore/utils.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index d17ab74393e8..9c10a0051633 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -463,10 +463,4 @@ def is_self_paced(course): """ Returns True if course is self-paced, False otherwise. """ - if course: - try: - return course.self_paced and SelfPacedConfiguration.current().enabled - except AttributeError: - # if course object has no self_paced attribute - pass - return False + return course.self_paced and SelfPacedConfiguration.current().enabled if course else False From 3559bd1e698d9cea5d5ba127c0c65aa472bc0300 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Mon, 15 Feb 2016 18:18:46 +0500 Subject: [PATCH 3/5] Refactor test to check support for both modulestore and is_self_paced method improvement --- cms/djangoapps/contentstore/utils.py | 2 +- .../contentstore/views/tests/test_item.py | 40 ++++++++++--------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 9c10a0051633..03a2b6aae259 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -463,4 +463,4 @@ def is_self_paced(course): """ Returns True if course is self-paced, False otherwise. """ - return course.self_paced and SelfPacedConfiguration.current().enabled if course else False + return course and course.self_paced and SelfPacedConfiguration.current().enabled diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 4711be650ee8..833c39d08c3e 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1848,6 +1848,7 @@ def test_no_add_advanced(self): self.assertFalse(lib.children) +@ddt.ddt class TestXBlockPublishingInfo(ItemTest): """ Unit tests for XBlock's outline handling. @@ -2171,7 +2172,8 @@ def test_locked_unit_staff_only_message(self): self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_SUBSECTION_PATH) self._verify_has_staff_only_message(xblock_info, True, path=self.FIRST_UNIT_PATH) - def test_self_paced_item_visibility_state(self): + @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) + def test_self_paced_item_visibility_state(self, store_type): """ Test that in self-paced course, item has `live` visibility state. Test that when item was initially in `scheduled` state in instructor mode, change course pacing to self-paced, @@ -2179,20 +2181,22 @@ def test_self_paced_item_visibility_state(self): """ SelfPacedConfiguration(enabled=True).save() - # Create chapter and setup future release date to make chapter in scheduled state - chapter = self._create_child(self.course, 'chapter', "Test Chapter") - self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) - - # Check that has scheduled state - xblock_info = self._get_xblock_info(chapter.location) - self._verify_visibility_state(xblock_info, VisibilityState.ready) - self.assertFalse(self.course.self_paced) - - # Change course pacing to self paced - self.course.self_paced = True - self.store.update_item(self.course, self.user.id) - self.assertTrue(self.course.self_paced) - - # Check that in self paced course content has live state now - xblock_info = self._get_xblock_info(chapter.location) - self._verify_visibility_state(xblock_info, VisibilityState.live) + with self.store.default_store(store_type): + # Create course, chapter and setup future release date to make chapter in scheduled state + course = CourseFactory.create() + chapter = self._create_child(course, 'chapter', "Test Chapter") + self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) + + # Check that has scheduled state + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.ready) + self.assertFalse(course.self_paced) + + # Change course pacing to self paced + course.self_paced = True + self.store.update_item(course, self.user.id) + self.assertTrue(course.self_paced) + + # Check that in self paced course content has live state now + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.live) From d3fc2625d035dc1eaa4d247a26ddb62bdb45f203 Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Wed, 17 Feb 2016 12:53:12 +0500 Subject: [PATCH 4/5] Code refactor pass store_type in course create method --- .../contentstore/views/tests/test_item.py | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 833c39d08c3e..03024fb587ab 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -2181,22 +2181,21 @@ def test_self_paced_item_visibility_state(self, store_type): """ SelfPacedConfiguration(enabled=True).save() - with self.store.default_store(store_type): - # Create course, chapter and setup future release date to make chapter in scheduled state - course = CourseFactory.create() - chapter = self._create_child(course, 'chapter', "Test Chapter") - self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) - - # Check that has scheduled state - xblock_info = self._get_xblock_info(chapter.location) - self._verify_visibility_state(xblock_info, VisibilityState.ready) - self.assertFalse(course.self_paced) - - # Change course pacing to self paced - course.self_paced = True - self.store.update_item(course, self.user.id) - self.assertTrue(course.self_paced) - - # Check that in self paced course content has live state now - xblock_info = self._get_xblock_info(chapter.location) - self._verify_visibility_state(xblock_info, VisibilityState.live) + # Create course, chapter and setup future release date to make chapter in scheduled state + course = CourseFactory.create(default_store=store_type) + chapter = self._create_child(course, 'chapter', "Test Chapter") + self._set_release_date(chapter.location, datetime.now(UTC) + timedelta(days=1)) + + # Check that chapter has scheduled state + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.ready) + self.assertFalse(course.self_paced) + + # Change course pacing to self paced + course.self_paced = True + self.store.update_item(course, self.user.id) + self.assertTrue(course.self_paced) + + # Check that in self paced course content has live state now + xblock_info = self._get_xblock_info(chapter.location) + self._verify_visibility_state(xblock_info, VisibilityState.live) From 5485d0c23686d5ce5f6009d05552bb639f6da66d Mon Sep 17 00:00:00 2001 From: Mushtaq Ali Date: Wed, 17 Feb 2016 15:54:54 +0500 Subject: [PATCH 5/5] make `is_course_self_paced` default to False --- cms/djangoapps/contentstore/views/item.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index b5348df581db..ada012c6ec18 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -1021,7 +1021,7 @@ class VisibilityState(object): gated = 'gated' -def _compute_visibility_state(xblock, child_info, is_unit_with_changes, is_course_self_paced): +def _compute_visibility_state(xblock, child_info, is_unit_with_changes, is_course_self_paced=False): """ Returns the current publish state for the specified xblock and its children """