From ecf9923e9664b478fe6e1a933231f985e84445e8 Mon Sep 17 00:00:00 2001
From: Greg Price <gprice@edx.org>
Date: Wed, 9 Apr 2014 16:34:09 -0400
Subject: [PATCH] Return 404 instead of 500 for missing forum thread

Also show a more specific error message in the front end. This change
only has an effect if using cs_comments_service commit 1d71330 or later.
---
 .../views/discussion_thread_view.coffee       |  9 ++++++--
 .../django_comment_client/forum/tests.py      | 22 +++++++++++++++++--
 .../django_comment_client/forum/views.py      | 17 +++++++++-----
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
index c753d15f3c..9f5a1a4ec8 100644
--- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee
+++ b/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.")
diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py
index 188aba479e..f13084646c 100644
--- a/lms/djangoapps/django_comment_client/forum/tests.py
+++ b/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')
diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py
index 407d0715a9..e70be6c1b6 100644
--- a/lms/djangoapps/django_comment_client/forum/views.py
+++ b/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"):