From 1272a7835026d34063cf971836e90931f0fd40f5 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 29 Aug 2024 12:13:36 +0500 Subject: [PATCH 1/7] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 72 ++++++++++++------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 36 ++++++++++ 3 files changed, 84 insertions(+), 26 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 1aa40b5e3376..d1883eb1d852 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -106,7 +106,7 @@ from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore from lms.djangoapps.instructor.views.serializer import ( - AccessSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, UserSerializer + AccessSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, UserSerializer, BlockDueDateSerializer ) 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 @@ -2933,37 +2933,59 @@ def change_due_date(request, course_id): due_date.strftime('%Y-%m-%d %H:%M'))) -@handle_dashboard_error -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.GIVE_STUDENT_EXTENSION) -@require_post_params('student', 'url') -def reset_due_date(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class ResetDueDate(APIView): """ Rescinds a due date extension for a student on a particular unit. """ - course = get_course_by_id(CourseKey.from_string(course_id)) - student = require_student_from_identifier(request.POST.get('student')) - unit = find_unit(course, request.POST.get('url')) - reason = strip_tags(request.POST.get('reason', '')) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.GIVE_STUDENT_EXTENSION + serializer_class = BlockDueDateSerializer + + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + reset a due date extension to a student for a particular unit. + params: + url (str): The URL related to the block that needs the due date update. + student (str): The email or username of the student whose access is being modified. + reason (str): Optional param. + """ + serializer_data = self.serializer_class(data=request.data, context={'make_due_datetime': True}) + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) - version = getattr(course, 'course_version', None) + student = serializer_data.validated_data.get('student') + if not student: + response_payload = { + 'error': f'Could not find student matching identifier: {request.data.get("student")}' + } + return JsonResponse(response_payload) - original_due_date = get_date_for_block(course_id, unit.location, published_version=version) + course = get_course_by_id(CourseKey.from_string(course_id)) + unit = find_unit(course, serializer_data.validated_data.get('url')) + reason = strip_tags(serializer_data.validated_data.get('reason', '')) - set_due_date_extension(course, unit, student, None, request.user, reason=reason) - if not original_due_date: - # It's possible the normal due date was deleted after an extension was granted: - return JsonResponse( - _("Successfully removed invalid due date extension (unit has no due date).") - ) + version = getattr(course, 'course_version', None) - original_due_date_str = original_due_date.strftime('%Y-%m-%d %H:%M') - return JsonResponse(_( - 'Successfully reset due date for student {0} for {1} ' - 'to {2}').format(student.profile.name, _display_unit(unit), - original_due_date_str)) + original_due_date = get_date_for_block(course_id, unit.location, published_version=version) + + try: + set_due_date_extension(course, unit, student, None, request.user, reason=reason) + if not original_due_date: + # It's possible the normal due date was deleted after an extension was granted: + return JsonResponse( + _("Successfully removed invalid due date extension (unit has no due date).") + ) + + original_due_date_str = original_due_date.strftime('%Y-%m-%d %H:%M') + return JsonResponse(_( + 'Successfully reset due date for student {0} for {1} ' + 'to {2}').format(student.profile.name, _display_unit(unit), + original_due_date_str)) + + except Exception as error: # pylint: disable=broad-except + return JsonResponse({'error': str(error)}, status=400) @handle_dashboard_error diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 18da0b63b218..841d03a736ec 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -51,7 +51,7 @@ path('update_forum_role_membership', api.update_forum_role_membership, name='update_forum_role_membership'), path('send_email', api.send_email, name='send_email'), path('change_due_date', api.change_due_date, name='change_due_date'), - path('reset_due_date', api.reset_due_date, name='reset_due_date'), + path('reset_due_date', api.ResetDueDate.as_view(), name='reset_due_date'), path('show_unit_extensions', api.show_unit_extensions, name='show_unit_extensions'), path('show_student_extensions', api.ShowStudentExtensions.as_view(), name='show_student_extensions'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 0697bed6832d..54ced3d5720e 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -77,3 +77,39 @@ def validate_student(self, value): return None return user + + +class BlockDueDateSerializer(serializers.Serializer): + """ + Serializer for handling block due date updates for a specific student. + Fields: + url (str): The URL related to the block that needs the due date update. + due_datetime (str): The new due date and time for the block. + student (str): The email or username of the student whose access is being modified. + reason (str): Reason why updating this. + """ + url = serializers.CharField() + due_datetime = serializers.CharField() + student = serializers.CharField( + max_length=255, + help_text="Email or username of user to change access" + ) + reason = serializers.CharField(required=False) + + def validate_student(self, value): + """ + Validate that the student corresponds to an existing user. + """ + try: + user = get_student_from_identifier(value) + except User.DoesNotExist: + return None + + return user + + def __init__(self, *args, **kwargs): + # Get context to check if `due_datetime` should be optional + make_due_datetime = kwargs.get('context', {}).get('make_due_datetime', False) + super().__init__(*args, **kwargs) + if make_due_datetime: + self.fields['due_datetime'].required = False From f3075a8cf2bd8079ec942cd2db6ebd359fd4fcf7 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 20 Sep 2024 14:22:44 +0500 Subject: [PATCH 2/7] feat!: fixing quality. --- lms/djangoapps/instructor/views/api_urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 841d03a736ec..aad169ce3219 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -50,7 +50,7 @@ path('list_forum_members', api.list_forum_members, name='list_forum_members'), path('update_forum_role_membership', api.update_forum_role_membership, name='update_forum_role_membership'), path('send_email', api.send_email, name='send_email'), - path('change_due_date', api.change_due_date, name='change_due_date'), + path('change_due_date', api.ChangeDueDate.as_view(), name='change_due_date'), path('reset_due_date', api.ResetDueDate.as_view(), name='reset_due_date'), path('show_unit_extensions', api.show_unit_extensions, name='show_unit_extensions'), path('show_student_extensions', api.ShowStudentExtensions.as_view(), name='show_student_extensions'), From 83a4e0336ec210f25dcb3e1312e40350472edd3c Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Fri, 20 Sep 2024 14:35:26 +0500 Subject: [PATCH 3/7] chore: Update api_urls.py --- lms/djangoapps/instructor/views/api_urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index e96bd908375e..a248b46ae531 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -51,7 +51,7 @@ path('update_forum_role_membership', api.update_forum_role_membership, name='update_forum_role_membership'), path('change_due_date', api.ChangeDueDate.as_view(), name='change_due_date'), path('send_email', api.SendEmail.as_view(), name='send_email'), - path('reset_due_date', api.ChangeDueDate.as_view(), name='reset_due_date'), + path('reset_due_date', api.ResetDueDate.as_view(), name='reset_due_date'), path('show_unit_extensions', api.show_unit_extensions, name='show_unit_extensions'), path('show_student_extensions', api.ShowStudentExtensions.as_view(), name='show_student_extensions'), From f7f4aa23e69c541202481407910f4c09a22e8802 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 20 Sep 2024 14:42:51 +0500 Subject: [PATCH 4/7] feat!: fixing quality. --- lms/djangoapps/instructor/views/serializer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 32516f06c98d..03df7500e55e 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -222,4 +222,3 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if make_due_datetime: self.fields['due_datetime'].required = False - \ No newline at end of file From e3aa6497856dbc98cdca19aa8740af358266378d Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 20 Sep 2024 15:11:31 +0500 Subject: [PATCH 5/7] chore: fixing quality. --- lms/djangoapps/instructor/views/api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 776f1401933f..a92510225410 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -137,7 +137,6 @@ handle_dashboard_error, keep_field_private, parse_datetime, - require_student_from_identifier, set_due_date_extension, strip_if_string, ) From e0066d0ff37345f34de859eb81e57cfec5b9d51b Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 20 Sep 2024 15:42:25 +0500 Subject: [PATCH 6/7] chore: fixing quality. --- lms/djangoapps/instructor/tests/test_api.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index e8bcc81318da..51fc514c4879 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -4175,6 +4175,16 @@ def test_change_due_date_with_reason(self): # This operation regenerates the cache, so we can use cached results from edx-when. assert get_date_for_block(self.course, self.week1, self.user1, use_cached=True) == due_date + def test_reset_due_date_with_reason(self): + url = reverse('reset_due_date', kwargs={'course_id': str(self.course.id)}) + response = self.client.post(url, { + 'student': self.user1.username, + 'url': str(self.week1.location), + 'reason': 'Testing reason.' # this is optional field. + }) + assert response.status_code == 200 + assert 'Successfully reset due date for student' in response.content.decode('utf-8') + def test_change_to_invalid_due_date(self): url = reverse('change_due_date', kwargs={'course_id': str(self.course.id)}) response = self.client.post(url, { From a98d39898e1a7347211354931d17954fde1dc469 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Fri, 20 Sep 2024 15:43:14 +0500 Subject: [PATCH 7/7] chore: fixing quality. --- lms/djangoapps/instructor/views/api.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index a92510225410..6978eaf3fe96 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -3052,7 +3052,7 @@ def post(self, request, course_id): student (str): The email or username of the student whose access is being modified. reason (str): Optional param. """ - serializer_data = self.serializer_class(data=request.data, context={'make_due_datetime': True}) + serializer_data = self.serializer_class(data=request.data, context={'disable_due_datetime': True}) if not serializer_data.is_valid(): return HttpResponseBadRequest(reason=serializer_data.errors) diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 03df7500e55e..5d123ad66c81 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -218,7 +218,7 @@ def validate_student(self, value): def __init__(self, *args, **kwargs): # Get context to check if `due_datetime` should be optional - make_due_datetime = kwargs.get('context', {}).get('make_due_datetime', False) + disable_due_datetime = kwargs.get('context', {}).get('disable_due_datetime', False) super().__init__(*args, **kwargs) - if make_due_datetime: + if disable_due_datetime: self.fields['due_datetime'].required = False