Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Track current chapter.

Browse Source 
- courseware index view now redirects to most recent chapter, or first
- simplify the view a bit
This commit is contained in:
Victor Shnayder
2012-09-04 00:16:15 -04:00
parent 6cfe52680b
commit 4481adb041
5 changed files with 193 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
common/lib/xmodule/xmodule/seq_module.py
View File

@@ -29,6 +29,8 @@ class SequenceModule(XModule):
shared_state=None, **kwargs):
XModule.__init__(self, system, location, definition, descriptor,
instance_state, shared_state, **kwargs)
# NOTE: Position is 1-indexed. This is silly, but there are now student
# positions saved on prod, so it's not easy to fix.
self.position = 1
if instance_state is not None:

10
common/lib/xmodule/xmodule/x_module.py
View File

@@ -465,6 +465,16 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
return self._child_instances
def get_child_by_url_name(self, url_name):
"""
Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise.
"""
for c in self.get_children():
if c.url_name == url_name:
return c
return None
def xmodule_constructor(self, system):
"""
Returns a constructor for an XModule. This constructor takes two

43
lms/djangoapps/courseware/module_render.py
View File

@@ -52,7 +52,7 @@ def make_track_function(request):
return f
def toc_for_course(user, request, course, active_chapter, active_section, course_id=None):
def toc_for_course(user, request, course, active_chapter, active_section):
'''
Create a table of contents from the module store
@@ -75,13 +75,13 @@ def toc_for_course(user, request, course, active_chapter, active_section, course
'''
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
course_id, user, course, depth=2)
course = get_module(user, request, course.location, student_module_cache, course_id)
if course is None:
course.id, user, course, depth=2)
course_module = get_module(user, request, course.location, student_module_cache, course.id)
if course_module is None:
return None
chapters = list()
for chapter in course.get_display_items():
for chapter in course_module.get_display_items():
hide_from_toc = chapter.metadata.get('hide_from_toc','false').lower() == 'true'
if hide_from_toc:
continue
@@ -109,36 +109,6 @@ def toc_for_course(user, request, course, active_chapter, active_section, course
return chapters
def get_section(course_module, chapter, section):
"""
Returns the xmodule descriptor for the name course > chapter > section,
or None if this doesn't specify a valid section
course: Course url
chapter: Chapter url_name
section: Section url_name
"""
if course_module is None:
return
chapter_module = None
for _chapter in course_module.get_children():
if _chapter.url_name == chapter:
chapter_module = _chapter
break
if chapter_module is None:
return
section_module = None
for _section in chapter_module.get_children():
if _section.url_name == section:
section_module = _section
break
return section_module
def get_module(user, request, location, student_module_cache, course_id, position=None):
"""
Get an instance of the xmodule class identified by location,
@@ -293,9 +263,10 @@ def _get_module(user, request, location, student_module_cache, course_id, positi
return module
# TODO (vshnayder): Rename this? It's very confusing.
def get_instance_module(course_id, user, module, student_module_cache):
"""
Returns instance_module is a StudentModule specific to this module for this student,
Returns the StudentModule specific to this module for this student,
or None if this is an anonymous user
"""
if user.is_authenticated():

104
lms/djangoapps/courseware/tests/tests.py
View File

@@ -7,6 +7,7 @@ import time
from nose import SkipTest
from path import path
from pprint import pprint
from urlparse import urlsplit, urlunsplit
from django.contrib.auth.models import User, Group
from django.test import TestCase
@@ -83,6 +84,27 @@ REAL_DATA_MODULESTORE = mongo_store_config(REAL_DATA_DIR)
class ActivateLoginTestCase(TestCase):
'''Check that we can activate and log in'''
def assertRedirectsNoFollow(self, response, expected_url):
"""
http://devblog.point2.com/2010/04/23/djangos-assertredirects-little-gotcha/
Don't check that the redirected-to page loads--there should be other tests for that.
Some of the code taken from django.test.testcases.py
"""
self.assertEqual(response.status_code, 302,
'Response status code was {0} instead of 302'.format(response.status_code))
url = response['Location']
e_scheme, e_netloc, e_path, e_query, e_fragment = urlsplit(
expected_url)
if not (e_scheme or e_netloc):
expected_url = urlunsplit(('http', 'testserver', e_path,
e_query, e_fragment))
self.assertEqual(url, expected_url, "Response redirected to '{0}', expected '{1}'".format(
url, expected_url))
def setUp(self):
email = 'view@test.com'
password = 'foo'
@@ -193,6 +215,25 @@ class PageLoader(ActivateLoginTestCase):
data = parse_json(resp)
self.assertTrue(data['success'])
def check_for_get_code(self, code, url):
"""
Check that we got the expected code. Hacks around our broken 404
handling.
"""
resp = self.client.get(url)
# HACK: workaround the bug that returns 200 instead of 404.
# TODO (vshnayder): once we're returning 404s, get rid of this if.
if code != 404:
self.assertEqual(resp.status_code, code)
# And 'page not found' shouldn't be in the returned page
self.assertTrue(resp.content.lower().find('page not found') == -1)
else:
# look for "page not found" instead of the status code
#print resp.content
self.assertTrue(resp.content.lower().find('page not found') != -1)
def check_pages_load(self, course_name, data_dir, modstore):
"""Make all locations in course load"""
print "Checking course {0} in {1}".format(course_name, data_dir)
@@ -204,7 +245,7 @@ class PageLoader(ActivateLoginTestCase):
course = courses[0]
self.enroll(course)
course_id = course.id
n = 0
num_bad = 0
all_ok = True
@@ -245,6 +286,54 @@ class TestCoursesLoadTestCase(PageLoader):
self.check_pages_load('full', TEST_DATA_DIR, modulestore())
@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
class TestNavigation(PageLoader):
"""Check that navigation state is saved properly"""
def setUp(self):
xmodule.modulestore.django._MODULESTORES = {}
courses = modulestore().get_courses()
def find_course(name):
"""Assumes the course is present"""
return [c for c in courses if c.location.course==name][0]
self.full = find_course("full")
self.toy = find_course("toy")
# Create two accounts
self.student = 'view@test.com'
self.student2 = 'view2@test.com'
self.password = 'foo'
self.create_account('u1', self.student, self.password)
self.create_account('u2', self.student2, self.password)
self.activate_user(self.student)
self.activate_user(self.student2)
def test_accordion_state(self):
"""Make sure that the accordion remembers where you were properly"""
self.login(self.student, self.password)
self.enroll(self.toy)
self.enroll(self.full)
# First request should redirect to Overview
resp = self.client.get(reverse('courseware', kwargs={'course_id': self.toy.id}))
self.assertRedirectsNoFollow(resp, reverse(
'courseware_chapter', kwargs={'course_id': self.toy.id, 'chapter': 'Overview'}))
# Now we directly navigate to a section in a different chapter
self.check_for_get_code(200, reverse('courseware_section',
kwargs={'course_id': self.toy.id,
'chapter':'secret:magic', 'section':'toyvideo'}))
# And now hitting the courseware tab should redirect to 'secret:magic'
resp = self.client.get(reverse('courseware', kwargs={'course_id': self.toy.id}))
self.assertRedirects(resp, reverse('courseware_chapter',
kwargs={'course_id': self.toy.id, 'chapter': 'secret:magic'}),
status_code = 302, target_status_code = 200)
@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
class TestViewAuth(PageLoader):
"""Check that view authentication works properly"""
@@ -272,19 +361,6 @@ class TestViewAuth(PageLoader):
self.activate_user(self.student)
self.activate_user(self.instructor)
def check_for_get_code(self, code, url):
resp = self.client.get(url)
# HACK: workaround the bug that returns 200 instead of 404.
# TODO (vshnayder): once we're returning 404s, get rid of this if.
if code != 404:
self.assertEqual(resp.status_code, code)
# And 'page not found' shouldn't be in the returned page
self.assertTrue(resp.content.lower().find('page not found') == -1)
else:
# look for "page not found" instead of the status code
#print resp.content
self.assertTrue(resp.content.lower().find('page not found') != -1)
def test_instructor_pages(self):
"""Make sure only instructors for the course or staff can load the instructor
dashboard, the grade views, and student profile pages"""

117
lms/djangoapps/courseware/views.py
View File

@@ -23,7 +23,7 @@ from courseware import grades
from courseware.access import has_access
from courseware.courses import (get_course_with_access, get_courses_by_university)
from models import StudentModuleCache
from module_render import toc_for_course, get_module, get_section
from module_render import toc_for_course, get_module, get_instance_module
from student.models import UserProfile
from multicourse import multicourse_settings
@@ -40,9 +40,6 @@ from xmodule.modulestore.search import path_to_location
import comment_client
log = logging.getLogger("mitx.courseware")
template_imports = {'urllib': urllib}
@@ -83,7 +80,7 @@ def courses(request):
return render_to_response("courses.html", {'universities': universities})
def render_accordion(request, course, chapter, section, course_id=None):
def render_accordion(request, course, chapter, section):
''' Draws navigation bar. Takes current position in accordion as
parameter.
@@ -94,7 +91,7 @@ def render_accordion(request, course, chapter, section, course_id=None):
Returns the html string'''
# grab the table of contents
toc = toc_for_course(request.user, request, course, chapter, section, course_id=course_id)
toc = toc_for_course(request.user, request, course, chapter, section)
context = dict([('toc', toc),
('course_id', course.id),
@@ -102,16 +99,55 @@ def render_accordion(request, course, chapter, section, course_id=None):
return render_to_string('accordion.html', context)
def redirect_to_course_position(course_module):
"""
Load the course state for the user, and return a redirect to the
appropriate place in the course: either the first element if there
is no state, or their previous place if there is.
"""
chapters = course_module.get_display_items()
# position is 1-indexed.
if course_module.position - 1 < len(chapters):
chapter = chapters[course_module.position - 1]
elif len(chapters) > 0:
# Something is wrong. Default to first chapter.
chapter = chapters[0]
else:
# oops. Something bad has happened.
raise Http404
return redirect(reverse('courseware_chapter', kwargs={'course_id': course_module.descriptor.id,
'chapter': chapter.url_name}))
def save_course_position(course_module, chapter, instance_module):
"""
chapter: url_name of the chapter
instance_module: the StudentModule object for the course_module
"""
for i, c in enumerate(course_module.get_display_items()):
if c.url_name == chapter:
# Position is 1-indexed
position = i + 1
# Only save if position changed
if position != course_module.position:
course_module.position = position
instance_module.state = course_module.get_instance_state()
instance_module.save()
@login_required
@ensure_csrf_cookie
@cache_control(no_cache=True, no_store=True, must_revalidate=True)
def index(request, course_id, chapter=None, section=None,
position=None):
"""
Displays courseware accordion, and any associated content.
If course, chapter, and section aren't all specified, just returns
the accordion. If they are specified, returns an error if they don't
point to a valid module.
Displays courseware accordion and associated content. If course, chapter,
and section are all specified, renders the page, or returns an error if they
are invalid.
If section is not specified, displays the accordion opened to the right chapter.
If neither chapter or section are specified, redirects to user's most recent
chapter, or the first chapter if this is the user's first visit.
Arguments:
@@ -134,9 +170,20 @@ def index(request, course_id, chapter=None, section=None,
return redirect(reverse('about_course', args=[course.id]))
try:
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
course.id, request.user, course, depth=1)
course_module = get_module(request.user, request, course.location, student_module_cache, course.id)
if course_module is None:
log.warning('If you see this, something went wrong: if we got this'
' far, should have gotten a course module for this user')
return redirect(reverse('about_course', args=[course.id]))
if chapter is None and section is None:
return redirect_to_course_position(course_module)
context = {
'csrf': csrf(request)['csrf_token'],
'accordion': render_accordion(request, course, chapter, section, course_id=course_id),
'accordion': render_accordion(request, course, chapter, section),
'COURSE_TITLE': course.title,
'course': course,
'init': '',
@@ -144,28 +191,32 @@ def index(request, course_id, chapter=None, section=None,
'staff_access': staff_access,
}
look_for_module = chapter is not None and section is not None
if look_for_module:
section_descriptor = get_section(course, chapter, section)
if section_descriptor is not None:
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
course_id, request.user, section_descriptor)
module = get_module(request.user, request,
section_descriptor.location,
student_module_cache, course_id, position)
if module is None:
# User is probably being clever and trying to access something
# they don't have access to.
raise Http404
context['content'] = module.get_html()
else:
log.warning("Couldn't find a section descriptor for course_id '{0}',"
"chapter '{1}', section '{2}'".format(
course_id, chapter, section))
else:
if request.user.is_staff:
# Add a list of all the errors...
context['course_errors'] = modulestore().get_item_errors(course.location)
chapter_descriptor = course.get_child_by_url_name(chapter)
if chapter_descriptor is not None:
instance_module = get_instance_module(course_id, request.user, course_module, student_module_cache)
save_course_position(course_module, chapter, instance_module)
if section is not None:
section_descriptor = chapter_descriptor.get_child_by_url_name(section)
if section_descriptor is None:
# Specifically asked-for section doesn't exist
raise Http404
student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(
course_id, request.user, section_descriptor)
module = get_module(request.user, request,
section_descriptor.location,
student_module_cache, course_id, position)
if module is None:
# User may be trying to be clever and access something
# they don't have access to.
raise Http404
context['content'] = module.get_html()
# if request.user.is_staff:
# # Add a list of all the errors...
# context['course_errors'] = modulestore().get_item_errors(course.location)
result = render_to_response('courseware/courseware.html', context)
except: