diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 42b427605d..446d7be982 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -37,9 +37,7 @@ def get_course_groupname_for_role(location, role): groupnames.append('{0}_{1}'.format(role, location.course)) elif isinstance(location, CourseLocator): old_location = loc_mapper().translate_locator_to_location(location) - if old_location is None: - groupnames.append('{0}_{1}'.format(role, location.as_old_location_course_id)) - else: + if old_location: groupnames.append('{0}_{1}'.format(role, old_location.course_id)) for groupname in groupnames: diff --git a/cms/djangoapps/contentstore/tests/test_course_index.py b/cms/djangoapps/contentstore/tests/test_course_index.py index f268eaf5f1..ec0c0b1f8e 100644 --- a/cms/djangoapps/contentstore/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/tests/test_course_index.py @@ -1,29 +1,48 @@ """ Unit tests for getting the list of courses and the course outline. """ -from django.core.urlresolvers import reverse +import json import lxml +from django.core.urlresolvers import reverse from contentstore.tests.utils import CourseTestCase from xmodule.modulestore.django import loc_mapper -from django.core.exceptions import PermissionDenied +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore import parsers class TestCourseIndex(CourseTestCase): """ Unit tests for getting the list of courses and the course outline. """ - def test_index(self): + def setUp(self): + """ + Add a course with odd characters in the fields + """ + super(TestCourseIndex, self).setUp() + # had a problem where index showed course but has_access failed to retrieve it for non-staff + self.odd_course = CourseFactory.create( + org='test.org_1-2', + number='test-2.3_course', + display_name='dotted.course.name-2', + ) + + + def check_index_and_outline(self, authed_client): """ Test getting the list of courses and then pulling up their outlines """ index_url = reverse('contentstore.views.index') - index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') + index_response = authed_client.get(index_url, {}, HTTP_ACCEPT='text/html') parsed_html = lxml.html.fromstring(index_response.content) course_link_eles = parsed_html.find_class('course-link') + self.assertGreaterEqual(len(course_link_eles), 2) for link in course_link_eles: - self.assertRegexpMatches(link.get("href"), r'course/\w+\.\w+\.\w+.*/branch/\w+/block/.*') + self.assertRegexpMatches( + link.get("href"), + r'course/{0}+/branch/{0}+/block/{0}+'.format(parsers.ALLOWED_ID_CHARS) + ) # now test that url - outline_response = self.client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') + outline_response = authed_client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') # ensure it has the expected 2 self referential links outline_parsed = lxml.html.fromstring(outline_response.content) outline_link = outline_parsed.find_class('course-link')[0] @@ -31,6 +50,12 @@ class TestCourseIndex(CourseTestCase): course_menu_link = outline_parsed.find_class('nav-course-courseware-outline')[0] self.assertEqual(course_menu_link.find("a").get("href"), link.get("href")) + def test_is_staff_access(self): + """ + Test that people with is_staff see the courses and can navigate into them + """ + self.check_index_and_outline(self.client) + def test_negative_conditions(self): """ Test the error conditions for the access @@ -38,6 +63,28 @@ class TestCourseIndex(CourseTestCase): locator = loc_mapper().translate_location(self.course.location.course_id, self.course.location, False, True) outline_url = reverse('contentstore.views.course_handler', kwargs={'course_url': unicode(locator)}) # register a non-staff member and try to delete the course branch - non_staff_client = self.createNonStaffAuthedUserClient() + non_staff_client, _ = self.createNonStaffAuthedUserClient() response = non_staff_client.delete(outline_url, {}, HTTP_ACCEPT='application/json') self.assertEqual(response.status_code, 403) + + def test_course_staff_access(self): + """ + Make and register an course_staff and ensure they can access the courses + """ + course_staff_client, course_staff = self.createNonStaffAuthedUserClient() + for course in [self.course, self.odd_course]: + permission_url = reverse("course_team_user", kwargs={ + "org": course.location.org, + "course": course.location.course, + "name": course.location.name, + "email": course_staff.email, + }) + self.client.post( + permission_url, + data=json.dumps({"role": "staff"}), + content_type="application/json", + HTTP_ACCEPT="application/json", + ) + + # test access + self.check_index_and_outline(course_staff_client) diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 6a227d9a80..29c9e59d82 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -78,4 +78,4 @@ class CourseTestCase(ModuleStoreTestCase): client = Client() client.login(username=uname, password=password) - return client + return client, nonstaff diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f043c09d73..cf6ef7f626 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -131,9 +131,9 @@ def course_index(request, course_url): lms_link = get_lms_link_for_item(old_location) upload_asset_callback_url = reverse('upload_asset', kwargs={ - 'org': location.as_old_location_org, - 'course': location.as_old_location_course, - 'coursename': location.as_old_location_run + 'org': old_location.org, + 'course': old_location.course, + 'coursename': old_location.name }) course = modulestore().get_item(old_location, depth=3) diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 749e2556ab..dffd182764 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -263,56 +263,6 @@ class CourseLocator(Locator): version_guid=self.version_guid, branch=self.branch) - OLD_COURSE_ID_RE = re.compile(r'^(?P[^.]+)\.?(?P.+)?\.(?P[^.]+)$') - @property - def as_old_location_course_id(self): - """ - The original Location type presented its course id as org/course/run. This function - assumes the course_id starts w/ org, has an arbitrarily long 'course' identifier, and then - ends w/ run all separated by periods. - - If this object does not have a course_id, this function returns None. - """ - if self.course_id is None: - return None - parsed = self.OLD_COURSE_ID_RE.match(self.course_id) - # check whether there are 2 or > 2 'fields' - if parsed.group('old_course_id'): - return '/'.join(parsed.groups()) - else: - return parsed.group('org') + '/' + parsed.group('run') - - def _old_location_field_helper(self, field): - """ - Parse course_id to get the old location field named field out - """ - if self.course_id is None: - return None - parsed = self.OLD_COURSE_ID_RE.match(self.course_id) - return parsed.group(field) - - @property - def as_old_location_org(self): - """ - Presume the first part of the course_id is the org and return it. - """ - return self._old_location_field_helper('org') - - @property - def as_old_location_course(self): - """ - Presume the middle part, if any, of the course_id is the old location scheme's - course id and return it. - """ - return self._old_location_field_helper('old_course_id') - - @property - def as_old_location_run(self): - """ - Presume the last part of the course_id is the old location scheme's run and return it. - """ - return self._old_location_field_helper('run') - def init_from_url(self, url): """ url must be a string beginning with 'edx://' and containing diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index be00f341b8..2db00f279d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -283,21 +283,6 @@ class LocatorTest(TestCase): Locator.to_locator_or_location("hello.world.not.a.url") self.assertIsNone(Locator.parse_url("unknown://foo.bar/baz")) - def test_as_old(self): - """ - Test the as_old_location_xxx accessors - """ - locator = CourseLocator(course_id='org.course.id.run', branch='mybranch') - self.assertEqual('org', locator.as_old_location_org) - self.assertEqual('course.id', locator.as_old_location_course) - self.assertEqual('run', locator.as_old_location_run) - self.assertEqual('org/course.id/run', locator.as_old_location_course_id) - locator = CourseLocator(course_id='org.course', branch='mybranch') - self.assertEqual('org', locator.as_old_location_org) - self.assertIsNone(locator.as_old_location_course) - self.assertEqual('course', locator.as_old_location_run) - self.assertEqual('org/course', locator.as_old_location_course_id) - def test_description_locator_url(self): object_id = '{:024x}'.format(random.randrange(16 ** 24)) definition_locator = DefinitionLocator(object_id) diff --git a/lms/templates/courseware/instructor_dashboard.html b/lms/templates/courseware/instructor_dashboard.html index 19a7e398c1..deded1d0ed 100644 --- a/lms/templates/courseware/instructor_dashboard.html +++ b/lms/templates/courseware/instructor_dashboard.html @@ -546,11 +546,13 @@ function goto( mode)

These email actions run in the background, and status for active email tasks will appear in a table below. diff --git a/lms/templates/registration/password_reset_complete.html b/lms/templates/registration/password_reset_complete.html index d8a9f0eedb..d1852cefaa 100644 --- a/lms/templates/registration/password_reset_complete.html +++ b/lms/templates/registration/password_reset_complete.html @@ -7,7 +7,10 @@ {% trans "Your Password Reset is Complete" %} - {% compressed_css 'application' %} + {% compressed_css 'style-vendor' %} + {% compressed_css 'style-app' %} + {% compressed_css 'style-app-extend1' %} + {% compressed_css 'style-app-extend2' %}