Skip to content

Integrate visual styling into the course outline #4549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cahrens merged 11 commits into from
Jul 29, 2014
Merged

Integrate visual styling into the course outline #4549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
22d996f
Studio: course outline revised styling with reference templates
talbs Jul 10, 2014
8db70ce
Integrate visual styling into the course outline
andy-armstrong Jul 16, 2014
31d8f51
Studio: outline UI revisions:
talbs Jul 24, 2014
5ab90d7
Fix most bokchoy tests
benmcmorran Jul 25, 2014
f809149
Apply code review comments and fix tests
andy-armstrong Jul 25, 2014
d828647
Correct publish titles, code review feedback.
Jul 26, 2014
beb448a
Studio: addressing outline UI PR feedback:
talbs Jul 28, 2014
c52b4f1
Add test fixes and fix expand collapse behavior
benmcmorran Jul 28, 2014
767487a
Studio: further outline UI revisions + PR feedback
talbs Jul 28, 2014
74f4c18
Include edited/published_by for container page only
dan-f Jul 28, 2014
bb9fae5
Add tests for course outline warning messages
benmcmorran Jul 29, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cms/djangoapps/contentstore/features/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ def create_a_course():


def add_section():
world.css_click('.course-outline .add-button')
assert_true(world.is_css_present('.outline-item-section .xblock-field-value'))
world.css_click('.outline .button-new')
assert_true(world.is_css_present('.outline-section .xblock-field-value'))


def set_date_and_time(date_css, desired_date, time_css, desired_time, key=None):
Expand Down Expand Up @@ -241,7 +241,7 @@ def create_unit_from_course_outline():
The end result is the page where the user is editing the new unit.
"""
css_selectors = [
'.outline-item-subsection .expand-collapse', '.outline-item-subsection .add-button'
'.outline-subsection .expand-collapse', '.outline-subsection .button-new'
]
for selector in css_selectors:
world.css_click(selector)
Expand Down
18 changes: 9 additions & 9 deletions cms/djangoapps/contentstore/features/course-outline.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def i_add_a_section(step):

@step(u'I press the "section" delete icon')
def i_press_the_section_delete_icon(step):
delete_locator = 'section .outline-item-section > .wrapper-xblock-header a.delete-button'
delete_locator = 'section .outline-section > .section-header a.delete-button'
world.css_click(delete_locator)


Expand All @@ -82,37 +82,37 @@ def i_confirm_all_alerts(step):
@step(u'I see the "([^"]*) All Sections" link$')
def i_see_the_collapse_expand_all_span(step, text):
if text == "Collapse":
span_locator = '.toggle-button-expand-collapse .collapse-all .label'
span_locator = '.button-toggle-expand-collapse .collapse-all .label'
elif text == "Expand":
span_locator = '.toggle-button-expand-collapse .expand-all .label'
span_locator = '.button-toggle-expand-collapse .expand-all .label'
assert_true(world.css_visible(span_locator))


@step(u'I do not see the "([^"]*) All Sections" link$')
def i_do_not_see_the_collapse_expand_all_span(step, text):
if text == "Collapse":
span_locator = '.toggle-button-expand-collapse .collapse-all .label'
span_locator = '.button-toggle-expand-collapse .collapse-all .label'
elif text == "Expand":
span_locator = '.toggle-button-expand-collapse .expand-all .label'
span_locator = '.button-toggle-expand-collapse .expand-all .label'
assert_false(world.css_visible(span_locator))


@step(u'I click the "([^"]*) All Sections" link$')
def i_click_the_collapse_expand_all_span(step, text):
if text == "Collapse":
span_locator = '.toggle-button-expand-collapse .collapse-all .label'
span_locator = '.button-toggle-expand-collapse .collapse-all .label'
elif text == "Expand":
span_locator = '.toggle-button-expand-collapse .expand-all .label'
span_locator = '.button-toggle-expand-collapse .expand-all .label'
assert_true(world.browser.is_element_present_by_css(span_locator))
world.css_click(span_locator)


@step(u'I ([^"]*) the first section$')
def i_collapse_expand_a_section(step, text):
if text == "collapse":
locator = 'section .outline-item-section .ui-toggle-expansion'
locator = 'section .outline-section .ui-toggle-expansion'
elif text == "expand":
locator = 'section .outline-item-section .ui-toggle-expansion'
locator = 'section .outline-section .ui-toggle-expansion'
world.css_click(locator)


Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/features/courses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,5 @@ def i_am_on_tab(step, tab_name):

@step('I see a link for adding a new section$')
def i_see_new_section_link(step):
link_css = '.course-outline .add-button'
link_css = '.outline .button-new'
assert world.css_has_text(link_css, 'New Section')
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/tests/test_contentstore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ def test_course_overview_view_with_course(self):
resp = self._show_course_overview(course.id)
self.assertContains(
resp,
'<article class="course-outline" data-locator="{locator}" data-course-key="{course_key}">'.format(
'<article class="outline outline-course" data-locator="{locator}" data-course-key="{course_key}">'.format(
locator='i4x://MITx/999/course/Robot_Super_Course',
course_key='MITx/999/Robot_Super_Course',
),
Expand Down
16 changes: 1 addition & 15 deletions cms/djangoapps/contentstore/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,21 +113,7 @@ def course_image_url(course):
path = loc.to_deprecated_string()
return path


def compute_publish_state(xblock):
"""
Returns whether this xblock is draft, public, or private.

