Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #22344 from edx/saad/PROD-736

Browse Source 
[PROD-736] Course outline links incorrect in subsections with staff-only units
This commit is contained in:
Saad Yousaf
2019-11-27 16:41:20 +05:00
committed by GitHub
parent 0497646ad9 a59363432c
commit 0186f5e175
6 changed files with 153 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
common/lib/xmodule/xmodule/modulestore/search.py
View File

@@ -5,12 +5,14 @@ from logging import getLogger
from six.moves import range
from lms.djangoapps.courseware.masquerade import MASQUERADE_SETTINGS_KEY
from student.roles import GlobalStaff
from .exceptions import ItemNotFoundError, NoPathToItem
LOGGER = getLogger(__name__)
def path_to_location(modulestore, usage_key, full_path=False):
def path_to_location(modulestore, usage_key, request=None, full_path=False):
'''
Try to find a course_id/chapter/section[/position] path to location in
modulestore. The courseware insists that the first level in the course is
@@ -19,6 +21,7 @@ def path_to_location(modulestore, usage_key, full_path=False):
Args:
modulestore: which store holds the relevant objects
usage_key: :class:`UsageKey` the id of the location to which to generate the path
request: Request object containing information about user and masquerade settings, Default is None
full_path: :class:`Bool` if True, return the full path to location. Default is False.
Raises
@@ -104,6 +107,7 @@ def path_to_location(modulestore, usage_key, full_path=False):
# (e.g. sequential and videosequence) currently deal with this form of
# representing nested positions. This needs to happen before jumping to a
# module nested in more than one positional module will work.
if n > 3:
position_list = []
for path_index in range(2, n - 1):
@@ -112,18 +116,54 @@ def path_to_location(modulestore, usage_key, full_path=False):
section_desc = modulestore.get_item(path[path_index])
# this calls get_children rather than just children b/c old mongo includes private children
# in children but not in get_children
child_locs = [c.location for c in section_desc.get_children()]
child_locs = get_child_locations(section_desc, request, course_id)
# positions are 1-indexed, and should be strings to be consistent with
# url parsing.
position_list.append(str(child_locs.index(path[path_index + 1]) + 1))
if path[path_index + 1] in child_locs:
position_list.append(str(child_locs.index(path[path_index + 1]) + 1))
position = "_".join(position_list)
return (course_id, chapter, section, vertical, position, path[-1])
def get_child_locations(section_desc, request, course_id):
"""
Returns all child locations for a section. If user is learner or masquerading as learner,
staff only blocks are excluded.
"""
is_staff_user = GlobalStaff().has_user(request.user) if request else False
def is_masquerading_as_student():
"""
Return True if user is masquerading as learner.
"""
masquerade_settings = request.session.get(MASQUERADE_SETTINGS_KEY, {})
course_info = masquerade_settings.get(course_id)
return masquerade_settings and course_info and getattr(course_info, 'role', '') == 'student'
def is_user_staff_and_not_masquerading_learner():
"""
Return True if user is staff and not masquerading as learner.
"""
return is_staff_user and not is_masquerading_as_student()
def is_child_appendable(child_instance):
"""
Return True if child is appendable based on request and request's user type.
"""
return (request and is_user_staff_and_not_masquerading_learner()) or not child_instance.visible_to_staff_only
child_locs = []
for child in section_desc.get_children():
if not is_child_appendable(child):
continue
child_locs.append(child.location)
return child_locs
def navigation_index(position):
"""
Get the navigation index from the position argument (where the position argument was recieved from a call to
Get the navigation index from the position argument (where the position argument was received from a call to
path_to_location)
Argument:

2
common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py
View File

@@ -1651,7 +1651,7 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup):
# 8-9. get vertical, compute inheritance
# 10-11. get other vertical_x1b (why?) and compute inheritance
# Split: active_versions & structure
@ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [2, 2], 0))
@ddt.data((ModuleStoreEnum.Type.mongo, [12, 3], 0), (ModuleStoreEnum.Type.split, [3, 2], 0))
@ddt.unpack
def test_path_to_location(self, default_ms, num_finds, num_sends):
"""

76
common/test/acceptance/tests/lms/test_lms_dashboard.py
View File

@@ -7,11 +7,13 @@ from __future__ import absolute_import
import datetime
import re
import six
from six.moves.urllib.parse import unquote # pylint: disable=import-error
from common.test.acceptance.fixtures.course import CourseFixture
from common.test.acceptance.fixtures.course import CourseFixture, XBlockFixtureDesc
from common.test.acceptance.pages.common.auto_auth import AutoAuthPage
from common.test.acceptance.pages.lms.course_home import CourseHomePage
from common.test.acceptance.pages.lms.dashboard import DashboardPage
from common.test.acceptance.pages.lms.problem import ProblemPage
from common.test.acceptance.pages.lms.staff_view import StaffPreviewPage
from common.test.acceptance.tests.helpers import UniqueCourseTest, generate_course_key
DEFAULT_SHORT_DATE_FORMAT = u'{dt:%b} {dt.day}, {dt.year}'
@@ -125,8 +127,24 @@ class BaseLmsDashboardTestMultiple(UniqueCourseTest):
u"social_sharing_url": {u"value": "http://custom/course/url"},
u"cert_name_long": {u"value": value['cert_name_long']}
})
course_fixture.install()
course_fixture.add_children(
XBlockFixtureDesc('chapter', 'Test Section 1').add_children(
XBlockFixtureDesc('sequential', 'Test Subsection 1,1').add_children(
XBlockFixtureDesc('problem', 'Test Problem 1', data='<problem>problem 1 dummy body</problem>'),
XBlockFixtureDesc('html', 'html 1', data="<html>html 1 dummy body</html>"),
XBlockFixtureDesc('problem', 'Test Problem 2', data="<problem>problem 2 dummy body</problem>"),
XBlockFixtureDesc('html', 'html 2', data="<html>html 2 dummy body</html>"),
),
XBlockFixtureDesc('sequential', 'Test Subsection 1,2').add_children(
XBlockFixtureDesc('problem', 'Test Problem 3', data='<problem>problem 3 dummy body</problem>'),
),
XBlockFixtureDesc(
'sequential', 'Test HIDDEN Subsection', metadata={'visible_to_staff_only': True}
).add_children(
XBlockFixtureDesc('problem', 'Test HIDDEN Problem', data='<problem>hidden problem</problem>'),
),
)
).install()
self.course_keys[key] = course_key
self.course_fixtures[key] = course_fixture
@@ -201,7 +219,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest):
Scenario:
Course Date should have the format 'Ended - Sep 23, 2015'
if the course on student dashboard has ended.
As a Student,
Given that I have enrolled to a course
And the course has ended in the past
@@ -233,7 +250,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest):
Scenario:
Course Date should have the format 'Started - Sep 23, 2015'
if the course on student dashboard is running.
As a Student,
Given that I have enrolled to a course
And the course has started
@@ -266,7 +282,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest):
Scenario:
Course Date should have the format 'Starts - Sep 23, 2015'
if the course on student dashboard starts in future.
As a Student,
Given that I have enrolled to a course
And the course starts in future
@@ -300,7 +315,6 @@ class LmsDashboardPageTest(BaseLmsDashboardTest):
Scenario:
Course Date should have the format 'Starts - Wednesday at 5am UTC'
if the course on student dashboard starts within 5 days.
As a Student,
Given that I have enrolled to a course
And the course starts within 5 days
@@ -436,3 +450,49 @@ class LmsDashboardA11yTest(BaseLmsDashboardTestMultiple):
course_listings = self.dashboard_page.get_courses()
self.assertEqual(len(course_listings), 3)
self.dashboard_page.a11y_audit.check_for_accessibility_errors()
class TestMasqueradeAndSwitchCourse(BaseLmsDashboardTestMultiple):
"""
Class to test lms dashboard accessibility of courses when masquerading as learner.
"""
def test_masquerade_and_switch_course(self):
"""
Scenario:
Staff user should be able to access other courses after
masquerading as student in one course
As Staff user, Select a course
When I click to change view from Staff to Learner
Then the first subsection from course outline should be visible as Learner
When I click to select a different course
Then the first subsection from new course outline should be visible as Staff
"""
AutoAuthPage(
self.browser,
username=self.username,
email=self.email,
staff=True
).visit()
self.dashboard_page.visit()
section_title = 'Test Section 1'
subsection_title = 'Test Subsection 1,1'
course_page = CourseHomePage(self.browser, str(self.course_keys['A']))
course_page.visit()
problem_name = u'Test Problem 1'
staff_page = StaffPreviewPage(self.browser)
staff_page.set_staff_view_mode('Learner')
course_page.outline.go_to_section(section_title, subsection_title)
self.assertEqual(staff_page.staff_view_mode, 'Learner')
self.assertEqual(ProblemPage(self.browser).problem_name, problem_name)
course_page.course_id = str(self.course_keys['B'])
course_page.visit()
course_page.outline.go_to_section(section_title, subsection_title)
self.assertNotEqual(staff_page.staff_view_mode, 'Learner')
self.assertEqual(ProblemPage(self.browser).problem_name, problem_name)

39
lms/djangoapps/courseware/tests/test_views.py
View File

@@ -25,7 +25,6 @@ from freezegun import freeze_time
from markupsafe import escape
from milestones.tests.utils import MilestonesTestCaseMixin
from mock import MagicMock, PropertyMock, create_autospec, patch
from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
from pytz import UTC
from six import text_type
@@ -62,6 +61,7 @@ from lms.djangoapps.commerce.utils import EcommerceService
from lms.djangoapps.grades.config.waffle import ASSUME_ZERO_GRADE_IF_ABSENT
from lms.djangoapps.grades.config.waffle import waffle as grades_waffle
from lms.djangoapps.verify_student.services import IDVerificationService
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx.core.djangoapps.catalog.tests.factories import CourseFactory as CatalogCourseFactory
from openedx.core.djangoapps.catalog.tests.factories import CourseRunFactory, ProgramFactory
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
@@ -104,6 +104,7 @@ FEATURES_WITH_DISABLE_HONOR_CERTIFICATE = settings.FEATURES.copy()
FEATURES_WITH_DISABLE_HONOR_CERTIFICATE['DISABLE_HONOR_CERTIFICATES'] = True
@ddt.ddt
class TestJumpTo(ModuleStoreTestCase):
"""
Check the jumpto link for a course.
@@ -191,7 +192,6 @@ class TestJumpTo(ModuleStoreTestCase):
module2 = ItemFactory.create(category='html', parent_location=nested_vertical2.location)
# internal position of module2 will be 1_2 (2nd item withing 1st item)
expected = '/courses/{course_id}/courseware/{chapter_id}/{section_id}/1?{activate_block_id}'.format(
course_id=six.text_type(course.id),
chapter_id=chapter.url_name,
@@ -215,6 +215,41 @@ class TestJumpTo(ModuleStoreTestCase):
response = self.client.get(jumpto_url)
self.assertEqual(response.status_code, 404)
@ddt.data(
(False, '1'),
(True, '2')
)
@ddt.unpack
def test_jump_to_for_learner_with_staff_only_content(self, is_staff_user, position):
"""
Test for checking correct position in redirect_url for learner when a course has staff-only units.
"""
course = CourseFactory.create()
request = RequestFactory().get('/')
request.user = UserFactory(is_staff=is_staff_user, username="staff")
request.session = {}
course_key = CourseKey.from_string(six.text_type(course.id))
chapter = ItemFactory.create(category='chapter', parent_location=course.location)
section = ItemFactory.create(category='sequential', parent_location=chapter.location)
__ = ItemFactory.create(category='vertical', parent_location=section.location)
staff_only_vertical = ItemFactory.create(category='vertical', parent_location=section.location,
metadata=dict(visible_to_staff_only=True))
__ = ItemFactory.create(category='vertical', parent_location=section.location)
usage_key = UsageKey.from_string(six.text_type(staff_only_vertical.location)).replace(course_key=course_key)
expected_url = reverse(
'courseware_position',
kwargs={
'course_id': six.text_type(course.id),
'chapter': chapter.url_name,
'section': section.url_name,
'position': position,
}
)
expected_url += "?{}".format(urlencode({'activate_block_id': six.text_type(staff_only_vertical.location)}))
self.assertEqual(expected_url, get_redirect_url(course_key, usage_key, request))
@ddt.ddt
class IndexQueryTestCase(ModuleStoreTestCase):

4
lms/djangoapps/courseware/url_helpers.py
View File

@@ -11,7 +11,7 @@ from xmodule.modulestore.django import modulestore
from xmodule.modulestore.search import navigation_index, path_to_location
def get_redirect_url(course_key, usage_key):
def get_redirect_url(course_key, usage_key, request=None):
""" Returns the redirect url back to courseware
Args:
@@ -28,7 +28,7 @@ def get_redirect_url(course_key, usage_key):
(
course_key, chapter, section, vertical_unused,
position, final_target_id
) = path_to_location(modulestore(), usage_key)
) = path_to_location(modulestore(), usage_key, request)
# choose the appropriate view (and provide the necessary args) based on the
# args provided by the redirect.

2
lms/djangoapps/courseware/views/views.py
View File

@@ -355,7 +355,7 @@ def jump_to(_request, course_id, location):
except InvalidKeyError:
raise Http404(u"Invalid course_key or usage_key")
try:
redirect_url = get_redirect_url(course_key, usage_key)
redirect_url = get_redirect_url(course_key, usage_key, _request)
except ItemNotFoundError:
raise Http404(u"No data at this location: {0}".format(usage_key))
except NoPathToItem: