Merge pull request #14378 from edx/ziafazal/feature-flag-for-library-creation

show button new library in studio depending on flags and user staff s…

cms/djangoapps/contentstore/tests/test_libraries.py

@@ -11,7 +11,7 @@ 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,
+    CourseInstructorRole, CourseStaffRole, LibraryUserRole,
     OrgStaffRole, OrgInstructorRole, OrgLibraryUserRole,
 )
 from xmodule.modulestore import ModuleStoreEnum
@@ -25,6 +25,7 @@ from xblock_django.user_service import DjangoXBlockUserService
 from xmodule.x_module import STUDIO_VIEW
 from student import auth
 from student.tests.factories import UserFactory
+from course_creators.views import add_user_with_status_granted
 
 
 class LibraryTestCase(ModuleStoreTestCase):
@@ -533,9 +534,13 @@ class TestLibraryAccess(SignalDisconnectTestMixin, LibraryTestCase):
         self.client.logout()
         self._assert_cannot_create_library(expected_code=302)  # 302 redirect to login expected
 
-        # Now check that logged-in users without CourseCreator role can still create libraries
+        # Now check that logged-in users without CourseCreator role cannot create libraries
         self._login_as_non_staff_user(logout_first=False)
-        self.assertFalse(CourseCreatorRole().has_user(self.non_staff_user))
+        with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}):
+            self._assert_cannot_create_library(expected_code=403)  # 403 user is not CourseCreator
+
+        # Now check that logged-in users with CourseCreator role can create libraries
+        add_user_with_status_granted(self.user, self.non_staff_user)
         with patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}):
             lib_key2 = self._create_library(library="lib2", display_name="Test Library 2")
             library2 = modulestore().get_library(lib_key2)

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,
@@ -465,6 +465,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
     libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else []
 
     def format_in_process_course_view(uca):
@@ -509,11 +510,11 @@ 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),
     })
@@ -1619,6 +1620,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
@@ -25,6 +25,7 @@ from xmodule.modulestore import ModuleStoreEnum
 from xmodule.modulestore.django import modulestore
 from .user import user_with_role
 
+from course_creators.views import get_course_creator_status
 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
@@ -39,6 +40,22 @@ 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 get_course_creator_status(user) == 'granted'
+    else:
+        return True
+
+
 @login_required
 @ensure_csrf_cookie
 @require_http_methods(('GET', 'POST'))
@@ -50,6 +67,10 @@ 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

@@ -24,6 +24,7 @@ from course_action_state.models import CourseRerunState
 from opaque_keys.edx.locator import CourseLocator
 from search.api import perform_search
 from student.auth import has_course_author_access
+from student.roles import LibraryUserRole
 from student.tests.factories import UserFactory
 from util.date_utils import get_default_time_display
 from xmodule.modulestore import ModuleStoreEnum
@@ -75,22 +76,37 @@ class TestCourseIndex(CourseTestCase):
         """
         Test getting the list of libraries from the course listing page
         """
+        def _assert_library_link_present(response, library):
+            """
+            Asserts there's a valid library link on libraries tab.
+            """
+            parsed_html = lxml.html.fromstring(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', library.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)
+
         # 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)
+        _assert_library_link_present(index_response, lib1)
+
+        # Make sure libraries are visible to non-staff users too
+        self.client.logout()
+        non_staff_user, non_staff_userpassword = self.create_non_staff_user()
+        lib2 = LibraryFactory.create(user_id=non_staff_user.id)
+        LibraryUserRole(lib2.location.library_key).add_users(non_staff_user)
+        self.client.login(username=non_staff_user.username, password=non_staff_userpassword)
+        index_response = self.client.get(index_url, {}, HTTP_ACCEPT='text/html')
+        _assert_library_link_present(index_response, lib2)
 
     def test_is_staff_access(self):
         """

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

@@ -5,12 +5,15 @@ 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 xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from contentstore.views.library import get_library_creator_status
+from course_creators.views import add_user_with_status_granted as grant_course_creator_status
 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.roles import LibraryUserRole
 
 LIBRARY_REST_URL = '/library/'  # URL for GET/POST requests involving libraries
@@ -24,7 +27,7 @@ def make_url_for_lib(key):
 
 
 @ddt.ddt
-class UnitTestLibraries(ModuleStoreTestCase):
+class UnitTestLibraries(CourseTestCase):
     """
     Unit tests for library views
     """
@@ -38,6 +41,27 @@ 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}):
+            grant_course_creator_status(self.user, 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 +111,45 @@ 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)
+        grant_course_creator_status(self.user, 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>