Returns:
PublishState.draft - content is in the process of being edited, but still has a previous
version deployed to LMS
PublishState.public - content is locked and deployed to LMS
PublishState.private - content is editable and not deployed to LMS
"""

return modulestore().compute_publish_state(xblock)


# pylint: disable=invalid-name
def is_currently_visible_to_students(xblock):
"""
Returns true if there is a published version of the xblock that is currently visible to students.
Expand Down
10 changes: 5 additions & 5 deletions cms/djangoapps/contentstore/views/component.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
from xmodule.modulestore.exceptions import ItemNotFoundError
from edxmako.shortcuts import render_to_response

from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import PublishState

from xblock.core import XBlock
from xblock.django.request import webob_to_django_response, django_to_webob_request
Expand All @@ -21,7 +21,7 @@
from xblock.plugin import PluginMissingError
from xblock.runtime import Mixologist

from contentstore.utils import get_lms_link_for_item, compute_publish_state
from contentstore.utils import get_lms_link_for_item
from contentstore.views.helpers import get_parent_xblock, is_unit, xblock_type_display_name
from contentstore.views.item import create_xblock_info

Expand Down Expand Up @@ -122,8 +122,8 @@ def subsection_handler(request, usage_key_string):
can_view_live = False
subsection_units = item.get_children()
for unit in subsection_units:
state = compute_publish_state(unit)
if state in (PublishState.public, PublishState.draft):
has_published = modulestore().has_item(unit.location, revision=ModuleStoreEnum.RevisionOption.published_only)
if has_published:
can_view_live = True
break

Expand Down Expand Up @@ -198,7 +198,7 @@ def container_handler(request, usage_key_string):

# Fetch the XBlock info for use by the container page. Note that it includes information
# about the block's ancestors and siblings for use by the Unit Outline.
xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page)
xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page, include_edited_by=True, include_published_by=True)

# Create the link for preview.
preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE')
Expand Down
4 changes: 2 additions & 2 deletions cms/djangoapps/contentstore/views/course.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def find_xblock_info(xblock_info, locator):
"""
if xblock_info['id'] == locator:
return xblock_info
children = xblock_info['child_info']['children'] if xblock_info['child_info'] else None
children = xblock_info['child_info']['children'] if xblock_info.get('child_info', None) else None
if children:
for child_xblock_info in children:
result = find_xblock_info(child_xblock_info, locator)
Expand All @@ -295,7 +295,7 @@ def collect_all_locators(locators, xblock_info):
Collect all the locators for an xblock and its children.
"""
locators.append(xblock_info['id'])
children = xblock_info['child_info']['children'] if xblock_info['child_info'] else None
children = xblock_info['child_info']['children'] if xblock_info.get('child_info', None) else None
if children:
for child_xblock_info in children:
collect_all_locators(locators, child_xblock_info)
Expand Down
117 changes: 102 additions & 15 deletions cms/djangoapps/contentstore/views/item.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ def _get_module_info(xblock, rewrite_static_links=True):


