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

add flag DISABLE_LIBRARY_CREATION

add comma

use CourseCreatorRole to determine if user can create a library

add disable library creation feature flag

Conflicts:
	cms/djangoapps/contentstore/views/course.py

ENABLE_CONTENT_LIBRARIES flag

check for course creator role for library creation

Conflicts:
	cms/djangoapps/contentstore/views/course.py

add unit tests

make check of creation of library a true/false for forntend, add security in api call, clean tests

update tests

fix docstring of tests

fixed quality violation

fixed broken unit test and quality violations

Feedback changes and unit test to assert libraries are visible to non staff users too

fixed quality violation and feedback changes

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>