Skip to content

E
edx-platform
Commit f4181663 by Don Mitchell
Options

RESTful refactoring of /course access continued 

Move index access into the url
Move course creation into the url
Add helper methods for testing to serialize json data and set accept header.
parent 7e6f22a4
Showing with 127 additions and 114 deletions
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_course_index.py
......@@ -31,7 +31,7 @@ class TestCourseIndex(CourseTestCase):
"""
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
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,10 +44,10 @@ class InternationalizationTest(ModuleStoreTestCase):
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,
......@@ -56,10 +55,10 @@ class InternationalizationTest(ModuleStoreTestCase):
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 @@ class InternationalizationTest(ModuleStoreTestCase):
@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
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 @@ class AuthTestCase(ContentStoreTestCase):
self.email = 'a@b.com'
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 @@ class AuthTestCase(ContentStoreTestCase):
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 @@ class AuthTestCase(ContentStoreTestCase):
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):
......
cms/djangoapps/contentstore/views/course.py
......@@ -3,9 +3,11 @@ Views related to operations on course objects
"""
import json
import random
from django.utils.translation import ugettext as _
import string # pylint: disable=W0402
import re
import bson
from django.utils.translation import ugettext as _
from django.contrib.auth.decorators import login_required
from django_future.csrf import ensure_csrf_cookie
from django.conf import settings
......@@ -16,6 +18,7 @@ from django.http import HttpResponseBadRequest, HttpResponseNotFound
from util.json_request import JsonResponse
from mitxmako.shortcuts import render_to_response
from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore, loc_mapper
from xmodule.modulestore.inheritance import own_metadata
from xmodule.contentstore.content import StaticContent
......@@ -49,9 +52,9 @@ from student.models import CourseEnrollment
from xmodule.html_module import AboutDescriptor
from xmodule.modulestore.locator import BlockUsageLocator
import re
import bson
__all__ = ['create_new_course', 'course_info', 'course_handler',
from course_creators.views import get_course_creator_status, add_user_with_status_unrequested
__all__ = ['course_info', 'course_handler',
'course_info_updates', 'get_course_settings',
'course_config_graders_page',
'course_config_advanced_page',
......@@ -69,11 +72,12 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid=
will typically be a 'course' object but may not be especially as we support modules.
GET
html: return html page overview for the given course
html: return course listing page if not given a course id
html: return html page overview for the given course if given a course id
json: return json representing the course branch's index entry as well as dag w/ all of the children
replaced w/ json docs where each doc has {'_id': , 'display_name': , 'children': }
POST
json: create (or update?) this course or branch in this course for this user, return resulting json
json: create a course, return resulting json
descriptor (same as in GET course/...). Leaving off /branch/draft would imply create the course w/ default
branches. Cannot change the structure contents ('_id', 'display_name', 'children') but can change the
index entry.
......@@ -86,13 +90,13 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid=
if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'):
if request.method == 'GET':
raise NotImplementedError('coming soon')
elif request.method == 'POST': # not sure if this is only post. If one will have ids, it goes after access
return create_new_course(request)
elif not has_access(
request.user,
BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block)
):
raise PermissionDenied()
elif request.method == 'POST':
raise NotImplementedError()
elif request.method == 'PUT':
raise NotImplementedError()
elif request.method == 'DELETE':
......@@ -100,13 +104,66 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid=
else:
return HttpResponseBadRequest()
elif request.method == 'GET': # assume html
return course_index(request, course_id, branch, version_guid, block)
if course_id is None:
return course_listing(request)
else:
return course_index(request, course_id, branch, version_guid, block)
else:
return HttpResponseNotFound()
@login_required
@ensure_csrf_cookie
def course_listing(request):
"""
List all courses available to the logged in user
"""
courses = modulestore('direct').get_items(['i4x', None, None, 'course', None])
# filter out courses that we don't have access too
def course_filter(course):
"""
Get courses to which this user has access
"""
return (has_access(request.user, course.location)
# pylint: disable=fixme
# TODO remove this condition when templates purged from db
and course.location.course != 'templates'
and course.location.org != ''
and course.location.course != ''
and course.location.name != '')
courses = filter(course_filter, courses)
def format_course_for_view(course):
"""
return tuple of the data which the view requires for each course
"""
# published = false b/c studio manipulates draft versions not b/c the course isn't pub'd
course_loc = loc_mapper().translate_location(
course.location.course_id, course.location, published=False, add_entry_if_missing=True
)
return (
course.display_name,
# note, couldn't get django reverse to work; so, wrote workaround
course_loc.url_reverse('course/', ''),
get_lms_link_for_item(
course.location
),
course.display_org_with_default,
course.display_number_with_default,
course.location.name
)
return render_to_response('index.html', {
'courses': [format_course_for_view(c) for c in courses if not isinstance(c, ErrorDescriptor)],
'user': request.user,
'request_course_creator_url': reverse('contentstore.views.request_course_creator'),
'course_creator_status': _get_course_creator_status(request.user),
})
@login_required
@ensure_csrf_cookie
def course_index(request, course_id, branch, version_guid, block):
"""
Display an editable course overview.
......@@ -142,7 +199,6 @@ def course_index(request, course_id, branch, version_guid, block):
})
@login_required
@expect_json
def create_new_course(request):
"""
......@@ -756,3 +812,28 @@ def textbook_by_id(request, org, course, name, tid):
own_metadata(course_module)
)
return JsonResponse()
def _get_course_creator_status(user):
"""
Helper method for returning the course creator status for a particular user,
taking into account the values of DISABLE_COURSE_CREATION and ENABLE_CREATOR_GROUP.
If the user passed in has not previously visited the index page, it will be
added with status 'unrequested' if the course creator group is in use.
"""
if user.is_staff:
course_creator_status = 'granted'
elif settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False):
course_creator_status = 'disallowed_for_this_site'
elif settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False):
course_creator_status = get_course_creator_status(user)
if course_creator_status is None:
# User not grandfathered in as an existing user, has not previously visited the dashboard page.
# Add the user to the course creator admin table with status 'unrequested'.
add_user_with_status_unrequested(user)
course_creator_status = get_course_creator_status(user)
else:
course_creator_status = 'granted'
return course_creator_status
cms/djangoapps/contentstore/views/public.py
......@@ -9,7 +9,6 @@ from django.conf import settings
from mitxmako.shortcuts import render_to_response
from external_auth.views import ssl_login_shortcut
from .user import index
__all__ = ['signup', 'old_login_redirect', 'login_page', 'howitworks']
......@@ -46,6 +45,6 @@ def login_page(request):
def howitworks(request):
"Proxy view"
if request.user.is_authenticated():
return index(request)
return redirect('/course')
else:
return render_to_response('howitworks.html', {})
cms/djangoapps/contentstore/views/user.py
import json
from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import reverse
from django.contrib.auth.models import User, Group
from django.contrib.auth.decorators import login_required
from django.views.decorators.http import require_http_methods
......@@ -9,17 +8,12 @@ from django.utils.translation import ugettext as _
from django.views.decorators.http import require_POST
from django_future.csrf import ensure_csrf_cookie
from mitxmako.shortcuts import render_to_response
from django.core.context_processors import csrf
from xmodule.modulestore.django import modulestore, loc_mapper
from xmodule.error_module import ErrorDescriptor
from contentstore.utils import get_lms_link_for_item
from util.json_request import JsonResponse
from auth.authz import (
STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME, get_course_groupname_for_role)
from course_creators.views import (
get_course_creator_status, add_user_with_status_unrequested,
user_requested_access)
from course_creators.views import user_requested_access
from .access import has_access
......@@ -28,51 +22,7 @@ from xmodule.modulestore.locator import BlockUsageLocator
from django.http import HttpResponseNotFound
__all__ = ['index', 'request_course_creator', 'course_team_handler']
@login_required
@ensure_csrf_cookie
def index(request):
"""
List all courses available to the logged in user
"""
courses = modulestore('direct').get_items(['i4x', None, None, 'course', None])
# filter out courses that we don't have access too
def course_filter(course):
return (has_access(request.user, course.location)
# TODO remove this condition when templates purged from db
and course.location.course != 'templates'
and course.location.org != ''
and course.location.course != ''
and course.location.name != '')
courses = filter(course_filter, courses)
def format_course_for_view(course):
# published = false b/c studio manipulates draft versions not b/c the course isn't pub'd
course_loc = loc_mapper().translate_location(
course.location.course_id, course.location, published=False, add_entry_if_missing=True
)
return (
course.display_name,
# note, couldn't get django reverse to work; so, wrote workaround
course_loc.url_reverse('course/', ''),
get_lms_link_for_item(
course.location
),
course.display_org_with_default,
course.display_number_with_default,
course.location.name
)
return render_to_response('index.html', {
'courses': [format_course_for_view(c) for c in courses if not isinstance(c, ErrorDescriptor)],
'user': request.user,
'request_course_creator_url': reverse('contentstore.views.request_course_creator'),
'course_creator_status': _get_course_creator_status(request.user),
'csrf': csrf(request)['csrf_token']
})
__all__ = ['request_course_creator', 'course_team_handler']
@require_POST
......@@ -254,27 +204,3 @@ def _course_team_user(request, location, email):
return JsonResponse()
def _get_course_creator_status(user):
"""
Helper method for returning the course creator status for a particular user,
taking into account the values of DISABLE_COURSE_CREATION and ENABLE_CREATOR_GROUP.
If the user passed in has not previously visited the index page, it will be
added with status 'unrequested' if the course creator group is in use.
"""
if user.is_staff:
course_creator_status = 'granted'
elif settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False):
course_creator_status = 'disallowed_for_this_site'
elif settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False):
course_creator_status = get_course_creator_status(user)
if course_creator_status is None:
# User not grandfathered in as an existing user, has not previously visited the dashboard page.
# Add the user to the course creator admin table with status 'unrequested'.
add_user_with_status_unrequested(user)
course_creator_status = get_course_creator_status(user)
else:
course_creator_status = 'granted'
return course_creator_status
cms/static/js/index.js
......@@ -32,7 +32,7 @@ require(["domReady", "jquery", "underscore", "js/utils/cancel_on_escape"],
'run': run
});
$.postJSON('/create_new_course', {
$.postJSON('/course', {
'org': org,
'number': number,
'display_name': display_name,
......
cms/templates/index.html
......@@ -212,7 +212,6 @@ require(["domReady!", "jquery", "jquery.form", "js/index"], function(doc, $) {
<h4 class="title">${_('Your Course Creator Request Status:')}</h4>
<form id="request-coursecreator" action="${request_course_creator_url}" method="post" enctype="multipart/form-data">
<input type="hidden" name="csrfmiddlewaretoken" value="${csrf}">
<div class="form-actions">
<button type="submit" id="request-coursecreator-submit" name="request-coursecreator-submit" class="action-primary action-request"><i class="icon-cog icon-inline icon-spin"></i> <span class="label">${_('Request the Ability to Create Courses')}</span></button>
</div>
......
cms/templates/signup.html
......@@ -124,7 +124,7 @@ require(["jquery", "jquery.cookie"], function($) {
submit_data,
function(json) {
if(json.success) {
location.href = "${reverse('index')}";
location.href = "${'/course'}";
} else {
$('#register_error').html(json.value).stop().addClass('is-shown');
}
......
cms/urls.py
......@@ -30,7 +30,6 @@ urlpatterns = patterns('', # nopep8
url(r'^create_draft$', 'contentstore.views.create_draft', name='create_draft'),
url(r'^publish_draft$', 'contentstore.views.publish_draft', name='publish_draft'),
url(r'^unpublish_unit$', 'contentstore.views.unpublish_unit', name='unpublish_unit'),
url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'),
url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'),
url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/export/(?P<name>[^/]+)$',
......@@ -116,12 +115,11 @@ urlpatterns += patterns(
urlpatterns += patterns(
'contentstore.views',
url(r'^course$', 'index', name='index'),
url(r'^request_course_creator$', 'request_course_creator'),
# (?ix) == ignore case and verbose (multiline regex)
url(r'(?ix)^course/{}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
url(r'(?ix)^checklists/{}(/)?(?P<checklist_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'),
url(r'(?ix)^course_team/{}(/)?(?P<email>.+)?$'.format(parsers.URL_RE_SOURCE), 'course_team_handler'),
url(r'(?ix)^course($|/){}$'.format(parsers.URL_RE_SOURCE), 'course_handler'),
url(r'(?ix)^checklists/{}(/)?(?P<checklist_index>\d+)?$'.format(parsers.URL_RE_SOURCE), 'checklists_handler'),
url(r'(?ix)^orphan/{}$'.format(parsers.URL_RE_SOURCE), 'orphan'),
url(r'(?ix)^assets/{}(/)?(?P<asset_id>.+)?$'.format(parsers.URL_RE_SOURCE), 'assets_handler'),
url(r'(?ix)^import/{}$'.format(parsers.URL_RE_SOURCE), 'import_handler'),
......
common/djangoapps/student/tests/test_userstanding.py
......@@ -53,7 +53,7 @@ class UserStandingTest(TestCase):
try:
self.some_url = reverse('dashboard')
except NoReverseMatch:
self.some_url = reverse('index')
self.some_url = '/course'
# since it's only possible to disable accounts from lms, we're going
# to skip tests for cms
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment