diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index d67e2f9703..70686f0353 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -26,7 +26,7 @@ from .component import ( ADVANCED_COMPONENT_TYPES, ) from .item import create_xblock_info -from .library import LIBRARIES_ENABLED, _get_library_creator_status +from .library import LIBRARIES_ENABLED from ccx_keys.locator import CCXLocator from contentstore.course_group_config import ( COHORT_SCHEME, @@ -467,10 +467,7 @@ def course_listing(request): List all courses available to the logged in user """ courses, in_process_course_actions = get_courses_accessible_to_user(request) - user = request.user - user_can_see_libraries =\ - user.is_active and (user.is_staff or CourseCreatorRole().has_user(user)) - libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED and user_can_see_libraries else [] + libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else [] programs_config = ProgramsApiConfig.current() raw_programs = get_programs(request.user) if programs_config.is_studio_tab_enabled else [] @@ -521,14 +518,14 @@ def course_listing(request): 'in_process_course_actions': in_process_course_actions, 'libraries_enabled': LIBRARIES_ENABLED, 'libraries': [format_library_for_view(lib) for lib in libraries], - 'show_new_library_button': _get_library_creator_status(user), - 'user': user, + 'show_new_library_button': LIBRARIES_ENABLED and request.user.is_active, + 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), - 'course_creator_status': _get_course_creator_status(user), - 'rerun_creator_status': GlobalStaff().has_user(user), + 'course_creator_status': _get_course_creator_status(request.user), + 'rerun_creator_status': GlobalStaff().has_user(request.user), 'allow_unicode_course_id': settings.FEATURES.get('ALLOW_UNICODE_COURSE_ID', False), 'allow_course_reruns': settings.FEATURES.get('ALLOW_COURSE_RERUNS', True), - 'is_programs_enabled': programs_config.is_studio_tab_enabled and user.is_staff, + 'is_programs_enabled': programs_config.is_studio_tab_enabled and request.user.is_staff, 'programs': programs, 'program_authoring_url': reverse('programs'), }) @@ -1634,7 +1631,6 @@ def _get_course_creator_status(user): If the user passed in has not previously visited the index page, it will be added with status 'unrequested' if the course creator group is in use. """ - if user.is_staff: course_creator_status = 'granted' elif settings.FEATURES.get('DISABLE_COURSE_CREATION', False): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 62f2bff17a..cb3cd4e18b 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -9,7 +9,7 @@ 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, HttpResponseForbidden +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 @@ -29,7 +29,7 @@ from .component import get_component_templates, CONTAINER_TEMPLATES 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 CourseInstructorRole, CourseStaffRole, LibraryUserRole, CourseCreatorRole +from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest __all__ = ['library_handler', 'manage_library_users'] @@ -38,21 +38,6 @@ log = logging.getLogger(__name__) LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) -def _get_library_creator_status(user): - """ - Helper method for returning the library creation status for a particular user, - taking into account the value LIBRARIES_ENABLED. - - """ - - if not LIBRARIES_ENABLED: - return False - elif user.is_staff: - return True - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - return CourseCreatorRole().has_user(user) - else: - return True @login_required @ensure_csrf_cookie @@ -65,11 +50,6 @@ def library_handler(request, library_key_string=None): 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 not _get_library_creator_status(request.user): - if not request.user.is_staff: - return HttpResponseForbidden() - - if library_key_string is not None and request.method == 'POST': return HttpResponseNotAllowed(("POST",)) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index e8756eb896..70b3e6387f 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -30,7 +30,6 @@ from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory -from student.roles import CourseCreatorRole class TestCourseIndex(CourseTestCase): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index 68b90d720b..2a704c002d 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -5,17 +5,13 @@ 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.tests.utils import CourseTestCase from contentstore.views.component import get_component_templates -from contentstore.views.course import _get_library_creator_status 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 -import mock -from student.auth import add_users -from student.roles import CourseCreatorRole, LibraryUserRole +from student.roles import LibraryUserRole LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries @@ -28,7 +24,7 @@ def make_url_for_lib(key): @ddt.ddt -class UnitTestLibraries(CourseTestCase): +class UnitTestLibraries(ModuleStoreTestCase): """ Unit tests for library views """ @@ -42,28 +38,6 @@ class UnitTestLibraries(CourseTestCase): ###################################################### # Tests for /library/ - list and create libraries: - @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", False) - def test_library_creator_status_libraries_not_enabled(self): - _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(_get_library_creator_status(nostaff_user), False) - - - @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) - def test_library_creator_status_with_is_staff_user(self): - self.assertEqual(_get_library_creator_status(self.user), True) - - @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) - def test_library_creator_status_with_course_creator_role(self): - _, nostaff_user = self.create_non_staff_authed_user_client() - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - add_users(self.user, CourseCreatorRole(), nostaff_user) - self.assertEqual(_get_library_creator_status(nostaff_user), True) - - @mock.patch("contentstore.views.library.LIBRARIES_ENABLED", True) - def test_library_creator_status_with_no_course_creator_role(self): - _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(_get_library_creator_status(nostaff_user), True) - @patch("contentstore.views.library.LIBRARIES_ENABLED", False) def test_with_libraries_disabled(self): """ @@ -113,43 +87,18 @@ class UnitTestLibraries(CourseTestCase): @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) def test_lib_create_permission(self): """ - Users who are given course creator roles should be able to create libraries. + Users who are not given course creator roles should still be able to + create libraries. """ self.client.logout() ns_user, password = self.create_non_staff_user() self.client.login(username=ns_user.username, password=password) - add_users(self.user, CourseCreatorRole(), ns_user) + response = self.client.ajax_post(LIBRARY_REST_URL, { 'org': 'org', 'library': 'lib', 'display_name': "New Library", }) self.assertEqual(response.status_code, 200) - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': False}) - def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group(self): - """ - Users who are not given course creator roles should still be able to create libraries if ENABLE_CREATOR_GROUP is not enabled. - """ - 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, 200) - - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) - def test_lib_create_permission_no_course_creator_role_and_course_creator_group(self): - """ - Users who are not given course creator roles should not be able to create libraries if ENABLE_CREATOR_GROUP is enabled. - """ - 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'}, diff --git a/cms/templates/index.html b/cms/templates/index.html index 8760d5951f..daf518d8a6 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -33,6 +33,7 @@ from openedx.core.djangolib.markup import HTML, Text % elif course_creator_status=='disallowed_for_this_site' and settings.FEATURES.get('STUDIO_REQUEST_EMAIL',''): ${_("Email staff to create course")} % endif + % if show_new_library_button: ${_("New Library")}