Skip to content

E
edx-platform
Commit 0bd2286e by Johnny Brown
Options

show a friendly message for an empty course rather than an error page

parent 85f1997f
Showing with 204 additions and 27 deletions
AUTHORS
......@@ -151,3 +151,4 @@ Steven Burch <stv@stanford.edu>
Waqas Khalid <wkhalid@edx.org>
Muhammad Ammar <mammar@edx.org>
Abdallah Nassif <abdoosh00@gmail.com>
Johnny Brown <johnnybrown7@gmail.com>
common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py
......@@ -506,6 +506,42 @@ class SplitModuleTest(unittest.TestCase):
return element
class TestHasChildrenAtDepth(SplitModuleTest):
"""Test the has_children_at_depth method of XModuleMixin. """
def test_has_children_at_depth(self):
course_locator = CourseLocator(
org='testx', offering='GreekHero', branch='draft'
)
block_locator = BlockUsageLocator(
course_locator, 'course', 'head12345'
)
block = modulestore().get_item(block_locator)
self.assertRaises(
ValueError, block.has_children_at_depth, -1,
)
self.assertTrue(block.has_children_at_depth(0))
self.assertTrue(block.has_children_at_depth(1))
self.assertFalse(block.has_children_at_depth(2))
ch1 = modulestore().get_item(
BlockUsageLocator(course_locator, 'chapter', block_id='chapter1')
)
self.assertFalse(ch1.has_children_at_depth(0))
ch2 = modulestore().get_item(
BlockUsageLocator(course_locator, 'chapter', block_id='chapter2')
)
self.assertFalse(ch2.has_children_at_depth(0))
ch3 = modulestore().get_item(
BlockUsageLocator(course_locator, 'chapter', block_id='chapter3')
)
self.assertTrue(ch3.has_children_at_depth(0))
self.assertFalse(ch3.has_children_at_depth(1))
class SplitModuleCourseTests(SplitModuleTest):
'''
Course CRUD operation tests
......
common/lib/xmodule/xmodule/x_module.py
......@@ -209,6 +209,29 @@ class XModuleMixin(XBlockMixin):
result[field.name] = field.read_json(self)
return result
def has_children_at_depth(self, depth):
"""
Returns true if self has children at the given depth. depth==0 returns
false if self is a leaf, true otherwise.
SELF
|
[child at depth 0]
/ \
[depth 1] [depth 1]
/ \
[depth 2] [depth 2]
So the example above would return True for `has_children_at_depth(2)`, and False
for depth > 2
"""
if depth < 0:
raise ValueError("negative depth argument is invalid")
elif depth == 0:
return bool(self.get_children())
else:
return any(child.has_children_at_depth(depth - 1) for child in self.get_children())
def get_content_titles(self):
"""
Returns list of content titles for all of self's children.
......
lms/djangoapps/courseware/tests/test_navigation.py
......@@ -13,6 +13,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from courseware.tests.helpers import LoginEnrollmentTestCase, check_for_get_code
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from courseware.tests.factories import GlobalStaffFactory
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
......@@ -35,6 +36,8 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
display_name='Welcome')
self.section9 = ItemFactory.create(parent_location=self.chapter9.location,
display_name='factory_section')
self.unit0 = ItemFactory.create(parent_location=self.section0.location,
display_name='New Unit')
# Create student accounts and activate them.
for i in range(len(self.STUDENT_INFO)):
......@@ -43,6 +46,8 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.create_account(username, email, password)
self.activate_user(email)
self.staff_user = GlobalStaffFactory()
@override_settings(SESSION_INACTIVITY_TIMEOUT_IN_SECONDS=1)
def test_inactive_session_timeout(self):
"""
......@@ -135,3 +140,55 @@ class TestNavigation(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.assertRedirects(resp, reverse('courseware_chapter',
kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': 'factory_chapter'}))
def test_incomplete_course(self):
email = self.staff_user.email
password = "test"
self.login(email, password)
self.enroll(self.test_course, True)
test_course_id = self.test_course.id.to_deprecated_string()
check_for_get_code(
self, 200,
reverse(
'courseware',
kwargs={'course_id': test_course_id}
)
)
section = ItemFactory.create(
parent_location=self.test_course.location,
display_name='New Section'
)
check_for_get_code(
self, 200,
reverse(
'courseware',
kwargs={'course_id': test_course_id}
)
)
subsection = ItemFactory.create(
parent_location=section.location,
display_name='New Subsection'
)
check_for_get_code(
self, 200,
reverse(
'courseware',
kwargs={'course_id': test_course_id}
)
)
ItemFactory.create(
parent_location=subsection.location,
display_name='New Unit'
)
check_for_get_code(
self, 302,
reverse(
'courseware',
kwargs={'course_id': test_course_id}
)
)
lms/djangoapps/courseware/tests/test_view_authentication.py
......@@ -112,23 +112,39 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
def setUp(self):
self.course = CourseFactory.create(number='999', display_name='Robot_Super_Course')
self.overview_chapter = ItemFactory.create(display_name='Overview')
self.courseware_chapter = ItemFactory.create(display_name='courseware')
self.overview_chapter = ItemFactory.create(
parent_location=self.course.location,
display_name='Super Overview'
)
self.welcome_section = ItemFactory.create(
parent_location=self.overview_chapter.location,
display_name='Super Welcome'
)
self.welcome_unit = ItemFactory.create(
parent_location=self.welcome_section.location,
display_name='Super Unit'
)
self.course = modulestore().get_course(self.course.id)
self.test_course = CourseFactory.create(number='666', display_name='Robot_Sub_Course')
self.other_org_course = CourseFactory.create(org='Other_Org_Course')
self.sub_courseware_chapter = ItemFactory.create(
parent_location=self.test_course.location, display_name='courseware'
parent_location=self.test_course.location,
display_name='courseware'
)
self.sub_overview_chapter = ItemFactory.create(
parent_location=self.sub_courseware_chapter.location,
display_name='Overview'
)
self.welcome_section = ItemFactory.create(
parent_location=self.overview_chapter.location,
self.sub_welcome_section = ItemFactory.create(
parent_location=self.sub_overview_chapter.location,
display_name='Welcome'
)
self.sub_welcome_unit = ItemFactory.create(
parent_location=self.sub_welcome_section.location,
display_name='New Unit'
)
self.test_course = modulestore().get_course(self.test_course.id)
self.global_staff_user = GlobalStaffFactory()
......@@ -151,9 +167,13 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
self.login(self.unenrolled_user)
response = self.client.get(reverse('courseware',
kwargs={'course_id': self.course.id.to_deprecated_string()}))
self.assertRedirects(response,
reverse('about_course',
args=[self.course.id.to_deprecated_string()]))
self.assertRedirects(
response,
reverse(
'about_course',
args=[self.course.id.to_deprecated_string()]
)
)
def test_redirection_enrolled(self):
"""
......@@ -162,14 +182,22 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
self.login(self.enrolled_user)
response = self.client.get(reverse('courseware',
kwargs={'course_id': self.course.id.to_deprecated_string()}))
response = self.client.get(
reverse(
'courseware',
kwargs={'course_id': self.course.id.to_deprecated_string()}
)
)
self.assertRedirects(response,
reverse('courseware_section',
kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': 'Overview',
'section': 'Welcome'}))
self.assertRedirects(
response,
reverse(
'courseware_section',
kwargs={'course_id': self.course.id.to_deprecated_string(),
'chapter': self.overview_chapter.url_name,
'section': self.welcome_section.url_name}
)
)
def test_instructor_page_access_nonstaff(self):
"""
......@@ -314,6 +342,7 @@ class TestViewAuth(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
now = datetime.datetime.now(pytz.UTC)
tomorrow = now + datetime.timedelta(days=1)
self.course.start = tomorrow
self.test_course.start = tomorrow
self.course = self.update_course(self.course)
......
lms/djangoapps/courseware/tests/test_views.py
......@@ -145,7 +145,7 @@ class ViewsTestCase(TestCase):
mock_module.position = 3
mock_module.get_display_items.return_value = []
self.assertRaises(Http404, views.redirect_to_course_position,
mock_module)
mock_module, views.CONTENT_DEPTH)
def test_registered_for_course(self):
self.assertFalse(views.registered_for_course('Basketweaving', None))
......
lms/djangoapps/courseware/views.py
......@@ -52,6 +52,7 @@ log = logging.getLogger("edx.courseware")
template_imports = {'urllib': urllib}
CONTENT_DEPTH = 2
def user_groups(user):
"""
......@@ -113,17 +114,36 @@ def render_accordion(request, course, chapter, section, field_data_cache):
return render_to_string('courseware/accordion.html', context)
def get_current_child(xmodule):
def get_current_child(xmodule, min_depth=None):
"""
Get the xmodule.position's display item of an xmodule that has a position and
children. If xmodule has no position or is out of bounds, return the first child.
children. If xmodule has no position or is out of bounds, return the first
child with children extending down to content_depth.
For example, if chapter_one has no position set, with two child sections,
section-A having no children and section-B having a discussion unit,
`get_current_child(chapter, min_depth=1)` will return section-B.
Returns None only if there are no children at all.
"""
def _get_default_child_module(child_modules):
"""Returns the first child of xmodule, subject to min_depth."""
if not child_modules:
default_child = None
elif not min_depth > 0:
default_child = child_modules[0]
else:
content_children = [child for child in child_modules if
child.has_children_at_depth(min_depth - 1)]
default_child = content_children[0] if content_children else None
return default_child
if not hasattr(xmodule, 'position'):
return None
if xmodule.position is None:
pos = 0
return _get_default_child_module(xmodule.get_display_items())
else:
# position is 1-indexed.
pos = xmodule.position - 1
......@@ -132,14 +152,15 @@ def get_current_child(xmodule):
if 0 <= pos < len(children):
child = children[pos]
elif len(children) > 0:
# Something is wrong. Default to first child
child = children[0]
# module has a set position, but the position is out of range.
# return default child.
child = _get_default_child_module(children)
else:
child = None
return child
def redirect_to_course_position(course_module):
def redirect_to_course_position(course_module, content_depth):
"""
Return a redirect to the user's current place in the course.
......@@ -153,7 +174,7 @@ def redirect_to_course_position(course_module):
"""
urlargs = {'course_id': course_module.id.to_deprecated_string()}
chapter = get_current_child(course_module)
chapter = get_current_child(course_module, min_depth=content_depth)
if chapter is None:
# oops. Something bad has happened.
raise Http404("No chapter found when loading current position in course")
......@@ -163,7 +184,7 @@ def redirect_to_course_position(course_module):
return redirect(reverse('courseware_chapter', kwargs=urlargs))
# Relying on default of returning first child
section = get_current_child(chapter)
section = get_current_child(chapter, min_depth=content_depth - 1)
if section is None:
raise Http404("No section found when loading current position in course")
......@@ -266,9 +287,6 @@ def index(request, course_id, chapter=None, section=None,
studio_url = get_studio_url(course_key, 'course')
if chapter is None:
return redirect_to_course_position(course_module)
context = {
'csrf': csrf(request)['csrf_token'],
'accordion': render_accordion(request, course, chapter, section, field_data_cache),
......@@ -283,6 +301,15 @@ def index(request, course_id, chapter=None, section=None,
'reverifications': fetch_reverify_banner_info(request, course_key),
}
has_content = course.has_children_at_depth(CONTENT_DEPTH)
if not has_content:
# Show empty courseware for a course with no units
return render_to_response('courseware/courseware.html', context)
elif chapter is None:
# passing CONTENT_DEPTH avoids returning 404 for a course with an
# empty first section and a second section with content
return redirect_to_course_position(course_module, CONTENT_DEPTH)
# Only show the chat if it's enabled by the course and in the
# settings.
show_chat = course.show_chat and settings.FEATURES['ENABLE_CHAT']
......
lms/templates/courseware/courseware.html
......@@ -195,7 +195,11 @@ ${fragment.foot_html()}
<div id="accordion" style="display: none">
<nav aria-label="${_('Course Navigation')}">
${accordion}
% if accordion.strip():
${accordion}
% else:
<div class="chapter">${_("No content has been added to this course")}</div>
% endif
</nav>
</div>
</div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment