diff --git a/cms/djangoapps/contentstore/features/textbooks.feature b/cms/djangoapps/contentstore/features/textbooks.feature index 85c9ee12a7..79b6c66d52 100644 --- a/cms/djangoapps/contentstore/features/textbooks.feature +++ b/cms/djangoapps/contentstore/features/textbooks.feature @@ -19,9 +19,9 @@ Feature: CMS.Textbooks And I upload the textbook "textbook.pdf" And I wait for "2" seconds And I save the textbook - Then I should see a textbook named "Economics" with a chapter path containing "/c4x/MITx/999/asset/textbook.pdf" + Then I should see a textbook named "Economics" with a chapter path containing "/static/textbook.pdf" And I reload the page - Then I should see a textbook named "Economics" with a chapter path containing "/c4x/MITx/999/asset/textbook.pdf" + Then I should see a textbook named "Economics" with a chapter path containing "/static/textbook.pdf" Scenario: Create a textbook with multiple chapters Given I have opened a new course in Studio diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 0b028cdca0..241a1cb320 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1792,10 +1792,10 @@ class ContentStoreTest(ModuleStoreTestCase): # first check PDF textbooks, to make sure the url paths got updated course_module = module_store.get_instance(target_course_id, target_location) - self.assertEquals(len(course_module.pdf_textbooks), 1) - self.assertEquals(len(course_module.pdf_textbooks[0]["chapters"]), 2) - self.assertEquals(course_module.pdf_textbooks[0]["chapters"][0]["url"], '/c4x/MITx/999/asset/Chapter1.pdf') - self.assertEquals(course_module.pdf_textbooks[0]["chapters"][1]["url"], '/c4x/MITx/999/asset/Chapter2.pdf') + self.assertEqual(len(course_module.pdf_textbooks), 1) + self.assertEqual(len(course_module.pdf_textbooks[0]["chapters"]), 2) + self.assertEqual(course_module.pdf_textbooks[0]["chapters"][0]["url"], '/static/Chapter1.pdf') + self.assertEqual(course_module.pdf_textbooks[0]["chapters"][1]["url"], '/static/Chapter2.pdf') def test_import_into_new_course_id_wiki_slug_renamespacing(self): module_store = modulestore('direct') diff --git a/cms/static/js/views/edit_chapter.js b/cms/static/js/views/edit_chapter.js index 2a6fcf4b60..c15eb1b19f 100644 --- a/cms/static/js/views/edit_chapter.js +++ b/cms/static/js/views/edit_chapter.js @@ -65,7 +65,7 @@ define(["js/views/baseview", "underscore", "underscore.string", "jquery", "gette if(!that.model.get('name')) { options.name = response.asset.displayname; } - options.asset_path = response.asset.url; + options.asset_path = response.asset.portable_url; that.model.set(options); } }); diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index b9dd91d09e..56780f0feb 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -517,12 +517,13 @@ def remap_namespace(module, target_location_namespace): # There is more re-namespacing work we have to do when # importing course modules - # remap pdf_textbook urls + # remap pdf_textbook urls to portable static URLs for entry in module.pdf_textbooks: for chapter in entry.get('chapters', []): if StaticContent.is_c4x_path(chapter.get('url', '')): - chapter['url'] = StaticContent.renamespace_c4x_path( - chapter['url'], target_location_namespace + chapter_loc = StaticContent.get_location_from_path(chapter['url']) + chapter['url'] = StaticContent.get_static_path_from_location( + chapter_loc ) # Original wiki_slugs had value location.course. To make them unique this was changed to 'org.course.name'. diff --git a/lms/djangoapps/staticbook/tests.py b/lms/djangoapps/staticbook/tests.py index 37294c517f..65ae5025c6 100644 --- a/lms/djangoapps/staticbook/tests.py +++ b/lms/djangoapps/staticbook/tests.py @@ -22,8 +22,17 @@ 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" }, + {"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"}, + ], +} + +PORTABLE_PDF_BOOK = { + "tab_title": "Textbook", + "title": "A PDF Textbook", + "chapters": [ + {"title": "Chapter 1 for PDF", "url": "/static/chap1.pdf"}, + {"title": "Chapter 2 for PDF", "url": "/static/chap2.pdf"}, ], } @@ -31,8 +40,8 @@ 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" }, + {"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"}, ], } @@ -194,6 +203,33 @@ class StaticPdfBookTest(StaticBookTest): with self.assertRaises(NoReverseMatch): self.make_url('pdf_book', book_index=0, chapter='fooey', page='xyzzy') + def test_static_url_map_contentstore(self): + """ + This ensure static URL mapping is happening properly for + a course that uses the contentstore + """ + self.make_course(pdf_textbooks=[PORTABLE_PDF_BOOK]) + url = self.make_url('pdf_book', book_index=0, chapter=1) + response = self.client.get(url) + self.assertNotContains(response, 'file={}'.format(PORTABLE_PDF_BOOK['chapters'][0]['url'])) + self.assertContains(response, 'file=/c4x/{0.org}/{0.course}/asset/{1}'.format( + self.course.location, + PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) + + def test_static_url_map_static_asset_path(self): + """ + Like above, but used when the course has set a static_asset_path + """ + self.make_course(pdf_textbooks=[PORTABLE_PDF_BOOK], static_asset_path='awesomesauce') + url = self.make_url('pdf_book', book_index=0, chapter=1) + response = self.client.get(url) + self.assertNotContains(response, 'file={}'.format(PORTABLE_PDF_BOOK['chapters'][0]['url'])) + self.assertNotContains(response, 'file=/c4x/{0.org}/{0.course}/asset/{1}'.format( + self.course.location, + PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) + self.assertContains(response, 'file=/static/awesomesauce/{}'.format( + PORTABLE_PDF_BOOK['chapters'][0]['url'].replace('/static/', ''))) + class StaticHtmlBookTest(StaticBookTest): """ diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py index 593ab248d5..17d886be11 100644 --- a/lms/djangoapps/staticbook/views.py +++ b/lms/djangoapps/staticbook/views.py @@ -51,6 +51,7 @@ def remap_static_url(original_url, course): input_url, getattr(course, 'data_dir', None), course_id=course.location.course_id, + static_asset_path=course.static_asset_path ) # strip off the quotes again... return output_url[1:-1]