From 62652c742260b221c1ab39953c2504d06503cd70 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Tue, 20 Jun 2023 15:30:27 +0500 Subject: [PATCH 1/6] enhancement: add support for caching programs for one site (#32380) (#32506) * perf!: add support for caching programs on per site bases --- .../management/commands/cache_programs.py | 13 +++- .../commands/tests/test_cache_programs.py | 77 +++++++++++++++---- 2 files changed, 72 insertions(+), 18 deletions(-) diff --git a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py index b815f8b4e4e5..9e0664f4930f 100644 --- a/openedx/core/djangoapps/catalog/management/commands/cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/cache_programs.py @@ -45,8 +45,17 @@ class Command(BaseCommand): """ help = "Rebuild the LMS' cache of program data." + def add_arguments(self, parser): + parser.add_argument( + '--domain', + dest='domain', + type=str, + help='Help in caching the programs for one site' + ) + # lint-amnesty, pylint: disable=bad-option-value, unicode-format-string def handle(self, *args, **options): # lint-amnesty, pylint: disable=too-many-statements + domain = options.get('domain', '') failure = False logger.info('populate-multitenant-programs switch is ON') @@ -68,7 +77,9 @@ def handle(self, *args, **options): # lint-amnesty, pylint: disable=too-many-st programs_by_type = {} programs_by_type_slug = {} organizations = {} - for site in Site.objects.all(): + + sites = Site.objects.filter(domain=domain) if domain else Site.objects.all() + for site in sites: site_config = getattr(site, 'configuration', None) if site_config is None or not site_config.get_value('COURSE_CATALOG_API_URL'): logger.info(f'Skipping site {site.domain}. No configuration.') diff --git a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py index 331d53a73042..4078b2d90d55 100644 --- a/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py +++ b/openedx/core/djangoapps/catalog/management/commands/tests/test_cache_programs.py @@ -48,29 +48,53 @@ def setUp(self): } ) + self.site_domain2 = 'testsite2.com' + self.site2 = self.set_up_site( + self.site_domain2, + { + 'COURSE_CATALOG_API_URL': self.catalog_integration.get_internal_api_url().rstrip('/') + } + ) + self.list_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/programs/' self.detail_tpl = self.list_url.rstrip('/') + '/{uuid}/' self.pathway_url = self.catalog_integration.get_internal_api_url().rstrip('/') + '/pathways/' self.programs = ProgramFactory.create_batch(3) + self.programs2 = ProgramFactory.create_batch(3) self.pathways = PathwayFactory.create_batch(3) + self.pathways2 = PathwayFactory.create_batch(3) self.child_program = ProgramFactory.create() + self.child_program2 = ProgramFactory.create() self.programs[0]['curricula'][0]['programs'].append(self.child_program) self.programs.append(self.child_program) - self.programs[0]['authoring_organizations'] = OrganizationFactory.create_batch(2) + self.programs2[0]['curricula'][0]['programs'].append(self.child_program2) + self.programs2.append(self.child_program2) + self.programs2[0]['authoring_organizations'] = OrganizationFactory.create_batch(2) + for pathway in self.pathways: self.programs += pathway['programs'] - self.uuids = [program['uuid'] for program in self.programs] + for pathway in self.pathways2: + self.programs2 += pathway['programs'] + + self.uuids = { + f"{self.site_domain}": [program["uuid"] for program in self.programs], + f"{self.site_domain2}": [program["uuid"] for program in self.programs2], + } # add some of the previously created programs to some pathways self.pathways[0]['programs'].extend([self.programs[0], self.programs[1]]) self.pathways[1]['programs'].append(self.programs[0]) - def mock_list(self): + # add some of the previously created programs to some pathways + self.pathways2[0]['programs'].extend([self.programs2[0], self.programs2[1]]) + self.pathways2[1]['programs'].append(self.programs2[0]) + + def mock_list(self, site=""): """ Mock the data returned by the program listing API endpoint. """ # pylint: disable=unused-argument def list_callback(request, uri, headers): @@ -81,8 +105,8 @@ def list_callback(request, uri, headers): 'uuids_only': ['1'] } assert request.querystring == expected - - return (200, headers, json.dumps(self.uuids)) + uuids = self.uuids[self.site_domain2] if site else self.uuids[self.site_domain] + return (200, headers, json.dumps(uuids)) httpretty.register_uri( httpretty.GET, @@ -130,17 +154,36 @@ def pathways_callback(request, uri, headers): # pylint: disable=unused-argument return (200, headers, json.dumps(body)) - # NOTE: httpretty does not properly match against query strings here (using match_querystring arg) - # as such, it does not actually look at the query parameters (for page num), but returns items in a LIFO order. - # this means that for multiple pages, you must call this function starting from the last page. - # we do assert the page number query param above, however httpretty.register_uri( httpretty.GET, - self.pathway_url, + self.pathway_url + f'?exclude_utm=1&page={page_number}', body=pathways_callback, content_type='application/json', + match_querystring=True, ) + def test_handle_domain(self): + """ + Verify that the command argument is working correct or not. + """ + UserFactory(username=self.catalog_integration.service_username) + + programs = { + PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']): program for program in self.programs2 + } + + self.mock_list(self.site2) + self.mock_pathways(self.pathways2) + + for uuid in self.uuids[self.site_domain2]: + program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] + self.mock_detail(uuid, program) + + call_command('cache_programs', f'--domain={self.site_domain2}') + + cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain2)) + assert set(cached_uuids) == set(self.uuids[self.site_domain2]) + def test_handle_programs(self): """ Verify that the command requests and caches program UUIDs and details. @@ -158,14 +201,14 @@ def test_handle_programs(self): self.mock_list() self.mock_pathways(self.pathways) - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) call_command('cache_programs') cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain)) - assert set(cached_uuids) == set(self.uuids) + assert set(cached_uuids) == set(self.uuids[self.site_domain]) program_keys = list(programs.keys()) cached_programs = cache.get_many(program_keys) @@ -228,7 +271,7 @@ def test_handle_pathways(self): self.mock_list() self.mock_pathways(self.pathways) - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -267,7 +310,7 @@ def test_pathways_multiple_pages(self): } self.mock_list() - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -337,7 +380,7 @@ def test_handle_missing_pathways(self): self.mock_list() - for uuid in self.uuids: + for uuid in self.uuids[self.site_domain]: program = programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -365,7 +408,7 @@ def test_handle_missing_programs(self): self.mock_list() - for uuid in self.uuids[:2]: + for uuid in self.uuids[self.site_domain][:2]: program = partial_programs[PROGRAM_CACHE_KEY_TPL.format(uuid=uuid)] self.mock_detail(uuid, program) @@ -375,7 +418,7 @@ def test_handle_missing_programs(self): assert context.value.code == 1 cached_uuids = cache.get(SITE_PROGRAM_UUIDS_CACHE_KEY_TPL.format(domain=self.site_domain)) - assert set(cached_uuids) == set(self.uuids) + assert set(cached_uuids) == set(self.uuids[self.site_domain]) program_keys = list(all_programs.keys()) cached_programs = cache.get_many(program_keys) From 42e84e386e833f6707eb2b6d4371ea8bd6cc693a Mon Sep 17 00:00:00 2001 From: ihor-romaniuk Date: Wed, 28 Sep 2022 18:03:46 +0300 Subject: [PATCH 2/6] fix: save scroll position on exit from video xblock fullscreen mode --- xmodule/js/src/video/04_video_full_screen.js | 23 ++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/xmodule/js/src/video/04_video_full_screen.js b/xmodule/js/src/video/04_video_full_screen.js index 0730300d105e..9da85d8dfb67 100644 --- a/xmodule/js/src/video/04_video_full_screen.js +++ b/xmodule/js/src/video/04_video_full_screen.js @@ -161,7 +161,21 @@ return this.videoFullScreen.height; } - /** + function notifyParent(fullscreenOpen) { + if (window !== window.parent) { + // This is used by the Learning MFE to know about changing fullscreen mode. + // The MFE is then able to respond appropriately and scroll window to the previous position. + window.parent.postMessage({ + type: 'plugin.videoFullScreen', + payload: { + open: fullscreenOpen + } + }, document.referrer + ); + } + } + + /** * Event handler to toggle fullscreen mode. * @param {jquery Event} event */ @@ -192,6 +206,8 @@ this.resizer.delta.reset().setMode('width'); } this.el.trigger('fullscreen', [this.isFullScreen]); + + this.videoFullScreen.notifyParent(false); } function handleEnter() { @@ -202,6 +218,8 @@ return; } + this.videoFullScreen.notifyParent(true); + this.videoFullScreen.fullScreenState = this.isFullScreen = true; fullScreenClassNameEl.addClass('video-fullscreen'); this.videoFullScreen.fullScreenEl @@ -267,7 +285,8 @@ handleFullscreenChange: handleFullscreenChange, toggle: toggle, toggleHandler: toggleHandler, - updateControlsHeight: updateControlsHeight + updateControlsHeight: updateControlsHeight, + notifyParent: notifyParent }; state.bindTo(methodsDict, state.videoFullScreen, state); From 9141a63d45318534c0b543f259c0fe41704fd1e3 Mon Sep 17 00:00:00 2001 From: Mariagabriela Jaimes Date: Fri, 14 Jul 2023 09:42:05 -0400 Subject: [PATCH 3/6] chore: upgrade Django to 3.2.20 (#32739) --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/testing.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a3bbb45cf8ed..0a2b02dd3ad1 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -177,7 +177,7 @@ defusedxml==0.7.1 # social-auth-core deprecated==1.2.13 # via jwcrypto -django==3.2.19 +django==3.2.20 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 58dc6727674c..b419cdf16d8a 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -273,7 +273,7 @@ distlib==0.3.6 # via # -r requirements/edx/testing.txt # virtualenv -django==3.2.19 +django==3.2.20 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 7bf33a176d50..39b22737bb2f 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -256,7 +256,7 @@ dill==0.3.6 # via pylint distlib==0.3.6 # via virtualenv -django==3.2.19 +django==3.2.20 # via # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt From 3efcd50a5b29f183582f0d4896e948719a82e4f2 Mon Sep 17 00:00:00 2001 From: Jesper Hodge <19345795+jesperhodge@users.noreply.github.com> Date: Tue, 25 Jul 2023 15:10:22 -0400 Subject: [PATCH 4/6] Merge pull request from GHSA-3q74-3rfh-g37j (#32838) This is a security fix that restricts capabilities to author libraries in studio to course authors/ members of an organization. Security advisory: https://github.com/openedx/edx-platform/security/advisories/GHSA-3q74-3rfh-g37j --- 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 +- 7 files changed, 142 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 023cb07199fe..664d1d34da4f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -53,7 +53,8 @@ CourseInstructorRole, CourseStaffRole, GlobalStaff, - UserBasedRole + UserBasedRole, + OrgStaffRole ) from common.djangoapps.util.course import get_link_for_about_page from common.djangoapps.util.date_utils import get_default_time_display @@ -597,6 +598,7 @@ 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), }) @@ -624,6 +626,7 @@ 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) @@ -1943,6 +1946,18 @@ 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. @@ -1950,6 +1965,18 @@ 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 8ec1e79d1932..328c92687b2c 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -35,14 +35,15 @@ from common.djangoapps.student.roles import ( CourseInstructorRole, CourseStaffRole, - LibraryUserRole + LibraryUserRole, + OrgStaffRole, + UserBasedRole, ) 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 @@ -79,10 +80,11 @@ def user_can_create_library(user, org=None): elif user.is_staff: return True elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - 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 + 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 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 cf9f2a4197ae..9fdbb425e51d 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, 20), - (False, 'staff', None, 0, 20), + (True, 'staff', None, 0, 21), + (False, 'staff', None, 0, 21), # Base user has global staff access - (True, 'user', ORG, 2, 20), - (False, 'user', ORG, 2, 20), - (True, 'user', None, 2, 20), - (False, 'user', None, 2, 20), + (True, 'user', ORG, 2, 21), + (False, 'user', ORG, 2, 21), + (True, 'user', None, 2, 21), + (False, 'user', None, 2, 21), ) @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 746e61cb3126..bac450b3bd26 100644 --- a/cms/djangoapps/contentstore/views/tests/test_library.py +++ b/cms/djangoapps/contentstore/views/tests/test_library.py @@ -19,11 +19,15 @@ 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 +from common.djangoapps.student.roles import LibraryUserRole, CourseStaffRole 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 @@ -51,26 +55,51 @@ 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(self): + 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) + # 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_no_course_creator_role(self): + def test_library_creator_status_with_course_staff_role_for_enabled_creator_group_setting(self): _, nostaff_user = self.create_non_staff_authed_user_client() - self.assertEqual(user_can_create_library(nostaff_user), True) + 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) @ddt.data( (False, False, True), @@ -188,9 +217,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_course_creator_group(self): + def test_lib_create_permission_no_course_creator_role_and_no_course_creator_group_and_no_course_staff_role(self): """ - Users who are not given course creator roles should not be able to create libraries + Users who are not given course creator roles or course staff role should not be able to create libraries if ENABLE_CREATOR_GROUP is enabled. """ self.client.logout() @@ -201,6 +230,23 @@ def test_lib_create_permission_no_course_creator_role_and_course_creator_group(s }) 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'}, @@ -405,3 +451,41 @@ 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 70b4f0781a0a..a9ea495b0841 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -215,6 +215,11 @@ # 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 1466d15b7e2c..58030a2fa914 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -156,7 +156,10 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing FEATURES['CERTIFICATES_HTML_VIEW'] = True ########################## AUTHOR PERMISSION ####################### -FEATURES['ENABLE_CREATOR_GROUP'] = False +FEATURES['ENABLE_CREATOR_GROUP'] = True + +########################## Library creation organizations restriction ####################### +FEATURES['ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES'] = True ################### 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 12e50215ba6b..dbd83b2ff518 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -167,7 +167,7 @@

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

% else: From 08f6fa66e9fc2bf89a9a69cbe973484f05969c4b Mon Sep 17 00:00:00 2001 From: Dmytro <98233552+DmytroAlipov@users.noreply.github.com> Date: Wed, 26 Jul 2023 00:54:30 +0300 Subject: [PATCH 5/6] fix: Incorrect symbols on wiki create article page for Palm (#32701) This is a backport from master: https://github.com/openedx/edx-platform/pull/32628 --- lms/templates/wiki/base.html | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/templates/wiki/base.html b/lms/templates/wiki/base.html index 8ca99f2d5d97..c28a38a2c7e7 100644 --- a/lms/templates/wiki/base.html +++ b/lms/templates/wiki/base.html @@ -76,11 +76,15 @@ {% 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 %}