Merge pull request #257 from edx/sarina/fix-unknown-user-profile

When user profile isn't found, throw a 404 rather than 500

lms/djangoapps/django_comment_client/forum/tests.py

+from django.test.utils import override_settings
+from django.test.client import Client
+from xmodule.modulestore.tests.factories import CourseFactory
+from student.tests.factories import UserFactory, CourseEnrollmentFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from django.core.urlresolvers import reverse
+from util.testing import UrlResetMixin
+
+from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
+from nose.tools import assert_true
+from mock import patch, Mock
+
+import logging
+
+log = logging.getLogger(__name__)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
+
+    @patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    def setUp(self):
+
+        # Patching the ENABLE_DISCUSSION_SERVICE value affects the contents of urls.py,
+        # so we need to call super.setUp() which reloads urls.py (because
+        # of the UrlResetMixin)
+        super(ViewsExceptionTestCase, self).setUp()
+
+        # create a course
+        self.course = CourseFactory.create(org='MITx', course='999',
+                                           display_name='Robot Super Course')
+
+        # Patch the comment client user save method so it does not try
+        # to create a new cc user when creating a django user
+        with patch('student.models.cc.User.save'):
+            uname = 'student'
+            email = 'student@edx.org'
+            password = 'test'
+
+            # Create the student
+            self.student = UserFactory(username=uname, password=password, email=email)
+
+            # Enroll the student in the course
+            CourseEnrollmentFactory(user=self.student, course_id=self.course.id)
+
+            # Log the student in
+            self.client = Client()
+            assert_true(self.client.login(username=uname, password=password))
+
+    @patch('student.models.cc.User.from_django_user')
+    @patch('student.models.cc.User.active_threads')
+    def test_user_profile_exception(self, mock_threads, mock_from_django_user):
+
+        # Mock the code that makes the HTTP requests to the cs_comment_service app
+        # for the profiled user's active threads
+        mock_threads.return_value = [], 1, 1
+
+        # Mock the code that makes the HTTP request to the cs_comment_service app
+        # that gets the current user's info
+        mock_from_django_user.return_value = Mock()
+
+        url = reverse('django_comment_client.forum.views.user_profile',
+                      kwargs={'course_id': self.course.id, 'user_id': '12345'})  # There is no user 12345
+        self.response = self.client.get(url)
+        self.assertEqual(self.response.status_code, 404)
+
+    @patch('student.models.cc.User.from_django_user')
+    @patch('student.models.cc.User.active_threads')
+    def test_user_followed_threads_exception(self, mock_threads, mock_from_django_user):
+
+        # Mock the code that makes the HTTP requests to the cs_comment_service app
+        # for the profiled user's active threads
+        mock_threads.return_value = [], 1, 1
+
+        # Mock the code that makes the HTTP request to the cs_comment_service app
+        # that gets the current user's info
+        mock_from_django_user.return_value = Mock()
+
+        url = reverse('django_comment_client.forum.views.followed_threads',
+                      kwargs={'course_id': self.course.id, 'user_id': '12345'})  # There is no user 12345
+        self.response = self.client.get(url)
+        self.assertEqual(self.response.status_code, 404)

lms/djangoapps/django_comment_client/forum/views.py

@@ -114,7 +114,7 @@ def inline_discussion(request, course_id, discussion_id):
         threads, query_params = get_threads(request, course_id, discussion_id, per_page=INLINE_THREADS_PER_PAGE)
         cc_user = cc.User.from_django_user(request.user)
         user_info = cc_user.to_dict()
-    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
+    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
         # TODO (vshnayder): since none of this code seems to be aware of the fact that
         # sometimes things go wrong, I suspect that the js client is also not
         # checking for errors on request.  Check and fix as needed.
@@ -141,8 +141,8 @@ def inline_discussion(request, course_id, discussion_id):
 
         if is_moderator:
             cohorts = get_course_cohorts(course_id)
-            for c in cohorts:
-                cohorts_list.append({'name': c.name, 'id': c.id})
+            for cohort in cohorts:
+                cohorts_list.append({'name': cohort.name, 'id': cohort.id})
 
         else:
             #students don't get to choose
@@ -174,11 +174,11 @@ def forum_form_discussion(request, course_id):
     try:
         unsafethreads, query_params = get_threads(request, course_id)   # This might process a search query
         threads = [utils.safe_content(thread) for thread in unsafethreads]
-    except (cc.utils.CommentClientMaintenanceError) as err:
+    except cc.utils.CommentClientMaintenanceError:
         log.warning("Forum is in maintenance mode")
         return render_to_response('discussion/maintenance.html', {})
     except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
-        log.error("Error loading forum discussion threads: %s" % str(err))
+        log.error("Error loading forum discussion threads: %s", str(err))
         raise Http404
 
     user = cc.User.from_django_user(request.user)
@@ -244,7 +244,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
 
     try:
         thread = cc.Thread.find(thread_id).retrieve(recursive=True, user_id=request.user.id)
-    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
+    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
         log.error("Error loading single thread.")
         raise Http404
 
@@ -269,7 +269,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
         try:
             threads, query_params = get_threads(request, course_id)
             threads.append(thread.to_dict())
-        except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
+        except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
             log.error("Error loading single thread.")
             raise Http404
 
@@ -369,7 +369,7 @@ def user_profile(request, course_id, user_id):
             }
 
             return render_to_response('discussion/user_profile.html', context)
-    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err:
+    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError, User.DoesNotExist):
         raise Http404
 
 
@@ -412,5 +412,5 @@ def followed_threads(request, course_id, user_id):
             }
 
             return render_to_response('discussion/user_profile.html', context)
-    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError):
+    except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError, User.DoesNotExist):
         raise Http404