Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RESTful refactoring of /course access continued #1542

Merged
merged 1 commit into from
Oct 30, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/features/signup.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
52 changes: 26 additions & 26 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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_"')

Expand Down Expand Up @@ -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')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why all of these are replaced with get_html. It's definitely nice for the cases where we need to specify HTML (for the refactored methods), but I don't think we should call it universally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It certainly doesn't hurt and makes the test more equivalent to actual use.

self.assertEqual(resp.status_code, 400)

def test_rewrite_nonportable_links_on_import(self):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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'

Expand All @@ -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.')
Expand All @@ -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):
Expand Down Expand Up @@ -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>',
Expand All @@ -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>',
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/tests/test_course_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
15 changes: 7 additions & 8 deletions cms/djangoapps/contentstore/tests/test_i18n.py
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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'
)
Expand All @@ -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'
)
Expand Down
15 changes: 7 additions & 8 deletions cms/djangoapps/contentstore/tests/tests.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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')
Expand All @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions cms/djangoapps/contentstore/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading