Merge pull request #1542 from edx/dhm/restful_course

RESTful refactoring of /course access continued

cms/djangoapps/contentstore/features/signup.feature

@@ -26,4 +26,4 @@ Feature: CMS.Sign in
     And I visit the url "/signin?next=http://www.google.com/"
     When I fill in and submit the signin form
     And I wait for "2" seconds
-    Then I should see that the path is "/"
+    Then I should see that the path is "/course"

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -133,7 +133,7 @@ def check_components_on_page(self, component_types, expected_types):
         # just pick one vertical
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
 
-        resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+        resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
         self.assertEqual(resp.status_code, 200)
 
         for expected in expected_types:
@@ -158,7 +158,7 @@ def test_malformed_edit_unit_request(self):
         descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0]
         location = descriptor.location.replace(name='.' + descriptor.location.name)
 
-        resp = self.client.get(reverse('edit_unit', kwargs={'location': location.url()}))
+        resp = self.client.get_html(reverse('edit_unit', kwargs={'location': location.url()}))
         self.assertEqual(resp.status_code, 400)
 
     def check_edit_unit(self, test_course_name):
@@ -167,7 +167,7 @@ def check_edit_unit(self, test_course_name):
         for descriptor in modulestore().get_items(Location(None, None, 'vertical', None, None)):
             print "Checking ", descriptor.location.url()
             print descriptor.__class__, descriptor.location
-            resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+            resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
             self.assertEqual(resp.status_code, 200)
 
     def lockAnAsset(self, content_store, course_location):
@@ -457,7 +457,7 @@ def test_module_preview_in_whitelist(self):
         # also try a custom response which will trigger the 'is this course in whitelist' logic
         problem_module_location = Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])
         url = reverse('preview_component', kwargs={'location': problem_module_location.url()})
-        resp = self.client.get(url)
+        resp = self.client.get_html(url)
         self.assertEqual(resp.status_code, 200)
 
     def test_video_module_caption_asset_path(self):
@@ -470,7 +470,7 @@ def test_video_module_caption_asset_path(self):
         # also try a custom response which will trigger the 'is this course in whitelist' logic
         video_module_location = Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None])
         url = reverse('preview_component', kwargs={'location': video_module_location.url()})
-        resp = self.client.get(url)
+        resp = self.client.get_html(url)
         self.assertEqual(resp.status_code, 200)
         self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"')
 
@@ -831,7 +831,7 @@ def test_illegal_draft_crud_ops(self):
         self.assertRaises(InvalidVersionError, draft_store.unpublish, location)
 
     def test_bad_contentstore_request(self):
-        resp = self.client.get('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png')
+        resp = self.client.get_html('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png')
         self.assertEqual(resp.status_code, 400)
 
     def test_rewrite_nonportable_links_on_import(self):
@@ -1022,7 +1022,7 @@ def check_import(self, module_store, root_dir, draft_store, content_store, stub_
             # the service in non-draft aware
             if getattr(descriptor, 'is_draft', False):
                 print "Checking {0}....".format(descriptor.location.url())
-                resp = self.client.get(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
+                resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()}))
                 self.assertEqual(resp.status_code, 200)
 
         # verify that we have the content in the draft store as well
@@ -1205,7 +1205,7 @@ def test_course_handouts_rewrites(self):
         handout_location = Location(['i4x', 'edX', 'toy', 'course_info', 'handouts'])
 
         # get module info
-        resp = self.client.get(reverse('module_info', kwargs={'module_location': handout_location}))
+        resp = self.client.get_html(reverse('module_info', kwargs={'module_location': handout_location}))
 
         # make sure we got a successful response
         self.assertEqual(resp.status_code, 200)
@@ -1392,7 +1392,7 @@ def test_forum_unseeding_with_multiple_courses(self):
 
     def test_create_course_duplicate_course(self):
         """Test new course creation - error path"""
-        self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        self.client.ajax_post('/course', self.course_data)
         self.assert_course_creation_failed('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.')
 
     def assert_course_creation_failed(self, error_message):
