From 78a31026298225dd97d61bdc022e230e5a331a6f Mon Sep 17 00:00:00 2001 From: Kaustav Banerjee Date: Mon, 31 Jul 2023 15:21:31 +0530 Subject: [PATCH] Revert "Sync opencraft-release/palm.1 with Upstream 20230728-1690565702 (#562)" This reverts commit dcf72e144429a010363aede55121505a017efd96. --- cms/djangoapps/contentstore/views/course.py | 29 +----- cms/djangoapps/contentstore/views/library.py | 14 ++- .../views/tests/test_course_index.py | 12 +-- .../contentstore/views/tests/test_library.py | 96 ++----------------- cms/envs/common.py | 5 - cms/envs/devstack.py | 5 +- cms/templates/index.html | 2 +- lms/templates/wiki/base.html | 6 +- 8 files changed, 22 insertions(+), 147 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index be83c164cc02..b583a954c6ef 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -53,8 +53,7 @@ CourseInstructorRole, CourseStaffRole, GlobalStaff, - UserBasedRole, - OrgStaffRole + UserBasedRole ) from common.djangoapps.util.course import get_link_for_about_page from common.djangoapps.util.date_utils import get_default_time_display @@ -600,7 +599,6 @@ def format_in_process_course_view(uca): 'optimization_enabled': optimization_enabled, 'active_tab': 'courses', 'allowed_organizations': get_allowed_organizations(user), - 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(user), 'can_create_organizations': user_can_create_organizations(user), }) @@ -628,7 +626,6 @@ def library_listing(request): 'split_studio_home': split_library_view_on_dashboard(), 'active_tab': 'libraries', 'allowed_organizations': get_allowed_organizations(request.user), - 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(request.user), 'can_create_organizations': user_can_create_organizations(request.user), } return render_to_response('index.html', data) @@ -1963,18 +1960,6 @@ def get_allowed_organizations(user): return [] -def get_allowed_organizations_for_libraries(user): - """ - Helper method for returning the list of organizations for which the user is allowed to create libraries. - """ - if settings.FEATURES.get('ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES', False): - return get_organizations_for_non_course_creators(user) - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - return get_organizations(user) - else: - return [] - - def user_can_create_organizations(user): """ Returns True if the user can create organizations. @@ -1982,18 +1967,6 @@ def user_can_create_organizations(user): return user.is_staff or not settings.FEATURES.get('ENABLE_CREATOR_GROUP', False) -def get_organizations_for_non_course_creators(user): - """ - Returns the list of organizations which the user is a staff member of, as a list of strings. - """ - orgs_map = set() - orgs = OrgStaffRole().get_orgs_for_user(user) - # deduplicate - for org in orgs: - orgs_map.add(org) - return list(orgs_map) - - def get_organizations(user): """ Returns the list of organizations for which the user is allowed to create courses. diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 328c92687b2c..8ec1e79d1932 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -35,15 +35,14 @@ from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, - LibraryUserRole, - OrgStaffRole, - UserBasedRole, + LibraryUserRole ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url from .component import CONTAINER_TEMPLATES, get_component_templates +from .helpers import is_content_creator from .block import create_xblock_info from .user import user_with_role @@ -80,11 +79,10 @@ def user_can_create_library(user, org=None): elif user.is_staff: return True elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - is_course_creator = get_course_creator_status(user) == 'granted' - 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() - - return is_course_creator or has_org_staff_role or has_course_staff_role + has_course_creator_role = True + if org: + has_course_creator_role = is_content_creator(user, org) + return get_course_creator_status(user) == 'granted' and has_course_creator_role else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index 9fdbb425e51d..cf9f2a4197ae 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -423,13 +423,13 @@ def check_index_page(self, separate_archived_courses, org): @ddt.data( # Staff user has course staff access - (True, 'staff', None, 0, 21), - (False, 'staff', None, 0, 21), + (True, 'staff', None, 0, 20), + (False, 'staff', None, 0, 20), # Base user has global staff access - (True, 'user', ORG, 2, 21), - (False, 'user', ORG, 2, 21), - (True, 'user', None, 2, 21), - (False, 'user', None, 2, 21), + (True, 'user', ORG, 2, 20), + (False, 'user', ORG, 2, 20), + (True, 'user', None, 2, 20), + (False, 'user', None, 2, 20), ) @ddt.unpack def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries): diff --git a/cms/djangoapps/contentstore/views/tests/test_library.py b/cms/djangoapps/contentstore/views/tests/test_library.py index bac450b3bd26..746e61cb3126 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -19,15 +19,11 @@ from cms.djangoapps.contentstore.tests.utils import AjaxEnabledTestClient, CourseTestCase, parse_json from cms.djangoapps.contentstore.utils import reverse_course_url, reverse_library_url from cms.djangoapps.course_creators.views import add_user_with_status_granted as grant_course_creator_status -from common.djangoapps.student.roles import LibraryUserRole, CourseStaffRole +from common.djangoapps.student.roles import LibraryUserRole from xmodule.modulestore.tests.factories import LibraryFactory # lint-amnesty, pylint: disable=wrong-import-order -from cms.djangoapps.course_creators.models import CourseCreator - -from common.djangoapps.student import auth from ..component import get_component_templates from ..library import user_can_create_library -from ..course import get_allowed_organizations_for_libraries LIBRARY_REST_URL = '/library/' # URL for GET/POST requests involving libraries @@ -55,51 +51,26 @@ def setUp(self): ###################################################### # Tests for /library/ - list and create libraries: - # When libraries are disabled, nobody can create libraries @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) - # 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) - - # 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) - - # Global staff can create libraries @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) - # When creator groups are enabled, global staff can create libraries - @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) - - # When creator groups are enabled, course creators can create libraries @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): + 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(user_can_create_library(nostaff_user), True) - # When creator groups are enabled, course staff members can create libraries @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): + def test_library_creator_status_with_no_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}): - 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), True) @ddt.data( (False, False, True), @@ -217,9 +188,9 @@ def test_lib_create_permission_no_course_creator_role_and_no_course_creator_grou 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_no_course_creator_group_and_no_course_staff_role(self): + def test_lib_create_permission_no_course_creator_role_and_course_creator_group(self): """ - Users who are not given course creator roles or course staff role should not be able to create libraries + Users who are not given course creator roles should not be able to create libraries if ENABLE_CREATOR_GROUP is enabled. """ self.client.logout() @@ -230,23 +201,6 @@ def test_lib_create_permission_no_course_creator_role_and_no_course_creator_grou }) self.assertEqual(response.status_code, 403) - @patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}) - def test_lib_create_permission_course_staff_role(self): - """ - Users who are staff on any existing course should 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) - - 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", - }) - self.assertEqual(response.status_code, 200) - @ddt.data( {}, {'org': 'org'}, @@ -451,41 +405,3 @@ def test_component_limits(self): response = self.client.ajax_post(reverse('xblock_handler'), data) self.assertEqual(response.status_code, 400) self.assertIn('cannot have more than 1 component', parse_json(response)['error']) - - def test_allowed_organizations_for_library(self): - """ - Test the different organizations that a user can select for creating a library, depending - on Feature Flags and on user role. - With organization staff access enabled, a user should be able to select organizations they - are a staff member of. Else, with creator groups enabled, the user should be able to select - organizations they are course creator for. - """ - course_creator = CourseCreator.objects.create(user=self.user, all_organizations=True) - with patch('cms.djangoapps.course_creators.models.CourseCreator.objects.filter') as mock_filter: - mock_filter.return_value.first.return_value = course_creator - with patch('organizations.models.Organization.objects.all') as mock_all: - mock_all.return_value.values_list.return_value = ['org1', 'org2'] - with patch('common.djangoapps.student.roles.OrgStaffRole.get_orgs_for_user') as get_user_orgs: - get_user_orgs.return_value = ['org3'] - # Call the method under test - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {"ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES": False} - ): - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {"ENABLE_CREATOR_GROUP": False} - ): - organizations = get_allowed_organizations_for_libraries(self.user) - # Assert that the method returned the expected value - self.assertEqual(organizations, []) - with mock.patch.dict('django.conf.settings.FEATURES', {"ENABLE_CREATOR_GROUP": True}): - organizations = get_allowed_organizations_for_libraries(self.user) - # Assert that the method returned the expected value - self.assertEqual(organizations, ['org1', 'org2']) - with mock.patch.dict( - 'django.conf.settings.FEATURES', - {"ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES": True} - ): - organizations = get_allowed_organizations_for_libraries(self.user) - self.assertEqual(organizations, ['org3']) diff --git a/cms/envs/common.py b/cms/envs/common.py index 7a0015b1a882..8bce01ff1c52 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -220,11 +220,6 @@ # an Open edX admin has added them to the course creator group. 'ENABLE_CREATOR_GROUP': True, - # If set to True, organization staff members can create libraries for their specific - # organization and no other organizations. They do not need to be course creators, - # even when ENABLE_CREATOR_GROUP is True. - 'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True, - # Turn off account locking if failed login attempts exceeds a limit 'ENABLE_MAX_FAILED_LOGIN_ATTEMPTS': False, diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 58030a2fa914..1466d15b7e2c 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -156,10 +156,7 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing FEATURES['CERTIFICATES_HTML_VIEW'] = True ########################## AUTHOR PERMISSION ####################### -FEATURES['ENABLE_CREATOR_GROUP'] = True - -########################## Library creation organizations restriction ####################### -FEATURES['ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES'] = True +FEATURES['ENABLE_CREATOR_GROUP'] = False ################### FRONTEND APPLICATION PUBLISHER URL ################### FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400' diff --git a/cms/templates/index.html b/cms/templates/index.html index dbd83b2ff518..12e50215ba6b 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -167,7 +167,7 @@

${_("Create a New Library")}

% else: diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index c28a38a2c7e7..8ca99f2d5d97 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -76,15 +76,11 @@ {% block wiki_breadcrumbs %}{% endblock %} {% if messages %} - {% comment %} - The message is not actually safe, but StatusAlertRenderer uses react which adds escaping, - so marking as safe keeps the message from being double-escaped. - {% endcomment %} {% for message in messages %}