diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index 41c6b7cffd..692ec695ce 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -171,7 +171,7 @@ def log_into_studio( world.log_in(username=uname, password=password, email=email, name=name) # Navigate to the studio dashboard world.visit('/') - assert_in(uname, world.css_text('h2.title', timeout=10)) + assert_in(uname, world.css_text('span.account-username', timeout=10)) def add_course_author(user, course): diff --git a/cms/djangoapps/contentstore/features/courses.py b/cms/djangoapps/contentstore/features/courses.py index 7b3ccbcbd2..218faded58 100644 --- a/cms/djangoapps/contentstore/features/courses.py +++ b/cms/djangoapps/contentstore/features/courses.py @@ -1,5 +1,6 @@ # pylint: disable=missing-docstring # pylint: disable=redefined-outer-name +# pylint: disable=unused-argument from lettuce import world, step from common import * @@ -33,15 +34,19 @@ def i_create_a_course(step): create_a_course() -@step('I click the course link in My Courses$') -def i_click_the_course_link_in_my_courses(step): +@step('I click the course link in Studio Home$') +def i_click_the_course_link_in_studio_home(step): # pylint: disable=invalid-name course_css = 'a.course-link' world.css_click(course_css) @step('I see an error about the length of the org/course/run tuple') def i_see_error_about_length(step): - assert world.css_has_text('#course_creation_error', 'The combined length of the organization, course number, and course run fields cannot be more than 65 characters.') + assert world.css_has_text( + '#course_creation_error', + 'The combined length of the organization, course number, ' + 'and course run fields cannot be more than 65 characters.' + ) ############ ASSERTIONS ################### @@ -52,8 +57,8 @@ def courseware_page_has_loaded_in_studio(step): assert world.is_css_present(course_title_css) -@step('I see the course listed in My Courses$') -def i_see_the_course_in_my_courses(step): +@step('I see the course listed in Studio Home$') +def i_see_the_course_in_studio_home(step): course_css = 'h3.class-title' assert world.css_has_text(course_css, world.scenario_dict['COURSE'].display_name) diff --git a/cms/djangoapps/contentstore/features/help.feature b/cms/djangoapps/contentstore/features/help.feature index eb0f872247..567a2f2526 100644 --- a/cms/djangoapps/contentstore/features/help.feature +++ b/cms/djangoapps/contentstore/features/help.feature @@ -11,7 +11,7 @@ Feature: CMS.Help Scenario: Users can access online help within a course Given I have opened a new course in Studio - And I click the course link in My Courses + And I click the course link in Studio Home Then I should see online help for "outline" And I go to the course updates page diff --git a/cms/djangoapps/contentstore/features/signup.feature b/cms/djangoapps/contentstore/features/signup.feature index 1fe254adaa..0954551059 100644 --- a/cms/djangoapps/contentstore/features/signup.feature +++ b/cms/djangoapps/contentstore/features/signup.feature @@ -26,7 +26,7 @@ Feature: CMS.Sign in And I visit the url "/signin?next=http://www.google.com/" When I fill in and submit the signin form And I wait for "2" seconds - Then I should see that the path is "/course/" + Then I should see that the path is "/home/" Scenario: Login with mistyped credentials Given I have opened a new course in Studio @@ -41,4 +41,4 @@ Feature: CMS.Sign in Then I should not see a login error message And I submit the signin form And I wait for "2" seconds - Then I should see that the path is "/course/" + Then I should see that the path is "/home/" diff --git a/cms/djangoapps/contentstore/features/signup.py b/cms/djangoapps/contentstore/features/signup.py index 014ff2eb69..b1c65edea5 100644 --- a/cms/djangoapps/contentstore/features/signup.py +++ b/cms/djangoapps/contentstore/features/signup.py @@ -25,7 +25,7 @@ def i_press_the_button_on_the_registration_form(step): @step('I should see an email verification prompt') def i_should_see_an_email_verification_prompt(step): - world.css_has_text('h1.page-header', u'My Courses') + world.css_has_text('h1.page-header', u'Studio Home') world.css_has_text('div.msg h3.title', u'We need to verify your email address') diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 27714230dc..a107d65449 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1166,11 +1166,10 @@ class ContentStoreTest(ContentStoreTestCase): def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" - # Create a course so there is something to view - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, - '

My Courses

', + '

Studio Home

', status_code=200, html=True ) @@ -1189,7 +1188,7 @@ class ContentStoreTest(ContentStoreTestCase): def test_course_index_view_with_course(self): """Test viewing the index page with an existing course""" CourseFactory.create(display_name='Robot Super Educational Course') - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains( resp, '

Robot Super Educational Course

', @@ -1604,7 +1603,7 @@ class RerunCourseTest(ContentStoreTestCase): Asserts that the given course key is in the accessible course listing section of the html and NOT in the unsucceeded course action section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 1) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 0) @@ -1613,7 +1612,7 @@ class RerunCourseTest(ContentStoreTestCase): Asserts that the given course key is in the unsucceeded course action section of the html and NOT in the accessible course listing section of the html. """ - course_listing = lxml.html.fromstring(self.client.get_html('/course/').content) + course_listing = lxml.html.fromstring(self.client.get_html('/home/').content) self.assertEqual(len(self.get_course_listing_elements(course_listing, course_key)), 0) self.assertEqual(len(self.get_unsucceeded_course_action_elements(course_listing, course_key)), 1) diff --git a/cms/djangoapps/contentstore/tests/test_i18n.py b/cms/djangoapps/contentstore/tests/test_i18n.py index e9e1739488..da79117220 100644 --- a/cms/djangoapps/contentstore/tests/test_i18n.py +++ b/cms/djangoapps/contentstore/tests/test_i18n.py @@ -44,9 +44,9 @@ class InternationalizationTest(ModuleStoreTestCase): self.client = AjaxEnabledTestClient() self.client.login(username=self.uname, password=self.password) - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertContains(resp, - '

My Courses

', + '

Studio Home

', status_code=200, html=True) @@ -56,13 +56,13 @@ class InternationalizationTest(ModuleStoreTestCase): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='en', ) self.assertContains(resp, - '

My Courses

', + '

Studio Home

', status_code=200, html=True) @@ -81,7 +81,7 @@ class InternationalizationTest(ModuleStoreTestCase): self.client.login(username=self.uname, password=self.password) resp = self.client.get_html( - '/course/', + '/home/', {}, HTTP_ACCEPT_LANGUAGE='eo' ) diff --git a/cms/djangoapps/contentstore/tests/test_libraries.py b/cms/djangoapps/contentstore/tests/test_libraries.py new file mode 100644 index 0000000000..dc6d8b976d --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_libraries.py @@ -0,0 +1,849 @@ +""" +Content library unit tests that require the CMS runtime. +""" +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.utils import reverse_url, reverse_usage_url, reverse_library_url +from contentstore.views.preview import _load_preview_module +from contentstore.views.tests.test_library import LIBRARY_REST_URL +import ddt +from mock import patch +from student.auth import has_studio_read_access, has_studio_write_access +from student.roles import ( + CourseInstructorRole, CourseStaffRole, CourseCreatorRole, LibraryUserRole, + OrgStaffRole, OrgInstructorRole, OrgLibraryUserRole, +) +from xmodule.library_content_module import LibraryVersionReference +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from mock import Mock +from opaque_keys.edx.locator import CourseKey, LibraryLocator + + +class LibraryTestCase(ModuleStoreTestCase): + """ + Common functionality for content libraries tests + """ + def setUp(self): + user_password = super(LibraryTestCase, self).setUp() + + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=user_password) + + self.lib_key = self._create_library() + self.library = modulestore().get_library(self.lib_key) + + self.session_data = {} # Used by _bind_module + + def _create_library(self, org="org", library="lib", display_name="Test Library"): + """ + Helper method used to create a library. Uses the REST API. + """ + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': org, + 'library': library, + 'display_name': display_name, + }) + self.assertEqual(response.status_code, 200) + lib_info = parse_json(response) + lib_key = CourseKey.from_string(lib_info['library_key']) + self.assertIsInstance(lib_key, LibraryLocator) + return lib_key + + def _add_library_content_block(self, course, library_key, other_settings=None): + """ + Helper method to add a LibraryContent block to a course. + The block will be configured to select content from the library + specified by library_key. + other_settings can be a dict of Scope.settings fields to set on the block. + """ + return ItemFactory.create( + category='library_content', + parent_location=course.location, + user_id=self.user.id, + publish_item=False, + source_libraries=[LibraryVersionReference(library_key)], + **(other_settings or {}) + ) + + def _add_simple_content_block(self): + """ Adds simple HTML block to library """ + return ItemFactory.create( + category="html", parent_location=self.library.location, + user_id=self.user.id, publish_item=False + ) + + def _refresh_children(self, lib_content_block, status_code_expected=200): + """ + Helper method: Uses the REST API to call the 'refresh_children' handler + of a LibraryContent block + """ + if 'user' not in lib_content_block.runtime._services: # pylint: disable=protected-access + lib_content_block.runtime._services['user'] = Mock(user_id=self.user.id) # pylint: disable=protected-access + handler_url = reverse_usage_url( + 'component_handler', + lib_content_block.location, + kwargs={'handler': 'refresh_children'} + ) + response = self.client.ajax_post(handler_url) + self.assertEqual(response.status_code, status_code_expected) + return modulestore().get_item(lib_content_block.location) + + def _bind_module(self, descriptor, user=None): + """ + Helper to use the CMS's module system so we can access student-specific fields. + """ + if user is None: + user = self.user + if user not in self.session_data: + self.session_data[user] = {} + request = Mock(user=user, session=self.session_data[user]) + _load_preview_module(request, descriptor) # pylint: disable=protected-access + + def _update_item(self, usage_key, metadata): + """ + Helper method: Uses the REST API to update the fields of an XBlock. + This will result in the XBlock's editor_saved() method being called. + """ + update_url = reverse_usage_url("xblock_handler", usage_key) + return self.client.ajax_post( + update_url, + data={ + 'metadata': metadata, + } + ) + + def _list_libraries(self): + """ + Use the REST API to get a list of libraries visible to the current user. + """ + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 200) + return parse_json(response) + + +@ddt.ddt +class TestLibraries(LibraryTestCase): + """ + High-level tests for libraries + """ + @ddt.data( + (2, 1, 1), + (2, 2, 2), + (2, 20, 2), + ) + @ddt.unpack + def test_max_items(self, num_to_create, num_to_select, num_expected): + """ + Test the 'max_count' property of LibraryContent blocks. + """ + for _ in range(0, num_to_create): + self._add_simple_content_block() + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': num_to_select}) + self.assertEqual(len(lc_block.children), 0) + lc_block = self._refresh_children(lc_block) + + # Now, we want to make sure that .children has the total # of potential + # children, and that get_child_descriptors() returns the actual children + # chosen for a given student. + # In order to be able to call get_child_descriptors(), we must first + # call bind_for_student: + self._bind_module(lc_block) + self.assertEqual(len(lc_block.children), num_to_create) + self.assertEqual(len(lc_block.get_child_descriptors()), num_expected) + + def test_consistent_children(self): + """ + Test that the same student will always see the same selected child block + """ + # Create many blocks in the library and add them to a course: + for num in range(0, 8): + ItemFactory.create( + data="This is #{}".format(num + 1), + category="html", parent_location=self.library.location, user_id=self.user.id, publish_item=False + ) + + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + lc_block = self._add_library_content_block(course, self.lib_key, {'max_count': 1}) + lc_block_key = lc_block.location + lc_block = self._refresh_children(lc_block) + + def get_child_of_lc_block(block): + """ + Fetch the child shown to the current user. + """ + children = block.get_child_descriptors() + self.assertEqual(len(children), 1) + return children[0] + + # Check which child a student will see: + self._bind_module(lc_block) + chosen_child = get_child_of_lc_block(lc_block) + chosen_child_defn_id = chosen_child.definition_locator.definition_id + lc_block.save() + + modulestore().update_item(lc_block, self.user.id) + + # Now re-load the block and try again: + def check(): + """ + Confirm that chosen_child is still the child seen by the test student + """ + for _ in range(0, 6): # Repeat many times b/c blocks are randomized + lc_block = modulestore().get_item(lc_block_key) # Reload block from the database + self._bind_module(lc_block) + current_child = get_child_of_lc_block(lc_block) + self.assertEqual(current_child.location, chosen_child.location) + self.assertEqual(current_child.data, chosen_child.data) + self.assertEqual(current_child.definition_locator.definition_id, chosen_child_defn_id) + + check() + # Refresh the children: + lc_block = self._refresh_children(lc_block) + # Now re-load the block and try yet again, in case refreshing the children changed anything: + check() + + def test_definition_shared_with_library(self): + """ + Test that the same block definition is used for the library and course[s] + """ + block1 = self._add_simple_content_block() + def_id1 = block1.definition_locator.definition_id + block2 = self._add_simple_content_block() + def_id2 = block2.definition_locator.definition_id + self.assertNotEqual(def_id1, def_id2) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + for child_key in lc_block.children: + child = modulestore().get_item(child_key) + def_id = child.definition_locator.definition_id + self.assertIn(def_id, (def_id1, def_id2)) + + def test_fields(self): + """ + Test that blocks used from a library have the same field values as + defined by the library author. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + lib_block = ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(lib_block.data, data_value) + self.assertEqual(lib_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + course_block = modulestore().get_item(lc_block.children[0]) + + self.assertEqual(course_block.data, data_value) + self.assertEqual(course_block.display_name, name_value) + + def test_block_with_children(self): + """ + Test that blocks used from a library can have children. + """ + data_value = "A Scope.content value" + name_value = "A Scope.settings value" + # In the library, create a vertical block with a child: + vert_block = ItemFactory.create( + category="vertical", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + ) + child_block = ItemFactory.create( + category="html", + parent_location=vert_block.location, + user_id=self.user.id, + publish_item=False, + display_name=name_value, + data=data_value, + ) + self.assertEqual(child_block.data, data_value) + self.assertEqual(child_block.display_name, name_value) + + # Next, create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + course_vert_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(len(course_vert_block.children), 1) + course_child_block = modulestore().get_item(course_vert_block.children[0]) + + self.assertEqual(course_child_block.data, data_value) + self.assertEqual(course_child_block.display_name, name_value) + + def test_change_after_first_sync(self): + """ + Check that nothing goes wrong if we (A) Set up a LibraryContent block + and use it successfully, then (B) Give it an invalid configuration. + No children should be deleted until the configuration is fixed. + """ + # Add a block to the library: + data_value = "Hello world!" + ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name="HTML BLock", + data=data_value, + ) + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + + # Now, change the block settings to have an invalid library key: + resp = self._update_item( + lc_block.location, + {"source_libraries": [["library-v1:NOT+FOUND", None]]}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + self.assertEqual(len(lc_block.children), 1) # Children should not be deleted due to a bad setting. + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.data, data_value) + + def test_refreshes_children_if_libraries_change(self): + """ Tests that children are automatically refreshed if libraries list changes """ + library2key = self._create_library("org2", "lib2", "Library2") + library2 = modulestore().get_library(library2key) + data1, data2 = "Hello world!", "Hello other world!" + ItemFactory.create( + category="html", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name="Lib1: HTML BLock", + data=data1, + ) + + ItemFactory.create( + category="html", + parent_location=library2.location, + user_id=self.user.id, + publish_item=False, + display_name="Lib 2: HTML BLock", + data=data2, + ) + + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 1) + + # Now, change the block settings to have an invalid library key: + resp = self._update_item( + lc_block.location, + {"source_libraries": [[str(library2key)]]}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.data, data2) + + def test_refreshes_children_if_capa_type_change(self): + """ Tests that children are automatically refreshed if capa type field changes """ + name1, name2 = "Option Problem", "Multiple Choice Problem" + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name1, + data="", + ) + ItemFactory.create( + category="problem", + parent_location=self.library.location, + user_id=self.user.id, + publish_item=False, + display_name=name2, + data="", + ) + + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 2) + + resp = self._update_item( + lc_block.location, + {"capa_type": 'optionresponse'}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.display_name, name1) + + resp = self._update_item( + lc_block.location, + {"capa_type": 'multiplechoiceresponse'}, + ) + self.assertEqual(resp.status_code, 200) + lc_block = modulestore().get_item(lc_block.location) + + self.assertEqual(len(lc_block.children), 1) + html_block = modulestore().get_item(lc_block.children[0]) + self.assertEqual(html_block.display_name, name2) + + def test_refresh_fails_for_unknown_library(self): + """ Tests that refresh children fails if unknown library is configured """ + # Create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + # Add a LibraryContent block to the course: + lc_block = self._add_library_content_block(course, self.lib_key) + lc_block = self._refresh_children(lc_block) + self.assertEqual(len(lc_block.children), 0) + + # Now, change the block settings to have an invalid library key: + resp = self._update_item( + lc_block.location, + {"source_libraries": [["library-v1:NOT+FOUND", None]]}, + ) + self.assertEqual(resp.status_code, 200) + with self.assertRaises(ValueError): + self._refresh_children(lc_block, status_code_expected=400) + + +@ddt.ddt +class TestLibraryAccess(LibraryTestCase): + """ + Test Roles and Permissions related to Content Libraries + """ + def setUp(self): + """ Create a library, staff user, and non-staff user """ + super(TestLibraryAccess, self).setUp() + self.non_staff_user, self.non_staff_user_password = self.create_non_staff_user() + + def _login_as_non_staff_user(self, logout_first=True): + """ Login as a user that starts out with no roles/permissions granted. """ + if logout_first: + self.client.logout() # We start logged in as a staff user + self.client.login(username=self.non_staff_user.username, password=self.non_staff_user_password) + + def _assert_cannot_create_library(self, org="org", library="libfail", expected_code=403): + """ Ensure the current user is not able to create a library. """ + self.assertTrue(expected_code >= 300) + response = self.client.ajax_post( + LIBRARY_REST_URL, + {'org': org, 'library': library, 'display_name': "Irrelevant"} + ) + self.assertEqual(response.status_code, expected_code) + key = LibraryLocator(org=org, library=library) + self.assertEqual(modulestore().get_library(key), None) + + def _can_access_library(self, library): + """ + Use the normal studio library URL to check if we have access + + `library` can be a LibraryLocator or the library's root XBlock + """ + if isinstance(library, (basestring, LibraryLocator)): + lib_key = library + else: + lib_key = library.location.library_key + response = self.client.get(reverse_library_url('library_handler', unicode(lib_key))) + self.assertIn(response.status_code, (200, 302, 403)) + return response.status_code == 200 + + def tearDown(self): + """ + Log out when done each test + """ + self.client.logout() + super(TestLibraryAccess, self).tearDown() + + def test_creation(self): + """ + The user that creates a library should have instructor (admin) and staff permissions + """ + # self.library has been auto-created by the staff user. + self.assertTrue(has_studio_write_access(self.user, self.lib_key)) + self.assertTrue(has_studio_read_access(self.user, self.lib_key)) + # Make sure the user was actually assigned the instructor role and not just using is_staff superpowers: + self.assertTrue(CourseInstructorRole(self.lib_key).has_user(self.user)) + + # Now log out and ensure we are forbidden from creating a library: + self.client.logout() + self._assert_cannot_create_library(expected_code=302) # 302 redirect to login expected + + # Now create a non-staff user with no permissions: + self._login_as_non_staff_user(logout_first=False) + self.assertFalse(CourseCreatorRole().has_user(self.non_staff_user)) + + # Now check that logged-in users without any permissions cannot create libraries + with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): + self._assert_cannot_create_library() + + @ddt.data( + CourseInstructorRole, + CourseStaffRole, + LibraryUserRole, + ) + def test_acccess(self, access_role): + """ + Test the various roles that allow viewing libraries are working correctly. + """ + # At this point, one library exists, created by the currently-logged-in staff user. + # Create another library as staff: + library2_key = self._create_library(library="lib2") + # Login as non_staff_user: + self._login_as_non_staff_user() + + # non_staff_user shouldn't be able to access any libraries: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 0) + self.assertFalse(self._can_access_library(self.library)) + self.assertFalse(self._can_access_library(library2_key)) + + # Now manually intervene to give non_staff_user access to library2_key: + access_role(library2_key).add_users(self.non_staff_user) + + # Now non_staff_user should be able to access library2_key only: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 1) + self.assertEqual(lib_list[0]["library_key"], unicode(library2_key)) + self.assertTrue(self._can_access_library(library2_key)) + self.assertFalse(self._can_access_library(self.library)) + + @ddt.data( + OrgStaffRole, + OrgInstructorRole, + OrgLibraryUserRole, + ) + def test_org_based_access(self, org_access_role): + """ + Test the various roles that allow viewing all of an organization's + libraries are working correctly. + """ + # Create some libraries as the staff user: + lib_key_pacific = self._create_library(org="PacificX", library="libP") + lib_key_atlantic = self._create_library(org="AtlanticX", library="libA") + + # Login as a non-staff: + self._login_as_non_staff_user() + + # Now manually intervene to give non_staff_user access to all "PacificX" libraries: + org_access_role(lib_key_pacific.org).add_users(self.non_staff_user) + + # Now non_staff_user should be able to access lib_key_pacific only: + lib_list = self._list_libraries() + self.assertEqual(len(lib_list), 1) + self.assertEqual(lib_list[0]["library_key"], unicode(lib_key_pacific)) + self.assertTrue(self._can_access_library(lib_key_pacific)) + self.assertFalse(self._can_access_library(lib_key_atlantic)) + self.assertFalse(self._can_access_library(self.lib_key)) + + @ddt.data(True, False) + def test_read_only_role(self, use_org_level_role): + """ + Test the read-only role (LibraryUserRole and its org-level equivalent) + """ + # As staff user, add a block to self.library: + block = self._add_simple_content_block() + + # Login as a non_staff_user: + self._login_as_non_staff_user() + self.assertFalse(self._can_access_library(self.library)) + + block_url = reverse_usage_url('xblock_handler', block.location) + + def can_read_block(): + """ Check if studio lets us view the XBlock in the library """ + response = self.client.get_json(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_edit_block(): + """ Check if studio lets us edit the XBlock in the library """ + response = self.client.ajax_post(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_delete_block(): + """ Check if studio lets us delete the XBlock in the library """ + response = self.client.delete(block_url) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_copy_block(): + """ Check if studio lets us duplicate the XBlock in the library """ + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(self.library.location), + 'duplicate_source_locator': unicode(block.location), + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + def can_create_block(): + """ Check if studio lets us make a new XBlock in the library """ + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(self.library.location), 'category': 'html', + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + return response.status_code == 200 + + # Check that we do not have read or write access to block: + self.assertFalse(can_read_block()) + self.assertFalse(can_edit_block()) + self.assertFalse(can_delete_block()) + self.assertFalse(can_copy_block()) + self.assertFalse(can_create_block()) + + # Give non_staff_user read-only permission: + if use_org_level_role: + OrgLibraryUserRole(self.lib_key.org).add_users(self.non_staff_user) + else: + LibraryUserRole(self.lib_key).add_users(self.non_staff_user) + + self.assertTrue(self._can_access_library(self.library)) + self.assertTrue(can_read_block()) + self.assertFalse(can_edit_block()) + self.assertFalse(can_delete_block()) + self.assertFalse(can_copy_block()) + self.assertFalse(can_create_block()) + + @ddt.data( + (LibraryUserRole, CourseStaffRole, True), + (CourseStaffRole, CourseStaffRole, True), + (None, CourseStaffRole, False), + (LibraryUserRole, None, False), + ) + @ddt.unpack + def test_duplicate_across_courses(self, library_role, course_role, expected_result): + """ + Test that the REST API will correctly allow/refuse when copying + from a library with (write, read, or no) access to a course with (write or no) access. + """ + # As staff user, add a block to self.library: + block = self._add_simple_content_block() + # And create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + self._login_as_non_staff_user() + + # Assign roles: + if library_role: + library_role(self.lib_key).add_users(self.non_staff_user) + if course_role: + course_role(course.location.course_key).add_users(self.non_staff_user) + + # Copy block to the course: + response = self.client.ajax_post(reverse_url('xblock_handler'), { + 'parent_locator': unicode(course.location), + 'duplicate_source_locator': unicode(block.location), + }) + self.assertIn(response.status_code, (200, 403)) # 400 would be ambiguous + duplicate_action_allowed = (response.status_code == 200) + self.assertEqual(duplicate_action_allowed, expected_result) + + @ddt.data( + (LibraryUserRole, CourseStaffRole, True), + (CourseStaffRole, CourseStaffRole, True), + (None, CourseStaffRole, False), + (LibraryUserRole, None, False), + ) + @ddt.unpack + def test_refresh_library_content_permissions(self, library_role, course_role, expected_result): + """ + Test that the LibraryContent block's 'refresh_children' handler will correctly + handle permissions and allow/refuse when updating its content with the latest + version of a library. We try updating from a library with (write, read, or no) + access to a course with (write or no) access. + """ + # As staff user, add a block to self.library: + self._add_simple_content_block() + # And create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + + self._login_as_non_staff_user() + + # Assign roles: + if library_role: + library_role(self.lib_key).add_users(self.non_staff_user) + if course_role: + course_role(course.location.course_key).add_users(self.non_staff_user) + + # Try updating our library content block: + lc_block = self._add_library_content_block(course, self.lib_key) + # We must use the CMS's module system in order to get permissions checks. + self._bind_module(lc_block, user=self.non_staff_user) + lc_block = self._refresh_children(lc_block, status_code_expected=200 if expected_result else 403) + self.assertEqual(len(lc_block.children), 1 if expected_result else 0) + + +class TestOverrides(LibraryTestCase): + """ + Test that overriding block Scope.settings fields from a library in a specific course works + """ + def setUp(self): + super(TestOverrides, self).setUp() + self.original_display_name = "A Problem Block" + self.original_weight = 1 + + # Create a problem block in the library: + self.problem = ItemFactory.create( + category="problem", + parent_location=self.library.location, + display_name=self.original_display_name, # display_name is a Scope.settings field + weight=self.original_weight, # weight is also a Scope.settings field + user_id=self.user.id, + publish_item=False, + ) + + # Also create a course: + with modulestore().default_store(ModuleStoreEnum.Type.split): + self.course = CourseFactory.create() + + # Add a LibraryContent block to the course: + self.lc_block = self._add_library_content_block(self.course, self.lib_key) + self.lc_block = self._refresh_children(self.lc_block) + self.problem_in_course = modulestore().get_item(self.lc_block.children[0]) + + def test_overrides(self): + """ + Test that we can override Scope.settings values in a course. + """ + new_display_name = "Modified Problem Title" + new_weight = 10 + self.problem_in_course.display_name = new_display_name + self.problem_in_course.weight = new_weight + modulestore().update_item(self.problem_in_course, self.user.id) + + # Add a second LibraryContent block to the course, with no override: + lc_block2 = self._add_library_content_block(self.course, self.lib_key) + lc_block2 = self._refresh_children(lc_block2) + # Re-load the two problem blocks - one with and one without an override: + self.problem_in_course = modulestore().get_item(self.lc_block.children[0]) + problem2_in_course = modulestore().get_item(lc_block2.children[0]) + + self.assertEqual(self.problem_in_course.display_name, new_display_name) + self.assertEqual(self.problem_in_course.weight, new_weight) + + self.assertEqual(problem2_in_course.display_name, self.original_display_name) + self.assertEqual(problem2_in_course.weight, self.original_weight) + + def test_reset_override(self): + """ + If we override a setting and then reset it, we should get the library value. + """ + new_display_name = "Modified Problem Title" + new_weight = 10 + self.problem_in_course.display_name = new_display_name + self.problem_in_course.weight = new_weight + modulestore().update_item(self.problem_in_course, self.user.id) + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + + self.assertEqual(self.problem_in_course.display_name, new_display_name) + self.assertEqual(self.problem_in_course.weight, new_weight) + + # Reset: + for field_name in ["display_name", "weight"]: + self.problem_in_course.fields[field_name].delete_from(self.problem_in_course) + + # Save, reload, and verify: + modulestore().update_item(self.problem_in_course, self.user.id) + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + + self.assertEqual(self.problem_in_course.display_name, self.original_display_name) + self.assertEqual(self.problem_in_course.weight, self.original_weight) + + def test_consistent_definitions(self): + """ + Make sure that the new child of the LibraryContent block + shares its definition with the original (self.problem). + + This test is specific to split mongo. + """ + definition_id = self.problem.definition_locator.definition_id + self.assertEqual(self.problem_in_course.definition_locator.definition_id, definition_id) + + # Now even if we change some Scope.settings fields and refresh, the definition should be unchanged + self.problem.weight = 20 + self.problem.display_name = "NEW" + modulestore().update_item(self.problem, self.user.id) + self.lc_block = self._refresh_children(self.lc_block) + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + + self.assertEqual(self.problem.definition_locator.definition_id, definition_id) + self.assertEqual(self.problem_in_course.definition_locator.definition_id, definition_id) + + def test_persistent_overrides(self): + """ + Test that when we override Scope.settings values in a course, + the override values persist even when the block is refreshed + with updated blocks from the library. + """ + new_display_name = "Modified Problem Title" + new_weight = 15 + self.problem_in_course.display_name = new_display_name + self.problem_in_course.weight = new_weight + + modulestore().update_item(self.problem_in_course, self.user.id) + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + self.assertEqual(self.problem_in_course.display_name, new_display_name) + self.assertEqual(self.problem_in_course.weight, new_weight) + + # Change the settings in the library version: + self.problem.display_name = "X" + self.problem.weight = 99 + new_data_value = "