def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False,
include_children_predicate=NEVER):
include_edited_by=False, include_published_by=False, include_children_predicate=NEVER):
"""
Creates the information needed for client-side XBlockInfo.

Expand All @@ -600,10 +600,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F
In addition, an optional include_children_predicate argument can be provided to define whether or
not a particular xblock should have its children included.
"""
published = modulestore().has_item(xblock.location, revision=ModuleStoreEnum.RevisionOption.published_only)

# Treat DEFAULT_START_DATE as a magic number that means the release date has not been set
release_date = get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None

def safe_get_username(user_id):
"""
Expand All @@ -622,36 +618,127 @@ def safe_get_username(user_id):

return None

# Compute the child info first so it can be included in aggregate information for the parent
if include_child_info and xblock.has_children:
child_info = _create_xblock_child_info(
xblock, include_children_predicate=include_children_predicate
)
else:
child_info = None

# Treat DEFAULT_START_DATE as a magic number that means the release date has not been set
release_date = get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None
published = modulestore().has_item(xblock.location, revision=ModuleStoreEnum.RevisionOption.published_only)
currently_visible_to_students = is_currently_visible_to_students(xblock)

is_xblock_unit = is_unit(xblock)
is_unit_with_changes = is_xblock_unit and modulestore().has_changes(xblock.location)

xblock_info = {
"id": unicode(xblock.location),
"display_name": xblock.display_name_with_default,
"category": xblock.category,
"has_changes": modulestore().has_changes(xblock.location),
"published": published,
"edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None,
"edited_by": safe_get_username(xblock.subtree_edited_by),
"published": published,
"published_on": get_default_time_display(xblock.published_date) if xblock.published_date else None,
"published_by": safe_get_username(xblock.published_by),
'studio_url': xblock_studio_url(xblock),
"released_to_students": datetime.now(UTC) > xblock.start,
"release_date": release_date,
"release_date_from": _get_release_date_from(xblock) if release_date else None,
"visible_to_staff_only": xblock.visible_to_staff_only,
"currently_visible_to_students": is_currently_visible_to_students(xblock),
"currently_visible_to_students": currently_visible_to_students,
"visibility_state": _compute_visibility_state(xblock, child_info, is_unit_with_changes) if not xblock.category == 'course' else None
}
if data is not None:
xblock_info["data"] = data
if metadata is not None:
xblock_info["metadata"] = metadata
if include_ancestor_info:
xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock)
if include_child_info and xblock.has_children:
xblock_info['child_info'] = _create_xblock_child_info(
xblock, include_children_predicate=include_children_predicate
)
if child_info:
xblock_info['child_info'] = child_info
# Currently, 'edited_by' and 'published_by' are only used by the container page. Only compute them when asked to do
# so, since safe_get_username() is expensive.
if include_edited_by:
xblock_info['edited_by'] = safe_get_username(xblock.subtree_edited_by)
if include_published_by:
xblock_info['published_by'] = safe_get_username(xblock.published_by)
# On the unit page only, add 'has_changes' to indicate when there are changes that can be discarded.
# We don't add it in general because it is an expensive operation.
if is_xblock_unit:
xblock_info['has_changes'] = is_unit_with_changes

return xblock_info


class VisibilityState(object):
"""
Represents the possible visibility states for an xblock:

live - the block and all of its descendants are live to students (excluding staff only items)
Note: Live means both published and released.

ready - the block is ready to go live and all of its descendants are live or ready (excluding staff only items)
Note: content is ready when it is published and scheduled with a release date in the future.

