Merge pull request #3307 from edx/gprice/deleted-thread-404

Return 404 instead of 500 for missing forum thread

common/static/coffee/src/discussion/views/discussion_thread_view.coffee

@@ -57,8 +57,13 @@ if Backbone?
           @responses.add(data['content']['children'])
           @renderResponseCountAndPagination(data['content']['resp_total'])
           @trigger "thread:responses:rendered"
-        error: =>
-          if firstLoad
+        error: (xhr) =>
+          if xhr.status == 404
+            DiscussionUtil.discussionAlert(
+              gettext("Sorry"),
+              gettext("The thread you selected has been deleted. Please select another thread.")
+            )
+          else if firstLoad
             DiscussionUtil.discussionAlert(
               gettext("Sorry"),
               gettext("We had some trouble loading responses. Please reload the page.")

lms/djangoapps/django_comment_client/forum/tests.py

@@ -1,4 +1,5 @@
 import json
+from django.http import Http404
 from django.test.utils import override_settings
 from django.test.client import Client, RequestFactory
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -108,19 +109,22 @@ def make_mock_thread_data(text, thread_id, include_children):
 def make_mock_request_impl(text, thread_id=None):
     def mock_request_impl(*args, **kwargs):
         url = args[1]
+        data = None
         if url.endswith("threads"):
             data = {
                 "collection": [make_mock_thread_data(text, "dummy_thread_id", False)]
             }
         elif thread_id and url.endswith(thread_id):
             data = make_mock_thread_data(text, thread_id, True)
-        else: # user query
+        elif "/users/" in url:
             data = {
                 "upvoted_ids": [],
                 "downvoted_ids": [],
                 "subscribed_thread_ids": [],
             }
-        return Mock(status_code=200, text=json.dumps(data), json=Mock(return_value=data))
+        if data:
+            return Mock(status_code=200, text=json.dumps(data), json=Mock(return_value=data))
+        return Mock(status_code=404)
     return mock_request_impl
 
 
@@ -233,6 +237,20 @@ class SingleThreadTestCase(ModuleStoreTestCase):
         )
         self.assertEquals(response.status_code, 405)
 
+    def test_not_found(self, mock_request):
+        request = RequestFactory().get("dummy_url")
+        request.user = self.student
+        # Mock request to return 404 for thread request
+        mock_request.side_effect = make_mock_request_impl("dummy", thread_id=None)
+        self.assertRaises(
+            Http404,
+            views.single_thread,
+            request,
+            self.course.id,
+            "test_discussion_id",
+            "test_thread_id"
+        )
+
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 @patch('requests.request')

lms/djangoapps/django_comment_client/forum/views.py

@@ -237,12 +237,17 @@ def single_thread(request, course_id, discussion_id, thread_id):
     # Currently, the front end always loads responses via AJAX, even for this
     # page; it would be a nice optimization to avoid that extra round trip to
     # the comments service.
-    thread = cc.Thread.find(thread_id).retrieve(
-        recursive=request.is_ajax(),
-        user_id=request.user.id,
-        response_skip=request.GET.get("resp_skip"),
-        response_limit=request.GET.get("resp_limit")
-    )
+    try:
+        thread = cc.Thread.find(thread_id).retrieve(
+            recursive=request.is_ajax(),
+            user_id=request.user.id,
+            response_skip=request.GET.get("resp_skip"),
+            response_limit=request.GET.get("resp_limit")
+        )
+    except cc.utils.CommentClientRequestError as e:
+        if e.status_code == 404:
+            raise Http404
+        raise
 
     if request.is_ajax():
         with newrelic.agent.FunctionTrace(nr_transaction, "get_annotated_content_infos"):