Skip to content

Commit af9ae77

Browse files
authored
feat: upgrading simple api to drf compatible. (#35260)
1 parent c3480b8 commit af9ae77

3 files changed

Lines changed: 95 additions & 63 deletions

File tree

lms/djangoapps/instructor/views/api.py

Lines changed: 63 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@
105105
from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError
106106
from lms.djangoapps.instructor_task.data import InstructorTaskTypes
107107
from lms.djangoapps.instructor_task.models import ReportStore
108-
from lms.djangoapps.instructor.views.serializer import RoleNameSerializer, UserSerializer
108+
from lms.djangoapps.instructor.views.serializer import RoleNameSerializer, UserSerializer, AccessSerializer
109109
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
110110
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
111111
from openedx.core.djangoapps.course_groups.models import CourseUserGroup
@@ -987,17 +987,8 @@ def bulk_beta_modify_access(request, course_id):
987987
return JsonResponse(response_payload)
988988

989989

990-
@require_POST
991-
@ensure_csrf_cookie
992-
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
993-
@require_course_permission(permissions.EDIT_COURSE_ACCESS)
994-
@require_post_params(
995-
unique_student_identifier="email or username of user to change access",
996-
rolename="'instructor', 'staff', 'beta', or 'ccx_coach'",
997-
action="'allow' or 'revoke'"
998-
)
999-
@common_exceptions_400
1000-
def modify_access(request, course_id):
990+
@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
991+
class ModifyAccess(APIView):
1001992
"""
1002993
Modify staff/instructor access of other user.
1003994
Requires instructor access.
@@ -1009,67 +1000,77 @@ def modify_access(request, course_id):
10091000
rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach']
10101001
action is one of ['allow', 'revoke']
10111002
"""
1012-
course_id = CourseKey.from_string(course_id)
1013-
course = get_course_with_access(
1014-
request.user, 'instructor', course_id, depth=None
1015-
)
1016-
unique_student_identifier = request.POST.get('unique_student_identifier')
1017-
try:
1018-
user = get_student_from_identifier(unique_student_identifier)
1019-
except User.DoesNotExist:
1020-
response_payload = {
1021-
'unique_student_identifier': unique_student_identifier,
1022-
'userDoesNotExist': True,
1023-
}
1024-
return JsonResponse(response_payload)
1003+
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
1004+
permission_name = permissions.EDIT_COURSE_ACCESS
1005+
serializer_class = AccessSerializer
10251006

1026-
# Check that user is active, because add_users
1027-
# in common/djangoapps/student/roles.py fails
1028-
# silently when we try to add an inactive user.
1029-
if not user.is_active:
1030-
response_payload = {
1031-
'unique_student_identifier': user.username,
1032-
'inactiveUser': True,
1033-
}
1034-
return JsonResponse(response_payload)
1007+
@method_decorator(ensure_csrf_cookie)
1008+
def post(self, request, course_id):
1009+
"""
1010+
Modify staff/instructor access of other user.
1011+
Requires instructor access.
1012+
"""
1013+
course_id = CourseKey.from_string(course_id)
1014+
course = get_course_with_access(
1015+
request.user, 'instructor', course_id, depth=None
1016+
)
10351017

1036-
rolename = request.POST.get('rolename')
1037-
action = request.POST.get('action')
1018+
serializer_data = AccessSerializer(data=request.data)
1019+
if not serializer_data.is_valid():
1020+
return HttpResponseBadRequest(reason=serializer_data.errors)
1021+
1022+
user = serializer_data.validated_data.get('unique_student_identifier')
1023+
if not user:
1024+
response_payload = {
1025+
'unique_student_identifier': request.data.get('unique_student_identifier'),
1026+
'userDoesNotExist': True,
1027+
}
1028+
return JsonResponse(response_payload)
10381029

1039-
if rolename not in ROLES:
1040-
error = strip_tags(f"unknown rolename '{rolename}'")
1041-
log.error(error)
1042-
return HttpResponseBadRequest(error)
1030+
if not user.is_active:
1031+
response_payload = {
1032+
'unique_student_identifier': user.username,
1033+
'inactiveUser': True,
1034+
}
1035+
return JsonResponse(response_payload)
1036+
1037+
rolename = serializer_data.data['rolename']
1038+
action = serializer_data.data['action']
1039+
1040+
if rolename not in ROLES:
1041+
error = strip_tags(f"unknown rolename '{rolename}'")
1042+
log.error(error)
1043+
return HttpResponseBadRequest(error)
1044+
1045+
# disallow instructors from removing their own instructor access.
1046+
if rolename == 'instructor' and user == request.user and action != 'allow':
1047+
response_payload = {
1048+
'unique_student_identifier': user.username,
1049+
'rolename': rolename,
1050+
'action': action,
1051+
'removingSelfAsInstructor': True,
1052+
}
1053+
return JsonResponse(response_payload)
1054+
1055+
if action == 'allow':
1056+
allow_access(course, user, rolename)
1057+
if not is_user_enrolled_in_course(user, course_id):
1058+
CourseEnrollment.enroll(user, course_id)
1059+
elif action == 'revoke':
1060+
revoke_access(course, user, rolename)
1061+
else:
1062+
return HttpResponseBadRequest(strip_tags(
1063+
f"unrecognized action u'{action}'"
1064+
))
10431065

1044-
# disallow instructors from removing their own instructor access.
1045-
if rolename == 'instructor' and user == request.user and action != 'allow':
10461066
response_payload = {
10471067
'unique_student_identifier': user.username,
10481068
'rolename': rolename,
10491069
'action': action,
1050-
'removingSelfAsInstructor': True,
1070+
'success': 'yes',
10511071
}
10521072
return JsonResponse(response_payload)
10531073

1054-
if action == 'allow':
1055-
allow_access(course, user, rolename)
1056-
if not is_user_enrolled_in_course(user, course_id):
1057-
CourseEnrollment.enroll(user, course_id)
1058-
elif action == 'revoke':
1059-
revoke_access(course, user, rolename)
1060-
else:
1061-
return HttpResponseBadRequest(strip_tags(
1062-
f"unrecognized action u'{action}'"
1063-
))
1064-
1065-
response_payload = {
1066-
'unique_student_identifier': user.username,
1067-
'rolename': rolename,
1068-
'action': action,
1069-
'success': 'yes',
1070-
}
1071-
return JsonResponse(response_payload)
1072-
10731074

10741075
@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
10751076
class ListCourseRoleMembersView(APIView):

lms/djangoapps/instructor/views/api_urls.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
path('students_update_enrollment', api.students_update_enrollment, name='students_update_enrollment'),
2525
path('register_and_enroll_students', api.RegisterAndEnrollStudents.as_view(), name='register_and_enroll_students'),
2626
path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'),
27-
path('modify_access', api.modify_access, name='modify_access'),
27+
path('modify_access', api.ModifyAccess.as_view(), name='modify_access'),
2828
path('bulk_beta_modify_access', api.bulk_beta_modify_access, name='bulk_beta_modify_access'),
2929
path('get_problem_responses', api.get_problem_responses, name='get_problem_responses'),
3030
path('get_grading_config', api.get_grading_config, name='get_grading_config'),

lms/djangoapps/instructor/views/serializer.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.core.exceptions import ValidationError
55
from django.utils.translation import gettext as _
66
from rest_framework import serializers
7+
from .tools import get_student_from_identifier
78

89
from lms.djangoapps.instructor.access import ROLES
910

@@ -28,3 +29,33 @@ class UserSerializer(serializers.ModelSerializer):
2829
class Meta:
2930
model = User
3031
fields = ['username', 'email', 'first_name', 'last_name']
32+
33+
34+
class AccessSerializer(serializers.Serializer):
35+
"""
36+
Serializer for managing user access changes.
37+
This serializer validates and processes the data required to modify
38+
user access within a system.
39+
"""
40+
unique_student_identifier = serializers.CharField(
41+
max_length=255,
42+
help_text="Email or username of user to change access"
43+
)
44+
rolename = serializers.CharField(
45+
help_text="Role name to assign to the user"
46+
)
47+
action = serializers.ChoiceField(
48+
choices=['allow', 'revoke'],
49+
help_text="Action to perform on the user's access"
50+
)
51+
52+
def validate_unique_student_identifier(self, value):
53+
"""
54+
Validate that the unique_student_identifier corresponds to an existing user.
55+
"""
56+
try:
57+
user = get_student_from_identifier(value)
58+
except User.DoesNotExist:
59+
return None
60+
61+
return user

0 commit comments

Comments
 (0)