Skip to content

E
edx-platform
Commit 10fd02f9 by Robert Raposa Committed by GitHub
Options

Merge pull request #15074 from edx/robrap/LEARNER-922 

LEARNER-922: Fix logic error in calculating last accessed.
parents 6c36980c 8569cb35
Showing with 166 additions and 53 deletions
openedx/features/course_experience/tests/views/test_course_outline.py
......@@ -13,6 +13,7 @@ from pyquery import PyQuery as pq
from courseware.tests.factories import StaffFactory
from student.models import CourseEnrollment
from student.tests.factories import UserFactory
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.course_module import DEFAULT_START_DATE
......@@ -26,11 +27,13 @@ PAST_DAY = datetime.datetime.now() - datetime.timedelta(days=30)
class TestCourseOutlinePage(SharedModuleStoreTestCase):
"""
Test the new course outline view.
Test the course outline view.
"""
@classmethod
def setUpClass(cls):
"""Set up the simplest course possible."""
"""
Set up an array of various courses to be tested.
"""
# setUpClassAndTestData() already calls setUpClass on SharedModuleStoreTestCase
# pylint: disable=super-method-not-called
with super(TestCourseOutlinePage, cls).setUpClassAndTestData():
......@@ -38,40 +41,40 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase):
course = CourseFactory.create()
with cls.store.bulk_operations(course.id):
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
section = ItemFactory.create(category='sequential', parent_location=chapter.location)
vertical = ItemFactory.create(category='vertical', parent_location=section.location)
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
vertical = ItemFactory.create(category='vertical', parent_location=sequential.location)
course.children = [chapter]
chapter.children = [section]
section.children = [vertical]
chapter.children = [sequential]
sequential.children = [vertical]
cls.courses.append(course)
course = CourseFactory.create()
with cls.store.bulk_operations(course.id):
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
section = ItemFactory.create(category='sequential', parent_location=chapter.location)
section2 = ItemFactory.create(category='sequential', parent_location=chapter.location)
vertical = ItemFactory.create(category='vertical', parent_location=section.location)
vertical2 = ItemFactory.create(category='vertical', parent_location=section2.location)
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
sequential2 = ItemFactory.create(category='sequential', parent_location=chapter.location)
vertical = ItemFactory.create(category='vertical', parent_location=sequential.location)
vertical2 = ItemFactory.create(category='vertical', parent_location=sequential2.location)
course.children = [chapter]
chapter.children = [section, section2]
section.children = [vertical]
section2.children = [vertical2]
chapter.children = [sequential, sequential2]
sequential.children = [vertical]
sequential2.children = [vertical2]
cls.courses.append(course)
course = CourseFactory.create()
with cls.store.bulk_operations(course.id):
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
section = ItemFactory.create(
sequential = ItemFactory.create(
category='sequential',
parent_location=chapter.location,
due=datetime.datetime.now(),
graded=True,
format='Homework',
)
vertical = ItemFactory.create(category='vertical', parent_location=section.location)
vertical = ItemFactory.create(category='vertical', parent_location=sequential.location)
course.children = [chapter]
chapter.children = [section]
section.children = [vertical]
chapter.children = [sequential]
sequential.children = [vertical]
cls.courses.append(course)
@classmethod
......@@ -100,15 +103,82 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase):
for chapter in course.children:
self.assertIn(chapter.display_name, response_content)
self.assertTrue(chapter.children)
for section in chapter.children:
self.assertIn(section.display_name, response_content)
if section.graded:
self.assertIn(section.due.strftime('%Y-%m-%d %H:%M:%S'), response_content)
self.assertIn(section.format, response_content)
self.assertTrue(section.children)
for vertical in section.children:
for sequential in chapter.children:
self.assertIn(sequential.display_name, response_content)
if sequential.graded:
self.assertIn(sequential.due.strftime('%Y-%m-%d %H:%M:%S'), response_content)
self.assertIn(sequential.format, response_content)
self.assertTrue(sequential.children)
for vertical in sequential.children:
self.assertNotIn(vertical.display_name, response_content)
class TestCourseOutlineResumeCourse(SharedModuleStoreTestCase):
"""
Test start course and resume course for the course outline view.
Technically, this mixes course home and course outline tests, but checking
the counts of start/resume course should be done together to avoid false
positives.
"""
@classmethod
def setUpClass(cls):
"""
Creates a test course that can be used for non-destructive tests
"""
# setUpClassAndTestData() already calls setUpClass on SharedModuleStoreTestCase
# pylint: disable=super-method-not-called
with super(TestCourseOutlineResumeCourse, cls).setUpClassAndTestData():
cls.course = cls.create_test_course()
@classmethod
def setUpTestData(cls):
"""Set up and enroll our fake user in the course."""
cls.user = UserFactory(password=TEST_PASSWORD)
CourseEnrollment.enroll(cls.user, cls.course.id)
@classmethod
def create_test_course(cls):
"""
Creates a test course.
"""
course = CourseFactory.create()
with cls.store.bulk_operations(course.id):
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
sequential2 = ItemFactory.create(category='sequential', parent_location=chapter.location)
vertical = ItemFactory.create(category='vertical', parent_location=sequential.location)
vertical2 = ItemFactory.create(category='vertical', parent_location=sequential2.location)
course.children = [chapter]
chapter.children = [sequential, sequential2]
sequential.children = [vertical]
sequential2.children = [vertical2]
if hasattr(cls, 'user'):
CourseEnrollment.enroll(cls.user, course.id)
return course
def setUp(self):
"""
Set up for the tests.
"""
super(TestCourseOutlineResumeCourse, self).setUp()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
def visit_sequential(self, course, chapter, sequential):
"""
Navigates to the provided sequential.
"""
last_accessed_url = reverse(
'courseware_section',
kwargs={
'course_id': course.id.to_deprecated_string(),
'chapter': chapter.url_name,
'section': sequential.url_name,
}
)
self.assertEqual(200, self.client.get(last_accessed_url).status_code)
def test_start_course(self):
"""
Tests that the start course button appears when the course has never been accessed.
......@@ -117,7 +187,7 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase):
start/resume course should be done together to not get a false positive.
"""
course = self.courses[0]
course = self.course
response = self.client.get(course_home_url(course))
self.assertEqual(response.status_code, 200)
......@@ -131,25 +201,42 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase):
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]
self.visit_sequential(course, chapter, sequential)
# check resume course buttons
response = self.client.get(course_home_url(course))
self.assertEqual(response.status_code, 200)
self.assertContains(response, 'Start Course', count=0)
self.assertContains(response, 'Resume Course', count=2)
Technically, this is a mix of a course home and course outline test, but checking the counts of start/resume
course should be done together to not get a false positive.
content = pq(response.content)
self.assertTrue(content('.action-resume-course').attr('href').endswith('/sequential/' + sequential.url_name))
def test_resume_course_deleted_sequential(self):
"""
Tests resume course when the last accessed sequential is deleted and
there is another sequential in the vertical.
"""
course = self.courses[0]
course = self.create_test_course()
# first navigate to a section to make it the last accessed
# first navigate to a sequential to make it the last accessed
chapter = course.children[0]
section = chapter.children[0]
last_accessed_url = reverse(
'courseware_section',
kwargs={
'course_id': course.id.to_deprecated_string(),
'chapter': chapter.url_name,
'section': section.url_name,
}
)
self.assertEqual(200, self.client.get(last_accessed_url).status_code)
self.assertGreaterEqual(len(chapter.children), 2)
sequential = chapter.children[0]
sequential2 = chapter.children[1]
self.visit_sequential(course, chapter, sequential)
# remove one of the sequentials from the chapter
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id):
self.store.delete_item(sequential.location, self.user.id) # pylint: disable=no-member
# check resume course buttons
response = self.client.get(course_home_url(course))
......@@ -159,7 +246,33 @@ class TestCourseOutlinePage(SharedModuleStoreTestCase):
self.assertContains(response, 'Resume Course', count=2)
content = pq(response.content)
self.assertTrue(content('.action-resume-course').attr('href').endswith('/sequential/' + section.url_name))
self.assertTrue(content('.action-resume-course').attr('href').endswith('/sequential/' + sequential2.url_name))
def test_resume_course_deleted_sequentials(self):
"""
Tests resume course when the last accessed sequential is deleted and
there are no sequentials left in the vertical.
"""
course = self.create_test_course()
# first navigate to a sequential to make it the last accessed
chapter = course.children[0]
self.assertEqual(len(chapter.children), 2)
sequential = chapter.children[0]
self.visit_sequential(course, chapter, sequential)
# remove all sequentials from chapter
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id):
for sequential in chapter.children:
self.store.delete_item(sequential.location, self.user.id) # pylint: disable=no-member
# check resume course buttons
response = self.client.get(course_home_url(course))
self.assertEqual(response.status_code, 200)
self.assertContains(response, 'Start Course', count=0)
self.assertContains(response, 'Resume Course', count=1)
class TestCourseOutlinePreview(SharedModuleStoreTestCase):
......@@ -184,8 +297,8 @@ class TestCourseOutlinePreview(SharedModuleStoreTestCase):
self.assertEqual(response.status_code, 200)
return response
# TODO: LEARNER-837: If you see this past 6/4/2017, please see why ticket is not yet closed.
@skip("testing skipping")
# TODO: LEARNER-837: Due 6/4/2017. Remove skip.
@skip("skipping test")
def test_preview(self):
"""
Verify the behavior of preview for the course outline.
......@@ -203,16 +316,16 @@ class TestCourseOutlinePreview(SharedModuleStoreTestCase):
parent_location=course.location,
display_name='First Chapter',
)
section = ItemFactory.create(category='sequential', parent_location=chapter.location)
ItemFactory.create(category='vertical', parent_location=section.location)
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
ItemFactory.create(category='vertical', parent_location=sequential.location)
chapter = ItemFactory.create(
category='chapter',
parent_location=course.location,
display_name='Future Chapter',
due=future_date,
)
section = ItemFactory.create(category='sequential', parent_location=chapter.location)
ItemFactory.create(category='vertical', parent_location=section.location)
sequential = ItemFactory.create(category='sequential', parent_location=chapter.location)
ItemFactory.create(category='vertical', parent_location=sequential.location)
# Verify that a staff user sees a chapter with a due date in the future
self.client.login(username=staff_user.username, password='test')
......
openedx/features/course_experience/utils.py
......@@ -32,15 +32,15 @@ def get_course_outline_block_tree(request, course_id):
return block
def set_lasted_accessed_default(block):
def set_last_accessed_default(block):
"""
Set default of False for last_accessed on all blocks.
"""
block['last_accessed'] = False
for child in block.get('children', []):
set_lasted_accessed_default(child)
set_last_accessed_default(child)
def mark_lasted_accessed(user, course_key, block):
def mark_last_accessed(user, course_key, block):
"""
Recursively marks the branch to the last accessed block.
"""
......@@ -49,10 +49,10 @@ def get_course_outline_block_tree(request, course_id):
last_accessed_child_position = student_module_dict.get('position')
if last_accessed_child_position and block.get('children'):
block['last_accessed'] = True
if len(block['children']) <= last_accessed_child_position:
if last_accessed_child_position <= len(block['children']):
last_accessed_child_block = block['children'][last_accessed_child_position - 1]
last_accessed_child_block['last_accessed'] = True
mark_lasted_accessed(user, course_key, last_accessed_child_block)
mark_last_accessed(user, course_key, last_accessed_child_block)
else:
# We should be using an id in place of position for last accessed. However, while using position, if
# the child block is no longer accessible we'll use the last child.
......@@ -72,7 +72,7 @@ def get_course_outline_block_tree(request, course_id):
course_outline_root_block = all_blocks['blocks'][all_blocks['root']]
populate_children(course_outline_root_block, all_blocks['blocks'])
set_lasted_accessed_default(course_outline_root_block)
mark_lasted_accessed(request.user, course_key, course_outline_root_block)
set_last_accessed_default(course_outline_root_block)
mark_last_accessed(request.user, course_key, course_outline_root_block)
return course_outline_root_block
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