diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c93efa34c2..4a26d09ae9 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -194,30 +194,35 @@ def course_image_url(course): return path -class UnitState(object): +class PublishState(object): + """ + The publish state for a given xblock-- either 'draft', 'private', or 'public'. + + Currently in CMS, an xblock can only be in 'draft' or 'private' if it is at or below the Unit level. + """ draft = 'draft' private = 'private' public = 'public' -def compute_unit_state(unit): +def compute_publish_state(xblock): """ - Returns whether this unit is 'draft', 'public', or 'private'. + Returns whether this xblock is 'draft', 'public', or 'private'. 'draft' content is in the process of being edited, but still has a previous version visible in the LMS 'public' content is locked and visible in the LMS - 'private' content is editabled and not visible in the LMS + 'private' content is editable and not visible in the LMS """ - if getattr(unit, 'is_draft', False): + if getattr(xblock, 'is_draft', False): try: - modulestore('direct').get_item(unit.location) - return UnitState.draft + modulestore('direct').get_item(xblock.location) + return PublishState.draft except ItemNotFoundError: - return UnitState.private + return PublishState.private else: - return UnitState.public + return PublishState.public def add_extra_panel_tab(tab_type, course): diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index a7946c55c9..75c66cb99a 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -26,7 +26,7 @@ from xblock.runtime import Mixologist from lms.lib.xblock.runtime import unquote_slashes -from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitState, get_modulestore +from contentstore.utils import get_lms_link_for_item, compute_publish_state, PublishState, get_modulestore from contentstore.views.helpers import get_parent_xblock from models.settings.course_grading import CourseGradingModel @@ -107,8 +107,8 @@ def subsection_handler(request, tag=None, package_id=None, branch=None, version_ can_view_live = False subsection_units = item.get_children() for unit in subsection_units: - state = compute_unit_state(unit) - if state == UnitState.public or state == UnitState.draft: + state = compute_publish_state(unit) + if state in (PublishState.public, PublishState.draft): can_view_live = True break @@ -282,7 +282,7 @@ def unit_handler(request, tag=None, package_id=None, branch=None, version_guid=N ), 'section': containing_section, 'new_unit_category': 'vertical', - 'unit_state': compute_unit_state(item), + 'unit_state': compute_publish_state(item), 'published_date': ( get_default_time_display(item.published_date) if item.published_date is not None else None @@ -322,6 +322,7 @@ def container_handler(request, tag=None, package_id=None, branch=None, version_g 'context_course': course, 'xblock': xblock, 'xblock_locator': locator, + 'unit': None if not ancestor_xblocks else ancestor_xblocks[0], 'ancestor_xblocks': ancestor_xblocks, }) else: diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index f351f481f5..946fa1637d 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -97,5 +97,5 @@ def xblock_studio_url(xblock, course=None): course_id = None if course: course_id = course.location.course_id - locator = loc_mapper().translate_location(course_id, xblock.location) + locator = loc_mapper().translate_location(course_id, xblock.location, published=False) return locator.url_reverse(prefix) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index bbbd71a2b8..ab2a2d60b4 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -297,9 +297,9 @@ def _save_item(request, usage_loc, item_location, data=None, children=None, meta if publish == 'make_private': _xmodule_recurse(existing_item, lambda i: modulestore().unpublish(i.location)) elif publish == 'create_draft': - # This clones the existing item location to a draft location (the draft is + # This recursively clones the existing item location to a draft location (the draft is # implicit, because modulestore is a Draft modulestore) - modulestore().convert_to_draft(item_location) + _xmodule_recurse(existing_item, lambda i: modulestore().convert_to_draft(i.location)) if data: # TODO Allow any scope.content fields not just "data" (exactly like the get below this) diff --git a/cms/djangoapps/contentstore/views/tests/test_container.py b/cms/djangoapps/contentstore/views/tests/test_container.py index 766b86f8f3..0752ca6141 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container.py +++ b/cms/djangoapps/contentstore/views/tests/test_container.py @@ -28,9 +28,9 @@ class ContainerViewTestCase(CourseTestCase): def test_container_html(self): self._test_html_content( self.child_vertical, - expected_section_tag='', + expected_section_tag='', expected_breadcrumbs=( - r'Unit\s*' r'Child Vertical'), ) @@ -46,11 +46,11 @@ class ContainerViewTestCase(CourseTestCase): category="html", display_name="Child HTML") self._test_html_content( xblock_with_child, - expected_section_tag='', + expected_section_tag='', expected_breadcrumbs=( - r'Unit\s*' - r'Child Vertical\s*' r'Wrapper'), ) @@ -67,3 +67,6 @@ class ContainerViewTestCase(CourseTestCase): self.assertIn(expected_section_tag, html) # Verify the navigation link at the top of the page is correct. self.assertRegexpMatches(html, expected_breadcrumbs) + # Verify the link that allows users to change publish status. + expected_unit_link = 'This content is published with unit Unit.' + self.assertIn(expected_unit_link, html) diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py index d0bf3ffc24..cfefeb3e81 100644 --- a/cms/djangoapps/contentstore/views/tests/test_helpers.py +++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py @@ -16,7 +16,7 @@ class HelpersTestCase(CourseTestCase): # Verify course URL self.assertEqual(xblock_studio_url(course), - u'/course/MITx.999.Robot_Super_Course/branch/published/block/Robot_Super_Course') + u'/course/MITx.999.Robot_Super_Course/branch/draft/block/Robot_Super_Course') # Verify chapter URL chapter = ItemFactory.create(parent_location=self.course.location, category='chapter', @@ -34,17 +34,17 @@ class HelpersTestCase(CourseTestCase): vertical = ItemFactory.create(parent_location=sequential.location, category='vertical', display_name='Unit') self.assertEqual(xblock_studio_url(vertical), - u'/unit/MITx.999.Robot_Super_Course/branch/published/block/Unit') + u'/unit/MITx.999.Robot_Super_Course/branch/draft/block/Unit') self.assertEqual(xblock_studio_url(vertical, course), - u'/unit/MITx.999.Robot_Super_Course/branch/published/block/Unit') + u'/unit/MITx.999.Robot_Super_Course/branch/draft/block/Unit') # Verify child vertical URL child_vertical = ItemFactory.create(parent_location=vertical.location, category='vertical', display_name='Child Vertical') self.assertEqual(xblock_studio_url(child_vertical), - u'/container/MITx.999.Robot_Super_Course/branch/published/block/Child_Vertical') + u'/container/MITx.999.Robot_Super_Course/branch/draft/block/Child_Vertical') self.assertEqual(xblock_studio_url(child_vertical, course), - u'/container/MITx.999.Robot_Super_Course/branch/published/block/Child_Vertical') + u'/container/MITx.999.Robot_Super_Course/branch/draft/block/Child_Vertical') # Verify video URL video = ItemFactory.create(parent_location=child_vertical.location, category="video", diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 06b067c13a..dbb5f6335d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -15,6 +15,7 @@ from django.test.client import RequestFactory from contentstore.views.component import component_handler from contentstore.tests.utils import CourseTestCase +from contentstore.utils import compute_publish_state, PublishState from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor from xmodule.modulestore.django import modulestore @@ -153,7 +154,7 @@ class GetItem(ItemTest): html, # The instance of the wrapper class will have an auto-generated ID (wrapperxxx). Allow anything # for the 3 characters after wrapper. - (r'"/container/MITx.999.Robot_Super_Course/branch/published/block/wrapper.{3}" class="action-button">\s*' + (r'"/container/MITx.999.Robot_Super_Course/branch/draft/block/wrapper.{3}" class="action-button">\s*' 'View') ) @@ -663,6 +664,7 @@ class TestEditItem(ItemTest): self.assertEqual(resp.status_code, 200) # Activate the editing view + view_url = '/xblock/{locator}/studio_view'.format(locator=self.problem_locator) resp = self.client.get(view_url, HTTP_ACCEPT='application/json') self.assertEqual(resp.status_code, 200) @@ -671,6 +673,49 @@ class TestEditItem(ItemTest): draft = self.get_item_from_modulestore(self.problem_locator, True) self.assertNotEqual(draft.data, published.data) + def test_publish_states_of_nested_xblocks(self): + """ Test publishing of a unit page containing a nested xblock """ + + resp = self.create_xblock(parent_locator=self.seq_locator, display_name='Test Unit', category='vertical') + unit_locator = self.response_locator(resp) + resp = self.create_xblock(parent_locator=unit_locator, category='wrapper') + wrapper_locator = self.response_locator(resp) + resp = self.create_xblock(parent_locator=wrapper_locator, category='html') + html_locator = self.response_locator(resp) + + # The unit and its children should be private initially + unit_update_url = '/xblock/' + unit_locator + unit = self.get_item_from_modulestore(unit_locator, True) + html = self.get_item_from_modulestore(html_locator, True) + self.assertEqual(compute_publish_state(unit), PublishState.private) + self.assertEqual(compute_publish_state(html), PublishState.private) + + # Make the unit public and verify that the problem is also made public + resp = self.client.ajax_post( + unit_update_url, + data={'publish': 'make_public'} + ) + self.assertEqual(resp.status_code, 200) + unit = self.get_item_from_modulestore(unit_locator, True) + html = self.get_item_from_modulestore(html_locator, True) + self.assertEqual(compute_publish_state(unit), PublishState.public) + self.assertEqual(compute_publish_state(html), PublishState.public) + + # Make a draft for the unit and verify that the problem also has a draft + resp = self.client.ajax_post( + unit_update_url, + data={ + 'id': unit_locator, + 'metadata': {}, + 'publish': 'create_draft' + } + ) + self.assertEqual(resp.status_code, 200) + unit = self.get_item_from_modulestore(unit_locator, True) + html = self.get_item_from_modulestore(html_locator, True) + self.assertEqual(compute_publish_state(unit), PublishState.draft) + self.assertEqual(compute_publish_state(html), PublishState.draft) + @ddt.ddt class TestComponentHandler(TestCase): diff --git a/cms/static/sass/views/_container.scss b/cms/static/sass/views/_container.scss index 4c50103b81..b7ac403e66 100644 --- a/cms/static/sass/views/_container.scss +++ b/cms/static/sass/views/_container.scss @@ -30,11 +30,22 @@ body.view-container { label { @extend %t-title8; } + + .bit-publishing { + margin-bottom: $baseline; + border-top: 5px solid $blue; + background-color: $white; + padding: ($baseline*.75) ($baseline*.75) ($baseline) ($baseline*.75); + + .copy { + @extend %t-copy-sub1; + } + } } } // UI: xblock rendering -body.view-container .content-primary{ +body.view-container .content-primary { .wrapper-xblock { @extend %wrap-xblock; diff --git a/cms/templates/container.html b/cms/templates/container.html index 83855250ed..8e5c7e5ab5 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -79,6 +79,15 @@ xblock_info = {