From 831dd593c9285356bad15e85d0c5ecb4f0a5c54b Mon Sep 17 00:00:00 2001 From: John Eskew Date: Tue, 14 Oct 2014 17:43:18 -0400 Subject: [PATCH] Add tests for empty analytics server info and url-encoded params. --- lms/djangoapps/instructor/tests/test_api.py | 82 ++++++++++++++++++--- lms/djangoapps/instructor/views/api.py | 4 +- 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 3c529ec077..a76d5ecff6 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -29,6 +29,8 @@ from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from courseware.tests.helpers import LoginEnrollmentTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore from student.tests.factories import UserFactory from courseware.tests.factories import StaffFactory, InstructorFactory, BetaTesterFactory from student.roles import CourseBetaTesterRole @@ -2563,6 +2565,7 @@ class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCas self.assertDictEqual(expected_info, returned_info) +@ddt.ddt @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(ANALYTICS_SERVER_URL="http://robotanalyticsserver.netbot:900/") @override_settings(ANALYTICS_API_KEY="robot_api_key") @@ -2590,25 +2593,84 @@ class TestInstructorAPIAnalyticsProxy(ModuleStoreTestCase, LoginEnrollmentTestCa self.instructor = InstructorFactory(course_key=self.course.id) self.client.login(username=self.instructor.username, password='test') + @ddt.data((ModuleStoreEnum.Type.mongo, False), (ModuleStoreEnum.Type.split, True)) + @ddt.unpack @patch.object(instructor.views.api.requests, 'get') - def test_analytics_proxy_url(self, act): + def test_analytics_proxy_url(self, store_type, assert_wo_encoding, act): """ Test legacy analytics proxy url generation. """ + with modulestore().default_store(store_type): + course = CourseFactory.create() + instructor_local = InstructorFactory(course_key=course.id) + self.client.login(username=instructor_local.username, password='test') + + act.return_value = self.FakeProxyResponse() + + url = reverse('proxy_legacy_analytics', kwargs={'course_id': course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'aname': 'ProblemGradeDistribution' + }) + self.assertEqual(response.status_code, 200) + + # Make request URL pattern - everything but course id. + url_pattern = "{url}get?aname={aname}&course_id={course_id}&apikey={api_key}".format( + url="http://robotanalyticsserver.netbot:900/", + aname="ProblemGradeDistribution", + course_id="{course_id!s}", + api_key="robot_api_key", + ) + + if assert_wo_encoding: + # Format url with no URL-encoding of parameters. + assert_url = url_pattern.format(course_id=course.id.to_deprecated_string()) + with self.assertRaises(AssertionError): + act.assert_called_once_with(assert_url) + + # Format url *with* URL-encoding of parameters. + expected_url = url_pattern.format(course_id=quote(course.id.to_deprecated_string())) + act.assert_called_once_with(expected_url) + + @override_settings(ANALYTICS_SERVER_URL="") + @patch.object(instructor.views.api.requests, 'get') + def test_analytics_proxy_server_url(self, act): + """ + Test legacy analytics when empty server url. + """ act.return_value = self.FakeProxyResponse() url = reverse('proxy_legacy_analytics', kwargs={'course_id': self.course.id.to_deprecated_string()}) response = self.client.get(url, { 'aname': 'ProblemGradeDistribution' }) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 501) - # check request url - expected_url = "{url}get?aname={aname}&course_id={course_id!s}&apikey={api_key}".format( - url="http://robotanalyticsserver.netbot:900/", - aname="ProblemGradeDistribution", - course_id=self.course.id.to_deprecated_string(), - api_key="robot_api_key", - ) - act.assert_called_once_with(expected_url) + @override_settings(ANALYTICS_API_KEY="") + @patch.object(instructor.views.api.requests, 'get') + def test_analytics_proxy_api_key(self, act): + """ + Test legacy analytics when empty server API key. + """ + act.return_value = self.FakeProxyResponse() + + url = reverse('proxy_legacy_analytics', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'aname': 'ProblemGradeDistribution' + }) + self.assertEqual(response.status_code, 501) + + @override_settings(ANALYTICS_SERVER_URL="") + @override_settings(ANALYTICS_API_KEY="") + @patch.object(instructor.views.api.requests, 'get') + def test_analytics_proxy_empty_url_and_api_key(self, act): + """ + Test legacy analytics when empty server url & API key. + """ + act.return_value = self.FakeProxyResponse() + + url = reverse('proxy_legacy_analytics', kwargs={'course_id': self.course.id.to_deprecated_string()}) + response = self.client.get(url, { + 'aname': 'ProblemGradeDistribution' + }) + self.assertEqual(response.status_code, 501) @patch.object(instructor.views.api.requests, 'get') def test_analytics_proxy(self, act): diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 8403942204..901dab3533 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1697,7 +1697,7 @@ def send_email(request, course_id): course_id, request.user, send_to, - subject,message, + subject, message, template_name=template_name, from_addr=from_addr ) @@ -1800,7 +1800,7 @@ def proxy_legacy_analytics(request, course_id): url = "{}get?aname={}&course_id={}&apikey={}".format( settings.ANALYTICS_SERVER_URL, analytics_name, - urllib.quote(course_id.to_deprecated_string()), + urllib.quote(unicode(course_id)), settings.ANALYTICS_API_KEY, )