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!: upgrade certificate_invalidation_view to drf ( 23 ) #35521

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
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
6 changes: 1 addition & 5 deletions lms/djangoapps/instructor/tests/test_certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,9 +1085,7 @@ def test_missing_username_and_email_error(self):
res_json = json.loads(response.content.decode('utf-8'))

# Assert Error Message
assert res_json['message'] == \
'Student username/email field is required and can not be empty.' \
' Kindly fill in username/email and then press "Invalidate Certificate" button.'
assert res_json['message'] == {'user': ['This field may not be blank.']}

def test_invalid_user_name_error(self):
"""
Expand All @@ -1106,7 +1104,6 @@ def test_invalid_user_name_error(self):
# Assert 400 status code in response
assert response.status_code == 400
res_json = json.loads(response.content.decode('utf-8'))

# Assert Error Message
assert res_json['message'] == f'{invalid_user} does not exist in the LMS. Please check your spelling and retry.'

Expand All @@ -1125,7 +1122,6 @@ def test_no_generated_certificate_error(self):
# Assert 400 status code in response
assert response.status_code == 400
res_json = json.loads(response.content.decode('utf-8'))

# Assert Error Message
assert res_json['message'] == f'The student {self.enrolled_user_2.username} does not have certificate for the course {self.course.number}. Kindly verify student username/email and the selected course are correct and try again.' # pylint: disable=line-too-long

Expand Down
101 changes: 76 additions & 25 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,16 @@
from lms.djangoapps.instructor_task.data import InstructorTaskTypes
from lms.djangoapps.instructor_task.models import ReportStore
from lms.djangoapps.instructor.views.serializer import (
AccessSerializer, BlockDueDateSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, UserSerializer,
SendEmailSerializer, StudentAttemptsSerializer, ListInstructorTaskInputSerializer, UniqueStudentIdentifierSerializer
AccessSerializer,
BlockDueDateSerializer,
CertificateSerializer,
ListInstructorTaskInputSerializer,
RoleNameSerializer,
SendEmailSerializer,
ShowStudentExtensionSerializer,
StudentAttemptsSerializer,
UserSerializer,
UniqueStudentIdentifierSerializer
)
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
Expand Down Expand Up @@ -3619,30 +3627,49 @@ def build_row_errors(key, _user, row_count):
return JsonResponse(results)


@transaction.non_atomic_requests
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_course_permission(permissions.CERTIFICATE_INVALIDATION_VIEW)
@require_http_methods(['POST', 'DELETE'])
def certificate_invalidation_view(request, course_id):
@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
@method_decorator(transaction.non_atomic_requests, name='dispatch')
class CertificateInvalidationView(APIView):
"""
Invalidate/Re-Validate students to/from certificate.

