Merge pull request #13914 from proversity-org/proversity/feature-flag-for-library-creation

Feature flag for library creation

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
+from .library import LIBRARIES_ENABLED, _get_library_creator_status
 from ccx_keys.locator import CCXLocator
 from contentstore.course_group_config import (
     COHORT_SCHEME,
@@ -467,7 +467,10 @@ def course_listing(request):
     List all courses available to the logged in user
     """
     courses, in_process_course_actions = get_courses_accessible_to_user(request)
-    libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else []
+    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 []
 
     programs_config = ProgramsApiConfig.current()
     raw_programs = get_programs(request.user) if programs_config.is_studio_tab_enabled else []
@@ -518,14 +521,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': LIBRARIES_ENABLED and request.user.is_active,
-        'user': request.user,
+        'show_new_library_button': _get_library_creator_status(user),
+        'user': user,
         'request_course_creator_url': reverse('contentstore.views.request_course_creator'),
-        'course_creator_status': _get_course_creator_status(request.user),
-        'rerun_creator_status': GlobalStaff().has_user(request.user),
+        'course_creator_status': _get_course_creator_status(user),
+        'rerun_creator_status': GlobalStaff().has_user(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 request.user.is_staff,
+        'is_programs_enabled': programs_config.is_studio_tab_enabled and user.is_staff,
         'programs': programs,
         'program_authoring_url': reverse('programs'),
     })
@@ -1631,6 +1634,7 @@ 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):

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
+from django.http import HttpResponseNotAllowed, Http404, HttpResponseForbidden
 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
+from student.roles import CourseInstructorRole, CourseStaffRole, LibraryUserRole, CourseCreatorRole
 from util.json_request import expect_json, JsonResponse, JsonResponseBadRequest
 
 __all__ = ['library_handler', 'manage_library_users']
@@ -38,6 +38,21 @@ 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
@@ -50,6 +65,11 @@ 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",))
 

cms/djangoapps/contentstore/views/tests/test_course_index.py

@@ -30,6 +30,7 @@ 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):

cms/djangoapps/contentstore/views/tests/test_library.py

@@ -5,13 +5,17 @@ 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
-from student.roles import LibraryUserRole
+import mock
+from student.auth import add_users
+from student.roles import CourseCreatorRole, LibraryUserRole
 
 LIBRARY_REST_URL = '/library/'  # URL for GET/POST requests involving libraries
 
@@ -24,7 +28,7 @@ def make_url_for_lib(key):
 
 
 @ddt.ddt
-class UnitTestLibraries(ModuleStoreTestCase):
+class UnitTestLibraries(CourseTestCase):
     """
     Unit tests for library views
     """
@@ -38,6 +42,28 @@ class UnitTestLibraries(ModuleStoreTestCase):
     ######################################################
     # 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):
         """
@@ -87,18 +113,43 @@ class UnitTestLibraries(ModuleStoreTestCase):
     @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True})
     def test_lib_create_permission(self):
         """
-        Users who are not given course creator roles should still be able to
-        create libraries.
+        Users who are given course creator roles should 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'},

cms/templates/index.html

@@ -33,7 +33,6 @@ from openedx.core.djangolib.markup import HTML, Text
           % elif course_creator_status=='disallowed_for_this_site' and settings.FEATURES.get('STUDIO_REQUEST_EMAIL',''):
           <a href="mailto:${settings.FEATURES.get('STUDIO_REQUEST_EMAIL','')}">${_("Email staff to create course")}</a>
           % endif
-
           % if show_new_library_button:
             <a href="#" class="button new-button new-library-button"><span class="icon fa fa-plus icon-inline" aria-hidden="true"></span>
             ${_("New Library")}</a>