From bb7288ee27856a3a5ca06fe0f5d5979df741f164 Mon Sep 17 00:00:00 2001 From: kingoftech-v01 Date: Fri, 10 Apr 2026 04:33:43 +0000 Subject: [PATCH] fix(contentstore): return 405 on PUT/POST to course_notifications_handler Dismissing the Discussions xblocks deprecation banner in Studio caused a 500 error because course_notifications_handler raised NotImplementedError on PUT/POST, which Django turns into a 500. The handler was designed in 2014 for CourseRerunState notifications with a GET/DELETE contract; PUT/POST were never part of the API. Return HttpResponseNotAllowed(['GET', 'DELETE']) so clients hitting those verbs get a proper 405 instead of crashing the server. Also allow null on the CourseIndexSerializer notification_dismiss_url field since it is None when there is no pending CourseRerunState. Closes #34483 --- .../rest_api/v1/serializers/course_index.py | 4 +- cms/djangoapps/contentstore/views/course.py | 14 ++-- .../views/tests/test_course_index.py | 69 +++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_index.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_index.py index 27b7ed857601..f8df60f0f7bf 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_index.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_index.py @@ -26,7 +26,9 @@ class CourseIndexSerializer(serializers.Serializer): language_code = serializers.CharField() lms_link = serializers.CharField() mfe_proctored_exam_settings_url = serializers.CharField() - notification_dismiss_url = serializers.CharField() + # notification_dismiss_url is None when there is no pending CourseRerunState + # for this course, so the field must accept null. See issue #34483. + notification_dismiss_url = serializers.CharField(allow_null=True, required=False) proctoring_errors = ProctoringErrorListSerializer(many=True) reindex_link = serializers.CharField() rerun_notification_id = serializers.IntegerField() diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 37e344e28c0b..b78fa9a04f83 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -17,7 +17,7 @@ from django.core.exceptions import FieldError, ImproperlyConfigured, PermissionDenied from django.core.exceptions import ValidationError as DjangoValidationError from django.db.models import QuerySet -from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound +from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotAllowed, HttpResponseNotFound from django.shortcuts import redirect from django.urls import reverse from django.utils.translation import gettext as _ @@ -211,9 +211,9 @@ def course_notifications_handler(request, course_key_string=None, action_state_i DELETE json: return json repressing success or failure of dismissal/deletion of the notification PUT - Raises a NotImplementedError. + Returns 405 Method Not Allowed — this handler only supports GET and DELETE. POST - Raises a NotImplementedError. + Returns 405 Method Not Allowed — this handler only supports GET and DELETE. """ # ensure that we have a course and an action state if not course_key_string or not action_state_id: @@ -231,10 +231,10 @@ def course_notifications_handler(request, course_key_string=None, action_state_i elif request.method == 'DELETE': # we assume any delete requests dismiss actions from the UI return _dismiss_notification(request, action_state_id) - elif request.method == 'PUT': - raise NotImplementedError() - elif request.method == 'POST': - raise NotImplementedError() + elif request.method in ('PUT', 'POST'): + # PUT/POST are not supported. Return 405 so Django does not raise + # an unhandled exception and produce a 500 (see issue #34483). + return HttpResponseNotAllowed(['GET', 'DELETE']) else: return HttpResponseBadRequest() else: diff --git a/cms/djangoapps/contentstore/views/tests/test_course_index.py b/cms/djangoapps/contentstore/views/tests/test_course_index.py index a9f53880c5f2..562d688cc598 100644 --- a/cms/djangoapps/contentstore/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/views/tests/test_course_index.py @@ -230,6 +230,75 @@ def test_number_of_calls_to_db(self): self.client.get(reverse_course_url('course_handler', self.course.id), content_type="application/json") +class TestCourseNotificationsHandler(CourseTestCase): + """ + Tests for course_notifications_handler behavior on unsupported HTTP methods. + + Regression tests for Bug #34483: dismissing the Discussions xblocks + deprecation banner in Studio used to cause a 500 error because the + handler raised NotImplementedError on PUT/POST. It must instead return + 405 Method Not Allowed. + """ + + MODULESTORE = TEST_DATA_SPLIT_MODULESTORE + + def _build_url(self): + """Build a URL for course_notifications_handler with an arbitrary action_state_id.""" + from django.urls import reverse + return reverse( + 'course_notifications_handler', + kwargs={ + 'course_key_string': str(self.course.id), + 'action_state_id': 1, + }, + ) + + def test_unit_post_returns_405_not_500(self): + """ + Unit: POST to course_notifications_handler must return 405 Method Not + Allowed instead of raising NotImplementedError (which Django turns + into a 500). Bug #34483. + """ + response = self.client.post(self._build_url(), HTTP_ACCEPT='application/json') + assert response.status_code == 405 + assert 'GET' in response['Allow'] + assert 'DELETE' in response['Allow'] + + def test_unit_put_returns_405_not_500(self): + """ + Unit: PUT to course_notifications_handler must return 405 Method Not + Allowed instead of raising NotImplementedError. Bug #34483. + """ + response = self.client.put(self._build_url(), HTTP_ACCEPT='application/json') + assert response.status_code == 405 + assert 'GET' in response['Allow'] + assert 'DELETE' in response['Allow'] + + def test_integration_dismiss_deprecation_banner_does_not_500(self): + """ + Integration: hitting the notification handler with POST or PUT (as + happens when the Studio MFE tries to dismiss the xblocks deprecation + banner) must not produce a 500. Bug #34483. + """ + url = self._build_url() + post_response = self.client.post(url, HTTP_ACCEPT='application/json') + put_response = self.client.put(url, HTTP_ACCEPT='application/json') + assert post_response.status_code != 500 + assert put_response.status_code != 500 + + def test_bug_34483_regression_dismiss_deprecation_banner(self): + """ + Regression for Bug #34483: Dismissing the Discussions xblocks + deprecation banner in Studio used to raise NotImplementedError on the + backend, resulting in a 500 error. After the fix, the handler returns + 405 Method Not Allowed with the correct Allow header. + """ + response = self.client.post(self._build_url(), HTTP_ACCEPT='application/json') + # Before the fix: response.status_code == 500 + # After the fix: response.status_code == 405 + assert response.status_code == 405 + + class TestCourseReIndex(CourseTestCase): """ Unit tests for the course outline.