Changed data to check that non-overriden fields *do* get updated.

" + self.problem.data = new_data_value + modulestore().update_item(self.problem, self.user.id) + + self.lc_block = self._refresh_children(self.lc_block) + self.problem_in_course = modulestore().get_item(self.problem_in_course.location) + + self.assertEqual(self.problem_in_course.display_name, new_display_name) + self.assertEqual(self.problem_in_course.weight, new_weight) + self.assertEqual(self.problem_in_course.data, new_data_value) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 9975764345..76429befa6 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -234,13 +234,13 @@ class AuthTestCase(ContentStoreTestCase): def test_private_pages_auth(self): """Make sure pages that do require login work.""" auth_pages = ( - '/course/', + '/home/', ) # These are pages that should just load when the user is logged in # (no data needed) simple_auth_pages = ( - '/course/', + '/home/', ) # need an activated user @@ -266,7 +266,7 @@ class AuthTestCase(ContentStoreTestCase): def test_index_auth(self): # not logged in. Should return a redirect. - resp = self.client.get_html('/course/') + resp = self.client.get_html('/home/') self.assertEqual(resp.status_code, 302) # Logged in should work. @@ -283,7 +283,7 @@ class AuthTestCase(ContentStoreTestCase): self.login(self.email, self.pw) # make sure we can access courseware immediately - course_url = '/course/' + course_url = '/home/' resp = self.client.get_html(course_url) self.assertEquals(resp.status_code, 200) @@ -293,7 +293,7 @@ class AuthTestCase(ContentStoreTestCase): resp = self.client.get_html(course_url) # re-request, and we should get a redirect to login page - self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/course/') + self.assertRedirects(resp, settings.LOGIN_REDIRECT_URL + '?next=/home/') class ForumTestCase(CourseTestCase): diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 914ad1ec65..e6c6d458cc 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -296,6 +296,13 @@ def reverse_course_url(handler_name, course_key, kwargs=None): return reverse_url(handler_name, 'course_key_string', course_key, kwargs) +def reverse_library_url(handler_name, library_key, kwargs=None): + """ + Creates the URL for handlers that use library_keys as URL parameters. + """ + return reverse_url(handler_name, 'library_key_string', library_key, kwargs) + + def reverse_usage_url(handler_name, usage_key, kwargs=None): """ Creates the URL for handlers that use usage_keys as URL parameters. diff --git a/cms/djangoapps/contentstore/views/__init__.py b/cms/djangoapps/contentstore/views/__init__.py index 26bb619fb3..9cceccd6e6 100644 --- a/cms/djangoapps/contentstore/views/__init__.py +++ b/cms/djangoapps/contentstore/views/__init__.py @@ -12,6 +12,7 @@ from .error import * from .helpers import * from .item import * from .import_export import * +from .library import * from .preview import * from .public import * from .export_git import * diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 70a470f9dc..90f1dde267 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -56,6 +56,15 @@ ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules' ADVANCED_PROBLEM_TYPES = settings.ADVANCED_PROBLEM_TYPES +CONTAINER_TEMPATES = [ + "basic-modal", "modal-button", "edit-xblock-modal", + "editor-mode-button", "upload-dialog", "image-modal", + "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", + "add-xblock-component-menu-problem", "xblock-string-field-editor", "publish-xblock", "publish-history", + "unit-outline", "container-message" +] + + def _advanced_component_types(): """ Return advanced component types which can be created. @@ -202,14 +211,15 @@ def container_handler(request, usage_key_string): 'xblock_info': xblock_info, 'draft_preview_link': preview_lms_link, 'published_preview_link': lms_link, + 'templates': CONTAINER_TEMPATES }) else: return HttpResponseBadRequest("Only supports HTML requests") -def get_component_templates(course): +def get_component_templates(courselike, library=False): """ - Returns the applicable component templates that can be used by the specified course. + Returns the applicable component templates that can be used by the specified course or library. """ def create_template_dict(name, cat, boilerplate_name=None, is_common=False): """ @@ -240,7 +250,13 @@ def get_component_templates(course): categories = set() # The component_templates array is in the order of "advanced" (if present), followed # by the components in the order listed in COMPONENT_TYPES. - for category in COMPONENT_TYPES: + component_types = COMPONENT_TYPES[:] + + # Libraries do not support discussions + if library: + component_types = [component for component in component_types if component != 'discussion'] + + for category in component_types: templates_for_category = [] component_class = _load_mixed_class(category) # add the default template with localized display name @@ -254,7 +270,7 @@ def get_component_templates(course): if hasattr(component_class, 'templates'): for template in component_class.templates(): filter_templates = getattr(component_class, 'filter_templates', None) - if not filter_templates or filter_templates(template, course): + if not filter_templates or filter_templates(template, courselike): templates_for_category.append( create_template_dict( _(template['metadata'].get('display_name')), @@ -279,11 +295,15 @@ def get_component_templates(course): "display_name": component_display_names[category] }) + # Libraries do not support advanced components at this time. + if library: + return component_templates + # Check if there are any advanced modules specified in the course policy. # These modules should be specified as a list of strings, where the strings # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be # enabled for the course. - course_advanced_keys = course.advanced_modules + course_advanced_keys = courselike.advanced_modules advanced_component_templates = {"type": "advanced", "templates": [], "display_name": _("Advanced")} advanced_component_types = _advanced_component_types() # Set component types according to course policy file diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index cbc40b6f31..f40e86f082 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1,6 +1,7 @@ """ Views related to operations on course objects """ +from django.shortcuts import redirect import json import random import string # pylint: disable=deprecated-module @@ -38,6 +39,7 @@ from contentstore.utils import ( add_extra_panel_tab, remove_extra_panel_tab, reverse_course_url, + reverse_library_url, reverse_usage_url, reverse_url, remove_all_instructors, @@ -47,7 +49,7 @@ from models.settings.course_grading import CourseGradingModel from models.settings.course_metadata import CourseMetadata from util.json_request import expect_json from util.string_utils import _has_non_ascii_characters -from student.auth import has_course_author_access +from student.auth import has_studio_write_access, has_studio_read_access from .component import ( OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES, @@ -56,6 +58,7 @@ from .component import ( ADVANCED_COMPONENT_TYPES, ) from contentstore.tasks import rerun_course +from .library import LIBRARIES_ENABLED from .item import create_xblock_info from course_creators.views import get_course_creator_status, add_user_with_status_unrequested from contentstore import utils @@ -69,7 +72,8 @@ from microsite_configuration import microsite from xmodule.course_module import CourseFields -__all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', +__all__ = ['course_info_handler', 'course_handler', 'course_listing', + 'course_info_update_handler', 'course_rerun_handler', 'settings_handler', 'grading_handler', @@ -94,7 +98,7 @@ def get_course_and_check_access(course_key, user, depth=0): Internal method used to calculate and return the locator and course module for the view functions in this file. """ - if not has_course_author_access(user, course_key): + if not has_studio_read_access(user, course_key): raise PermissionDenied() course_module = modulestore().get_course(course_key, depth=depth) return course_module @@ -128,7 +132,7 @@ def course_notifications_handler(request, course_key_string=None, action_state_i course_key = CourseKey.from_string(course_key_string) if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): - if not has_course_author_access(request.user, course_key): + if not has_studio_write_access(request.user, course_key): raise PermissionDenied() if request.method == 'GET': return _course_notifications_json_get(action_state_id) @@ -218,7 +222,7 @@ def course_handler(request, course_key_string=None): return JsonResponse(_course_outline_json(request, course_module)) elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access return _create_or_rerun_course(request) - elif not has_course_author_access(request.user, CourseKey.from_string(course_key_string)): + elif not has_studio_write_access(request.user, CourseKey.from_string(course_key_string)): raise PermissionDenied() elif request.method == 'PUT': raise NotImplementedError() @@ -228,7 +232,7 @@ def course_handler(request, course_key_string=None): return HttpResponseBadRequest() elif request.method == 'GET': # assume html if course_key_string is None: - return course_listing(request) + return redirect(reverse("home")) else: return course_index(request, CourseKey.from_string(course_key_string)) else: @@ -290,7 +294,7 @@ def _accessible_courses_list(request): if course.location.course == 'templates': return False - return has_course_author_access(request.user, course.id) + return has_studio_read_access(request.user, course.id) courses = filter(course_filter, modulestore().get_courses()) in_process_course_actions = [ @@ -298,7 +302,7 @@ def _accessible_courses_list(request): CourseRerunState.objects.find_all( exclude_args={'state': CourseRerunUIStateManager.State.SUCCEEDED}, should_display=True ) - if has_course_author_access(request.user, course.course_key) + if has_studio_read_access(request.user, course.course_key) ] return courses, in_process_course_actions @@ -341,6 +345,14 @@ def _accessible_courses_list_from_groups(request): return courses_list.values(), in_process_course_actions +def _accessible_libraries_list(user): + """ + List all libraries available to the logged in user by iterating through all libraries + """ + # No need to worry about ErrorDescriptors - split's get_libraries() never returns them. + return [lib for lib in modulestore().get_libraries() if has_studio_read_access(user, lib.location.library_key)] + + @login_required @ensure_csrf_cookie def course_listing(request): @@ -360,6 +372,8 @@ def course_listing(request): # so fallback to iterating through all courses courses, in_process_course_actions = _accessible_courses_list(request) + libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else [] + def format_course_for_view(course): """ Return a dict of the data which the view requires for each course @@ -396,6 +410,19 @@ def course_listing(request): ) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } + def format_library_for_view(library): + """ + Return a dict of the data which the view requires for each library + """ + return { + 'display_name': library.display_name, + 'library_key': unicode(library.location.library_key), + 'url': reverse_library_url('library_handler', unicode(library.location.library_key)), + 'org': library.display_org_with_default, + 'number': library.display_number_with_default, + 'can_edit': has_studio_write_access(request.user, library.location.library_key), + } + # remove any courses in courses that are also in the in_process_course_actions list in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions] courses = [ @@ -409,6 +436,8 @@ def course_listing(request): return render_to_response('index.html', { 'courses': courses, 'in_process_course_actions': in_process_course_actions, + 'libraries_enabled': LIBRARIES_ENABLED, + 'libraries': [format_library_for_view(lib) for lib in libraries], 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), @@ -621,7 +650,7 @@ def _rerun_course(request, org, number, run, fields): source_course_key = CourseKey.from_string(request.json.get('source_course_key')) # verify user has access to the original course - if not has_course_author_access(request.user, source_course_key): + if not has_studio_write_access(request.user, source_course_key): raise PermissionDenied() # create destination course key @@ -702,7 +731,7 @@ def course_info_update_handler(request, course_key_string, provided_id=None): provided_id = None # check that logged in user has permissions to this item (GET shouldn't require this level?) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_write_access(request.user, usage_key.course_key): raise PermissionDenied() if request.method == 'GET': diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 34ef869f17..3769c81978 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -13,7 +13,7 @@ from django.utils.translation import ugettext as _ from edxmako.shortcuts import render_to_string, render_to_response from xblock.core import XBlock from xmodule.modulestore.django import modulestore -from contentstore.utils import reverse_course_url, reverse_usage_url +from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url __all__ = ['edge', 'event', 'landing'] @@ -106,6 +106,9 @@ def xblock_studio_url(xblock, parent_xblock=None): url=reverse_course_url('course_handler', xblock.location.course_key), usage_key=urllib.quote(unicode(xblock.location)) ) + elif category == 'library': + library_key = xblock.location.course_key + return reverse_library_url('library_handler', library_key) else: return reverse_usage_url('container_handler', xblock.location) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 2e6528ed0d..e390811096 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -37,7 +37,7 @@ from util.date_utils import get_default_time_display from util.json_request import expect_json, JsonResponse -from student.auth import has_course_author_access +from student.auth import has_studio_write_access, has_studio_read_access from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ ancestor_has_staff_lock from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ @@ -47,6 +47,7 @@ from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url from opaque_keys.edx.keys import UsageKey, CourseKey +from opaque_keys.edx.locator import LibraryUsageLocator __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -129,7 +130,8 @@ def xblock_handler(request, usage_key_string): if usage_key_string: usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + access_check = has_studio_read_access if request.method == 'GET' else has_studio_write_access + if not access_check(request.user, usage_key.course_key): raise PermissionDenied() if request.method == 'GET': @@ -165,6 +167,14 @@ def xblock_handler(request, usage_key_string): parent_usage_key = usage_key_with_run(request.json['parent_locator']) duplicate_source_usage_key = usage_key_with_run(request.json['duplicate_source_locator']) + source_course = duplicate_source_usage_key.course_key + dest_course = parent_usage_key.course_key + if ( + not has_studio_write_access(request.user, dest_course) or + not has_studio_read_access(request.user, source_course) + ): + raise PermissionDenied() + dest_usage_key = _duplicate_item( parent_usage_key, duplicate_source_usage_key, @@ -196,7 +206,7 @@ def xblock_view_handler(request, usage_key_string, view_name): the second is the resource description """ usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_read_access(request.user, usage_key.course_key): raise PermissionDenied() accept_header = request.META.get('HTTP_ACCEPT', 'application/json') @@ -204,7 +214,7 @@ def xblock_view_handler(request, usage_key_string, view_name): if 'application/json' in accept_header: store = modulestore() xblock = store.get_item(usage_key) - container_views = ['container_preview', 'reorderable_container_child_preview'] + container_views = ['container_preview', 'reorderable_container_child_preview', 'container_child_preview'] # wrap the generated fragment in the xmodule_editor div so that the javascript # can bind to it correctly @@ -227,6 +237,7 @@ def xblock_view_handler(request, usage_key_string, view_name): elif view_name in (PREVIEW_VIEWS + container_views): is_pages_view = view_name == STUDENT_VIEW # Only the "Pages" view uses student view in Studio + can_edit = has_studio_write_access(request.user, usage_key.course_key) # Determine the items to be shown as reorderable. Note that the view # 'reorderable_container_child_preview' is only rendered for xblocks that @@ -236,12 +247,34 @@ def xblock_view_handler(request, usage_key_string, view_name): if view_name == 'reorderable_container_child_preview': reorderable_items.add(xblock.location) + paging = None + try: + if request.REQUEST.get('enable_paging', 'false') == 'true': + paging = { + 'page_number': int(request.REQUEST.get('page_number', 0)), + 'page_size': int(request.REQUEST.get('page_size', 0)), + } + except ValueError: + # pylint: disable=too-many-format-args + return HttpResponse( + content="Couldn't parse paging parameters: enable_paging: " + "%s, page_number: %s, page_size: %s".format( + request.REQUEST.get('enable_paging', 'false'), + request.REQUEST.get('page_number', 0), + request.REQUEST.get('page_size', 0) + ), + status=400, + content_type="text/plain", + ) + # Set up the context to be passed to each XBlock's render method. context = { 'is_pages_view': is_pages_view, # This setting disables the recursive wrapping of xblocks 'is_unit_page': is_unit(xblock), + 'can_edit': can_edit, 'root_xblock': xblock if (view_name == 'container_preview') else None, - 'reorderable_items': reorderable_items + 'reorderable_items': reorderable_items, + 'paging': paging, } fragment = get_preview_fragment(request, xblock, context) @@ -283,7 +316,7 @@ def xblock_outline_handler(request, usage_key_string): a course. """ usage_key = usage_key_with_run(usage_key_string) - if not has_course_author_access(request.user, usage_key.course_key): + if not has_studio_read_access(request.user, usage_key.course_key): raise PermissionDenied() response_format = request.REQUEST.get('format', 'html') @@ -405,8 +438,11 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, else: try: value = field.from_json(value) - except ValueError: - return JsonResponse({"error": "Invalid data"}, 400) + except ValueError as verr: + reason = _("Invalid data") + if verr.message: + reason = _("Invalid data ({details})").format(details=verr.message) + return JsonResponse({"error": reason}, 400) field.write_to(xblock, value) # update the xblock and call any xblock callbacks @@ -452,12 +488,18 @@ def _save_xblock(user, xblock, data=None, children_strings=None, metadata=None, def _create_item(request): """View for create items.""" usage_key = usage_key_with_run(request.json['parent_locator']) - category = request.json['category'] + if not has_studio_write_access(request.user, usage_key.course_key): + raise PermissionDenied() + category = request.json['category'] display_name = request.json.get('display_name') - if not has_course_author_access(request.user, usage_key.course_key): - raise PermissionDenied() + if isinstance(usage_key, LibraryUsageLocator): + # Only these categories are supported at this time. + if category not in ['html', 'problem', 'video']: + return HttpResponseBadRequest( + "Category '%s' not supported for Libraries" % category, content_type='text/plain' + ) store = modulestore() with store.bulk_operations(usage_key.course_key): @@ -508,7 +550,9 @@ def _create_item(request): ) store.update_item(course, request.user.id) - return JsonResponse({"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)}) + return JsonResponse( + {"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)} + ) def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None): @@ -523,7 +567,12 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ category = dest_usage_key.block_type # Update the display name to indicate this is a duplicate (unless display name provided). - duplicate_metadata = own_metadata(source_item) + # Can't use own_metadata(), b/c it converts data for JSON serialization - + # not suitable for setting metadata of the new block + duplicate_metadata = {} + for field in source_item.fields.values(): + if field.scope == Scope.settings and field.is_set_on(source_item): + duplicate_metadata[field.name] = field.read_from(source_item) if display_name is not None: duplicate_metadata['display_name'] = display_name else: @@ -548,7 +597,8 @@ def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_ dest_module.children = [] for child in source_item.children: dupe = _duplicate_item(dest_module.location, child, user=user) - dest_module.children.append(dupe) + if dupe not in dest_module.children: # _duplicate_item may add the child for us. + dest_module.children.append(dupe) store.update_item(dest_module, user.id) if 'detached' not in source_item.runtime.load_block_type(category)._class_tags: @@ -598,7 +648,7 @@ def orphan_handler(request, course_key_string): """ course_usage_key = CourseKey.from_string(course_key_string) if request.method == 'GET': - if has_course_author_access(request.user, course_usage_key): + if has_studio_read_access(request.user, course_usage_key): return JsonResponse([unicode(item) for item in modulestore().get_orphans(course_usage_key)]) else: raise PermissionDenied() @@ -660,7 +710,9 @@ def _get_module_info(xblock, rewrite_static_links=True): ) # Pre-cache has changes for the entire course because we'll need it for the ancestor info - modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) + # Except library blocks which don't [yet] use draft/publish + if not isinstance(xblock.location, LibraryUsageLocator): + modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) # Note that children aren't being returned until we have a use case. return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True) @@ -701,12 +753,18 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F return None + is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) - # this should not be calculated for Sections and Subsections on Unit page - has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) else None + # this should not be calculated for Sections and Subsections on Unit page or for library blocks + has_changes = None + if (is_xblock_unit or course_outline) and not is_library_block: + has_changes = modulestore().has_changes(xblock) if graders is None: - graders = CourseGradingModel.fetch(xblock.location.course_key).graders + if not is_library_block: + graders = CourseGradingModel.fetch(xblock.location.course_key).graders + else: + graders = [] # Compute the child info first so it can be included in aggregate information for the parent should_visit_children = include_child_info and (course_outline and not is_xblock_unit or not course_outline) @@ -726,7 +784,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes) else: visibility_state = None - published = modulestore().has_published_version(xblock) + published = modulestore().has_published_version(xblock) if not is_library_block else None xblock_info = { "id": unicode(xblock.location), @@ -734,7 +792,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F "category": xblock.category, "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, "published": published, - "published_on": get_default_time_display(xblock.published_on) if xblock.published_on else None, + "published_on": get_default_time_display(xblock.published_on) if published and xblock.published_on else None, "studio_url": xblock_studio_url(xblock, parent_xblock), "released_to_students": datetime.now(UTC) > xblock.start, "release_date": release_date, diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py new file mode 100644 index 0000000000..2bd433c83f --- /dev/null +++ b/cms/djangoapps/contentstore/views/library.py @@ -0,0 +1,235 @@ +""" +Views related to content libraries. +A content library is a structure containing XBlocks which can be re-used in the +multiple courses. +""" +from __future__ import absolute_import + +import json +import logging + +from contentstore.views.item import create_xblock_info +from contentstore.utils import reverse_library_url, add_instructor +from django.http import HttpResponseNotAllowed, Http404 +from django.contrib.auth.decorators import login_required +from django.core.exceptions import PermissionDenied +from django.conf import settings +from django.utils.translation import ugettext as _ +from django.views.decorators.http import require_http_methods +from django_future.csrf import ensure_csrf_cookie +from edxmako.shortcuts import render_to_response +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator +from xmodule.modulestore.exceptions import DuplicateCourseError +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore + +from .component import get_component_templates, CONTAINER_TEMPATES +from student.auth import ( + STUDIO_VIEW_USERS, STUDIO_EDIT_ROLES, get_user_permissions, has_studio_read_access, has_studio_write_access +) +from student.roles import CourseCreatorRole, CourseInstructorRole, CourseStaffRole, LibraryUserRole +from student import auth +from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest + +__all__ = ['library_handler', 'manage_library_users'] + +log = logging.getLogger(__name__) + +LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) + + +@login_required +@ensure_csrf_cookie +@require_http_methods(('GET', 'POST')) +def library_handler(request, library_key_string=None): + """ + RESTful interface to most content library related functionality. + """ + if not LIBRARIES_ENABLED: + log.exception("Attempted to use the content library API when the libraries feature is disabled.") + raise Http404 # Should never happen because we test the feature in urls.py also + + if library_key_string is not None and request.method == 'POST': + return HttpResponseNotAllowed(("POST",)) + + if request.method == 'POST': + return _create_library(request) + + # request method is get, since only GET and POST are allowed by @require_http_methods(('GET', 'POST')) + if library_key_string: + return _display_library(library_key_string, request) + + return _list_libraries(request) + + +def _display_library(library_key_string, request): + """ + Displays single library + """ + library_key = CourseKey.from_string(library_key_string) + if not isinstance(library_key, LibraryLocator): + log.exception("Non-library key passed to content libraries API.") # Should never happen due to url regex + raise Http404 # This is not a library + if not has_studio_read_access(request.user, library_key): + log.exception( + u"User %s tried to access library %s without permission", + request.user.username, unicode(library_key) + ) + raise PermissionDenied() + + library = modulestore().get_library(library_key) + if library is None: + log.exception(u"Library %s not found", unicode(library_key)) + raise Http404 + + response_format = 'html' + if ( + request.REQUEST.get('format', 'html') == 'json' or + 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html') + ): + response_format = 'json' + + return library_blocks_view(library, request.user, response_format) + + +def _list_libraries(request): + """ + List all accessible libraries + """ + lib_info = [ + { + "display_name": lib.display_name, + "library_key": unicode(lib.location.library_key), + } + for lib in modulestore().get_libraries() + if has_studio_read_access(request.user, lib.location.library_key) + ] + return JsonResponse(lib_info) + + +@expect_json +def _create_library(request): + """ + Helper method for creating a new library. + """ + if not auth.has_access(request.user, CourseCreatorRole()): + log.exception(u"User %s tried to create a library without permission", request.user.username) + raise PermissionDenied() + display_name = None + try: + display_name = request.json['display_name'] + org = request.json['org'] + library = request.json.get('number', None) + if library is None: + library = request.json['library'] + store = modulestore() + with store.default_store(ModuleStoreEnum.Type.split): + new_lib = store.create_library( + org=org, + library=library, + user_id=request.user.id, + fields={"display_name": display_name}, + ) + # Give the user admin ("Instructor") role for this library: + add_instructor(new_lib.location.library_key, request.user, request.user) + except KeyError as error: + log.exception("Unable to create library - missing required JSON key.") + return JsonResponseBadRequest({ + "ErrMsg": _("Unable to create library - missing required field '{field}'".format(field=error.message)) + }) + except InvalidKeyError as error: + log.exception("Unable to create library - invalid key.") + return JsonResponseBadRequest({ + "ErrMsg": _("Unable to create library '{name}'.\n\n{err}").format(name=display_name, err=error.message) + }) + except DuplicateCourseError: + log.exception("Unable to create library - one already exists with the same key.") + return JsonResponseBadRequest({ + 'ErrMsg': _( + 'There is already a library defined with the same ' + 'organization and library code. Please ' + 'change your library code so that it is unique within your organization.' + ) + }) + + lib_key_str = unicode(new_lib.location.library_key) + return JsonResponse({ + 'url': reverse_library_url('library_handler', lib_key_str), + 'library_key': lib_key_str, + }) + + +def library_blocks_view(library, user, response_format): + """ + The main view of a course's content library. + Shows all the XBlocks in the library, and allows adding/editing/deleting + them. + Can be called with response_format="json" to get a JSON-formatted list of + the XBlocks in the library along with library metadata. + + Assumes that read permissions have been checked before calling this. + """ + assert isinstance(library.location.library_key, LibraryLocator) + assert isinstance(library.location, LibraryUsageLocator) + + children = library.children + if response_format == "json": + # The JSON response for this request is short and sweet: + prev_version = library.runtime.course_entry.structure['previous_version'] + return JsonResponse({ + "display_name": library.display_name, + "library_id": unicode(library.location.library_key), + "version": unicode(library.runtime.course_entry.course_key.version), + "previous_version": unicode(prev_version) if prev_version else None, + "blocks": [unicode(x) for x in children], + }) + + can_edit = has_studio_write_access(user, library.location.library_key) + + xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) + component_templates = get_component_templates(library, library=True) if can_edit else [] + + return render_to_response('library.html', { + 'can_edit': can_edit, + 'context_library': library, + 'component_templates': json.dumps(component_templates), + 'xblock_info': xblock_info, + 'templates': CONTAINER_TEMPATES, + 'lib_users_url': reverse_library_url('manage_library_users', unicode(library.location.library_key)), + }) + + +def manage_library_users(request, library_key_string): + """ + Studio UI for editing the users within a library. + + Uses the /course_team/:library_key/:user_email/ REST API to make changes. + """ + library_key = CourseKey.from_string(library_key_string) + if not isinstance(library_key, LibraryLocator): + raise Http404 # This is not a library + user_perms = get_user_permissions(request.user, library_key) + if not user_perms & STUDIO_VIEW_USERS: + raise PermissionDenied() + library = modulestore().get_library(library_key) + if library is None: + raise Http404 + + # Segment all the users explicitly associated with this library, ensuring each user only has one role listed: + instructors = set(CourseInstructorRole(library_key).users_with_role()) + staff = set(CourseStaffRole(library_key).users_with_role()) - instructors + users = set(LibraryUserRole(library_key).users_with_role()) - instructors - staff + all_users = instructors | staff | users + + return render_to_response('manage_users_lib.html', { + 'context_library': library, + 'staff': staff, + 'instructors': instructors, + 'users': users, + 'all_users': all_users, + 'allow_actions': bool(user_perms & STUDIO_EDIT_ROLES), + 'library_key': unicode(library_key), + 'lib_users_url': reverse_library_url('manage_library_users', library_key_string), + }) diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index 71d6056f42..63e1a6b792 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -14,6 +14,7 @@ from xmodule.x_module import PREVIEW_VIEWS, STUDENT_VIEW, AUTHOR_VIEW from xmodule.contentstore.django import contentstore from xmodule.error_module import ErrorDescriptor from xmodule.exceptions import NotFoundError, ProcessingError +from xmodule.library_tools import LibraryToolsService from xmodule.modulestore.django import modulestore, ModuleI18nService from opaque_keys.edx.keys import UsageKey from xmodule.x_module import ModuleSystem @@ -21,6 +22,7 @@ from xblock.runtime import KvsFieldData from xblock.django.request import webob_to_django_response, django_to_webob_request from xblock.exceptions import NoSuchHandlerError from xblock.fragment import Fragment +from student.auth import has_studio_read_access, has_studio_write_access from lms.djangoapps.lms_xblock.field_data import LmsFieldData from cms.lib.xblock.field_data import CmsFieldData @@ -123,6 +125,28 @@ class StudioUserService(object): return self._request.user.id +class StudioPermissionsService(object): + """ + Service that can provide information about a user's permissions. + + Deprecated. To be replaced by a more general authorization service. + + Only used by LibraryContentDescriptor (and library_tools.py). + """ + + def __init__(self, request): + super(StudioPermissionsService, self).__init__() + self._request = request + + def can_read(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_read_access(self._request.user, course_key) + + def can_write(self, course_key): + """ Does the user have read access to the given course/library? """ + return has_studio_write_access(self._request.user, course_key) + + def _preview_module_system(request, descriptor, field_data): """ Returns a ModuleSystem for the specified descriptor that is specialized for @@ -152,6 +176,7 @@ def _preview_module_system(request, descriptor, field_data): ] descriptor.runtime._services['user'] = StudioUserService(request) # pylint: disable=protected-access + descriptor.runtime._services['studio_user_permissions'] = StudioPermissionsService(request) # pylint: disable=protected-access return PreviewModuleSystem( static_url=settings.STATIC_URL, @@ -177,6 +202,7 @@ def _preview_module_system(request, descriptor, field_data): services={ "i18n": ModuleI18nService(), "field-data": field_data, + "library_tools": LibraryToolsService(modulestore()), }, ) @@ -224,6 +250,7 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'content': frag.content, 'is_root': is_root, 'is_reorderable': is_reorderable, + 'can_edit': context.get('can_edit', True), } html = render_to_string('studio_xblock_wrapper.html', template_context) frag = wrap_fragment(frag, html) diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 597bb5e187..7bc6545868 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -66,6 +66,6 @@ def login_page(request): def howitworks(request): "Proxy view" if request.user.is_authenticated(): - return redirect('/course/') + return redirect('/home/') else: return render_to_response('howitworks.html', {}) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index b0ea997b4d..32b5ccbf2c 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -6,7 +6,7 @@ import lxml import datetime from contentstore.tests.utils import CourseTestCase -from contentstore.utils import reverse_course_url, add_instructor +from contentstore.utils import reverse_course_url, reverse_library_url, add_instructor from student.auth import has_course_author_access from contentstore.views.course import course_outline_initial_state from contentstore.views.item import create_xblock_info, VisibilityState @@ -14,7 +14,7 @@ from course_action_state.models import CourseRerunState from util.date_utils import get_default_time_display from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory from opaque_keys.edx.locator import CourseLocator from student.tests.factories import UserFactory from course_action_state.managers import CourseRerunUIStateManager @@ -42,7 +42,7 @@ class TestCourseIndex(CourseTestCase): """ Test getting the list of courses and then pulling up their outlines """ - index_url = '/course/' + index_url = '/home/' 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') @@ -61,6 +61,27 @@ 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_libraries_on_course_index(self): + """ + Test getting the list of libraries from the course listing page + """ + # Add a library: + lib1 = LibraryFactory.create() + + index_url = '/home/' + index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html') + parsed_html = lxml.html.fromstring(index_response.content) + library_link_elements = parsed_html.find_class('library-link') + self.assertEqual(len(library_link_elements), 1) + link = library_link_elements[0] + self.assertEqual( + link.get("href"), + reverse_library_url('library_handler', lib1.location.library_key), + ) + # now test that url + outline_response = self.client.get(link.get("href"), {}, HTTP_ACCEPT='text/html') + self.assertEqual(outline_response.status_code, 200) + def test_is_staff_access(self): """ Test that people with is_staff see the courses and can navigate into them diff --git a/cms/djangoapps/contentstore/views/tests/test_helpers.py b/cms/djangoapps/contentstore/views/tests/test_helpers.py index 034a9002fb..576ea38808 100644 --- a/cms/djangoapps/contentstore/views/tests/test_helpers.py +++ b/cms/djangoapps/contentstore/views/tests/test_helpers.py @@ -4,7 +4,7 @@ Unit tests for helpers.py. from contentstore.tests.utils import CourseTestCase from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name -from xmodule.modulestore.tests.factories import ItemFactory +from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory from django.utils import http @@ -50,6 +50,11 @@ class HelpersTestCase(CourseTestCase): display_name="My Video") self.assertIsNone(xblock_studio_url(video)) + # Verify library URL + library = LibraryFactory.create() + expected_url = u'/library/{}'.format(unicode(library.location.library_key)) + self.assertEqual(xblock_studio_url(library), expected_url) + def test_xblock_type_display_name(self): # Verify chapter type display name diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index b496a7ffc4..c322233319 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -3,7 +3,7 @@ import json from datetime import datetime, timedelta import ddt -from mock import patch +from mock import patch, Mock, PropertyMock from pytz import UTC from webob import Response @@ -18,13 +18,15 @@ from contentstore.views.component import ( component_handler, get_component_templates ) + from contentstore.views.item import create_xblock_info, ALWAYS, VisibilityState, _xblock_type_and_display_name from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.factories import ItemFactory, check_mongo_calls +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import ItemFactory, LibraryFactory, check_mongo_calls from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW from xblock.exceptions import NoSuchHandlerError from opaque_keys.edx.keys import UsageKey, CourseKey @@ -85,12 +87,18 @@ class ItemTest(CourseTestCase): class GetItemTest(ItemTest): """Tests for '/xblock' GET url.""" - def _get_container_preview(self, usage_key): + def _get_preview(self, usage_key, data=None): + """ Makes a request to xblock preview handler """ + preview_url = reverse_usage_url("xblock_view_handler", usage_key, {'view_name': 'container_preview'}) + data = data if data else {} + resp = self.client.get(preview_url, data, HTTP_ACCEPT='application/json') + return resp + + def _get_container_preview(self, usage_key, data=None): """ Returns the HTML and resources required for the xblock at the specified UsageKey """ - preview_url = reverse_usage_url("xblock_view_handler", usage_key, {'view_name': 'container_preview'}) - resp = self.client.get(preview_url, HTTP_ACCEPT='application/json') + resp = self._get_preview(usage_key, data) self.assertEqual(resp.status_code, 200) resp_content = json.loads(resp.content) html = resp_content['html'] @@ -99,6 +107,14 @@ class GetItemTest(ItemTest): self.assertIsNotNone(resources) return html, resources + def _get_container_preview_with_error(self, usage_key, expected_code, data=None, content_contains=None): + """ Make request and asserts on response code and response contents """ + resp = self._get_preview(usage_key, data) + self.assertEqual(resp.status_code, expected_code) + if content_contains: + self.assertIn(content_contains, resp.content) + return resp + @ddt.data( (1, 21, 23, 35, 37), (2, 22, 24, 38, 39), @@ -246,6 +262,40 @@ class GetItemTest(ItemTest): self.assertIn('New_NAME_A', html) self.assertIn('New_NAME_B', html) + def test_valid_paging(self): + """ + Tests that valid paging is passed along to underlying block + """ + with patch('contentstore.views.item.get_preview_fragment') as patched_get_preview_fragment: + retval = Mock() + type(retval).content = PropertyMock(return_value="Some content") + type(retval).resources = PropertyMock(return_value=[]) + patched_get_preview_fragment.return_value = retval + + root_usage_key = self._create_vertical() + _, _ = self._get_container_preview( + root_usage_key, + {'enable_paging': 'true', 'page_number': 0, 'page_size': 2} + ) + call_args = patched_get_preview_fragment.call_args[0] + _, _, context = call_args + self.assertIn('paging', context) + self.assertEqual({'page_number': 0, 'page_size': 2}, context['paging']) + + @ddt.data([1, 'invalid'], ['invalid', 2]) + @ddt.unpack + def test_invalid_paging(self, page_number, page_size): + """ + Tests that valid paging is passed along to underlying block + """ + root_usage_key = self._create_vertical() + self._get_container_preview_with_error( + root_usage_key, + 400, + data={'enable_paging': 'true', 'page_number': page_number, 'page_size': page_size}, + content_contains="Couldn't parse paging parameters" + ) + class DeleteItem(ItemTest): """Tests for '/xblock' DELETE url.""" @@ -893,6 +943,29 @@ class TestEditItem(ItemTest): self._verify_published_with_draft(unit_usage_key) self._verify_published_with_draft(html_usage_key) + def test_field_value_errors(self): + """ + Test that if the user's input causes a ValueError on an XBlock field, + we provide a friendly error message back to the user. + """ + response = self.create_xblock(parent_usage_key=self.seq_usage_key, category='video') + video_usage_key = self.response_usage_key(response) + update_url = reverse_usage_url('xblock_handler', video_usage_key) + + response = self.client.ajax_post( + update_url, + data={ + 'id': unicode(video_usage_key), + 'metadata': { + 'saved_video_position': "Not a valid relative time", + }, + } + ) + self.assertEqual(response.status_code, 400) + parsed = json.loads(response.content) + self.assertIn("error", parsed) + self.assertIn("Incorrect RelativeTime value", parsed["error"]) # See xmodule/fields.py + class TestEditSplitModule(ItemTest): """ @@ -1420,6 +1493,90 @@ class TestXBlockInfo(ItemTest): self.assertIsNone(xblock_info.get('edited_by', None)) +class TestLibraryXBlockInfo(ModuleStoreTestCase): + """ + Unit tests for XBlock Info for XBlocks in a content library + """ + def setUp(self): + super(TestLibraryXBlockInfo, self).setUp() + user_id = self.user.id + self.library = LibraryFactory.create() + self.top_level_html = ItemFactory.create( + parent_location=self.library.location, category='html', user_id=user_id, publish_item=False + ) + self.vertical = ItemFactory.create( + parent_location=self.library.location, category='vertical', user_id=user_id, publish_item=False + ) + self.child_html = ItemFactory.create( + parent_location=self.vertical.location, category='html', display_name='Test HTML Child Block', + user_id=user_id, publish_item=False + ) + + def test_lib_xblock_info(self): + html_block = modulestore().get_item(self.top_level_html.location) + xblock_info = create_xblock_info(html_block) + self.validate_component_xblock_info(xblock_info, html_block) + self.assertIsNone(xblock_info.get('child_info', None)) + + def test_lib_child_xblock_info(self): + html_block = modulestore().get_item(self.child_html.location) + xblock_info = create_xblock_info(html_block, include_ancestor_info=True, include_child_info=True) + self.validate_component_xblock_info(xblock_info, html_block) + self.assertIsNone(xblock_info.get('child_info', None)) + ancestors = xblock_info['ancestor_info']['ancestors'] + self.assertEqual(len(ancestors), 2) + self.assertEqual(ancestors[0]['category'], 'vertical') + self.assertEqual(ancestors[0]['id'], unicode(self.vertical.location)) + self.assertEqual(ancestors[1]['category'], 'library') + + def validate_component_xblock_info(self, xblock_info, original_block): + """ + Validate that the xblock info is correct for the test component. + """ + self.assertEqual(xblock_info['category'], original_block.category) + self.assertEqual(xblock_info['id'], unicode(original_block.location)) + self.assertEqual(xblock_info['display_name'], original_block.display_name) + self.assertIsNone(xblock_info.get('has_changes', None)) + self.assertIsNone(xblock_info.get('published', None)) + self.assertIsNone(xblock_info.get('published_on', None)) + self.assertIsNone(xblock_info.get('graders', None)) + + +class TestLibraryXBlockCreation(ItemTest): + """ + Tests the adding of XBlocks to Library + """ + def test_add_xblock(self): + """ + Verify we can add an XBlock to a Library. + """ + lib = LibraryFactory.create() + self.create_xblock(parent_usage_key=lib.location, display_name='Test', category="html") + lib = self.store.get_library(lib.location.library_key) + self.assertTrue(lib.children) + xblock_locator = lib.children[0] + self.assertEqual(self.store.get_item(xblock_locator).display_name, 'Test') + + def test_no_add_discussion(self): + """ + Verify we cannot add a discussion module to a Library. + """ + lib = LibraryFactory.create() + response = self.create_xblock(parent_usage_key=lib.location, display_name='Test', category='discussion') + self.assertEqual(response.status_code, 400) + lib = self.store.get_library(lib.location.library_key) + self.assertFalse(lib.children) + + def test_no_add_advanced(self): + lib = LibraryFactory.create() + lib.advanced_modules = ['lti'] + lib.save() + response = self.create_xblock(parent_usage_key=lib.location, display_name='Test', category='lti') + self.assertEqual(response.status_code, 400) + lib = self.store.get_library(lib.location.library_key) + self.assertFalse(lib.children) + + class TestXBlockPublishingInfo(ItemTest): """ Unit tests for XBlock's outline handling. diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py new file mode 100644 index 0000000000..7289022e8f --- /dev/null +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -0,0 +1,228 @@ +""" +Unit tests for contentstore.views.library + +More important high-level tests are in contentstore/tests/test_libraries.py +""" +from contentstore.tests.utils import AjaxEnabledTestClient, parse_json +from contentstore.utils import reverse_course_url, reverse_library_url +from contentstore.views.component import get_component_templates +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import LibraryFactory +from mock import patch +from opaque_keys.edx.locator import CourseKey, LibraryLocator +import ddt +from student.roles import LibraryUserRole + +LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries + + +def make_url_for_lib(key): + """ Get the RESTful/studio URL for testing the given library """ + if isinstance(key, LibraryLocator): + key = unicode(key) + return LIBRARY_REST_URL + key + + +@ddt.ddt +class UnitTestLibraries(ModuleStoreTestCase): + """ + Unit tests for library views + """ + + def setUp(self): + user_password = super(UnitTestLibraries, self).setUp() + + self.client = AjaxEnabledTestClient() + self.client.login(username=self.user.username, password=user_password) + + ###################################################### + # Tests for /library/ - list and create libraries: + + @patch("contentstore.views.library.LIBRARIES_ENABLED", False) + def test_with_libraries_disabled(self): + """ + The library URLs should return 404 if libraries are disabled. + """ + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 404) + + def test_list_libraries(self): + """ + Test that we can GET /library/ to list all libraries visible to the current user. + """ + # Create some more libraries + libraries = [LibraryFactory.create() for _ in range(0, 3)] + lib_dict = dict([(lib.location.library_key, lib) for lib in libraries]) + + response = self.client.get_json(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 200) + lib_list = parse_json(response) + self.assertEqual(len(lib_list), len(libraries)) + for entry in lib_list: + self.assertIn("library_key", entry) + self.assertIn("display_name", entry) + key = CourseKey.from_string(entry["library_key"]) + self.assertIn(key, lib_dict) + self.assertEqual(entry["display_name"], lib_dict[key].display_name) + del lib_dict[key] # To ensure no duplicates are matched + + @ddt.data("delete", "put") + def test_bad_http_verb(self, verb): + """ + We should get an error if we do weird requests to /library/ + """ + response = getattr(self.client, verb)(LIBRARY_REST_URL) + self.assertEqual(response.status_code, 405) + + def test_create_library(self): + """ Create a library. """ + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', + 'library': 'lib', + 'display_name': "New Library", + }) + self.assertEqual(response.status_code, 200) + # That's all we check. More detailed tests are in contentstore.tests.test_libraries... + + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) + def test_lib_create_permission(self): + """ + Users who aren't given course creator roles shouldn't be able to create + libraries either. + """ + self.client.logout() + ns_user, password = self.create_non_staff_user() + self.client.login(username=ns_user.username, password=password) + + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': 'org', 'library': 'lib', 'display_name': "New Library", + }) + self.assertEqual(response.status_code, 403) + + @ddt.data( + {}, + {'org': 'org'}, + {'library': 'lib'}, + {'org': 'C++', 'library': 'lib', 'display_name': 'Lib with invalid characters in key'}, + {'org': 'Org', 'library': 'Wh@t?', 'display_name': 'Lib with invalid characters in key'}, + ) + def test_create_library_invalid(self, data): + """ + Make sure we are prevented from creating libraries with invalid keys/data + """ + response = self.client.ajax_post(LIBRARY_REST_URL, data) + self.assertEqual(response.status_code, 400) + + def test_no_duplicate_libraries(self): + """ + We should not be able to create multiple libraries with the same key + """ + lib = LibraryFactory.create() + lib_key = lib.location.library_key + response = self.client.ajax_post(LIBRARY_REST_URL, { + 'org': lib_key.org, + 'library': lib_key.library, + 'display_name': "A Duplicate key, same as 'lib'", + }) + self.assertIn('already a library defined', parse_json(response)['ErrMsg']) + self.assertEqual(response.status_code, 400) + + ###################################################### + # Tests for /library/:lib_key/ - get a specific library as JSON or HTML editing view + + def test_get_lib_info(self): + """ + Test that we can get data about a library (in JSON format) using /library/:key/ + """ + # Create a library + lib_key = LibraryFactory.create().location.library_key + # Re-load the library from the modulestore, explicitly including version information: + lib = self.store.get_library(lib_key, remove_version=False, remove_branch=False) + version = lib.location.library_key.version_guid + self.assertNotEqual(version, None) + + response = self.client.get_json(make_url_for_lib(lib_key)) + self.assertEqual(response.status_code, 200) + info = parse_json(response) + self.assertEqual(info['display_name'], lib.display_name) + self.assertEqual(info['library_id'], unicode(lib_key)) + self.assertEqual(info['previous_version'], None) + self.assertNotEqual(info['version'], None) + self.assertNotEqual(info['version'], '') + self.assertEqual(info['version'], unicode(version)) + + def test_get_lib_edit_html(self): + """ + Test that we can get the studio view for editing a library using /library/:key/ + """ + lib = LibraryFactory.create() + + response = self.client.get(make_url_for_lib(lib.location.library_key)) + self.assertEqual(response.status_code, 200) + self.assertIn(" "instructor" > "staff" (in a course) + is_library = isinstance(course_key, LibraryLocator) + # Ordered list of roles: can always move self to the right, but need STUDIO_EDIT_ROLES to move any user left + if is_library: + role_hierarchy = (CourseInstructorRole, CourseStaffRole, LibraryUserRole) + else: + role_hierarchy = (CourseInstructorRole, CourseStaffRole) + if request.method == "GET": # just return info about the user msg = { @@ -117,12 +121,17 @@ def _course_team_user(request, course_key, email): "role": None, } # what's the highest role that this user has? (How should this report global staff?) - for role in [CourseInstructorRole(course_key), CourseStaffRole(course_key)]: - if role.has_user(user): + for role in role_hierarchy: + if role(course_key).has_user(user): msg["role"] = role.ROLE break return JsonResponse(msg) + # All of the following code is for editing/promoting/deleting users. + # Check that the user has STUDIO_EDIT_ROLES permission or is editing themselves: + if not ((requester_perms & STUDIO_EDIT_ROLES) or (user.id == request.user.id)): + return permissions_error_response + # can't modify an inactive user if not user.is_active: msg = { @@ -131,60 +140,44 @@ def _course_team_user(request, course_key, email): return JsonResponse(msg, 400) if request.method == "DELETE": - try: - try_remove_instructor(request, course_key, user) - except CannotOrphanCourse as oops: - return JsonResponse(oops.msg, 400) + new_role = None + else: + # only other operation supported is to promote/demote a user by changing their role: + # role may be None or "" (equivalent to a DELETE request) but must be set. + # Check that the new role was specified: + if "role" in request.json or "role" in request.POST: + new_role = request.json.get("role", request.POST.get("role")) + else: + return JsonResponse({"error": _("No `role` specified.")}, 400) - auth.remove_users(request.user, CourseStaffRole(course_key), user) - return JsonResponse() + old_roles = set() + role_added = False + for role_type in role_hierarchy: + role = role_type(course_key) + if role_type.ROLE == new_role: + if (requester_perms & STUDIO_EDIT_ROLES) or (user.id == request.user.id and old_roles): + # User has STUDIO_EDIT_ROLES permission or + # is currently a member of a higher role, and is thus demoting themself + auth.add_users(request.user, role, user) + role_added = True + else: + return permissions_error_response + elif role.has_user(user): + # Remove the user from this old role: + old_roles.add(role) - # all other operations require the requesting user to specify a role - role = request.json.get("role", request.POST.get("role")) - if role is None: - return JsonResponse({"error": _("`role` is required")}, 400) + if new_role and not role_added: + return JsonResponse({"error": _("Invalid `role` specified.")}, 400) - if role == "instructor": - if not has_course_author_access(request.user, course_key, role=CourseInstructorRole): - msg = { - "error": _("Only instructors may create other instructors") - } + for role in old_roles: + if isinstance(role, CourseInstructorRole) and role.users_with_role().count() == 1: + msg = {"error": _("You may not remove the last Admin. Add another Admin first.")} return JsonResponse(msg, 400) - auth.add_users(request.user, CourseInstructorRole(course_key), user) - # auto-enroll the course creator in the course so that "View Live" will work. - CourseEnrollment.enroll(user, course_key) - elif role == "staff": - # add to staff regardless (can't do after removing from instructors as will no longer - # be allowed) - auth.add_users(request.user, CourseStaffRole(course_key), user) - try: - try_remove_instructor(request, course_key, user) - except CannotOrphanCourse as oops: - return JsonResponse(oops.msg, 400) + auth.remove_users(request.user, role, user) - # auto-enroll the course creator in the course so that "View Live" will work. + if new_role and not is_library: + # The user may be newly added to this course. + # auto-enroll the user in the course so that "View Live" will work. CourseEnrollment.enroll(user, course_key) return JsonResponse() - - -class CannotOrphanCourse(Exception): - """ - Exception raised if an attempt is made to remove all responsible instructors from course. - """ - def __init__(self, msg): - self.msg = msg - Exception.__init__(self) - - -def try_remove_instructor(request, course_key, user): - - # remove all roles in this course from this user: but fail if the user - # is the last instructor in the course team - instructors = CourseInstructorRole(course_key) - if instructors.has_user(user): - if instructors.users_with_role().count() == 1: - msg = {"error": _("You may not remove the last instructor from a course")} - raise CannotOrphanCourse(msg) - else: - auth.remove_users(request.user, instructors, user) diff --git a/cms/envs/bok_choy.env.json b/cms/envs/bok_choy.env.json index 204515802c..685d2f7b63 100644 --- a/cms/envs/bok_choy.env.json +++ b/cms/envs/bok_choy.env.json @@ -72,7 +72,8 @@ "SUBDOMAIN_BRANDING": false, "SUBDOMAIN_COURSE_LISTINGS": false, "ALLOW_ALL_ADVANCED_COMPONENTS": true, - "ALLOW_COURSE_RERUNS": true + "ALLOW_COURSE_RERUNS": true, + "ENABLE_CONTENT_LIBRARIES": true }, "FEEDBACK_SUBMISSION_EMAIL": "", "GITHUB_REPO_ROOT": "** OVERRIDDEN **", diff --git a/cms/envs/common.py b/cms/envs/common.py index a3e1e95034..19d6b55b2d 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -766,6 +766,7 @@ ADVANCED_COMPONENT_TYPES = [ 'word_cloud', 'graphical_slider_tool', 'lti', + 'library_content', # XBlocks from pmitros repos are prototypes. They should not be used # except for edX Learning Sciences experiments on edge.edx.org without # further work to make them robust, maintainable, finalize data formats, diff --git a/cms/envs/test.py b/cms/envs/test.py index 2d16ded81a..cf77fd0115 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -226,3 +226,6 @@ FEATURES['USE_MICROSITES'] = True # For consistency in user-experience, keep the value of this setting in sync with # the one in lms/envs/test.py FEATURES['ENABLE_DISCUSSION_SERVICE'] = False + +# Enable content libraries code for the tests +FEATURES['ENABLE_CONTENT_LIBRARIES'] = True diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 3a7b2c046a..33a8162031 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -239,6 +239,7 @@ define([ "js/spec/views/assets_spec", "js/spec/views/baseview_spec", "js/spec/views/container_spec", + "js/spec/views/paged_container_spec", "js/spec/views/group_configuration_spec", "js/spec/views/paging_spec", "js/spec/views/unit_outline_spec", @@ -255,6 +256,7 @@ define([ "js/spec/views/pages/course_outline_spec", "js/spec/views/pages/course_rerun_spec", "js/spec/views/pages/index_spec", + "js/spec/views/pages/library_users_spec", "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", diff --git a/cms/static/js/factories/container.js b/cms/static/js/factories/container.js index 93cdeb8fd9..429ae58f51 100644 --- a/cms/static/js/factories/container.js +++ b/cms/static/js/factories/container.js @@ -1,22 +1,20 @@ define([ - 'jquery', 'js/models/xblock_info', 'js/views/pages/container', + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', 'xblock/cms.runtime.v1' ], -function($, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { +function($, _, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { 'use strict'; - return function (componentTemplates, XBlockInfoJson, action, isUnitPage) { - var templates = new ComponentTemplates(componentTemplates, {parse: true}), - mainXBlockInfo = new XBlockInfo(XBlockInfoJson, {parse: true}); + return function (componentTemplates, XBlockInfoJson, action, options) { + var main_options = { + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + action: action, + templates: new ComponentTemplates(componentTemplates, {parse: true}) + }; xmoduleLoader.done(function () { - var view = new ContainerPage({ - el: $('#content'), - model: mainXBlockInfo, - action: action, - templates: templates, - isUnitPage: isUnitPage - }); + var view = new ContainerPage(_.extend(main_options, options)); view.render(); }); }; diff --git a/cms/static/js/factories/library.js b/cms/static/js/factories/library.js new file mode 100644 index 0000000000..59f447dead --- /dev/null +++ b/cms/static/js/factories/library.js @@ -0,0 +1,23 @@ +define([ + 'jquery', 'underscore', 'js/models/xblock_info', 'js/views/pages/paged_container', + 'js/views/library_container', 'js/collections/component_template', 'xmodule', 'coffee/src/main', + 'xblock/cms.runtime.v1' +], +function($, _, XBlockInfo, PagedContainerPage, LibraryContainerView, ComponentTemplates, xmoduleLoader) { + 'use strict'; + return function (componentTemplates, XBlockInfoJson, options) { + var main_options = { + el: $('#content'), + model: new XBlockInfo(XBlockInfoJson, {parse: true}), + templates: new ComponentTemplates(componentTemplates, {parse: true}), + action: 'view', + viewClass: LibraryContainerView, + canEdit: true + }; + + xmoduleLoader.done(function () { + var view = new PagedContainerPage(_.extend(main_options, options)); + view.render(); + }); + }; +}); diff --git a/cms/static/js/factories/manage_users.js b/cms/static/js/factories/manage_users.js index 5886f5eab9..2f4e5b406f 100644 --- a/cms/static/js/factories/manage_users.js +++ b/cms/static/js/factories/manage_users.js @@ -32,7 +32,7 @@ define(['jquery', 'underscore', 'gettext', 'js/views/feedback_prompt'], function msg = new PromptView.Warning({ title: gettext('Already a course team member'), message: _.template( - gettext("{email} is already on the “{course}” team. If you're trying to add a new member, please double-check the email address you provided."), { + gettext("{email} is already on the {course} team. Recheck the email address if you want to add a new member."), { email: email, course: course.escape('name') }, {interpolate: /\{(.+?)\}/g} diff --git a/cms/static/js/factories/manage_users_lib.js b/cms/static/js/factories/manage_users_lib.js new file mode 100644 index 0000000000..388ec56c73 --- /dev/null +++ b/cms/static/js/factories/manage_users_lib.js @@ -0,0 +1,159 @@ +/* + Code for editing users and assigning roles within a library context. +*/ +define(['jquery', 'underscore', 'gettext', 'js/views/feedback_prompt', 'js/views/utils/view_utils'], +function($, _, gettext, PromptView, ViewUtils) { + 'use strict'; + return function (libraryName, allUserEmails, tplUserURL) { + var unknownErrorMessage = gettext('Unknown'), + $createUserForm = $('#create-user-form'), + $createUserFormWrapper = $createUserForm.closest('.wrapper-create-user'), + $cancelButton; + + // Our helper method that calls the RESTful API to add/remove/change user roles: + var changeRole = function(email, newRole, opts) { + var url = tplUserURL.replace('@@EMAIL@@', email); + var errMessage = opts.errMessage || gettext("There was an error changing the user's role"); + var onSuccess = opts.onSuccess || function(data){ ViewUtils.reload(); }; + var onError = opts.onError || function(){}; + $.ajax({ + url: url, + type: newRole ? 'POST' : 'DELETE', + dataType: 'json', + contentType: 'application/json', + notifyOnError: false, + data: JSON.stringify({role: newRole}), + success: onSuccess, + error: function(jqXHR, textStatus, errorThrown) { + var message, prompt; + try { + message = JSON.parse(jqXHR.responseText).error || unknownErrorMessage; + } catch (e) { + message = unknownErrorMessage; + } + prompt = new PromptView.Error({ + title: errMessage, + message: message, + actions: { + primary: { text: gettext('OK'), click: function(view) { view.hide(); onError(); } } + } + }); + prompt.show(); + } + }); + }; + + $createUserForm.bind('submit', function(event) { + event.preventDefault(); + var email = $('#user-email-input').val().trim(); + var msg; + + if(!email) { + msg = new PromptView.Error({ + title: gettext('A valid email address is required'), + message: gettext('You must enter a valid email address in order to add an instructor'), + actions: { + primary: { + text: gettext('Return and add email address'), + click: function(view) { view.hide(); $('#user-email-input').focus(); } + } + } + }); + msg.show(); + return; + } + + if(_.contains(allUserEmails, email)) { + msg = new PromptView.Warning({ + title: gettext('Already a library team member'), + message: _.template( + gettext("{email} is already on the {course} team. Recheck the email address if you want to add a new member."), { + email: email, + course: libraryName + }, {interpolate: /\{(.+?)\}/g} + ), + actions: { + primary: { + text: gettext('Return to team listing'), + click: function(view) { view.hide(); $('#user-email-input').focus(); } + } + } + }); + msg.show(); + return; + } + + // Use the REST API to create the user, giving them a role of "library_user" for now: + changeRole( + $('#user-email-input').val().trim(), + 'library_user', + { + errMessage: gettext('Error adding user'), + onError: function() { $('#user-email-input').focus(); } + } + ); + }); + + $cancelButton = $createUserForm.find('.action-cancel'); + $cancelButton.on('click', function(event) { + event.preventDefault(); + $('.create-user-button').toggleClass('is-disabled'); + $createUserFormWrapper.toggleClass('is-shown'); + $('#user-email-input').val(''); + }); + + $('.create-user-button').on('click', function(event) { + event.preventDefault(); + $('.create-user-button').toggleClass('is-disabled'); + $createUserFormWrapper.toggleClass('is-shown'); + $createUserForm.find('#user-email-input').focus(); + }); + + $('body').on('keyup', function(event) { + if(event.which == jQuery.ui.keyCode.ESCAPE && $createUserFormWrapper.is('.is-shown')) { + $cancelButton.click(); + } + }); + + $('.remove-user').click(function() { + var email = $(this).closest('li[data-email]').data('email'), + msg = new PromptView.Warning({ + title: gettext('Are you sure?'), + message: _.template(gettext('Are you sure you want to delete {email} from the library “{library}”?'), {email: email, library: libraryName}, {interpolate: /\{(.+?)\}/g}), + actions: { + primary: { + text: gettext('Delete'), + click: function(view) { + // User the REST API to delete the user: + changeRole(email, null, { errMessage: gettext('Error removing user') }); + } + }, + secondary: { + text: gettext('Cancel'), + click: function(view) { view.hide(); } + } + } + }); + msg.show(); + }); + + $('.user-actions .make-instructor').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'instructor', {}); + }); + + $('.user-actions .make-staff').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'staff', {}); + }); + + $('.user-actions .make-user').click(function(event) { + event.preventDefault(); + var email = $(this).closest('li[data-email]').data('email'); + changeRole(email, 'library_user', {}); + }); + + }; +}); diff --git a/cms/static/js/index.js b/cms/static/js/index.js index e54be1c991..e1c91d03a1 100644 --- a/cms/static/js/index.js +++ b/cms/static/js/index.js @@ -1,16 +1,17 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/views/utils/create_course_utils", - "js/views/utils/view_utils"], - function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory, ViewUtils) { - var CreateCourseUtils = CreateCourseUtilsFactory({ + "js/views/utils/create_library_utils", "js/views/utils/view_utils"], + function (domReady, $, _, CancelOnEscape, CreateCourseUtilsFactory, CreateLibraryUtilsFactory, ViewUtils) { + "use strict"; + var CreateCourseUtils = new CreateCourseUtilsFactory({ name: '.new-course-name', org: '.new-course-org', number: '.new-course-number', run: '.new-course-run', save: '.new-course-save', - errorWrapper: '.wrap-error', + errorWrapper: '.create-course .wrap-error', errorMessage: '#course_creation_error', - tipError: 'span.tip-error', - error: '.error', + tipError: '.create-course span.tip-error', + error: '.create-course .error', allowUnicode: '.allow-unicode-course-id' }, { shown: 'is-shown', @@ -20,6 +21,24 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie error: 'error' }); + var CreateLibraryUtils = new CreateLibraryUtilsFactory({ + name: '.new-library-name', + org: '.new-library-org', + number: '.new-library-number', + save: '.new-library-save', + errorWrapper: '.create-library .wrap-error', + errorMessage: '#library_creation_error', + tipError: '.create-library span.tip-error', + error: '.create-library .error', + allowUnicode: '.allow-unicode-library-id' + }, { + shown: 'is-shown', + showing: 'is-showing', + hiding: 'is-hiding', + disabled: 'is-disabled', + error: 'error' + }); + var saveNewCourse = function (e) { e.preventDefault(); @@ -33,7 +52,7 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie var number = $newCourseForm.find('.new-course-number').val(); var run = $newCourseForm.find('.new-course-run').val(); - course_info = { + var course_info = { org: org, number: number, display_name: display_name, @@ -41,27 +60,24 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie }; analytics.track('Created a Course', course_info); - CreateCourseUtils.createCourse(course_info, function (errorMessage) { - $('.wrap-error').addClass('is-shown'); + CreateCourseUtils.create(course_info, function (errorMessage) { + $('.create-course .wrap-error').addClass('is-shown'); $('#course_creation_error').html('

