From 09501a62e298cad8afea8f143b2f5639423941da Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Mon, 24 Jun 2013 18:02:04 -0400 Subject: [PATCH 1/3] Handle exception with a 404 --- lms/djangoapps/django_comment_client/forum/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index b04bd787d8..558542e804 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -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) as err: 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) as err: raise Http404 From b1424a75b8ae477cbee839a56f97578b15c77374 Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Thu, 27 Jun 2013 13:03:51 -0400 Subject: [PATCH 2/3] Add a test for 404 raised when requesting the profile page of a user that does not exist --- .../django_comment_client/forum/tests.py | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 lms/djangoapps/django_comment_client/forum/tests.py diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py new file mode 100644 index 0000000000..bd18ab80d6 --- /dev/null +++ b/lms/djangoapps/django_comment_client/forum/tests.py @@ -0,0 +1,82 @@ +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) From e5f97b372b1bdd5dcb561d22a6fbd9ae550bfe59 Mon Sep 17 00:00:00 2001 From: Sarina Canelake Date: Fri, 28 Jun 2013 18:00:41 -0400 Subject: [PATCH 3/3] pep8 fixes --- .../django_comment_client/forum/views.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 558542e804..24305a214a 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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, User.DoesNotExist) 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, User.DoesNotExist) as err: + except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError, User.DoesNotExist): raise Http404