@@ -1401,7 +1401,7 @@ def assert_course_creation_failed(self, error_message):
         """
         course_id = _get_course_id(self.course_data)
         initially_enrolled = CourseEnrollment.is_enrolled(self.user, course_id)
-        resp = self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        resp = self.client.ajax_post('/course', self.course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
         self.assertEqual(data['ErrMsg'], error_message)
@@ -1411,7 +1411,7 @@ def assert_course_creation_failed(self, error_message):
 
     def test_create_course_duplicate_number(self):
         """Test new course creation - error path"""
-        self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        self.client.ajax_post('/course', self.course_data)
         self.course_data['display_name'] = 'Robot Super Course Two'
         self.course_data['run'] = '2013_Summer'
 
@@ -1420,13 +1420,13 @@ def test_create_course_duplicate_number(self):
     def test_create_course_case_change(self):
         """Test new course creation - error path due to case insensitive name equality"""
         self.course_data['number'] = 'capital'
-        self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        self.client.ajax_post('/course', self.course_data)
         cache_current = self.course_data['org']
         self.course_data['org'] = self.course_data['org'].lower()
         self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
         self.course_data['org'] = cache_current
 
-        self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        self.client.ajax_post('/course', self.course_data)
         cache_current = self.course_data['number']
         self.course_data['number'] = self.course_data['number'].upper()
         self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
@@ -1435,14 +1435,14 @@ def test_course_substring(self):
         """
         Test that a new course can be created whose name is a substring of an existing course
         """
-        self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        self.client.ajax_post('/course', self.course_data)
         cache_current = self.course_data['number']
         self.course_data['number'] = '{}a'.format(self.course_data['number'])
-        resp = self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        resp = self.client.ajax_post('/course', self.course_data)
         self.assertEqual(resp.status_code, 200)
         self.course_data['number'] = cache_current
         self.course_data['org'] = 'a{}'.format(self.course_data['org'])
-        resp = self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        resp = self.client.ajax_post('/course', self.course_data)
         self.assertEqual(resp.status_code, 200)
 
     def test_create_course_with_bad_organization(self):
@@ -1485,13 +1485,13 @@ def assert_course_permission_denied(self):
         """
         Checks that the course did not get created due to a PermissionError.
         """
-        resp = self.client.ajax_post(reverse('create_new_course'), self.course_data)
+        resp = self.client.ajax_post('/course', self.course_data)
         self.assertEqual(resp.status_code, 403)
 
     def test_course_index_view_with_no_courses(self):
         """Test viewing the index page with no courses"""
         # Create a course so there is something to view
-        resp = self.client.get(reverse('index'))
+        resp = self.client.get_html('/course')
         self.assertContains(
             resp,
             '<h1 class="page-header">My Courses</h1>',
@@ -1513,7 +1513,7 @@ def test_item_factory(self):
     def test_course_index_view_with_course(self):
         """Test viewing the index page with an existing course"""
         CourseFactory.create(display_name='Robot Super Educational Course')
-        resp = self.client.get(reverse('index'))
+        resp = self.client.get_html('/course')
         self.assertContains(
             resp,
             '<h3 class="course-title">Robot Super Educational Course</h3>',
@@ -1590,7 +1590,7 @@ def test_cms_imported_course_walkthrough(self):
         # go to various pages
 
         # import page
-        resp = self.client.get(new_location.url_reverse('import/', ''), HTTP_ACCEPT='text/html')
+        resp = self.client.get_html(new_location.url_reverse('import/', ''))
         self.assertEqual(resp.status_code, 200)
 
         # export page
@@ -1602,7 +1602,7 @@ def test_cms_imported_course_walkthrough(self):
 
         # course team
         url = new_location.url_reverse('course_team/', '')
-        resp = self.client.get(url, HTTP_ACCEPT='text/html')
+        resp = self.client.get_html(url)
         self.assertEqual(resp.status_code, 200)
 
         # course info
@@ -1628,18 +1628,18 @@ def test_cms_imported_course_walkthrough(self):
 
         # assets_handler (HTML for full page content)
         url = new_location.url_reverse('assets/', '')
-        resp = self.client.get(url, HTTP_ACCEPT='text/html')
+        resp = self.client.get_html(url)
         self.assertEqual(resp.status_code, 200)
 
         # go look at a subsection page
         subsection_location = loc.replace(category='sequential', name='test_sequence')
-        resp = self.client.get(reverse('edit_subsection',
+        resp = self.client.get_html(reverse('edit_subsection',
                                        kwargs={'location': subsection_location.url()}))
         self.assertEqual(resp.status_code, 200)
 
         # go look at the Edit page
         unit_location = loc.replace(category='vertical', name='test_vertical')
-        resp = self.client.get(reverse('edit_unit',
+        resp = self.client.get_html(reverse('edit_unit',
                                        kwargs={'location': unit_location.url()}))
         self.assertEqual(resp.status_code, 200)
 
@@ -1853,7 +1853,7 @@ def _show_course_overview(self, location):
         Show the course overview page.
         """
         new_location = loc_mapper().translate_location(location.course_id, location, False, True)
-        return self.client.get(new_location.url_reverse('course/', ''), HTTP_ACCEPT='text/html')
+        return self.client.get_html(new_location.url_reverse('course/', ''))
 
 
 @override_settings(MODULESTORE=TEST_MODULESTORE)
@@ -1927,7 +1927,7 @@ def _create_course(test, course_data):
     course_id = _get_course_id(course_data)
     new_location = loc_mapper().translate_location(course_id, CourseDescriptor.id_to_location(course_id), False, True)
 
-    response = test.client.ajax_post(reverse('create_new_course'), course_data)
+    response = test.client.ajax_post('/course', course_data)
     test.assertEqual(response.status_code, 200)
     data = parse_json(response)
     test.assertNotIn('ErrMsg', data)

cms/djangoapps/contentstore/tests/test_course_index.py

@@ -31,7 +31,7 @@ def check_index_and_outline(self, authed_client):
         """
         Test getting the list of courses and then pulling up their outlines
         """