' + errorMessage + '

'); $('.new-course-save').addClass('is-disabled').attr('aria-disabled', true); }); }; - var cancelNewCourse = function (e) { - e.preventDefault(); - $('.new-course-button').removeClass('is-disabled').attr('aria-disabled', false); - $('.wrapper-create-course').removeClass('is-shown'); - // Clear out existing fields and errors - _.each( - ['.new-course-name', '.new-course-org', '.new-course-number', '.new-course-run'], - function (field) { - $(field).val(''); - } - ); - $('#course_creation_error').html(''); - $('.wrap-error').removeClass('is-shown'); - $('.new-course-save').off('click'); + var makeCancelHandler = function (addType) { + return function(e) { + e.preventDefault(); + $('.new-'+addType+'-button').removeClass('is-disabled').attr('aria-disabled', false); + $('.wrapper-create-'+addType).removeClass('is-shown'); + // Clear out existing fields and errors + $('#create-'+addType+'-form input[type=text]').val(''); + $('#'+addType+'_creation_error').html(''); + $('.create-'+addType+' .wrap-error').removeClass('is-shown'); + $('.new-'+addType+'-save').off('click'); + }; }; var addNewCourse = function (e) { @@ -73,18 +89,70 @@ define(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape", "js/vie var $courseName = $('.new-course-name'); $courseName.focus().select(); $('.new-course-save').on('click', saveNewCourse); - $cancelButton.bind('click', cancelNewCourse); + $cancelButton.bind('click', makeCancelHandler('course')); CancelOnEscape($cancelButton); CreateCourseUtils.configureHandlers(); }; + var saveNewLibrary = function (e) { + e.preventDefault(); + + if (CreateLibraryUtils.hasInvalidRequiredFields()) { + return; + } + + var $newLibraryForm = $(this).closest('#create-library-form'); + var display_name = $newLibraryForm.find('.new-library-name').val(); + var org = $newLibraryForm.find('.new-library-org').val(); + var number = $newLibraryForm.find('.new-library-number').val(); + + var lib_info = { + org: org, + number: number, + display_name: display_name, + }; + + analytics.track('Created a Library', lib_info); + CreateLibraryUtils.create(lib_info, function (errorMessage) { + $('.create-library .wrap-error').addClass('is-shown'); + $('#library_creation_error').html('

