Skip to content

Commit

Permalink
Merge pull request #614 from open-craft/0x29a/bb8366/quince-shared-br…
Browse files Browse the repository at this point in the history
…anch

feat: port code drift for open-craft/edx-platform [BB-8366]
  • Loading branch information
0x29a authored Jan 3, 2024
2 parents 4e15e5e + a46281a commit 14497bf
Show file tree
Hide file tree
Showing 39 changed files with 519 additions and 57 deletions.
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

0 comments on commit 14497bf

Please sign in to comment.