From f52c08a0dd41c3bd677b22390d0002e4c88d749e Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Mon, 9 Sep 2024 14:52:47 +0500 Subject: [PATCH] feat: upgrade send_email api to drf ( 15th ) (#35387) * feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/tests/test_api.py | 9 ++ lms/djangoapps/instructor/views/api.py | 146 ++++++++++-------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 21 +++ 4 files changed, 112 insertions(+), 66 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index b0e533ee6f..65d35221d4 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -3515,6 +3515,14 @@ class TestInstructorSendEmail(SiteMixin, SharedModuleStoreTestCase, LoginEnrollm self.client.logout() url = reverse('send_email', kwargs={'course_id': str(self.course.id)}) response = self.client.post(url, self.full_test_message) + assert response.status_code == 401 + + def test_send_email_logged_in_but_no_perms(self): + self.client.logout() + user = UserFactory() + self.client.login(username=user.username, password=self.TEST_PASSWORD) + url = reverse('send_email', kwargs={'course_id': str(self.course.id)}) + response = self.client.post(url, self.full_test_message) assert response.status_code == 403 def test_send_email_but_not_staff(self): @@ -3635,6 +3643,7 @@ class TestInstructorSendEmail(SiteMixin, SharedModuleStoreTestCase, LoginEnrollm url = reverse('send_email', kwargs={'course_id': str(self.course.id)}) with LogCapture() as log: + response = self.client.post(url, self.full_test_message) assert response.status_code == 400 diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 0c329b09cb..c6db96b3ca 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -107,7 +107,8 @@ from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, Queue 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, SendEmailSerializer ) 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 @@ -2769,81 +2770,96 @@ def list_forum_members(request, course_id): return JsonResponse(response_payload) -@transaction.non_atomic_requests -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.EMAIL) -@require_post_params(send_to="sending to whom", subject="subject line", message="message text") -@common_exceptions_400 -def send_email(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 SendEmail(DeveloperErrorViewMixin, APIView): """ Send an email to self, staff, cohorts, or everyone involved in a course. - Query Parameters: - - 'send_to' specifies what group the email should be sent to - Options are defined by the CourseEmail model in - lms/djangoapps/bulk_email/models.py - - 'subject' specifies email's subject - - 'message' specifies email's content """ - course_id = CourseKey.from_string(course_id) - course_overview = CourseOverview.get_from_id(course_id) + http_method_names = ['post'] + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.EMAIL + serializer_class = SendEmailSerializer - if not is_bulk_email_feature_enabled(course_id): - log.warning(f"Email is not enabled for course {course_id}") - return HttpResponseForbidden("Email is not enabled for this course.") + @method_decorator(ensure_csrf_cookie) + @method_decorator(transaction.non_atomic_requests) + def post(self, request, course_id): + """ + Query Parameters: + - 'send_to' specifies what group the email should be sent to + Options are defined by the CourseEmail model in + lms/djangoapps/bulk_email/models.py + - 'subject' specifies email's subject + - 'message' specifies email's content + """ + course_id = CourseKey.from_string(course_id) + course_overview = CourseOverview.get_from_id(course_id) - targets = json.loads(request.POST.get("send_to")) - subject = request.POST.get("subject") - message = request.POST.get("message") - # optional, this is a date and time in the form of an ISO8601 string - schedule = request.POST.get("schedule", "") + if not is_bulk_email_feature_enabled(course_id): + log.warning(f"Email is not enabled for course {course_id}") + return HttpResponseForbidden("Email is not enabled for this course.") - schedule_dt = None - if schedule: + serializer_data = self.serializer_class(data=request.data) + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) + + # Skipping serializer validation to avoid potential disruptions. + # The API handles numerous input variations, and changes here could introduce breaking issues. + + targets = json.loads(request.POST.get("send_to")) + + subject = serializer_data.validated_data.get("subject") + message = serializer_data.validated_data.get("message") + # optional, this is a date and time in the form of an ISO8601 string + schedule = serializer_data.validated_data.get("schedule", "") + + schedule_dt = None + if schedule: + try: + # convert the schedule from a string to a datetime, then check if its a + # valid future date and time, dateutil + # will throw a ValueError if the schedule is no good. + schedule_dt = dateutil.parser.parse(schedule).replace(tzinfo=pytz.utc) + if schedule_dt < datetime.datetime.now(pytz.utc): + raise ValueError("the requested schedule is in the past") + except ValueError as value_error: + error_message = ( + f"Error occurred creating a scheduled bulk email task. Schedule provided: '{schedule}'. Error: " + f"{value_error}" + ) + log.error(error_message) + return HttpResponseBadRequest(error_message) + + # Retrieve the customized email "from address" and email template from site configuration for the c + # ourse/partner. + # If there is no site configuration enabled for the current site then we use system defaults for both. + from_addr = _get_branded_email_from_address(course_overview) + template_name = _get_branded_email_template(course_overview) + + # Create the CourseEmail object. This is saved immediately so that any transaction that has been + # pending up to this point will also be committed. try: - # convert the schedule from a string to a datetime, then check if its a valid future date and time, dateutil - # will throw a ValueError if the schedule is no good. - schedule_dt = dateutil.parser.parse(schedule).replace(tzinfo=pytz.utc) - if schedule_dt < datetime.datetime.now(pytz.utc): - raise ValueError("the requested schedule is in the past") - except ValueError as value_error: - error_message = ( - f"Error occurred creating a scheduled bulk email task. Schedule provided: '{schedule}'. Error: " - f"{value_error}" + email = create_course_email( + course_id, + request.user, + targets, + subject, + message, + template_name=template_name, + from_addr=from_addr, ) - log.error(error_message) - return HttpResponseBadRequest(error_message) + except ValueError as err: + return HttpResponseBadRequest(repr(err)) - # Retrieve the customized email "from address" and email template from site configuration for the course/partner. If - # there is no site configuration enabled for the current site then we use system defaults for both. - from_addr = _get_branded_email_from_address(course_overview) - template_name = _get_branded_email_template(course_overview) + # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) + task_api.submit_bulk_course_email(request, course_id, email.id, schedule_dt) - # Create the CourseEmail object. This is saved immediately so that any transaction that has been pending up to this - # point will also be committed. - try: - email = create_course_email( - course_id, - request.user, - targets, - subject, - message, - template_name=template_name, - from_addr=from_addr, - ) - except ValueError as err: - return HttpResponseBadRequest(repr(err)) + response_payload = { + 'course_id': str(course_id), + 'success': True, + } - # Submit the task, so that the correct InstructorTask object gets created (for monitoring purposes) - task_api.submit_bulk_course_email(request, course_id, email.id, schedule_dt) - - response_payload = { - 'course_id': str(course_id), - 'success': True, - } - - return JsonResponse(response_payload) + return JsonResponse(response_payload) @require_POST diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 274e3b5ea0..d4719b90b4 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -49,7 +49,7 @@ urlpatterns = [ path('list_email_content', api.ListEmailContent.as_view(), name='list_email_content'), 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('send_email', api.SendEmail.as_view(), 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('show_unit_extensions', api.show_unit_extensions, name='show_unit_extensions'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 0697bed683..ff79124d02 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -77,3 +77,24 @@ class ShowStudentExtensionSerializer(serializers.Serializer): return None return user + + +class SendEmailSerializer(serializers.Serializer): + """ + Serializer for sending an email with optional scheduling. + + Fields: + send_to (str): The email address of the recipient. This field is required. + subject (str): The subject line of the email. This field is required. + message (str): The body of the email. This field is required. + schedule (str, optional): + An optional field to specify when the email should be sent. + If provided, this should be a string that can be parsed into a + datetime format or some other scheduling logic. + """ + send_to = serializers.CharField(write_only=True, required=True) + + # set max length as per model field. + subject = serializers.CharField(max_length=128, write_only=True, required=True) + message = serializers.CharField(required=True) + schedule = serializers.CharField(required=False)