-        index_url = reverse('contentstore.views.index')
+        index_url = '/course'
         index_response = authed_client.get(index_url, {}, HTTP_ACCEPT='text/html')
         parsed_html = lxml.html.fromstring(index_response.content)
         course_link_eles = parsed_html.find_class('course-link')

cms/djangoapps/contentstore/tests/test_i18n.py

@@ -1,12 +1,11 @@
 from unittest import skip
 
-from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
-from django.test.client import Client
 from django.test.utils import override_settings
 
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from contentstore.tests.modulestore_config import TEST_MODULESTORE
+from contentstore.tests.utils import AjaxEnabledTestClient
 
 
 @override_settings(MODULESTORE=TEST_MODULESTORE)
@@ -45,21 +44,21 @@ def setUp(self):
 
     def test_course_plain_english(self):
         """Test viewing the index page with no courses"""
-        self.client = Client()
+        self.client = AjaxEnabledTestClient()
         self.client.login(username=self.uname, password=self.password)
 
-        resp = self.client.get(reverse('index'))
+        resp = self.client.get_html('/course')
         self.assertContains(resp,
                             '<h1 class="page-header">My Courses</h1>',
                             status_code=200,
                             html=True)
 
     def test_course_explicit_english(self):
         """Test viewing the index page with no courses"""
-        self.client = Client()
+        self.client = AjaxEnabledTestClient()
         self.client.login(username=self.uname, password=self.password)
 
-        resp = self.client.get(reverse('index'),
+        resp = self.client.get_html('/course',
                                {},
                                HTTP_ACCEPT_LANGUAGE='en'
                                )
@@ -80,10 +79,10 @@ def test_course_explicit_english(self):
     @skip
     def test_course_with_accents(self):
         """Test viewing the index page with no courses"""
-        self.client = Client()
+        self.client = AjaxEnabledTestClient()
         self.client.login(username=self.uname, password=self.password)
 
-        resp = self.client.get(reverse('index'),
+        resp = self.client.get_html('/course',
                                {},
                                HTTP_ACCEPT_LANGUAGE='fr'
                                )

cms/djangoapps/contentstore/tests/tests.py

@@ -1,9 +1,8 @@
-from django.test.client import Client
 from django.test.utils import override_settings
 from django.core.cache import cache
 from django.core.urlresolvers import reverse
 
-from contentstore.tests.utils import parse_json, user, registration
+from contentstore.tests.utils import parse_json, user, registration, AjaxEnabledTestClient
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from contentstore.tests.test_course_settings import CourseTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -82,12 +81,12 @@ def setUp(self):
         self.email = '[email protected]'
         self.pw = 'xyz'
         self.username = 'testuser'
-        self.client = Client()
+        self.client = AjaxEnabledTestClient()
         # clear the cache so ratelimiting won't affect these tests
         cache.clear()
 
     def check_page_get(self, url, expected):
-        resp = self.client.get(url)
+        resp = self.client.get_html(url)
         self.assertEqual(resp.status_code, expected)
         return resp
 
@@ -152,20 +151,20 @@ def test_login_link_on_activation_age(self):
     def test_private_pages_auth(self):
         """Make sure pages that do require login work."""
         auth_pages = (
-            reverse('index'),
+            '/course',
         )
 
         # These are pages that should just load when the user is logged in
         # (no data needed)
         simple_auth_pages = (
-            reverse('index'),
+            '/course',
         )
 
         # need an activated user
         self.test_create_account()
 
         # Create a new session
-        self.client = Client()
+        self.client = AjaxEnabledTestClient()
 
         # Not logged in.  Should redirect to login.
         print('Not logged in')
@@ -184,7 +183,7 @@ def test_private_pages_auth(self):
     def test_index_auth(self):
 
         # not logged in.  Should return a redirect.
-        resp = self.client.get(reverse('index'))
+        resp = self.client.get_html('/course')
         self.assertEqual(resp.status_code, 302)
 
         # Logged in should work.

cms/djangoapps/contentstore/tests/utils.py

@@ -30,12 +30,24 @@ def registration(email):
 
 
 class AjaxEnabledTestClient(Client):
+    """
+    Convenience class to make testing easier.
+    """
     def ajax_post(self, path, data=None, content_type="application/json", **kwargs):
+        """
+        Convenience method for client post which serializes the data into json and sets the accept type
+        to json
+        """
         if not isinstance(data, basestring):
             data = json.dumps(data or {})
         kwargs.setdefault("HTTP_X_REQUESTED_WITH", "XMLHttpRequest")
         return self.post(path=path, data=data, content_type=content_type, **kwargs)
 
+    def get_html(self, path, data=None, follow=False, **extra):
+        """
+        Convenience method for client.get which sets the accept type to html
+        """
+        return self.get(path, data or {}, follow, HTTP_ACCEPT="text/html", **extra)
 
 @override_settings(MODULESTORE=TEST_MODULESTORE)
 class CourseTestCase(ModuleStoreTestCase):