Skip to content

Commit

Permalink
Merge tag 'open-release/palm.2' of github.com:EduTrigger/edx-platform…
Browse files Browse the repository at this point in the history
… into chuan-test2
  • Loading branch information
chuan-edutrigger committed Sep 14, 2023
2 parents 81788e6 + 085987e commit 2956108
Show file tree
Hide file tree
Showing 16 changed files with 270 additions and 60 deletions.
29 changes: 28 additions & 1 deletion cms/djangoapps/contentstore/views/course.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
})

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1943,13 +1946,37 @@ 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.
"""
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.
Expand Down
14 changes: 8 additions & 6 deletions cms/djangoapps/contentstore/views/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
12 changes: 6 additions & 6 deletions cms/djangoapps/contentstore/views/tests/test_course_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
96 changes: 90 additions & 6 deletions cms/djangoapps/contentstore/views/tests/test_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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()
Expand All @@ -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'},
Expand Down Expand Up @@ -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'])
5 changes: 5 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
5 changes: 4 additions & 1 deletion cms/envs/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion cms/templates/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ <h3 class="title">${_("Create a New Library")}</h3>
<input class="new-library-org" id="new-library-org" type="text" name="new-library-org" required placeholder="${_('e.g. UniversityX or OrganizationX')}" aria-describedby="tip-new-library-org tip-error-new-library-org" />
% else:
<select class="new-library-org" id="new-library-org" name="new-library-org" required aria-describedby="tip-new-library-org tip-error-new-library-org">
% for org in allowed_organizations:
% for org in allowed_organizations_for_libraries:
<option value="${org}">${org}</option>
% endfor
</select>
Expand Down
7 changes: 7 additions & 0 deletions lms/djangoapps/courseware/tests/test_discussion_xblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

from unittest import mock
import ddt
from django.conf import settings
from django.test.utils import override_settings
from django.urls import reverse
from opaque_keys.edx.keys import CourseKey
from web_fragments.fragment import Fragment
Expand Down Expand Up @@ -167,6 +169,7 @@ def test_studio_view(self):
}
)

@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
@ddt.data(
(False, False, False),
(True, False, False),
Expand Down Expand Up @@ -231,6 +234,7 @@ def test_studio_view(self):
fragment = self.block.author_view({})
assert f'data-discussion-id="{self.discussion_id}"' in fragment.content

@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
@ddt.data(
(True, False, False),
(False, True, False),
Expand Down Expand Up @@ -290,6 +294,7 @@ def get_root(self, block):
block = block.get_parent()
return block

@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
def test_html_with_user(self):
"""
Test rendered DiscussionXBlock permissions.
Expand All @@ -308,6 +313,7 @@ def test_html_with_user(self):
assert 'data-user-create-comment="false"' in html
assert 'data-user-create-subcomment="false"' in html

@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
def test_discussion_render_successfully_with_orphan_parent(self):
"""
Test that discussion xblock render successfully
Expand Down Expand Up @@ -407,6 +413,7 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase):
Test the number of queries executed when rendering the XBlock.
"""

@override_settings(FEATURES=dict(settings.FEATURES, ENABLE_DISCUSSION_SERVICE='True'))
def test_permissions_query_load(self):
"""
Tests that the permissions queries are cached when rendering numerous discussion XBlocks.
Expand Down
6 changes: 5 additions & 1 deletion lms/templates/wiki/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
<div id="alert_stat_bar"></div>
<script type="text/javascript">
new StatusAlertRenderer(
"{{ message }}",
"{{ message|safe }}",
"#alert_stat_bar",
".nav nav-tabs"
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand All @@ -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.')
Expand Down
Loading

0 comments on commit 2956108

Please sign in to comment.