' + errorMessage + '

'); + $('.new-library-save').addClass('is-disabled').attr('aria-disabled', true); + }); + }; + + var addNewLibrary = function (e) { + e.preventDefault(); + $('.new-library-button').addClass('is-disabled').attr('aria-disabled', true); + $('.new-library-save').addClass('is-disabled').attr('aria-disabled', true); + var $newLibrary = $('.wrapper-create-library').addClass('is-shown'); + var $cancelButton = $newLibrary.find('.new-library-cancel'); + var $libraryName = $('.new-library-name'); + $libraryName.focus().select(); + $('.new-library-save').on('click', saveNewLibrary); + $cancelButton.bind('click', makeCancelHandler('library')); + CancelOnEscape($cancelButton); + + CreateLibraryUtils.configureHandlers(); + }; + + var showTab = function(tab) { + return function(e) { + e.preventDefault(); + $('.courses-tab').toggleClass('active', tab === 'courses'); + $('.libraries-tab').toggleClass('active', tab === 'libraries'); + }; + }; + var onReady = function () { $('.new-course-button').bind('click', addNewCourse); + $('.new-library-button').bind('click', addNewLibrary); $('.dismiss-button').bind('click', ViewUtils.deleteNotificationHandler(function () { ViewUtils.reload(); })); $('.action-reload').bind('click', ViewUtils.reload); + $('#course-index-tabs .courses-tab').bind('click', showTab('courses')); + $('#course-index-tabs .libraries-tab').bind('click', showTab('libraries')); }; domReady(onReady); diff --git a/cms/static/js/spec/views/modals/edit_xblock_spec.js b/cms/static/js/spec/views/modals/edit_xblock_spec.js index 99de1404c8..b159998e7f 100644 --- a/cms/static/js/spec/views/modals/edit_xblock_spec.js +++ b/cms/static/js/spec/views/modals/edit_xblock_spec.js @@ -49,7 +49,7 @@ define(["jquery", "underscore", "js/common_helpers/ajax_helpers", "js/spec_helpe var requests = AjaxHelpers.requests(this); modal = showModal(requests, mockXBlockEditorHtml); expect(modal.$('.action-save')).not.toBeVisible(); - expect(modal.$('.action-cancel').text()).toBe('OK'); + expect(modal.$('.action-cancel').text()).toBe('Close'); }); it('shows the correct title', function() { diff --git a/cms/static/js/spec/views/paged_container_spec.js b/cms/static/js/spec/views/paged_container_spec.js new file mode 100644 index 0000000000..524f88e552 --- /dev/null +++ b/cms/static/js/spec/views/paged_container_spec.js @@ -0,0 +1,489 @@ +define([ "jquery", "underscore", "js/common_helpers/ajax_helpers", "URI", "js/models/xblock_info", + "js/views/paged_container", "js/views/paging_header", "js/views/paging_footer"], + function ($, _, AjaxHelpers, URI, XBlockInfo, PagedContainer, PagingHeader, PagingFooter) { + + var htmlResponseTpl = _.template('' + + '
' + ); + + function getResponseHtml(options){ + return '
' + + '
' + + htmlResponseTpl(options) + + '' + + '
' + } + + var PAGE_SIZE = 3; + + var mockFirstPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: PAGE_SIZE, + total: PAGE_SIZE + 1 + }) + }; + + var mockSecondPage = { + resources: [], + html: getResponseHtml({ + start: PAGE_SIZE, + displayed: 1, + total: PAGE_SIZE + 1 + }) + }; + + var mockEmptyPage = { + resources: [], + html: getResponseHtml({ + start: 0, + displayed: 0, + total: 0 + }) + }; + + var respondWithMockPage = function(requests) { + var requestIndex = requests.length - 1; + var request = requests[requestIndex]; + var url = new URI(request.url); + var queryParameters = url.query(true); // Returns an object with each query parameter stored as a value + var page = queryParameters.page_number; + var response = page === "0" ? mockFirstPage : mockSecondPage; + AjaxHelpers.respondWithJson(requests, response, requestIndex); + }; + + var MockPagingView = PagedContainer.extend({ + view: 'container_preview', + el: $("
"), + model: new XBlockInfo({}, {parse: true}) + }); + + describe("Paging Container", function() { + var pagingContainer; + + beforeEach(function () { + var feedbackTpl = readFixtures('system-feedback.underscore'); + setFixtures($(" + + +
+ +
+
+
+
+ +
+
    +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ +
