diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index a7f0a71a59..9e71fad279 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -19,14 +19,13 @@ class XModuleCourseFactory(Factory): ABSTRACT_FACTORY = True @classmethod - def _create(cls, target_class, *args, **kwargs): + def _create(cls, target_class, **kwargs): template = Location('i4x', 'edx', 'templates', 'course', 'Empty') - org = kwargs.get('org') - number = kwargs.get('number') - display_name = kwargs.get('display_name') - location = Location('i4x', org, number, - 'course', Location.clean(display_name)) + org = kwargs.pop('org', None) + number = kwargs.pop('number', None) + display_name = kwargs.pop('display_name', None) + location = Location('i4x', org, number, 'course', Location.clean(display_name)) try: store = modulestore('direct') @@ -41,7 +40,7 @@ class XModuleCourseFactory(Factory): new_course.display_name = display_name new_course.lms.start = datetime.datetime.now(UTC) - new_course.tabs = kwargs.get( + new_course.tabs = kwargs.pop( 'tabs', [ {"type": "courseware"}, @@ -51,15 +50,18 @@ class XModuleCourseFactory(Factory): {"type": "progress", "name": "Progress"} ] ) - new_course.discussion_link = kwargs.get('discussion_link') + + data = kwargs.pop('data', None) + if data is not None: + store.update_item(new_course.location, data) + + # The rest of kwargs become attributes on the course: + for k, v in kwargs.iteritems(): + setattr(new_course, k, v) # Update the data in the mongo datastore store.update_metadata(new_course.location.url(), own_metadata(new_course)) - data = kwargs.get('data') - if data is not None: - store.update_item(new_course.location, data) - # update_item updates the the course as it exists in the modulestore, but doesn't # update the instance we are working with, so have to refetch the course after updating it. new_course = store.get_instance(new_course.id, new_course.location) @@ -101,7 +103,7 @@ class XModuleItemFactory(Factory): return parent._replace(category=attr.category, name=dest_name) @classmethod - def _create(cls, target_class, *args, **kwargs): + def _create(cls, target_class, **kwargs): """ Uses *kwargs*: diff --git a/lms/djangoapps/staticbook/tests.py b/lms/djangoapps/staticbook/tests.py new file mode 100644 index 0000000000..0786aa58b9 --- /dev/null +++ b/lms/djangoapps/staticbook/tests.py @@ -0,0 +1,152 @@ +""" +Test the lms/staticbook views. +""" + +from django.test.utils import override_settings +from django.core.urlresolvers import reverse + +from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE +from student.tests.factories import UserFactory, CourseEnrollmentFactory +from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +PDF_BOOK = { + "tab_title": "Textbook", + "title": "A PDF Textbook", + "chapters": [ + { "title": "Chapter 1 for PDF", "url": "https://somehost.com/the_book/chap1.pdf" }, + { "title": "Chapter 2 for PDF", "url": "https://somehost.com/the_book/chap2.pdf" }, + ], +} + +HTML_BOOK = { + "tab_title": "Textbook", + "title": "An HTML Textbook", + "chapters": [ + { "title": "Chapter 1 for HTML", "url": "https://somehost.com/the_book/chap1.html" }, + { "title": "Chapter 2 for HTML", "url": "https://somehost.com/the_book/chap2.html" }, + ], +} + +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) +class StaticBookTest(ModuleStoreTestCase): + """ + Helpers for the static book tests. + """ + + def __init__(self, *args, **kwargs): + super(StaticBookTest, self).__init__(*args, **kwargs) + self.course = None + + def make_course(self, **kwargs): + """ + Make a course with an enrolled logged-in student. + """ + self.course = CourseFactory.create(**kwargs) + user = UserFactory.create() + CourseEnrollmentFactory.create(user=user, course_id=self.course.id) + self.client.login(username=user.username, password='test') + + def make_url(self, url_name, **kwargs): + """ + Make a URL for a `url_name` using keyword args for url slots. + + Automatically provides the course id. + + """ + kwargs['course_id'] = self.course.id + url = reverse(url_name, kwargs=kwargs) + return url + + +class StaticPdfBookTest(StaticBookTest): + """ + Test the PDF static book view. + """ + + def test_book(self): + # We can access a book. + self.make_course(pdf_textbooks=[PDF_BOOK]) + url = self.make_url('pdf_book', book_index=0) + response = self.client.get(url) + self.assertContains(response, "Chapter 1 for PDF") + self.assertNotContains(response, "options.chapterNum =") + self.assertNotContains(response, "options.pageNum =") + + def test_book_chapter(self): + # We can access a book at a particular chapter. + self.make_course(pdf_textbooks=[PDF_BOOK]) + url = self.make_url('pdf_book', book_index=0, chapter=2) + response = self.client.get(url) + self.assertContains(response, "Chapter 2 for PDF") + self.assertContains(response, "options.chapterNum = 2;") + self.assertNotContains(response, "options.pageNum =") + + def test_book_page(self): + # We can access a book at a particular page. + self.make_course(pdf_textbooks=[PDF_BOOK]) + url = self.make_url('pdf_book', book_index=0, page=17) + response = self.client.get(url) + self.assertContains(response, "Chapter 1 for PDF") + self.assertNotContains(response, "options.chapterNum =") + self.assertContains(response, "options.pageNum = 17;") + + def test_book_chapter_page(self): + # We can access a book at a particular chapter and page. + self.make_course(pdf_textbooks=[PDF_BOOK]) + url = self.make_url('pdf_book', book_index=0, chapter=2, page=17) + response = self.client.get(url) + self.assertContains(response, "Chapter 2 for PDF") + self.assertContains(response, "options.chapterNum = 2;") + self.assertContains(response, "options.pageNum = 17;") + + def test_bad_book_id(self): + # If we have one book, asking for the second book will fail with a 404. + self.make_course(pdf_textbooks=[PDF_BOOK]) + url = self.make_url('pdf_book', book_index=1, chapter=1) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + def test_no_book(self): + # If we have no books, asking for the first book will fail with a 404. + self.make_course() + url = self.make_url('pdf_book', book_index=0, chapter=1) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + +class StaticHtmlBookTest(StaticBookTest): + """ + Test the HTML static book view. + """ + + def test_book(self): + # We can access a book. + self.make_course(html_textbooks=[HTML_BOOK]) + url = self.make_url('html_book', book_index=0) + response = self.client.get(url) + self.assertContains(response, "Chapter 1 for HTML") + self.assertNotContains(response, "options.chapterNum =") + + def test_book_chapter(self): + # We can access a book at a particular chapter. + self.make_course(html_textbooks=[HTML_BOOK]) + url = self.make_url('html_book', book_index=0, chapter=2) + response = self.client.get(url) + self.assertContains(response, "Chapter 2 for HTML") + self.assertContains(response, "options.chapterNum = 2;") + + def test_bad_book_id(self): + # If we have one book, asking for the second book will fail with a 404. + self.make_course(html_textbooks=[HTML_BOOK]) + url = self.make_url('html_book', book_index=1, chapter=1) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) + + def test_no_book(self): + # If we have no books, asking for the first book will fail with a 404. + self.make_course() + url = self.make_url('html_book', book_index=0, chapter=1) + response = self.client.get(url) + self.assertEqual(response.status_code, 404) diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index fcfba9e22c..9ed14bfb6c 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -1,3 +1,7 @@ +""" +Views for serving static textbooks. +""" + from django.contrib.auth.decorators import login_required from django.http import Http404 from mitxmako.shortcuts import render_to_response @@ -10,6 +14,9 @@ from static_replace import replace_static_urls @login_required def index(request, course_id, book_index, page=None): + """ + Serve static image-based textbooks. + """ course = get_course_with_access(request.user, course_id, 'load') staff_access = has_access(request.user, course, 'staff') @@ -22,18 +29,31 @@ def index(request, course_id, book_index, page=None): if page is None: page = textbook.start_page - return render_to_response('staticbook.html', - {'book_index': book_index, 'page': int(page), - 'course': course, - 'book_url': textbook.book_url, - 'table_of_contents': table_of_contents, - 'start_page': textbook.start_page, - 'end_page': textbook.end_page, - 'staff_access': staff_access}) + return render_to_response( + 'staticbook.html', + { + 'book_index': book_index, 'page': int(page), + 'course': course, + 'book_url': textbook.book_url, + 'table_of_contents': table_of_contents, + 'start_page': textbook.start_page, + 'end_page': textbook.end_page, + 'staff_access': staff_access, + }, + ) -def index_shifted(request, course_id, page): - return index(request, course_id=course_id, page=int(page) + 24) +def remap_static_url(original_url, course): + """Remap a URL in the ways the course requires.""" + # Ick: this should be possible without having to quote and unquote the URL... + input_url = "'" + original_url + "'" + output_url = replace_static_urls( + input_url, + getattr(course, 'data_dir', None), + course_namespace=course.location, + ) + # strip off the quotes again... + return output_url[1:-1] @login_required @@ -60,16 +80,6 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): raise Http404("Invalid book index value: {0}".format(book_index)) textbook = course.pdf_textbooks[book_index] - def remap_static_url(original_url, course): - input_url = "'" + original_url + "'" - output_url = replace_static_urls( - input_url, - getattr(course, 'data_dir', None), - course_namespace=course.location - ) - # strip off the quotes again... - return output_url[1:-1] - if 'url' in textbook: textbook['url'] = remap_static_url(textbook['url'], course) # then remap all the chapter URLs as well, if they are provided. @@ -77,13 +87,17 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None): for entry in textbook['chapters']: entry['url'] = remap_static_url(entry['url'], course) - return render_to_response('static_pdfbook.html', - {'book_index': book_index, - 'course': course, - 'textbook': textbook, - 'chapter': chapter, - 'page': page, - 'staff_access': staff_access}) + return render_to_response( + 'static_pdfbook.html', + { + 'book_index': book_index, + 'course': course, + 'textbook': textbook, + 'chapter': chapter, + 'page': page, + 'staff_access': staff_access, + }, + ) @login_required @@ -109,16 +123,6 @@ def html_index(request, course_id, book_index, chapter=None): raise Http404("Invalid book index value: {0}".format(book_index)) textbook = course.html_textbooks[book_index] - def remap_static_url(original_url, course): - input_url = "'" + original_url + "'" - output_url = replace_static_urls( - input_url, - getattr(course, 'data_dir', None), - course_namespace=course.location - ) - # strip off the quotes again... - return output_url[1:-1] - if 'url' in textbook: textbook['url'] = remap_static_url(textbook['url'], course) # then remap all the chapter URLs as well, if they are provided. @@ -126,10 +130,14 @@ def html_index(request, course_id, book_index, chapter=None): for entry in textbook['chapters']: entry['url'] = remap_static_url(entry['url'], course) - return render_to_response('static_htmlbook.html', - {'book_index': book_index, - 'course': course, - 'textbook': textbook, - 'chapter': chapter, - 'staff_access': staff_access, - 'notes_enabled': notes_enabled}) + return render_to_response( + 'static_htmlbook.html', + { + 'book_index': book_index, + 'course': course, + 'textbook': textbook, + 'chapter': chapter, + 'staff_access': staff_access, + 'notes_enabled': notes_enabled, + }, + ) diff --git a/lms/urls.py b/lms/urls.py index 4f9af149bc..51c6ba13b7 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -227,23 +227,21 @@ if settings.COURSEWARE_ENABLED: 'staticbook.views.index', name="book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book/(?P[^/]*)/(?P[^/]*)$', 'staticbook.views.index'), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/book-shifted/(?P[^/]*)$', - 'staticbook.views.index_shifted'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/pdfbook/(?P[^/]*)/$', 'staticbook.views.pdf_index', name="pdf_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/pdfbook/(?P[^/]*)/(?P[^/]*)$', - 'staticbook.views.pdf_index'), + 'staticbook.views.pdf_index', name="pdf_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/pdfbook/(?P[^/]*)/chapter/(?P[^/]*)/$', - 'staticbook.views.pdf_index'), + 'staticbook.views.pdf_index', name="pdf_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/pdfbook/(?P[^/]*)/chapter/(?P[^/]*)/(?P[^/]*)$', - 'staticbook.views.pdf_index'), + 'staticbook.views.pdf_index', name="pdf_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/htmlbook/(?P[^/]*)/$', 'staticbook.views.html_index', name="html_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/htmlbook/(?P[^/]*)/chapter/(?P[^/]*)/$', - 'staticbook.views.html_index'), + 'staticbook.views.html_index', name="html_book"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/?$', 'courseware.views.index', name="courseware"),