Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: port code drift for open-craft/edx-platform [BB-8366] #614

Merged
merged 14 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions cms/djangoapps/api/v1/serializers/course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.db import transaction
from django.utils.translation import gettext_lazy as _
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from rest_framework import serializers
from rest_framework.fields import empty

Expand Down Expand Up @@ -203,3 +204,35 @@ def update(self, instance, validated_data):
course_run = get_course_and_check_access(new_course_run_key, user)
self.update_team(course_run, team)
return course_run


class CourseCloneSerializer(serializers.Serializer): # lint-amnesty, pylint: disable=abstract-method, missing-class-docstring
source_course_id = serializers.CharField()
destination_course_id = serializers.CharField()

def validate(self, attrs):
source_course_id = attrs.get('source_course_id')
destination_course_id = attrs.get('destination_course_id')
store = modulestore()
source_key = CourseKey.from_string(source_course_id)
dest_key = CourseKey.from_string(destination_course_id)

# Check if the source course exists
if not store.has_course(source_key):
raise serializers.ValidationError('Source course does not exist.')

# Check if the destination course already exists
if store.has_course(dest_key):
raise serializers.ValidationError('Destination course already exists.')
return attrs

def create(self, validated_data):
source_course_id = validated_data.get('source_course_id')
destination_course_id = validated_data.get('destination_course_id')
user_id = self.context['request'].user.id
store = modulestore()
source_key = CourseKey.from_string(source_course_id)
dest_key = CourseKey.from_string(destination_course_id)
with store.default_store('split'):
new_course = store.clone_course(source_key, dest_key, user_id)
return new_course
51 changes: 51 additions & 0 deletions cms/djangoapps/api/v1/tests/test_views/test_course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,3 +402,54 @@ def test_rerun_invalid_number(self):
assert response.data == {'non_field_errors': [
'Invalid key supplied. Ensure there are no special characters in the Course Number.'
]}

def test_clone_course(self):
course = CourseFactory()
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': str(course.id),
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 201
self.assertEqual(response.data, {"message": "Course cloned successfully."})