+
    +
  • + +
  • +
+
+
+
+ +
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+
+
+
+
+
+
+
+
+
+
+ +
+ diff --git a/cms/templates/js/mock/mock-index-page.underscore b/cms/templates/js/mock/mock-index-page.underscore index b185b2aba2..737234ff8d 100644 --- a/cms/templates/js/mock/mock-index-page.underscore +++ b/cms/templates/js/mock/mock-index-page.underscore @@ -1,6 +1,6 @@
-

My Courses

+

Studio Home

@@ -17,13 +21,6 @@
-
-

Welcome, user!

-
-

Here are all of the courses you currently have access to in Studio:

-
-
-
@@ -78,6 +75,53 @@
+
+
+
+ +
+ +
+

Create a New Library

+ +
+ Required Information to Create a New Library + +
    +
  1. + + + The public display name for your library. + +
  2. +
  3. + + + The public organization name for your library. This cannot be changed. + +
  4. + +
  5. + + + The major version number of your library. Minor revisions are tracked as edits happen within a library. + +
  6. +
+ +
+
+ +
+ + + +
+
+
+

Courses Being Processed

@@ -163,6 +207,15 @@
+ + + +
+
+
diff --git a/cms/templates/js/mock/mock-manage-users-lib.underscore b/cms/templates/js/mock/mock-manage-users-lib.underscore new file mode 100644 index 0000000000..fb0ea5e67d --- /dev/null +++ b/cms/templates/js/mock/mock-manage-users-lib.underscore @@ -0,0 +1,146 @@ +
+
+

