Merge pull request #311 from edx/ned/staticbook-tests

Ned/staticbook tests

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')
 
-        # Update the data in the mongo datastore
-        store.update_metadata(new_course.location.url(), own_metadata(new_course))
-
-        data = kwargs.get('data')
+        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))
+
         # 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*:
 

lms/djangoapps/staticbook/tests.py

+"""
+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)

lms/djangoapps/staticbook/views.py

+"""
+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})
-
-
-def index_shifted(request, course_id, page):
-    return index(request, course_id=course_id, page=int(page) + 24)
+    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 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,
+        },
+    )

lms/urls.py

@@ -227,23 +227,21 @@ if settings.COURSEWARE_ENABLED:
             'staticbook.views.index', name="book"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/book/(?P<book_index>[^/]*)/(?P<page>[^/]*)$',
             'staticbook.views.index'),
-        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/book-shifted/(?P<page>[^/]*)$',
-            'staticbook.views.index_shifted'),
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/pdfbook/(?P<book_index>[^/]*)/$',
             'staticbook.views.pdf_index', name="pdf_book"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/pdfbook/(?P<book_index>[^/]*)/(?P<page>[^/]*)$',
-            'staticbook.views.pdf_index'),
+            'staticbook.views.pdf_index', name="pdf_book"),
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/pdfbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/$',
-            'staticbook.views.pdf_index'),
+            'staticbook.views.pdf_index', name="pdf_book"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/pdfbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/(?P<page>[^/]*)$',
-            'staticbook.views.pdf_index'),
+            'staticbook.views.pdf_index', name="pdf_book"),
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/$',
             'staticbook.views.html_index', name="html_book"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/$',
-            'staticbook.views.html_index'),
+            'staticbook.views.html_index', name="html_book"),
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/?$',
             'courseware.views.index', name="courseware"),