From df635e0fae5dea48e5a0bf835393919f5aa60ad6 Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Thu, 15 Feb 2024 18:04:49 +0000 Subject: [PATCH 1/2] fix: libraries across orgs --- cms/djangoapps/contentstore/utils.py | 4 +- cms/djangoapps/contentstore/views/library.py | 53 +++++++++++++++---- .../contentstore/views/tests/test_library.py | 39 +++++++++----- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 9ed3f1ce4b36..d75ebdb3f23f 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1670,7 +1670,7 @@ def get_home_context(request, no_course=False): LIBRARY_AUTHORING_MICROFRONTEND_URL, LIBRARIES_ENABLED, should_redirect_to_library_authoring_mfe, - user_can_create_library, + user_can_view_create_library_button, ) active_courses = [] @@ -1699,7 +1699,7 @@ def get_home_context(request, no_course=False): 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_create_library(user) and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user) and not should_redirect_to_library_authoring_mfe(), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 17aa24c5712a..8ea6bf1e3657 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -10,7 +10,7 @@ from django.conf import settings from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied -from django.http import Http404, HttpResponseForbidden, HttpResponseNotAllowed +from django.http import Http404, HttpResponseNotAllowed from django.utils.translation import gettext as _ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods @@ -68,13 +68,10 @@ def should_redirect_to_library_authoring_mfe(): REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() ) - -def user_can_create_library(user, org=None): +def user_can_view_create_library_button(user): """ - Helper method for returning the library creation status for a particular user, - taking into account the value LIBRARIES_ENABLED. + Helper method for displaying the visibilty of the create_library_button. """ - if not LIBRARIES_ENABLED: return False elif user.is_staff: @@ -84,8 +81,48 @@ def user_can_create_library(user, org=None): has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).exists() has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role().exists() has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role().exists() + return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role + else: + # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. + disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) + disable_course_creation = settings.FEATURES.get('DISABLE_COURSE_CREATION', False) + if disable_library_creation is not None: + return not disable_library_creation + else: + return not disable_course_creation + + +def user_can_create_library(user, org): + """ + Helper method for returning the library creation status for a particular user, + taking into account the value LIBRARIES_ENABLED. + + users can only create libraries in orgs they are a part of. + """ + if org is None: + return False + if not LIBRARIES_ENABLED: + return False + elif user.is_staff: + return True + if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + is_course_creator = get_course_creator_status(user) == 'granted' + has_org_staff_role = org in OrgStaffRole().get_orgs_for_user(user) + has_course_staff_role = ( + UserBasedRole(user=user, role=CourseStaffRole.ROLE) + .courses_with_role() + .filter(org=org) + .exists() + ) + has_course_admin_role = ( + UserBasedRole(user=user, role=CourseInstructorRole.ROLE) + .courses_with_role() + .filter(org=org) + .exists() + ) return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role + else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) @@ -108,12 +145,8 @@ def library_handler(request, library_key_string=None): raise Http404 # Should never happen because we test the feature in urls.py also if request.method == 'POST': - if not user_can_create_library(request.user): - return HttpResponseForbidden() - if library_key_string is not None: return HttpResponseNotAllowed(("POST",)) - return _create_library(request) else: diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index f6b7a48a68e1..fdbb9d905c62 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -59,55 +59,66 @@ def setUp(self): @mock.patch("cms.djangoapps.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(user_can_create_library(nostaff_user), False) + self.assertEqual(user_can_create_library(nostaff_user, None), False) # When creator group is disabled, non-staff users can create libraries @mock.patch("cms.djangoapps.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(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, 'An Org'), True) # When creator group is enabled, Non staff users cannot create libraries @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_for_enabled_creator_group_setting_for_non_staff_users(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertEqual(user_can_create_library(nostaff_user), False) + self.assertEqual(user_can_create_library(nostaff_user, None), False) - # Global staff can create libraries + # Global staff can create libraries for any org, even ones that don't exist. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_is_staff_user(self): - self.assertEqual(user_can_create_library(self.user), True) + print(self.user.is_staff) + self.assertEqual(user_can_create_library(self.user, 'aNyOrg'), True) - # When creator groups are enabled, global staff can create libraries + # Global staff can create libraries for any org, but an org has to be supplied. + @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) + def test_library_creator_status_with_is_staff_user_no_org(self): + print(self.user.is_staff) + self.assertEqual(user_can_create_library(self.user, None), False) + + # When creator groups are enabled, global staff can create libraries in any org @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_for_enabled_creator_group_setting_with_is_staff_user(self): with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - self.assertEqual(user_can_create_library(self.user), True) + self.assertEqual(user_can_create_library(self.user, 'RandomOrg'), True) - # When creator groups are enabled, course creators can create libraries + # When creator groups are enabled, course creators can create libraries in any org. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_creator_role_for_enabled_creator_group_setting(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(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, 'soMeRandOmoRg'), True) # When creator groups are enabled, course staff members can create libraries + # but only in the org they are course staff for. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_staff_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): auth.add_users(self.user, CourseStaffRole(self.course.id), nostaff_user) - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) # When creator groups are enabled, course instructor members can create libraries + # but only in the org they are course staff for. @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) def test_library_creator_status_with_course_instructor_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): auth.add_users(self.user, CourseInstructorRole(self.course.id), nostaff_user) - self.assertEqual(user_can_create_library(nostaff_user), True) + self.assertEqual(user_can_create_library(nostaff_user, self.course.org), True) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOtherOrg'), False) @ddt.data( (False, False, True), @@ -131,7 +142,7 @@ def test_library_creator_status_settings(self, disable_course, disable_library, "DISABLE_LIBRARY_CREATION": disable_library } ): - self.assertEqual(user_can_create_library(nostaff_user), expected_status) + self.assertEqual(user_can_create_library(nostaff_user, 'SomEOrg'), expected_status) @mock.patch.dict('django.conf.settings.FEATURES', {'DISABLE_COURSE_CREATION': True}) @mock.patch("cms.djangoapps.contentstore.views.library.LIBRARIES_ENABLED", True) @@ -140,7 +151,7 @@ def test_library_creator_status_with_no_course_creator_role_and_disabled_nonstaf Ensure that `DISABLE_COURSE_CREATION` feature works with libraries as well. """ nostaff_client, nostaff_user = self.create_non_staff_authed_user_client() - self.assertFalse(user_can_create_library(nostaff_user)) + self.assertFalse(user_can_create_library(nostaff_user, 'SomEOrg')) # To be explicit, this user can GET, but not POST get_response = nostaff_client.get_json(LIBRARY_REST_URL) @@ -251,7 +262,7 @@ def test_lib_create_permission_course_staff_role(self): auth.add_users(self.user, CourseStaffRole(self.course.id), ns_user) self.assertTrue(auth.user_has_role(ns_user, CourseStaffRole(self.course.id))) response = self.client.ajax_post(LIBRARY_REST_URL, { - 'org': 'org', 'library': 'lib', 'display_name': "New Library", + 'org': self.course.org, 'library': 'lib', 'display_name': "New Library", }) self.assertEqual(response.status_code, 200) From 25437d2a728ea2fbb08c7023df2b5bed319a01a4 Mon Sep 17 00:00:00 2001 From: connorhaugh Date: Thu, 15 Feb 2024 21:26:45 +0000 Subject: [PATCH 2/2] docs: imporved comment --- cms/djangoapps/contentstore/views/library.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8ea6bf1e3657..7ba80bcab9f6 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -98,7 +98,16 @@ def user_can_create_library(user, org): Helper method for returning the library creation status for a particular user, taking into account the value LIBRARIES_ENABLED. - users can only create libraries in orgs they are a part of. + if the ENABLE_CREATOR_GROUP value is False, then any user can create a library (in any org), + if library creation is enabled. + + if the ENABLE_CREATOR_GROUP value is true, then what a user can do varies by thier role. + + Global Staff: can make libraries in any org. + Course Creator Group Members: can make libraries in any org. + Organization Staff: Can make libraries in the organization for which they are staff. + Course Staff: Can make libraries in the organization which has courses of which they are staff. + Course Admin: Can make libraries in the organization which has courses of which they are Admin. """ if org is None: return False