+ Settings + > Instructor Access +

+ + +
+
+ +
+
+
+
+
+
+

Grant Instructor Access to This Library

+ +
+ New Instructor Information + +
    +
  1. + + + Please provide the email address of the instructor you'd like to add +
  2. +
+
+
+ +
+ + +
+
+
+ +
    + +
  1. + + + + Current Role: + + Staff + + + + + + + + +
  2. + +
  3. + + + + Current Role: + + Admin + + + + + + + + +
  4. + +
  5. + + + + Current Role: + + User + + + + + + + + +
  6. +
+ +
+
+
diff --git a/cms/templates/js/mock/mock-xblock-paged.underscore b/cms/templates/js/mock/mock-xblock-paged.underscore new file mode 100644 index 0000000000..c6c2c881d8 --- /dev/null +++ b/cms/templates/js/mock/mock-xblock-paged.underscore @@ -0,0 +1,21 @@ +
+
+
+ Mock XBlock +
+ +
+
+
+

Mock XBlock

+
+
+
diff --git a/cms/templates/js/system-feedback.underscore b/cms/templates/js/system-feedback.underscore index 57f768ce3f..0d9a692879 100644 --- a/cms/templates/js/system-feedback.underscore +++ b/cms/templates/js/system-feedback.underscore @@ -4,6 +4,7 @@ id="<%= type %>-<%= intent %>" aria-hidden="<% if(obj.shown) { %>false<% } else { %>true<% } %>" aria-labelledby="<%= type %>-<%= intent %>-title" + tabindex="-1" <% if (obj.message) { %>aria-describedby="<%= type %>-<%= intent %>-description" <% } %> <% if (obj.actions) { %>role="dialog"<% } %> > diff --git a/cms/templates/library.html b/cms/templates/library.html new file mode 100644 index 0000000000..9a7d8fda9a --- /dev/null +++ b/cms/templates/library.html @@ -0,0 +1,101 @@ +<%inherit file="base.html" /> +<%def name="online_help_token()"><% return "content_libraries" %> +<%! +import json + +from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name +from django.utils.translation import ugettext as _ +%> +<%block name="title">${context_library.display_name_with_default} ${xblock_type_display_name(context_library)} +<%block name="bodyclass">is-signedin course container view-container view-library + +<%namespace name='static' file='static_content.html'/> + +<%block name="header_extras"> +% for template_name in templates: + +% endfor + + +<%block name="requirejs"> + require(["js/factories/library"], function(LibraryFactory) { + LibraryFactory( + ${component_templates | n}, + ${json.dumps(xblock_info) | n}, + { + isUnitPage: false, + page_size: 10, + canEdit: ${"true" if can_edit else "false"} + } + ); + }); + + +<%block name="content"> + + +
+
+ + + +
+
+ +
+
+
+ +
+
+ +
+