unscheduled - the block and all of its descendants have no release date (excluding staff only items)
Note: it is valid for items to be published with no release date in which case they are still unscheduled.

needs_attention - the block or its descendants are not fully live, ready or unscheduled (excluding staff only items)
For example: one subsection has draft content, or there's both unreleased and released content in one section.

staff_only - all of the block's content is to be shown to staff only
Note: staff only items do not affect their parent's state.
"""
live = 'live'
ready = 'ready'
unscheduled = 'unscheduled'
needs_attention = 'needs_attention'
staff_only = 'staff_only'


def _compute_visibility_state(xblock, child_info, is_unit_with_changes):
"""
Returns the current publish state for the specified xblock and its children
"""
if xblock.visible_to_staff_only:
return VisibilityState.staff_only
elif 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
Copy link

@cahrens cahrens Jul 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note (somewhat for myself) that a never-published unit will "have changes".

is_unscheduled = xblock.start == DEFAULT_START_DATE
is_live = datetime.now(UTC) > xblock.start
children = child_info and child_info['children']
if children and len(children) > 0:
all_staff_only = True
all_unscheduled = True
all_live = True
for child in child_info['children']:
child_state = child['visibility_state']
if child_state == VisibilityState.needs_attention:
return child_state
elif not child_state == VisibilityState.staff_only:
all_staff_only = False
if not child_state == VisibilityState.unscheduled:
all_unscheduled = False
if not child_state == VisibilityState.live:
all_live = False
if all_staff_only:
return VisibilityState.staff_only
elif all_unscheduled:
return VisibilityState.unscheduled if is_unscheduled else VisibilityState.needs_attention
elif all_live:
return VisibilityState.live if is_live else VisibilityState.needs_attention
else:
return VisibilityState.ready if not is_unscheduled else VisibilityState.needs_attention
if is_unscheduled:
return VisibilityState.unscheduled
elif is_live:
return VisibilityState.live
else:
return VisibilityState.ready


def _create_xblock_ancestor_info(xblock):
"""
Returns information about the ancestors of an xblock. Note that the direct parent will also return
Expand Down
8 changes: 5 additions & 3 deletions cms/djangoapps/contentstore/views/tests/test_course_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from contentstore.tests.utils import CourseTestCase
from contentstore.utils import reverse_course_url
from contentstore.views.course import course_outline_initial_state
from contentstore.views.item import create_xblock_info
from contentstore.views.item import create_xblock_info, VisibilityState
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory

Expand Down Expand Up @@ -123,6 +123,7 @@ def test_json_responses(self):
self.assertEqual(json_response['id'], 'i4x://MITx/999/course/Robot_Super_Course')
self.assertEqual(json_response['display_name'], 'Robot Super Course')
self.assertTrue(json_response['published'])
Copy link

@cahrens cahrens Jul 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore checks of "published" (here and below).

self.assertIsNone(json_response['visibility_state'])

# Now verify the first child
children = json_response['child_info']['children']
Expand All @@ -131,7 +132,8 @@ def test_json_responses(self):
self.assertEqual(first_child_response['category'], 'chapter')
self.assertEqual(first_child_response['id'], 'i4x://MITx/999/chapter/Week_1')
self.assertEqual(first_child_response['display_name'], 'Week 1')
self.assertTrue(first_child_response['published'])
self.assertTrue(json_response['published'])
self.assertEqual(first_child_response['visibility_state'], VisibilityState.unscheduled)
self.assertTrue(len(first_child_response['child_info']['children']) > 0)

# Finally, validate the entire response for consistency
Expand All @@ -144,7 +146,7 @@ def assert_correct_json_response(self, json_response):
self.assertIsNotNone(json_response['display_name'])
self.assertIsNotNone(json_response['id'])
self.assertIsNotNone(json_response['category'])
self.assertIsNotNone(json_response['published'])
self.assertTrue(json_response['published'])
if json_response.get('child_info', None):
for child_response in json_response['child_info']['children']:
self.assert_correct_json_response(child_response)
Expand Down
Loading