def test_clone_course_with_missing_source_id(self):
url = reverse('api:v1:course_run-clone')
data = {
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
self.assertEqual(response.data, {'source_course_id': ['This field is required.']})

def test_clone_course_with_missing_dest_id(self):
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': 'course-v1:source+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
self.assertEqual(response.data, {'destination_course_id': ['This field is required.']})

def test_clone_course_with_nonexistent_source_course(self):
url = reverse('api:v1:course_run-clone')
data = {
'source_course_id': 'course-v1:nonexistent+source+course_id',
'destination_course_id': 'course-v1:destination+course+id',
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
assert str(response.data.get('non_field_errors')[0]) == 'Source course does not exist.'

def test_clone_course_with_existing_dest_course(self):
url = reverse('api:v1:course_run-clone')
course = CourseFactory()
existing_dest_course = CourseFactory()
data = {
'source_course_id': str(course.id),
'destination_course_id': str(existing_dest_course.id),
}
response = self.client.post(url, data, format='json')
assert response.status_code == 400
assert str(response.data.get('non_field_errors')[0]) == 'Destination course already exists.'
8 changes: 8 additions & 0 deletions cms/djangoapps/api/v1/views/course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from cms.djangoapps.contentstore.views.course import _accessible_courses_iter, get_course_and_check_access

from ..serializers.course_runs import (
CourseCloneSerializer,
CourseRunCreateSerializer,
CourseRunImageSerializer,
CourseRunRerunSerializer,
Expand Down Expand Up @@ -93,3 +94,10 @@ def rerun(self, request, *args, **kwargs): # lint-amnesty, pylint: disable=miss
new_course_run = serializer.save()
serializer = self.get_serializer(new_course_run)
return Response(serializer.data, status=status.HTTP_201_CREATED)

@action(detail=False, methods=['post'])
def clone(self, request, *args, **kwargs): # lint-amnesty, pylint: disable=missing-function-docstring, unused-argument
serializer = CourseCloneSerializer(data=request.data, context=self.get_serializer_context())
serializer.is_valid(raise_exception=True)
serializer.save()
return Response({"message": "Course cloned successfully."}, status=status.HTTP_201_CREATED)
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,8 @@ def get_course_grading(course_key):
'grading_url': reverse_course_url('grading_handler', course_key),
'is_credit_course': is_credit_course(course_key),
'mfe_proctored_exam_settings_url': get_proctored_exam_settings_url(course_key),
'course_assignment_lists': dict(course_assignment_lists)
'course_assignment_lists': dict(course_assignment_lists),
'default_grade_designations': settings.DEFAULT_GRADE_DESIGNATIONS
}

return grading_context
Expand Down
19 changes: 19 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,17 @@
# .. toggle_creation_date: 2023-03-31
# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/32015
'DISABLE_ADVANCED_SETTINGS': 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: ENABLE_COPPA_COMPLIANCE
Expand Down Expand Up @@ -2366,6 +2377,14 @@
# Rate limit for regrading tasks that a grading policy change can kick off
POLICY_CHANGE_TASK_RATE_LIMIT = '900/h'

# .. setting_name: DEFAULT_GRADE_DESIGNATIONS
# .. setting_default: ['A', 'B', 'C', 'D']
# .. setting_description: The default 'pass' grade cutoff designations to be used. The failure grade
# is always 'F' and should not be included in this list.
# .. setting_warning: The DEFAULT_GRADE_DESIGNATIONS list must have more than one designation,
# or else ['A', 'B', 'C', 'D'] will be used as the default grade designations.
DEFAULT_GRADE_DESIGNATIONS = ['A', 'B', 'C', 'D']

############## Settings for CourseGraph ############################

# .. setting_name: COURSEGRAPH_JOB_QUEUE
Expand Down
5 changes: 3 additions & 2 deletions cms/static/js/factories/settings_graders.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ define([
], function($, GradingView, CourseGradingPolicyModel) {
'use strict';

return function(courseDetails, gradingUrl, courseAssignmentLists) {
return function(courseDetails, gradingUrl, gradeDesignations, courseAssignmentLists) {
var model, editor;

$('form :input')
Expand All @@ -19,7 +19,8 @@ define([
editor = new GradingView({
el: $('.settings-grading'),
model: model,
courseAssignmentLists: courseAssignmentLists
courseAssignmentLists: courseAssignmentLists,
gradeDesignations: gradeDesignations
});
editor.render();
};
Expand Down
7 changes: 5 additions & 2 deletions cms/static/js/views/settings/grading.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) {
$('#course_grade_cutoff-tpl').text()
);
this.setupCutoffs();
this.setupGradeDesignations(options.gradeDesignations);

this.listenTo(this.model, 'invalid', this.handleValidationError);
this.listenTo(this.model, 'change', this.showNotificationBar);
Expand Down Expand Up @@ -318,7 +319,7 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) {
addNewGrade: function(e) {
e.preventDefault();
var gradeLength = this.descendingCutoffs.length; // cutoffs doesn't include fail/f so this is only the passing grades
if (gradeLength > 3) {
if (gradeLength > this.GRADES.length - 1) {
// TODO shouldn't we disable the button
return;
}
Expand Down Expand Up @@ -399,7 +400,9 @@ function(ValidatingView, _, $, ui, GraderView, StringUtils, HtmlUtils) {
this.descendingCutoffs = _.sortBy(this.descendingCutoffs,
function(gradeEle) { return -gradeEle.cutoff; });
},
revertView: function() {
setupGradeDesignations: function(gradeDesignations) {
if (Array.isArray(gradeDesignations) && gradeDesignations.length > 1) { this.GRADES = gradeDesignations; }
},revertView: function() {
var self = this;
this.model.fetch({
success: function() {
Expand Down
10 changes: 5 additions & 5 deletions cms/static/sass/views/_settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -777,23 +777,23 @@
height: 17px;
}

&:nth-child(1) {
&:nth-child(5n+1) {
background: #4fe696;
}

&:nth-child(2) {
&:nth-child(5n+2) {
background: #ffdf7e;
}

&:nth-child(3) {
&:nth-child(5n+3) {
background: #ffb657;
}

&:nth-child(4) {
&:nth-child(5n+4) {
background: #ef54a1;
}

&:nth-child(5),
&:nth-child(5n+5),
&.bar-fail {
background: #fb336c;
}
Expand Down
1 change: 1 addition & 0 deletions cms/templates/settings_graders.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
{ is_credit_course: ${is_credit_course | n, dump_js_escaped_json} }
),
"${grading_url | n, js_escaped_string}",
${default_grade_designations | n, dump_js_escaped_json},
${course_assignment_lists | n, dump_js_escaped_json},
);
});
Expand Down
11 changes: 9 additions & 2 deletions common/djangoapps/student/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,16 @@ def get_user_permissions(user, course_key, org=None):
return all_perms
if course_key and user_has_role(user, CourseInstructorRole(course_key)):
return all_perms
# Limited Course Staff does not have access to Studio.
# HACK: Limited Staff should not have studio read access. However, since many LMS views depend on the
# `has_course_author_access` check and `course_author_access_required` decorator, we have to allow write access
# until the permissions become more granular. For example, there could be STUDIO_VIEW_COHORTS and
# STUDIO_EDIT_COHORTS specifically for the cohorts endpoint, which is used to display the "Cohorts" tab of the
# Instructor Dashboard.
# The permissions matrix from the RBAC project (https://github.com/openedx/platform-roadmap/issues/246) shows that
# 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)):
return STUDIO_NO_PERMISSIONS
return STUDIO_EDIT_CONTENT
# Staff have all permissions except EDIT_ROLES:
if OrgStaffRole(org=org).has_user(user) or (course_key and user_has_role(user, CourseStaffRole(course_key))):
return STUDIO_VIEW_USERS | STUDIO_EDIT_CONTENT | STUDIO_VIEW_CONTENT
Expand Down
13 changes: 11 additions & 2 deletions common/djangoapps/student/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
30 changes: 25 additions & 5 deletions common/djangoapps/student/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import logging
from abc import ABCMeta, abstractmethod
from collections import defaultdict
from contextlib import contextmanager

from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
from opaque_keys.edx.django.models import CourseKeyField
Expand Down Expand Up @@ -44,6 +45,17 @@ def register_access_role(cls):
return cls


@contextmanager
def strict_role_checking():
"""
Context manager that temporarily disables role inheritance.
"""
OLD_ACCESS_ROLES_INHERITANCE = ACCESS_ROLES_INHERITANCE.copy()
ACCESS_ROLES_INHERITANCE.clear()
yield
ACCESS_ROLES_INHERITANCE.update(OLD_ACCESS_ROLES_INHERITANCE)


class BulkRoleCache: # lint-amnesty, pylint: disable=missing-class-docstring
CACHE_NAMESPACE = "student.roles.BulkRoleCache"
CACHE_KEY = 'roles_by_user'
Expand Down Expand Up @@ -78,7 +90,7 @@ def __init__(self, user):
)

@staticmethod
def _get_roles(role):
def get_roles(role):
"""
Return the roles that should have the same permissions as the specified role.
"""
Expand All @@ -90,7 +102,7 @@ def has_role(self, role, course_id, org):
or a role that inherits from the specified role, course_id and org.
"""
return any(
access_role.role in self._get_roles(role) and
access_role.role in self.get_roles(role) and
access_role.course_id == course_id and
access_role.org == org
for access_role in self._roles
Expand Down Expand Up @@ -136,7 +148,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:
Expand Down Expand Up @@ -286,6 +298,14 @@ class CourseLimitedStaffRole(CourseStaffRole):
BASE_ROLE = CourseStaffRole.ROLE


@register_access_role
class eSHEInstructorRole(CourseStaffRole):
"""A Staff member of a course without access to Studio."""

ROLE = 'eshe_instructor'
BASE_ROLE = CourseStaffRole.ROLE


@register_access_role
class CourseInstructorRole(CourseRole):
"""A course Instructor"""
Expand Down Expand Up @@ -463,11 +483,11 @@ def remove_courses(self, *course_keys):

def courses_with_role(self):
"""
Return a django QuerySet for all of the courses with this user x role. You can access
Return a django QuerySet for all of the courses with this user x (or derived from x) role. You can access
any of these properties on each result record:
* user (will be self.user--thus uninteresting)
* org
* course_id
* role (will be self.role--thus uninteresting)
"""
return CourseAccessRole.objects.filter(role=self.role, user=self.user)
return CourseAccessRole.objects.filter(role__in=RoleCache.get_roles(self.role), user=self.user)
6 changes: 3 additions & 3 deletions common/djangoapps/student/tests/test_authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,14 +285,14 @@ def test_remove_user_from_course_group_permission_denied(self):
with pytest.raises(PermissionDenied):
remove_users(self.staff, CourseStaffRole(self.course_key), another_staff)

def test_no_limited_staff_read_or_write_access(self):
def test_limited_staff_no_studio_read_access(self):
"""
Test that course limited staff have no read or write access.
Verifies that course limited staff have no read, but have write access.
"""
add_users(self.global_admin, CourseLimitedStaffRole(self.course_key), self.limited_staff)

assert not has_studio_read_access(self.limited_staff, self.course_key)
assert not has_studio_write_access(self.limited_staff, self.course_key)
assert has_studio_write_access(self.limited_staff, self.course_key)


class CourseOrgGroupTest(TestCase):
Expand Down
Loading
Loading