${_("Loading")}

+
+
+ +
+
+
+ diff --git a/cms/templates/manage_users_lib.html b/cms/templates/manage_users_lib.html new file mode 100644 index 0000000000..a0deb0b307 --- /dev/null +++ b/cms/templates/manage_users_lib.html @@ -0,0 +1,170 @@ +<%! import json %> +<%! from django.utils.translation import ugettext as _ %> +<%! from django.core.urlresolvers import reverse %> +<%inherit file="base.html" /> +<%def name="online_help_token()"><% return "team" %> +<%block name="title">${_("Library User Access")} +<%block name="bodyclass">is-signedin course users view-team + +<%block name="content"> + +
+
+

+ ${_("Settings")} + > ${_("User Access")} +

+ + +
+
+ +
+
+
+ %if allow_actions: +
+
+
+

${_("Grant Access to This Library")}

+ +
+ ${_("New Team Member Information")} + +
    +
  1. + + + ${_("Provide the email address of the user you want to add")} +
  2. +
+
+
+ +
+ + +
+
+
+ %endif + +
    + % for user in all_users: + <% + is_instructor = user in instructors + is_staff = user in staff + role_id = 'admin' if is_instructor else ('staff' if is_staff else 'user') + role_desc = _("Admin") if is_instructor else (_("Staff") if is_staff else _("User")) + %> + +
  1. + + + + ${_("Current Role:")} + + ${role_desc} + % if request.user.id == user.id: + ${_("You!")} + % endif + + + + + + + % if allow_actions: + + % elif request.user.id == user.id: + + % endif + +
  2. + % endfor +
