Skip to content

Commit

Permalink
feat!: upgrade certificate_exception_view to DRF ( 28 ) (openedx#35594)
Browse files Browse the repository at this point in the history
* feat!: upgrading api to DRF.
  • Loading branch information
awais786 authored and salman2013 committed Dec 2, 2024
1 parent f6e2ae6 commit c32598a
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 32 deletions.
32 changes: 29 additions & 3 deletions lms/djangoapps/instructor/tests/test_certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,9 +488,7 @@ def test_certificate_exception_missing_username_and_email_error(self):
assert not res_json['success']

# 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 "Add to Exception List" button.'
assert res_json['message'] == {'user': ['This field may not be blank.']}

def test_certificate_exception_duplicate_user_error(self):
"""
Expand Down Expand Up @@ -604,6 +602,34 @@ def test_certificate_exception_removed_successfully(self):
# Verify that certificate exception does not exist
assert not certs_api.is_on_allowlist(self.user2, self.course.id)

def test_certificate_exception_removed_successfully_form_url(self):
"""
In case of deletion front-end is sending content-type x-www-form-urlencoded.
Just to handle that some logic added in api and this test is for that part.
Test certificates exception removal api endpoint returns success status
when called with valid course key and certificate exception id
"""
GeneratedCertificateFactory.create(
user=self.user2,
course_id=self.course.id,
status=CertificateStatuses.downloadable,
grade='1.0'
)
# Verify that certificate exception exists
assert certs_api.is_on_allowlist(self.user2, self.course.id)

response = self.client.post(
self.url,
data=json.dumps(self.certificate_exception_in_db),
content_type='application/x-www-form-urlencoded',
REQUEST_METHOD='DELETE'
)
# Assert successful request processing
assert response.status_code == 204

# Verify that certificate exception does not exist
assert not certs_api.is_on_allowlist(self.user2, self.course.id)

def test_remove_certificate_exception_invalid_request_error(self):
"""
Test certificates exception removal api endpoint returns error
Expand Down
105 changes: 78 additions & 27 deletions lms/djangoapps/instructor/views/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, PermissionDenied, ValidationError
from django.core.validators import validate_email
from django.db import IntegrityError, transaction
from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound
from django.http import QueryDict, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound
from django.shortcuts import redirect
from django.urls import reverse
from django.utils.decorators import method_decorator
from django.utils.html import strip_tags
from django.utils.translation import gettext as _
from django.views.decorators.cache import cache_control
from django.views.decorators.csrf import ensure_csrf_cookie
from django.views.decorators.http import require_POST, require_http_methods
from django.views.decorators.http import require_POST
from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
from edx_when.api import get_date_for_block
Expand Down Expand Up @@ -3350,42 +3350,93 @@ def start_certificate_regeneration(request, course_id):
return JsonResponse(response_payload)


@transaction.non_atomic_requests
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
@require_course_permission(permissions.CERTIFICATE_EXCEPTION_VIEW)
@require_http_methods(['POST', 'DELETE'])
def certificate_exception_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 CertificateExceptionView(DeveloperErrorViewMixin, APIView):
"""
Add/Remove students to/from the certificate allowlist.
: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 exception data.
"""
course_key = CourseKey.from_string(course_id)
# Validate request data and return error response in case of invalid data
try:
certificate_exception, student = parse_request_data_and_get_user(request)
except ValueError as error:
return JsonResponse({'success': False, 'message': str(error)}, status=400)
permission_classes = (IsAuthenticated, permissions.InstructorPermission)
permission_name = permissions.CERTIFICATE_EXCEPTION_VIEW
serializer_class = CertificateSerializer
http_method_names = ['post', 'delete']

@method_decorator(transaction.non_atomic_requests, name='dispatch')
@method_decorator(ensure_csrf_cookie)
def post(self, request, course_id):
"""
Add certificate exception for a student.
"""
return self._handle_certificate_exception(request, course_id, action="post")

@method_decorator(ensure_csrf_cookie)
@method_decorator(transaction.non_atomic_requests)
def delete(self, request, course_id):
"""
Remove certificate exception for a student.
"""
return self._handle_certificate_exception(request, course_id, action="delete")

def _handle_certificate_exception(self, request, course_id, action):
"""
Handles adding or removing certificate exceptions.
"""
course_key = CourseKey.from_string(course_id)
try:
data = request.data
except Exception: # pylint: disable=broad-except
return JsonResponse(
{
'success': False,
'message':
_('The record is not in the correct format. Please add a valid username or email address.')},
status=400
)

# Extract and validate the student information
student, error_response = self._get_and_validate_user(data)

if error_response:
return error_response

# Add new Certificate Exception for the student passed in request data
if request.method == 'POST':
try:
exception = add_certificate_exception(course_key, student, certificate_exception)
if action == "post":
exception = add_certificate_exception(course_key, student, data)
return JsonResponse(exception)
elif action == "delete":
remove_certificate_exception(course_key, student)
return JsonResponse({}, status=204)
except ValueError as error:
return JsonResponse({'success': False, 'message': str(error)}, status=400)
return JsonResponse(exception)

# Remove Certificate Exception for the student passed in request data
elif request.method == 'DELETE':
def _get_and_validate_user(self, raw_data):
"""
Extracts the user data from the request and validates the student.
"""
# This is only happening in case of delete.
# because content-type is coming as x-www-form-urlencoded from front-end.
if isinstance(raw_data, QueryDict):
raw_data = list(raw_data.keys())[0]
try:
raw_data = json.loads(raw_data)
except Exception as error: # pylint: disable=broad-except
return None, JsonResponse({'success': False, 'message': str(error)}, status=400)

try:
remove_certificate_exception(course_key, student)
user_data = raw_data.get('user_name', '') or raw_data.get('user_email', '')
except ValueError as error:
return JsonResponse({'success': False, 'message': str(error)}, status=400)
return None, JsonResponse({'success': False, 'message': str(error)}, status=400)

return JsonResponse({}, status=204)
serializer_data = self.serializer_class(data={'user': user_data})
if not serializer_data.is_valid():
return None, JsonResponse({'success': False, 'message': serializer_data.errors}, status=400)

student = serializer_data.validated_data.get('user')
if not student:
response_payload = f'{user_data} does not exist in the LMS. Please check your spelling and retry.'
return None, JsonResponse({'success': False, 'message': response_payload}, status=400)

return student, None


def add_certificate_exception(course_key, student, certificate_exception):
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/instructor/views/api_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
path('enable_certificate_generation', api.enable_certificate_generation, name='enable_certificate_generation'),
path('start_certificate_generation', api.StartCertificateGeneration.as_view(), name='start_certificate_generation'),
path('start_certificate_regeneration', api.start_certificate_regeneration, name='start_certificate_regeneration'),
path('certificate_exception_view/', api.certificate_exception_view, name='certificate_exception_view'),
path('certificate_exception_view/', api.CertificateExceptionView.as_view(), name='certificate_exception_view'),
re_path(r'^generate_certificate_exceptions/(?P<generate_for>[^/]*)', api.GenerateCertificateExceptions.as_view(),
name='generate_certificate_exceptions'),
path('generate_bulk_certificate_exceptions', api.generate_bulk_certificate_exceptions,
Expand Down
6 changes: 5 additions & 1 deletion lms/djangoapps/instructor/views/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,11 @@ def __init__(self, *args, **kwargs):

class CertificateSerializer(serializers.Serializer):
"""
Serializer for resetting a students attempts counter or starts a task to reset all students
Serializer for multiple operations related with certificates.
resetting a students attempts counter or starts a task to reset all students
attempts counters
Also Add/Remove students to/from the certificate allowlist.
Also For resetting a students attempts counter or starts a task to reset all students
attempts counters.
"""
user = serializers.CharField(
Expand Down

0 comments on commit c32598a

Please sign in to comment.