From e868db2e4d52712e2a11c2fdcb012cc8ab057b5b Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Mon, 2 Feb 2015 19:25:24 +0500 Subject: [PATCH] Fixed multiple textbooks links to only last uploaded textbook. TNL-1304 --- common/lib/xmodule/xmodule/tabs.py | 12 +++-- common/test/acceptance/fixtures/course.py | 23 ++++++++ common/test/acceptance/tests/lms/test_lms.py | 40 ++++++++++++++ lms/djangoapps/courseware/tests/test_tabs.py | 56 +++++++++++++++++++- 4 files changed, 126 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 1e960f3d69..fb0926950a 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -558,7 +558,9 @@ class TextbookTabs(TextbookTabsBase): yield SingleTextbookTab( name=textbook.title, tab_id='textbook/{0}'.format(index), - link_func=lambda course, reverse_func: reverse_func('book', args=[course.id.to_deprecated_string(), index]), + link_func=lambda course, reverse_func, index=index: reverse_func( + 'book', args=[course.id.to_deprecated_string(), index] + ), ) @@ -578,7 +580,9 @@ class PDFTextbookTabs(TextbookTabsBase): yield SingleTextbookTab( name=textbook['tab_title'], tab_id='pdftextbook/{0}'.format(index), - link_func=lambda course, reverse_func: reverse_func('pdf_book', args=[course.id.to_deprecated_string(), index]), + link_func=lambda course, reverse_func, index=index: reverse_func( + 'pdf_book', args=[course.id.to_deprecated_string(), index] + ), ) @@ -598,7 +602,9 @@ class HtmlTextbookTabs(TextbookTabsBase): yield SingleTextbookTab( name=textbook['tab_title'], tab_id='htmltextbook/{0}'.format(index), - link_func=lambda course, reverse_func: reverse_func('html_book', args=[course.id.to_deprecated_string(), index]), + link_func=lambda course, reverse_func, index=index: reverse_func( + 'html_book', args=[course.id.to_deprecated_string(), index] + ), ) diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index 656a12a965..b7c61e818c 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -137,6 +137,7 @@ class CourseFixture(XBlockContainerFixture): self._updates = [] self._handouts = [] self._assets = [] + self._textbooks = [] self._advanced_settings = {} self._course_key = None @@ -165,6 +166,12 @@ class CourseFixture(XBlockContainerFixture): """ self._assets.extend(asset_name) + def add_textbook(self, book_title, chapters): + """ + Add textbook to the list of textbooks to be added when the install method is called. + """ + self._textbooks.append({"chapters": chapters, "tab_title": book_title}) + def add_advanced_settings(self, settings): """ Adds advanced settings to be set on the course when the install method is called. @@ -181,6 +188,7 @@ class CourseFixture(XBlockContainerFixture): self._create_course() self._install_course_updates() self._install_course_handouts() + self._install_course_textbooks() self._configure_course() self._upload_assets() self._add_advanced_settings() @@ -352,6 +360,21 @@ class CourseFixture(XBlockContainerFixture): raise FixtureError('Could not upload {asset_name} with {url}. Status code: {code}'.format( asset_name=asset_name, url=url, code=upload_response.status_code)) + def _install_course_textbooks(self): + """ + Add textbooks to the course, if any are configured. + """ + url = STUDIO_BASE_URL + '/textbooks/' + self._course_key + + for book in self._textbooks: + payload = json.dumps(book) + response = self.session.post(url, headers=self.headers, data=payload) + + if not response.ok: + raise FixtureError( + "Could not add book to course: {0} with {1}. Status was {2}".format( + book, url, response.status_code)) + def _add_advanced_settings(self): """ Add advanced settings. diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index cb10f6bea2..20d05e6232 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -496,6 +496,46 @@ class HighLevelTabTest(UniqueCourseTest): self.assertIn(expected, actual_items) +class PDFTextBooksTabTest(UniqueCourseTest): + """ + Tests that verify each of the textbook tabs available within a course. + """ + + def setUp(self): + """ + Initialize pages and install a course fixture. + """ + super(PDFTextBooksTabTest, self).setUp() + + self.course_info_page = CourseInfoPage(self.browser, self.course_id) + self.tab_nav = TabNavPage(self.browser) + + # Install a course with TextBooks + course_fix = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) + + # Add PDF textbooks to course fixture. + for i in range(1, 3): + course_fix.add_textbook("PDF Book {}".format(i), [{"title": "Chapter Of Book {}".format(i), "url": ""}]) + + course_fix.install() + + # Auto-auth register for the course + AutoAuthPage(self.browser, course_id=self.course_id).visit() + + def test_verify_textbook_tabs(self): + """ + Test multiple pdf textbooks loads correctly in lms. + """ + self.course_info_page.visit() + + # Verify each PDF textbook tab by visiting, it will fail if correct tab is not loaded. + for i in range(1, 3): + self.tab_nav.go_to_tab("PDF Book {}".format(i)) + + class VideoTest(UniqueCourseTest): """ Navigate to a video in the courseware and play it. diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 26fadb0634..21512205a3 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -11,12 +11,12 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.courses import get_course_by_id from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase +from xmodule import tabs from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MIXED_TOY_MODULESTORE, TEST_DATA_MIXED_CLOSED_MODULESTORE ) from courseware.views import get_static_tab_contents, static_tab from student.tests.factories import UserFactory -from xmodule.tabs import CourseTabList from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -59,7 +59,7 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): def test_get_static_tab_contents(self): course = get_course_by_id(self.toy_course_key) request = get_request_for_user(UserFactory.create()) - tab = CourseTabList.get_tab_by_slug(course.tabs, 'resources') + tab = tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') # Test render works okay tab_content = get_static_tab_contents(request, course, tab) @@ -170,3 +170,55 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.assertEqual(course_tab_list[0]['tab_id'], 'courseware') self.assertEqual(course_tab_list[0]['name'], 'Entrance Exam') self.assertEqual(course_tab_list[1]['tab_id'], 'instructor') + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_TOY_MODULESTORE) +class TextBookTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): + """ + Validate tab behavior when dealing with textbooks. + """ + + def setUp(self): + self.course = CourseFactory.create() + self.set_up_books(2) + self.course.tabs = [ + tabs.CoursewareTab(), + tabs.CourseInfoTab(), + tabs.TextbookTabs(), + tabs.PDFTextbookTabs(), + tabs.HtmlTextbookTabs(), + ] + self.setup_user() + self.enroll(self.course) + self.num_textbook_tabs = sum(1 for tab in self.course.tabs if isinstance(tab, tabs.TextbookTabsBase)) + self.num_textbooks = self.num_textbook_tabs * len(self.books) + + def set_up_books(self, num_books): + """Initializes the textbooks in the course and adds the given number of books to each textbook""" + self.books = [MagicMock() for _ in range(num_books)] + for book_index, book in enumerate(self.books): + book.title = 'Book{0}'.format(book_index) + self.course.textbooks = self.books + self.course.pdf_textbooks = self.books + self.course.html_textbooks = self.books + + def test_pdf_textbook_tabs(self): + """ + Test that all textbooks tab links generating correctly. + """ + type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} + + course_tab_list = get_course_tab_list(self.course, self.user) + num_of_textbooks_found = 0 + for tab in course_tab_list: + # Verify links of all textbook type tabs. + if isinstance(tab, tabs.SingleTextbookTab): + book_type, book_index = tab.tab_id.split("/", 1) + expected_link = reverse( + type_to_reverse_name[book_type], + args=[self.course.id.to_deprecated_string(), book_index] + ) + tab_link = tab.link_func(self.course, reverse) + self.assertEqual(tab_link, expected_link) + num_of_textbooks_found += 1 + self.assertEqual(num_of_textbooks_found, self.num_textbooks)