+ + % if allow_actions and len(all_users) == 1: +
+
+

${_('Add More Users to This Library')}

+
+

${_('Grant other members of your course team access to this library. New library users must have an active {studio_name} account.').format(studio_name=settings.STUDIO_SHORT_NAME)}

+
+
+ + +
+ %endif +
+ + +
+
+ + +<%block name="requirejs"> + require(["js/factories/manage_users_lib"], function(ManageUsersFactory) { + ManageUsersFactory( + "${context_library.display_name_with_default | h}", + ${json.dumps([user.email for user in all_users])}, + "${reverse('contentstore.views.course_team_handler', kwargs={'course_key_string': library_key, 'email': '@@EMAIL@@'})}" + ); + }); + diff --git a/cms/templates/studio_xblock_wrapper.html b/cms/templates/studio_xblock_wrapper.html index 914f8ec6e0..44ec7298fa 100644 --- a/cms/templates/studio_xblock_wrapper.html +++ b/cms/templates/studio_xblock_wrapper.html @@ -55,30 +55,38 @@ messages = json.dumps(xblock.validate().to_json())
diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 76c5cea17a..97b005a8ae 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -45,7 +45,7 @@ + % elif context_library: + <% + library_key = context_library.location.course_key + index_url = reverse('contentstore.views.library_handler', kwargs={'library_key_string': unicode(library_key)}) + %> +

+ ${_("Current Library:")} + + ${context_library.display_org_with_default | h}${context_library.display_number_with_default | h} + ${context_library.display_name_with_default} + +

+ + % endif
@@ -151,7 +183,7 @@