:param request: HttpRequest object
:param course_id: course identifier of the course for whom to add/remove certificates exception.
:return: JsonResponse object with success/error message or certificate invalidation data.
"""
course_key = CourseKey.from_string(course_id)
# Validate request data and return error response in case of invalid data
try:
certificate_invalidation_data = parse_request_data(request)
student = _get_student_from_request_data(certificate_invalidation_data)
certificate = _get_certificate_for_user(course_key, student)
except ValueError as error:
return JsonResponse({'message': str(error)}, status=400)
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
permission_name = permissions.CERTIFICATE_INVALIDATION_VIEW
serializer_class = CertificateSerializer
http_method_names = ['post', 'delete']

# Invalidate certificate of the given student for the course course
if request.method == 'POST':
@method_decorator(ensure_csrf_cookie)
@method_decorator(transaction.non_atomic_requests)
def post(self, request, course_id):
"""
Invalidate/Re-Validate students to/from certificate.
"""
course_key = CourseKey.from_string(course_id)
# Validate request data and return error response in case of invalid data
serializer_data = self.serializer_class(data=request.data)
if not serializer_data.is_valid():
# return HttpResponseBadRequest(reason=serializer_data.errors)
return JsonResponse({'message': serializer_data.errors}, status=400)

student = serializer_data.validated_data.get('user')
notes = serializer_data.validated_data.get('notes')

if not student:
invalid_user = request.data.get('user')
response_payload = f'{invalid_user} does not exist in the LMS. Please check your spelling and retry.'

return JsonResponse({'message': response_payload}, status=400)

try:
certificate = _get_certificate_for_user(course_key, student)
except Exception as ex: # pylint: disable=broad-except
return JsonResponse({'message': str(ex)}, status=400)

# Invalidate certificate of the given student for the course course
try:
if certs_api.is_on_allowlist(student, course_key):
log.warning(f"Invalidating certificate for student {student.id} in course {course_key} failed. "
Expand All @@ -3655,15 +3682,39 @@ def certificate_invalidation_view(request, course_id):
certificate_invalidation = invalidate_certificate(
request,
certificate,
certificate_invalidation_data,
notes,
student
)

except ValueError as error:
return JsonResponse({'message': str(error)}, status=400)
return JsonResponse(certificate_invalidation)

# Re-Validate student certificate for the course course
elif request.method == 'DELETE':
@method_decorator(ensure_csrf_cookie)
@method_decorator(transaction.non_atomic_requests)
def delete(self, request, course_id):
"""
Invalidate/Re-Validate students to/from certificate.
"""
# Re-Validate student certificate for the course course
course_key = CourseKey.from_string(course_id)
try:
data = json.loads(self.request.body.decode('utf8') or '{}')
except Exception: # pylint: disable=broad-except
data = {}

serializer_data = self.serializer_class(data=data)

if not serializer_data.is_valid():
return HttpResponseBadRequest(reason=serializer_data.errors)

student = serializer_data.validated_data.get('user')

try:
certificate = _get_certificate_for_user(course_key, student)
except Exception as ex: # pylint: disable=broad-except
return JsonResponse({'message': str(ex)}, status=400)

try:
re_validate_certificate(request, course_key, certificate, student)
except ValueError as error:
Expand All @@ -3672,13 +3723,13 @@ def certificate_invalidation_view(request, course_id):
return JsonResponse({}, status=204)


def invalidate_certificate(request, generated_certificate, certificate_invalidation_data, student):
def invalidate_certificate(request, generated_certificate, notes, student):
"""
Invalidate given GeneratedCertificate and add CertificateInvalidation record for future reference or re-validation.

:param request: HttpRequest object
:param generated_certificate: GeneratedCertificate object, the certificate we want to invalidate
:param certificate_invalidation_data: dict object containing data for CertificateInvalidation.
:param notes: notes values.
:param student: User object, this user is tied to the generated_certificate we are going to invalidate
:return: dict object containing updated certificate invalidation data.
"""
Expand All @@ -3701,7 +3752,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat
certificate_invalidation = certs_api.create_certificate_invalidation_entry(
generated_certificate,
request.user,
certificate_invalidation_data.get("notes", ""),
notes,
)

# Invalidate the certificate
Expand All @@ -3712,7 +3763,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat
'user': student.username,
'invalidated_by': certificate_invalidation.invalidated_by.username,
'created': certificate_invalidation.created.strftime("%B %d, %Y"),
'notes': certificate_invalidation.notes,
'notes': notes,
}


Expand Down
6 changes: 5 additions & 1 deletion lms/djangoapps/instructor/views/api_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,5 +89,9 @@
name='generate_certificate_exceptions'),
path('generate_bulk_certificate_exceptions', api.generate_bulk_certificate_exceptions,
name='generate_bulk_certificate_exceptions'),
path('certificate_invalidation_view/', api.certificate_invalidation_view, name='certificate_invalidation_view'),
path(
'certificate_invalidation_view/',
api.CertificateInvalidationView.as_view(),
name='certificate_invalidation_view'
),
]
22 changes: 22 additions & 0 deletions lms/djangoapps/instructor/views/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,25 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
if disable_due_datetime:
self.fields['due_datetime'].required = False


class CertificateSerializer(serializers.Serializer):
"""
Serializer for resetting a students attempts counter or starts a task to reset all students
attempts counters.
"""
user = serializers.CharField(
help_text="Email or username of student.", required=True
)
notes = serializers.CharField(required=False, allow_null=True, allow_blank=True)

def validate_user(self, value):
"""
Validate that the user corresponds to an existing user.
"""
try:
user = get_student_from_identifier(value)
except User.DoesNotExist:
return None

return user
Loading