diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index 1afc51ed77af..0aa06d8b8dcc 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -51,6 +51,7 @@ class CourseHomeSerializer(serializers.Serializer): allow_empty=True ) archived_courses = CourseCommonSerializer(required=False, many=True) + can_access_advanced_settings = serializers.BooleanField() can_create_organizations = serializers.BooleanField() course_creator_status = serializers.CharField() courses = CourseCommonSerializer(required=False, many=True) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index d41ceb2647c5..d72042cff611 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -52,6 +52,7 @@ def get(self, request: Request): "allow_unicode_course_id": false, "allowed_organizations": [], "archived_courses": [], + "can_access_advanced_settings": true, "can_create_organizations": true, "course_creator_status": "granted", "courses": [], diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py index eafc2b37aa0c..4ac97b8e943d 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py @@ -61,8 +61,8 @@ def test_course_index_response(self): "blocks": [], "advance_settings_url": f"/settings/advanced/{self.course.id}" }, - "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_feedback_url": "https://discuss.openedx.org/t/new-and-improved-discussions-forum/9183", # lint-amnesty, pylint: disable=line-too-long + "discussions_incontext_learnmore_url": "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/open-release-redwood.master/manage_discussions/discussions.html", # lint-amnesty, pylint: disable=line-too-long "is_custom_relative_dates_active": True, "initial_state": None, "initial_user_clipboard": { @@ -102,8 +102,8 @@ def test_course_index_response_with_show_locators(self): "blocks": [], "advance_settings_url": f"/settings/advanced/{self.course.id}" }, - "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_feedback_url": "https://discuss.openedx.org/t/new-and-improved-discussions-forum/9183", # lint-amnesty, pylint: disable=line-too-long + "discussions_incontext_learnmore_url": "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/open-release-redwood.master/manage_discussions/discussions.html", # lint-amnesty, pylint: disable=line-too-long "is_custom_relative_dates_active": False, "initial_state": { "expanded_locators": [ diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index 1b8bfaa84728..a8b4cf5e3933 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -44,6 +44,7 @@ def test_home_page_courses_response(self): "allow_unicode_course_id": False, "allowed_organizations": [], "archived_courses": [], + "can_access_advanced_settings": True, "can_create_organizations": True, "course_creator_status": "granted", "courses": [], diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index faaf9dc7e1ca..863863238036 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -456,11 +456,11 @@ def sync_discussion_settings(course_key, user): if ( ENABLE_NEW_STRUCTURE_DISCUSSIONS.is_enabled() and not course.discussions_settings['provider_type'] == Provider.OPEN_EDX + and not course.discussions_settings['provider'] == Provider.OPEN_EDX ): LOGGER.info(f"New structure is enabled, also updating {course_key} to use new provider") course.discussions_settings['enable_graded_units'] = False course.discussions_settings['unit_level_visibility'] = True - course.discussions_settings['provider'] = Provider.OPEN_EDX course.discussions_settings['provider_type'] = Provider.OPEN_EDX modulestore().update_item(course, user.id) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 39e826b3e4f3..697645fd825e 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1372,6 +1372,16 @@ def test_create_course_with_unicode_in_id_disabled(self): self.course_data['run'] = '����������' self.assert_create_course_failed(error_message) + @override_settings(DEFAULT_COURSE_INVITATION_ONLY=True) + def test_create_course_invitation_only(self): + """ + Test new course creation with setting: DEFAULT_COURSE_INVITATION_ONLY=True. + """ + test_course_data = self.assert_created_course() + course_id = _get_course_id(self.store, test_course_data) + course = self.store.get_course(course_id) + self.assertEqual(course.invitation_only, True) + def assert_course_permission_denied(self): """ Checks that the course did not get created due to a PermissionError. diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index ad68ea0c4318..3080d3582568 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -1713,6 +1713,7 @@ def get_home_context(request, no_course=False): 'allowed_organizations': get_allowed_organizations(user), 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(user), 'can_create_organizations': user_can_create_organizations(user), + 'can_access_advanced_settings': auth.has_studio_advanced_settings_access(user), } return home_context diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index dce82809dfad..0cda97012c94 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -990,8 +990,9 @@ def create_new_course_in_store(store, user, org, number, run, fields): # Set default language from settings and enable web certs fields.update({ - 'language': getattr(settings, 'DEFAULT_COURSE_LANGUAGE', 'en'), 'cert_html_view_enabled': True, + 'invitation_only': getattr(settings, 'DEFAULT_COURSE_INVITATION_ONLY', False), + 'language': getattr(settings, 'DEFAULT_COURSE_LANGUAGE', 'en'), }) with modulestore().default_store(store): diff --git a/cms/envs/common.py b/cms/envs/common.py index d8e5105e6720..ac29857ac5a8 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -591,6 +591,25 @@ # .. toggle_use_cases: open_edx # .. toggle_creation_date: 2024-04-10 'BADGES_ENABLED': False, + + # .. toggle_name: FEATURES['IN_CONTEXT_DISCUSSION_ENABLED_DEFAULT'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: True + # .. toggle_description: Set to False to not enable in-context discussion for units by default. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2024-09-02 + 'IN_CONTEXT_DISCUSSION_ENABLED_DEFAULT': True, + + # .. toggle_name: FEATURES['ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the legacy MD5 hashing algorithm to generate anonymous user id + # instead of the newer SHAKE128 hashing algorithm + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-08-08 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832' + 'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False, } # .. toggle_name: ENABLE_COPPA_COMPLIANCE @@ -2880,8 +2899,9 @@ BRAZE_COURSE_ENROLLMENT_CANVAS_ID = '' -DISCUSSIONS_INCONTEXT_FEEDBACK_URL = '' -DISCUSSIONS_INCONTEXT_LEARNMORE_URL = '' +# pylint: disable=line-too-long +DISCUSSIONS_INCONTEXT_FEEDBACK_URL = "https://discuss.openedx.org/t/new-and-improved-discussions-forum/9183" +DISCUSSIONS_INCONTEXT_LEARNMORE_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/open-release-redwood.master/manage_discussions/discussions.html" #### django-simple-history## # disable indexing on date field its coming django-simple-history. @@ -3003,3 +3023,7 @@ def _should_send_learning_badge_events(settings): # See https://www.meilisearch.com/docs/learn/security/tenant_tokens MEILISEARCH_INDEX_PREFIX = "" MEILISEARCH_API_KEY = "devkey" + +############## Default value for invitation_only when creating courses ############## + +DEFAULT_COURSE_INVITATION_ONLY = False diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index d3dc51616c75..de9ccfc7bf2f 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -73,7 +73,7 @@ def user_has_role(user, role): return False -def get_user_permissions(user, course_key, org=None): +def get_user_permissions(user, course_key, org=None, service_variant=None): """ Get the bitmask of permissions that this user has in the given course context. Can also set course_key=None and pass in an org to get the user's @@ -103,7 +103,7 @@ def get_user_permissions(user, course_key, org=None): # the LMS and Studio permissions will be separated as a part of this project. Once this is done (and this code is # not removed during its implementation), we can replace the Limited Staff permissions with more granular ones. if course_key and user_has_role(user, CourseLimitedStaffRole(course_key)): - if settings.SERVICE_VARIANT == 'lms': + if (service_variant or settings.SERVICE_VARIANT) == 'lms': return STUDIO_EDIT_CONTENT else: return STUDIO_NO_PERMISSIONS @@ -119,7 +119,7 @@ def get_user_permissions(user, course_key, org=None): return STUDIO_NO_PERMISSIONS -def has_studio_write_access(user, course_key): +def has_studio_write_access(user, course_key, service_variant=None): """ Return True if user has studio write access to the given course. Note that the CMS permissions model is with respect to courses. @@ -131,15 +131,17 @@ def has_studio_write_access(user, course_key): :param user: :param course_key: a CourseKey + :param service_variant: the variant of the service (lms or cms). Permissions may differ between the two, + see the comment in get_user_permissions for more details. """ - return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key)) + return bool(STUDIO_EDIT_CONTENT & get_user_permissions(user, course_key, service_variant=service_variant)) -def has_course_author_access(user, course_key): +def has_course_author_access(user, course_key, service_variant=None): """ Old name for has_studio_write_access """ - return has_studio_write_access(user, course_key) + return has_studio_write_access(user, course_key, service_variant) def has_studio_advanced_settings_access(user): diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 2a4a9deb3664..cb806fed4467 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -154,12 +154,21 @@ def anonymous_id_for_user(user, course_id): # function: Rotate at will, since the hashes are stored and # will not change. # include the secret key as a salt, and to make the ids unique across different LMS installs. - hasher = hashlib.shake_128() + legacy_hash_enabled = settings.FEATURES.get('ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID', False) + if legacy_hash_enabled: + # Use legacy MD5 algorithm if flag enabled + hasher = hashlib.md5() + else: + hasher = hashlib.shake_128() hasher.update(settings.SECRET_KEY.encode('utf8')) hasher.update(str(user.id).encode('utf8')) if course_id: hasher.update(str(course_id).encode('utf-8')) - anonymous_user_id = hasher.hexdigest(16) + + if legacy_hash_enabled: + anonymous_user_id = hasher.hexdigest() + else: + anonymous_user_id = hasher.hexdigest(16) # pylint: disable=too-many-function-args try: AnonymousUserId.objects.create( diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py index 971433c9c523..cd7ced4e9fdd 100644 --- a/common/djangoapps/student/roles.py +++ b/common/djangoapps/student/roles.py @@ -211,7 +211,7 @@ class GlobalStaff(AccessRole): The global staff role """ def has_user(self, user): - return bool(user and user.is_staff) + return bool(user and (user.is_superuser or user.is_staff)) def add_users(self, *users): for user in users: @@ -361,6 +361,25 @@ class CourseLimitedStaffRole(CourseStaffRole): BASE_ROLE = CourseStaffRole.ROLE +@register_access_role +class eSHEInstructorRole(CourseStaffRole): + """A Staff member of a course without access to the membership tab and enrollment-related operations.""" + + ROLE = 'eshe_instructor' + BASE_ROLE = CourseStaffRole.ROLE + + +@register_access_role +class TeachingAssistantRole(CourseStaffRole): + """ + A Staff member of a course without access to the membership tab, enrollment-related operations and + grade-related operations. + """ + + ROLE = 'teaching_assistant' + BASE_ROLE = CourseStaffRole.ROLE + + @register_access_role class CourseInstructorRole(CourseRole): """A course Instructor""" diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py index da1aad19a803..816e20fa198b 100644 --- a/common/djangoapps/student/tests/test_roles.py +++ b/common/djangoapps/student/tests/test_roles.py @@ -17,6 +17,8 @@ CourseStaffRole, CourseFinanceAdminRole, CourseSalesAdminRole, + eSHEInstructorRole, + TeachingAssistantRole, LibraryUserRole, CourseDataResearcherRole, GlobalStaff, @@ -199,6 +201,8 @@ class RoleCacheTestCase(TestCase): # lint-amnesty, pylint: disable=missing-clas ROLES = ( (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX')), (CourseLimitedStaffRole(IN_KEY), ('limited_staff', IN_KEY, 'edX')), + (eSHEInstructorRole(IN_KEY), ('eshe_instructor', IN_KEY, 'edX')), + (TeachingAssistantRole(IN_KEY), ('teaching_assistant', IN_KEY, 'edX')), (CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX')), (OrgStaffRole(IN_KEY.org), ('staff', None, 'edX')), (CourseFinanceAdminRole(IN_KEY), ('finance_admin', IN_KEY, 'edX')), diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 9e61095260b6..5d393149c406 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -1064,6 +1064,17 @@ def test_anonymous_id_secret_key_changes_result_in_diff_values_for_same_new_user assert anonymous_id != new_anonymous_id assert self.user == user_by_anonymous_id(new_anonymous_id) + def test_enable_legacy_hash_flag(self): + """Test that different anonymous id returned if ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID enabled.""" + CourseEnrollment.enroll(self.user, self.course.id) + anonymous_id = anonymous_id_for_user(self.user, self.course.id) + with patch.dict(settings.FEATURES, ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID=True): + # Recreate user object to clear cached anonymous id. + self.user = User.objects.get(pk=self.user.id) + AnonymousUserId.objects.filter(user=self.user).filter(course_id=self.course.id).delete() + new_anonymous_id = anonymous_id_for_user(self.user, self.course.id) + assert anonymous_id != new_anonymous_id + @skip_unless_lms @patch('openedx.core.djangoapps.programs.utils.get_programs') diff --git a/lms/djangoapps/course_home_api/course_metadata/serializers.py b/lms/djangoapps/course_home_api/course_metadata/serializers.py index 7683a9089453..ccb22e86b9fa 100644 --- a/lms/djangoapps/course_home_api/course_metadata/serializers.py +++ b/lms/djangoapps/course_home_api/course_metadata/serializers.py @@ -58,3 +58,4 @@ class CourseHomeMetadataSerializer(VerifiedModeSerializer): can_view_certificate = serializers.BooleanField() course_modes = CourseModeSerrializer(many=True) is_new_discussion_sidebar_view_enabled = serializers.BooleanField() + has_course_author_access = serializers.BooleanField() diff --git a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py index 4b3524eb1ee2..86aeee883a7c 100644 --- a/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py +++ b/lms/djangoapps/course_home_api/course_metadata/tests/test_views.py @@ -12,7 +12,12 @@ from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment -from common.djangoapps.student.roles import CourseInstructorRole +from common.djangoapps.student.roles import ( + CourseBetaTesterRole, + CourseInstructorRole, + CourseLimitedStaffRole, + CourseStaffRole +) from common.djangoapps.student.tests.factories import UserFactory from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests from lms.djangoapps.courseware.toggles import ( @@ -248,3 +253,32 @@ def test_discussion_tab_visible(self, visible): assert 'discussion' in tab_ids else: assert 'discussion' not in tab_ids + + @ddt.data( + { + 'course_team_role': None, + 'has_course_author_access': False + }, + { + 'course_team_role': CourseBetaTesterRole, + 'has_course_author_access': False + }, + { + 'course_team_role': CourseStaffRole, + 'has_course_author_access': True + }, + { + 'course_team_role': CourseLimitedStaffRole, + 'has_course_author_access': False + }, + ) + @ddt.unpack + def test_has_course_author_access_for_staff_roles(self, course_team_role, has_course_author_access): + CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED) + + if course_team_role: + course_team_role(self.course.id).add_users(self.user) + + response = self.client.get(self.url) + assert response.status_code == 200 + assert response.data['has_course_author_access'] == has_course_author_access diff --git a/lms/djangoapps/course_home_api/course_metadata/views.py b/lms/djangoapps/course_home_api/course_metadata/views.py index 248f90389d40..7ad2a2537000 100644 --- a/lms/djangoapps/course_home_api/course_metadata/views.py +++ b/lms/djangoapps/course_home_api/course_metadata/views.py @@ -16,6 +16,7 @@ from openedx.core.djangoapps.courseware_api.utils import get_celebrations_dict from common.djangoapps.course_modes.models import CourseMode +from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.student.models import CourseEnrollment from lms.djangoapps.course_api.api import course_detail from lms.djangoapps.course_goals.models import UserActivity @@ -139,6 +140,12 @@ def get(self, request, *args, **kwargs): 'can_view_certificate': certificates_viewable_for_course(course), 'course_modes': course_modes, 'is_new_discussion_sidebar_view_enabled': new_discussion_sidebar_view_is_enabled(course_key), + # We check the course author access in the context of CMS here because this field is used + # to determine whether the user can access the course authoring tools in the CMS. + # This is a temporary solution until the course author role is split into "Course Author" and + # "Course Editor" as described in the permission matrix here: + # https://github.com/openedx/platform-roadmap/issues/246 + 'has_course_author_access': has_course_author_access(request.user, course_key, 'cms'), } context = self.get_serializer_context() context['course'] = course diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 2a67b6454e42..de38871832c1 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -315,6 +315,12 @@ def link_func(course, _reverse_func): tab_dict['link_func'] = link_func super().__init__(tab_dict) + @classmethod + def is_enabled(cls, course, user=None): + if settings.FEATURES.get('DISABLE_DATES_TAB'): + return False + return super().is_enabled(course, user) + def get_course_tab_list(user, course): """ diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 6ad7ef73de01..e882efd3cbbf 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -885,3 +885,16 @@ def test_singular_dates_tab(self): if tab.type == 'dates': num_dates_tabs += 1 assert num_dates_tabs == 1 + + def test_dates_tab_is_enabled_by_default(self): + """Test dates tab is enabled by default.""" + tab = DatesTab({'type': DatesTab.type, 'name': 'dates'}) + user = self.create_mock_user() + assert self.is_tab_enabled(tab, self.course, user) + + @patch.dict("django.conf.settings.FEATURES", {"DISABLE_DATES_TAB": True}) + def test_dates_tab_disabled_by_feature_flag(self): + """Test dates tab is disabled by the feature flag.""" + tab = DatesTab({'type': DatesTab.type, 'name': 'dates'}) + user = self.create_mock_user() + assert not self.is_tab_enabled(tab, self.course, user) diff --git a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py index cd2107ec7c29..656e7e6b4396 100644 --- a/lms/djangoapps/grades/rest_api/v1/tests/test_views.py +++ b/lms/djangoapps/grades/rest_api/v1/tests/test_views.py @@ -302,7 +302,7 @@ def setUpClass(cls): + [ { 'category': 'Homework', - 'detail': 'Homework Average = 0%', + 'detail': 'Homework Average = 0.00%', 'label': 'HW Avg', 'percent': 0.0, 'prominent': True } @@ -332,21 +332,21 @@ def setUpClass(cls): }, { 'category': 'Lab', - 'detail': 'Lab Average = 0%', + 'detail': 'Lab Average = 0.00%', 'label': 'Lab Avg', 'percent': 0.0, 'prominent': True }, { 'category': 'Midterm Exam', - 'detail': 'Midterm Exam = 0%', + 'detail': 'Midterm Exam = 0.00%', 'label': 'Midterm', 'percent': 0.0, 'prominent': True }, { 'category': 'Final Exam', - 'detail': 'Final Exam = 0%', + 'detail': 'Final Exam = 0.00%', 'label': 'Final', 'percent': 0.0, 'prominent': True diff --git a/lms/djangoapps/grades/scores.py b/lms/djangoapps/grades/scores.py index f621d85ea17b..38dd0dc18926 100644 --- a/lms/djangoapps/grades/scores.py +++ b/lms/djangoapps/grades/scores.py @@ -162,8 +162,8 @@ def compute_percent(earned, possible): Returns the percentage of the given earned and possible values. """ if possible > 0: - # Rounds to two decimal places. - return around(earned / possible, decimals=2) + # Rounds to four decimal places. + return around(earned / possible, decimals=4) else: return 0.0 diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index 4e3bcde0ad95..2066b6cd0d7c 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/lms/djangoapps/grades/tests/test_course_grade_factory.py @@ -185,26 +185,26 @@ def test_course_grade_summary(self): 'section_breakdown': [ { 'category': 'Homework', - 'detail': 'Homework 1 - Test Sequential X with an & Ampersand - 50% (1/2)', + 'detail': 'Homework 1 - Test Sequential X with an & Ampersand - 50.00% (1/2)', 'label': 'HW 01', 'percent': 0.5 }, { 'category': 'Homework', - 'detail': 'Homework 2 - Test Sequential A - 0% (0/1)', + 'detail': 'Homework 2 - Test Sequential A - 0.00% (0/1)', 'label': 'HW 02', 'percent': 0.0 }, { 'category': 'Homework', - 'detail': 'Homework Average = 25%', + 'detail': 'Homework Average = 25.00%', 'label': 'HW Avg', 'percent': 0.25, 'prominent': True }, { 'category': 'NoCredit', - 'detail': 'NoCredit Average = 0%', + 'detail': 'NoCredit Average = 0.00%', 'label': 'NC Avg', 'percent': 0, 'prominent': True diff --git a/lms/djangoapps/grades/tests/test_subsection_grade.py b/lms/djangoapps/grades/tests/test_subsection_grade.py index 7dd39af4ece2..2398e7a71000 100644 --- a/lms/djangoapps/grades/tests/test_subsection_grade.py +++ b/lms/djangoapps/grades/tests/test_subsection_grade.py @@ -14,7 +14,7 @@ @ddt class SubsectionGradeTest(GradeTestBase): # lint-amnesty, pylint: disable=missing-class-docstring - @data((50, 100, .50), (59.49, 100, .59), (59.51, 100, .60), (59.50, 100, .60), (60.5, 100, .60)) + @data((50, 100, .5), (.5949, 100, .0059), (.5951, 100, .006), (.595, 100, .0059), (.605, 100, .006)) @unpack def test_create_and_read(self, mock_earned, mock_possible, expected_result): with mock_get_score(mock_earned, mock_possible): diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index 9255d113f038..c5bc5fb83370 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -19,6 +19,8 @@ CourseInstructorRole, CourseLimitedStaffRole, CourseStaffRole, + eSHEInstructorRole, + TeachingAssistantRole, ) from lms.djangoapps.instructor.enrollment import enroll_email, get_email_params from openedx.core.djangoapps.django_comment_common.models import Role @@ -30,6 +32,8 @@ 'instructor': CourseInstructorRole, 'staff': CourseStaffRole, 'limited_staff': CourseLimitedStaffRole, + 'eshe_instructor': eSHEInstructorRole, + 'teaching_assistant': TeachingAssistantRole, 'ccx_coach': CourseCcxCoachRole, 'data_researcher': CourseDataResearcherRole, } diff --git a/lms/djangoapps/instructor/permissions.py b/lms/djangoapps/instructor/permissions.py index e1a1cbf466f6..a06905a0a35d 100644 --- a/lms/djangoapps/instructor/permissions.py +++ b/lms/djangoapps/instructor/permissions.py @@ -36,6 +36,21 @@ VIEW_ENROLLMENTS = 'instructor.view_enrollments' VIEW_FORUM_MEMBERS = 'instructor.view_forum_members' +# Due to how the roles iheritance is implemented currently, eshe_instructor and teaching_assistant have implicit +# staff access, but unlike staff, they shouldn't be able to enroll and do grade-related operations as per client's +# requirements. At the same time, all other staff-derived roles, like Limited Staff, should be able to enroll students. +# This solution is far from perfect, but it's probably the best we can do untill the roles system is reworked. +_is_teaching_assistant = HasRolesRule('teaching_assistant') +_is_eshe_instructor = HasRolesRule('eshe_instructor') +_is_eshe_instructor_or_teaching_assistant = _is_teaching_assistant | _is_eshe_instructor +is_staff_but_not_teaching_assistant = ( + (_is_teaching_assistant & HasAccessRule('staff', strict=True)) | + (~_is_teaching_assistant & HasAccessRule('staff')) +) +is_staff_but_not_eshe_instructor_or_teaching_assistant = ( + (_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff', strict=True)) | + (~_is_eshe_instructor_or_teaching_assistant & HasAccessRule('staff')) +) perms[ALLOW_STUDENT_TO_BYPASS_ENTRANCE_EXAM] = HasAccessRule('staff') perms[ASSIGN_TO_COHORTS] = HasAccessRule('staff') @@ -49,17 +64,17 @@ perms[START_CERTIFICATE_REGENERATION] = is_staff | HasAccessRule('instructor') perms[CERTIFICATE_EXCEPTION_VIEW] = is_staff | HasAccessRule('instructor') perms[CERTIFICATE_INVALIDATION_VIEW] = is_staff | HasAccessRule('instructor') -perms[GIVE_STUDENT_EXTENSION] = HasAccessRule('staff') +perms[GIVE_STUDENT_EXTENSION] = is_staff_but_not_teaching_assistant perms[VIEW_ISSUED_CERTIFICATES] = HasAccessRule('staff') | HasRolesRule('data_researcher') # only global staff or those with the data_researcher role can access the data download tab # HasAccessRule('staff') also includes course staff perms[CAN_RESEARCH] = is_staff | HasRolesRule('data_researcher') -perms[CAN_ENROLL] = HasAccessRule('staff') +perms[CAN_ENROLL] = is_staff_but_not_eshe_instructor_or_teaching_assistant perms[CAN_BETATEST] = HasAccessRule('instructor') perms[ENROLLMENT_REPORT] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[VIEW_COUPONS] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[EXAM_RESULTS] = HasAccessRule('staff') -perms[OVERRIDE_GRADES] = HasAccessRule('staff') +perms[OVERRIDE_GRADES] = is_staff_but_not_teaching_assistant perms[SHOW_TASKS] = HasAccessRule('staff') | HasRolesRule('data_researcher') perms[EMAIL] = HasAccessRule('staff') perms[RESCORE_EXAMS] = HasAccessRule('instructor') diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 400e9b556283..67bffae5991c 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -30,6 +30,7 @@ from lms.djangoapps.courseware.tabs import get_course_tab_list from lms.djangoapps.courseware.tests.factories import StudentModuleFactory from lms.djangoapps.courseware.tests.helpers import LoginEnrollmentTestCase +from lms.djangoapps.discussion.django_comment_client.tests.factories import RoleFactory from lms.djangoapps.grades.config.waffle import WRITABLE_GRADEBOOK from lms.djangoapps.instructor.toggles import DATA_DOWNLOAD_V2 from lms.djangoapps.instructor.views.gradebook_api import calculate_page_info @@ -38,6 +39,7 @@ ENABLE_PAGES_AND_RESOURCES_MICROFRONTEND, OVERRIDE_DISCUSSION_LEGACY_SETTINGS_FLAG ) +from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_ADMINISTRATOR from openedx.core.djangoapps.site_configuration.models import SiteConfiguration @@ -314,6 +316,103 @@ def test_membership_reason_field_visibility(self, enbale_reason_field): else: self.assertNotContains(response, reason_field) + @ddt.data('eshe_instructor', 'teaching_assistant') + def test_membership_tab_content(self, role): + """ + Verify that eSHE Instructors and Teaching Assistants don't have access to membership tab and + work correctly with other roles. + """ + + membership_section = ( + '
  • ' + '' + '
    • ' + ) + batch_enrollment = ( + '
    ' + ) + + user = UserFactory.create() + self.client.login(username=user.username, password=self.TEST_PASSWORD) + + # eSHE Instructors / Teaching Assistants shouldn't have access to membership tab + CourseAccessRoleFactory( + course_id=self.course.id, + user=user, + role=role, + org=self.course.id.org + ) + response = self.client.get(self.url) + self.assertNotContains(response, membership_section) + + # However if combined with forum_admin, they should have access to this + # tab, but not to batch enrollment + forum_admin_role = RoleFactory(name=FORUM_ROLE_ADMINISTRATOR, course_id=self.course.id) + forum_admin_role.users.add(user) + response = self.client.get(self.url) + self.assertContains(response, membership_section) + self.assertNotContains(response, batch_enrollment) + + # Combined with course staff, should have union of all three roles + # permissions sets + CourseAccessRoleFactory( + course_id=self.course.id, + user=user, + role='staff', + org=self.course.id.org + ) + response = self.client.get(self.url) + self.assertContains(response, membership_section) + self.assertContains(response, batch_enrollment) + + def test_student_admin_tab_content(self): + """ + Verify that Teaching Assistants don't have access to the gradebook-related sections + of the student admin tab. + """ + + # Should be visible to Teaching Assistants + view_enrollment_status = '
    ' + view_progress = '
    ' + + # Should not be visible to Teaching Assistants + view_gradebook = '
    ' + adjust_learner_grade = '
    ' + adjust_all_learners_grades = '
    ' + + user = UserFactory.create() + self.client.login(username=user.username, password=self.TEST_PASSWORD) + + # Teaching Assistants shouldn't have access to the gradebook-related sections + CourseAccessRoleFactory( + course_id=self.course.id, + user=user, + role='teaching_assistant', + org=self.course.id.org + ) + response = self.client.get(self.url) + self.assertContains(response, view_enrollment_status) + self.assertContains(response, view_progress) + self.assertNotContains(response, view_gradebook) + self.assertNotContains(response, adjust_learner_grade) + self.assertNotContains(response, adjust_all_learners_grades) + + # However if combined with instructor, they should have access to all sections + CourseAccessRoleFactory( + course_id=self.course.id, + user=user, + role='instructor', + org=self.course.id.org + ) + response = self.client.get(self.url) + self.assertContains(response, view_enrollment_status) + self.assertContains(response, view_progress) + self.assertContains(response, view_gradebook) + self.assertContains(response, adjust_learner_grade) + self.assertContains(response, adjust_all_learners_grades) + def test_student_admin_staff_instructor(self): """ Verify that staff users are not able to see course-wide options, while still diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index a068ffdf1c9d..4e976b18932d 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -31,7 +31,10 @@ CourseFinanceAdminRole, CourseInstructorRole, CourseSalesAdminRole, - CourseStaffRole + CourseStaffRole, + eSHEInstructorRole, + TeachingAssistantRole, + strict_role_checking, ) from common.djangoapps.util.json_request import JsonResponse from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled @@ -122,6 +125,8 @@ def instructor_dashboard_2(request, course_id): # lint-amnesty, pylint: disable access = { 'admin': request.user.is_staff, 'instructor': bool(has_access(request.user, 'instructor', course)), + 'eshe_instructor': eSHEInstructorRole(course_key).has_user(request.user), + 'teaching_assistant': TeachingAssistantRole(course_key).has_user(request.user), 'finance_admin': CourseFinanceAdminRole(course_key).has_user(request.user), 'sales_admin': CourseSalesAdminRole(course_key).has_user(request.user), 'staff': bool(has_access(request.user, 'staff', course)), @@ -129,6 +134,9 @@ def instructor_dashboard_2(request, course_id): # lint-amnesty, pylint: disable 'data_researcher': request.user.has_perm(permissions.CAN_RESEARCH, course_key), } + with strict_role_checking(): + access['explicit_staff'] = bool(has_access(request.user, 'staff', course)) + if not request.user.has_perm(permissions.VIEW_DASHBOARD, course_key): raise Http404() @@ -514,7 +522,19 @@ def _section_membership(course, access): 'update_forum_role_membership', kwargs={'course_id': str(course_key)} ), - 'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False) + 'is_reason_field_enabled': configuration_helpers.get_value('ENABLE_MANUAL_ENROLLMENT_REASON_FIELD', False), + + # Membership section should be hidden for eSHE instructors. + # Since they get Course Staff role implicitly, we need to hide this + # section if the user doesn't have the Course Staff role set explicitly + # or have the Discussion Admin role. + 'is_hidden': ( + not access['forum_admin'] + and ( + (access['eshe_instructor'] or access['teaching_assistant']) + and not access['explicit_staff'] + ) + ), } return section_data diff --git a/lms/envs/common.py b/lms/envs/common.py index 5f8714ca9cf7..1208a9199ddf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1085,6 +1085,46 @@ # .. toggle_creation_date: 2024-04-02 # .. toggle_target_removal_date: None 'BADGES_ENABLED': False, + + # .. toggle_name: FEATURES['ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the legacy MD5 hashing algorithm to generate anonymous user id + # instead of the newer SHAKE128 hashing algorithm + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2022-08-08 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/openedx/edx-platform/pull/30832' + 'ENABLE_LEGACY_MD5_HASH_FOR_ANONYMOUS_USER_ID': False, + + # .. toggle_name: FEATURES['ENABLE_ESHE_INSTRUCTOR_ROLE'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the ESHE Instructor role + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2023-07-31 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/561/files' + 'ENABLE_ESHE_INSTRUCTOR_ROLE': False, + + # .. toggle_name: FEATURES['ENABLE_TEACHING_ASSISTANT_ROLE'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Whether to enable the Teaching Assistant role + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2024-02-12 + # .. toggle_target_removal_date: None + # .. toggle_tickets: 'https://github.com/open-craft/edx-platform/pull/632/files' + 'ENABLE_TEACHING_ASSISTANT_ROLE': False, + + # .. toggle_name: FEATURES['DISABLE_DATES_TAB'] + # .. toggle_implementation: DjangoSetting + # .. toggle_default: False + # .. toggle_description: Disables dates tab for all courses. + # .. toggle_use_cases: open_edx + # .. toggle_creation_date: 2024-04-15 + # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/34511 + 'DISABLE_DATES_TAB': False, } # Specifies extra XBlock fields that should available when requested via the Course Blocks API diff --git a/lms/static/completion/js/ViewedEvent.js b/lms/static/completion/js/ViewedEvent.js index 1ad936f8a158..b2b9d5ff5bc4 100644 --- a/lms/static/completion/js/ViewedEvent.js +++ b/lms/static/completion/js/ViewedEvent.js @@ -109,7 +109,13 @@ export class ViewedEventTracker { constructor() { this.elementViewings = new Set(); this.handlers = []; - this.registerDomHandlers(); + if (window === window.parent) { + // Preview (legacy LMS frontend). + this.registerDomHandlers(); + } else { + // Learning MFE. + window.addEventListener('message', this.handleVisibilityMessage.bind(this)); + } } /** Add an element to track. */ @@ -121,7 +127,11 @@ export class ViewedEventTracker { (el, event) => this.callHandlers(el, event), ), ); - this.updateVisible(); + // Update visibility status immediately after adding the element (in case it's already visible). + // We don't need this for the Learning MFE because it will send a message once the iframe is loaded. + if (window === window.parent) { + this.updateVisible(); + } } /** Register a new handler to be called when an element has been viewed. */ @@ -177,4 +187,36 @@ export class ViewedEventTracker { handler(el, event); }); } + + /** Handle a unit.visibilityStatus message from the Learning MFE. */ + handleVisibilityMessage(event) { + if (event.data.type === 'unit.visibilityStatus') { + const { topPosition, viewportHeight } = event.data.data; + + this.elementViewings.forEach((elv) => { + const rect = elv.getBoundingRect(); + let visible = false; + + // Convert iframe-relative rect coordinates to be relative to the parent's viewport. + const elTopPosition = rect.top + topPosition; + const elBottomPosition = rect.bottom + topPosition; + + // Check if the element is visible in the parent's viewport. + if (elTopPosition < viewportHeight && elTopPosition >= 0) { + elv.markTopSeen(); + visible = true; + } + if (elBottomPosition <= viewportHeight && elBottomPosition > 0) { + elv.markBottomSeen(); + visible = true; + } + + if (visible) { + elv.handleVisible(); + } else { + elv.handleNotVisible(); + } + }); + } + } } diff --git a/lms/static/js/fixtures/instructor_dashboard/membership.html b/lms/static/js/fixtures/instructor_dashboard/membership.html index 85ab52c9665e..383552f7d83f 100644 --- a/lms/static/js/fixtures/instructor_dashboard/membership.html +++ b/lms/static/js/fixtures/instructor_dashboard/membership.html @@ -11,6 +11,8 @@

    Course Team Management

    % for form_name, form_value in submit_disabled_cta['form_values'].items(): @@ -76,13 +77,14 @@

    ${submit_disabled_cta['link_name']} - - + % endif + + - (${submit_disabled_cta['description']}) - + + (${submit_disabled_cta['description']}) + % endif % endif
    diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index 0e52e3c7f426..7df5b8b5b46e 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -44,7 +44,7 @@

    ${unit_title}

    - % else: + % elif vertical_banner_cta.get('link'):