diff --git a/.gitignore b/.gitignore index b3f7473dc0..1a23b36b2a 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ cms/envs/private.py /nbproject .idea/ .redcar/ +codekit-config.json ### OS X artifacts *.DS_Store @@ -48,14 +49,18 @@ reports/ .prereqs_cache .vagrant/ node_modules +.bundle/ +bin/ ### Static assets pipeline artifacts *.scssc +lms/static/css/ lms/static/sass/*.css lms/static/sass/application.scss lms/static/sass/application-extend1.scss lms/static/sass/application-extend2.scss lms/static/sass/course.scss +cms/static/css/ cms/static/sass/*.css ### Logging artifacts diff --git a/AUTHORS b/AUTHORS index 9326b6781a..60ac912d49 100644 --- a/AUTHORS +++ b/AUTHORS @@ -97,3 +97,4 @@ Iain Dunning Olivier Marquez Florian Dufour Manuel Freire +Daniel Cebrián Robles diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b0239dc86b..9680759f8b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,9 +13,36 @@ Blades: Added grading support for LTI module. LTI providers can now grade student's work and send edX scores. OAuth1 based authentication implemented. BLD-384. -LMS: Beta-tester status is now set on a per-course-run basis, rather than being valid - across all runs with the same course name. Old group membership will still work - across runs, but new beta-testers will only be added to a single course run. +LMS: Beta-tester status is now set on a per-course-run basis, rather than being + valid across all runs with the same course name. Old group membership will + still work across runs, but new beta-testers will only be added to a single + course run. + +Blades: Enabled several Video Jasmine tests. BLD-463. + +Studio: Continued modification of Studio pages to follow a RESTful framework. +includes Settings pages, edit page for Subsection and Unit, and interfaces +for updating xblocks (xmodules) and getting their editing HTML. + +Blades: Put 2nd "Hide output" button at top of test box & increase text size for +code response questions. BLD-126. + +Blades: Update the calculator hints tooltip with full information. BLD-400. + +Blades: Fix transcripts 500 error in studio (BLD-530) + +LMS: Add error recovery when a user loads or switches pages in an +inline discussion. + +Blades: Allow multiple strings as the correct answer to a string response +question. BLD-474. + +Blades: a11y - Videos will alert screenreaders when the video is over. + +LMS: Trap focus on the loading element when a user loads more threads +in the forum sidebar to improve accessibility. + +LMS: Add error recovery when a user loads more threads in the forum sidebar. LMS: Add a user-visible alert modal when a forums AJAX request fails. @@ -36,7 +63,8 @@ text like with bold or italics. (BLD-449) LMS: Beta instructor dashboard will only count actively enrolled students for course enrollment numbers. -Blades: Fix speed menu that is not rendered correctly when YouTube is unavailable. (BLD-457). +Blades: Fix speed menu that is not rendered correctly when YouTube is +unavailable. (BLD-457). LMS: Users with is_staff=True no longer have the STAFF label appear on their forum posts. @@ -54,6 +82,9 @@ key in course settings. (BLD-426) Blades: Fix bug when the speed can only be changed when the video is playing. +LMS: The dialogs on the wiki "changes" page are now accessible to screen +readers. Now all wiki pages have been made accessible. (LMS-1337) + LMS: Change bulk email implementation to use less memory, and to better handle duplicate tasks in celery. @@ -70,8 +101,8 @@ client error are correctly passed through to the client. LMS: Improve performance of page load and thread list load for discussion tab -LMS: The wiki markup cheatsheet dialog is now accessible to people with -disabilites. (LMS-1303) +LMS: The wiki markup cheatsheet dialog is now accessible to screen readers. +(LMS-1303) Common: Add skip links for accessibility to CMS and LMS. (LMS-1311) diff --git a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py index 5473438571..d3293c474e 100644 --- a/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py +++ b/cms/djangoapps/contentstore/features/component_settings_editor_helpers.py @@ -6,14 +6,6 @@ from nose.tools import assert_equal, assert_in # pylint: disable=E0611 from terrain.steps import reload_the_page -def _is_expected_element_count(css, expected_number): - """ - Returns whether the number of elements found on the page by css locator - the same number that you expected. - """ - return len(world.css_find(css)) == expected_number - - @world.absorb def create_component_instance(step, category, component_type=None, is_advanced=False): """ @@ -47,8 +39,11 @@ def create_component_instance(step, category, component_type=None, is_advanced=F world.wait_for_invisible(component_button_css) click_component_from_menu(category, component_type, is_advanced) - world.wait_for(lambda _: _is_expected_element_count(module_css, - module_count_before + 1)) + expected_count = module_count_before + 1 + world.wait_for( + lambda _: len(world.css_find(module_css)) == expected_count, + timeout=20 + ) @world.absorb diff --git a/cms/djangoapps/contentstore/features/course-updates.feature b/cms/djangoapps/contentstore/features/course-updates.feature index 6f24fba68c..152da9c349 100644 --- a/cms/djangoapps/contentstore/features/course-updates.feature +++ b/cms/djangoapps/contentstore/features/course-updates.feature @@ -76,3 +76,17 @@ Feature: CMS.Course updates Then I see the handout "/c4x/MITx/999/asset/modified.jpg" And when I reload the page Then I see the handout "/c4x/MITx/999/asset/modified.jpg" + + Scenario: Users cannot save handouts with bad html until edit or update it properly + Given I have opened a new course in Studio + And I go to the course updates page + When I modify the handout to "

[LINK TEXT]

" + Then I see the handout error text + And I see handout save button disabled + When I edit the handout to "

home

" + Then I see handout save button re-enabled + When I save handout edit + # Can only do partial text matches because of the quotes with in quotes (and regexp step matching). + Then I see the handout "https://www.google.com.pk/" + And when I reload the page + Then I see the handout "https://www.google.com.pk/" diff --git a/cms/djangoapps/contentstore/features/course-updates.py b/cms/djangoapps/contentstore/features/course-updates.py index da74f5aa4b..b41578c907 100644 --- a/cms/djangoapps/contentstore/features/course-updates.py +++ b/cms/djangoapps/contentstore/features/course-updates.py @@ -90,6 +90,35 @@ def check_handout(_step, handout): assert handout in world.css_html(handout_css) +@step(u'I see the handout error text') +def check_handout_error(_step): + handout_error_css = 'div#handout_error' + assert world.css_has_class(handout_error_css, 'is-shown') + + +@step(u'I see handout save button disabled') +def check_handout_error(_step): + handout_save_button = 'form.edit-handouts-form a.save-button' + assert world.css_has_class(handout_save_button, 'is-disabled') + + +@step(u'I edit the handout to "([^"]*)"$') +def edit_handouts(_step, text): + type_in_codemirror(0, text) + + +@step(u'I see handout save button re-enabled') +def check_handout_error(_step): + handout_save_button = 'form.edit-handouts-form a.save-button' + assert not world.css_has_class(handout_save_button, 'is-disabled') + + +@step(u'I save handout edit') +def check_handout_error(_step): + save_css = 'a.save-button' + world.css_click(save_css) + + def change_text(text): type_in_codemirror(0, text) save_css = 'a.save-button' diff --git a/cms/djangoapps/contentstore/features/static-pages.feature b/cms/djangoapps/contentstore/features/static-pages.feature index 54d23d985d..39399fb207 100644 --- a/cms/djangoapps/contentstore/features/static-pages.feature +++ b/cms/djangoapps/contentstore/features/static-pages.feature @@ -9,10 +9,8 @@ Feature: CMS.Static Pages Then I should see a static page named "Empty" Scenario: Users can delete static pages - Given I have opened a new course in Studio - And I go to the static pages page - And I add a new page - And I "delete" the static page + Given I have created a static page + When I "delete" the static page Then I am shown a prompt When I confirm the prompt Then I should not see any static pages @@ -20,9 +18,16 @@ Feature: CMS.Static Pages # Safari won't update the name properly @skip_safari Scenario: Users can edit static pages - Given I have opened a new course in Studio - And I go to the static pages page - And I add a new page + Given I have created a static page When I "edit" the static page And I change the name to "New" Then I should see a static page named "New" + + # Safari won't update the name properly + @skip_safari + Scenario: Users can reorder static pages + Given I have created two different static pages + When I reorder the tabs + Then the tabs are in the reverse order + And I reload the page + Then the tabs are in the reverse order diff --git a/cms/djangoapps/contentstore/features/static-pages.py b/cms/djangoapps/contentstore/features/static-pages.py index 58932ad8e2..0adb4b1e54 100644 --- a/cms/djangoapps/contentstore/features/static-pages.py +++ b/cms/djangoapps/contentstore/features/static-pages.py @@ -48,3 +48,47 @@ def change_name(step, new_name): world.trigger_event(input_css) save_button = 'a.save-button' world.css_click(save_button) + + +@step(u'I reorder the tabs') +def reorder_tabs(_step): + # For some reason, the drag_and_drop method did not work in this case. + draggables = world.css_find('.drag-handle') + source = draggables.first + target = draggables.last + source.action_chains.click_and_hold(source._element).perform() + source.action_chains.move_to_element_with_offset(target._element, 0, 50).perform() + source.action_chains.release().perform() + + +@step(u'I have created a static page') +def create_static_page(step): + step.given('I have opened a new course in Studio') + step.given('I go to the static pages page') + step.given('I add a new page') + + +@step(u'I have created two different static pages') +def create_two_pages(step): + step.given('I have created a static page') + step.given('I "edit" the static page') + step.given('I change the name to "First"') + step.given('I add a new page') + # Verify order of tabs + _verify_tab_names('First', 'Empty') + + +@step(u'the tabs are in the reverse order') +def tabs_in_reverse_order(step): + _verify_tab_names('Empty', 'First') + + +def _verify_tab_names(first, second): + world.wait_for( + func=lambda _: len(world.css_find('.xmodule_StaticTabModule')) == 2, + timeout=200, + timeout_msg="Timed out waiting for two tabs to be present" + ) + tabs = world.css_find('.xmodule_StaticTabModule') + assert tabs[0].text == first + assert tabs[1].text == second diff --git a/cms/djangoapps/contentstore/features/transcripts.feature b/cms/djangoapps/contentstore/features/transcripts.feature index dad9cbb49e..159c8a3c5a 100644 --- a/cms/djangoapps/contentstore/features/transcripts.feature +++ b/cms/djangoapps/contentstore/features/transcripts.feature @@ -641,6 +641,7 @@ Feature: Video Component Editor And I save changes Then when I view the video it does show the captions + And I see "好 各位同学" text in the captions And I edit the component And I open tab "Advanced" diff --git a/cms/djangoapps/contentstore/features/transcripts.py b/cms/djangoapps/contentstore/features/transcripts.py index 5cbb65dc9d..4fac5e5b93 100644 --- a/cms/djangoapps/contentstore/features/transcripts.py +++ b/cms/djangoapps/contentstore/features/transcripts.py @@ -116,6 +116,7 @@ def i_see_status_message(_step, status): world.wait(DELAY) world.wait_for_ajax_complete() + assert not world.css_visible(SELECTORS['error_bar']) assert world.css_has_text(SELECTORS['status_bar'], STATUSES[status.strip()]) diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index 06bf36747e..9f08e98f0d 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -53,30 +53,33 @@ Feature: CMS.Video Component Then Captions become "invisible" # 8 - Scenario: Open captions never become invisible - Given I have created a Video component with subtitles - And Make sure captions are open - Then Captions are "visible" - And I hover over button "CC" - Then Captions are "visible" - And I hover over button "volume" - Then Captions are "visible" + # Disabled 11/26 due to flakiness in master + #Scenario: Open captions never become invisible + # Given I have created a Video component with subtitles + # And Make sure captions are open + # Then Captions are "visible" + # And I hover over button "CC" + # Then Captions are "visible" + # And I hover over button "volume" + # Then Captions are "visible" # 9 - Scenario: Closed captions are invisible when mouse doesn't hover on CC button - Given I have created a Video component with subtitles - And Make sure captions are closed - Then Captions become "invisible" - And I hover over button "volume" - Then Captions are "invisible" + # Disabled 11/26 due to flakiness in master + #Scenario: Closed captions are invisible when mouse doesn't hover on CC button + # Given I have created a Video component with subtitles + # And Make sure captions are closed + # Then Captions become "invisible" + # And I hover over button "volume" + # Then Captions are "invisible" # 10 - Scenario: When enter key is pressed on a caption shows an outline around it - Given I have created a Video component with subtitles - And Make sure captions are opened - Then I focus on caption line with data-index "0" - Then I press "enter" button on caption line with data-index "0" - And I see caption line with data-index "0" has class "focused" + # Disabled 11/26 due to flakiness in master + #Scenario: When enter key is pressed on a caption shows an outline around it + # Given I have created a Video component with subtitles + # And Make sure captions are opened + # Then I focus on caption line with data-index "0" + # Then I press "enter" button on caption line with data-index "0" + # And I see caption line with data-index "0" has class "focused" # 11 Scenario: When start end end times are specified, a range on slider is shown diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 5408c48290..c97dba10b9 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -181,7 +181,7 @@ def click_on_the_caption(_step, index): @step('I see caption line with data-index "([^"]*)" has class "([^"]*)"$') def caption_line_has_class(_step, index, className): SELECTOR = ".subtitles > li[data-index='{index}']".format(index=int(index.strip())) - world.css_has_class(SELECTOR, className.strip()) + assert world.css_has_class(SELECTOR, className.strip()) @step('I see a range on slider$') diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index 58cf3be70b..0aaf2dfb29 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,6 +1,5 @@ #pylint: disable=E1101 -import json import shutil import mock @@ -15,6 +14,7 @@ from fs.osfs import OSFS import copy from json import loads from datetime import timedelta +from django.test import TestCase from django.contrib.auth.models import User from django.dispatch import Signal @@ -42,6 +42,7 @@ from xmodule.capa_module import CapaDescriptor from xmodule.course_module import CourseDescriptor from xmodule.seq_module import SequenceDescriptor from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.locator import BlockUsageLocator from contentstore.views.component import ADVANCED_COMPONENT_TYPES from xmodule.exceptions import NotFoundError @@ -53,6 +54,7 @@ from pytz import UTC from uuid import uuid4 from pymongo import MongoClient from student.models import CourseEnrollment +import re from contentstore.utils import delete_course_and_groups from xmodule.modulestore.django import loc_mapper @@ -132,9 +134,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # just pick one vertical descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] - - resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) + locator = loc_mapper().translate_location(course.location.course_id, descriptor.location, False, True) + resp = self.client.get_html(locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when video transcripts no longer require IDs. + # _test_no_locations(self, resp) for expected in expected_types: self.assertIn(expected, resp.content) @@ -152,25 +156,24 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_malformed_edit_unit_request(self): store = modulestore('direct') - import_from_xml(store, 'common/test/data/', ['simple']) + _, course_items = import_from_xml(store, 'common/test/data/', ['simple']) # just pick one vertical descriptor = store.get_items(Location('i4x', 'edX', 'simple', 'vertical', None, None))[0] location = descriptor.location.replace(name='.' + descriptor.location.name) + locator = loc_mapper().translate_location(course_items[0].location.course_id, location, False, True) - resp = self.client.get_html(reverse('edit_unit', kwargs={'location': location.url()})) + resp = self.client.get_html(locator.url_reverse('unit')) self.assertEqual(resp.status_code, 400) + _test_no_locations(self, resp, status_code=400) def check_edit_unit(self, test_course_name): - import_from_xml(modulestore('direct'), 'common/test/data/', [test_course_name]) + _, course_items = import_from_xml(modulestore('direct'), 'common/test/data/', [test_course_name]) - for descriptor in modulestore().get_items(Location(None, None, 'vertical', None, None)): - print "Checking ", descriptor.location.url() - print descriptor.__class__, descriptor.location - resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) - self.assertEqual(resp.status_code, 200) + items = modulestore().get_items(Location('i4x', 'edX', test_course_name, 'vertical', None, None)) + self._check_verticals(items, course_items[0].location.course_id) - def lockAnAsset(self, content_store, course_location): + def _lock_an_asset(self, content_store, course_location): """ Lock an arbitrary asset in the course :param course_location: @@ -398,15 +401,63 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(course.tabs, expected_tabs) def test_static_tab_reordering(self): - def get_tab_locator(tab): - tab_location = 'i4x://MITx/999/static_tab/{0}'.format(tab['url_slug']) - return unicode(loc_mapper().translate_location( - course.location.course_id, Location(tab_location), False, True - )) + module_store, course_location, new_location = self._create_static_tabs() + course = module_store.get_item(course_location) + + # reverse the ordering + reverse_tabs = [] + for tab in course.tabs: + if tab['type'] == 'static_tab': + reverse_tabs.insert(0, unicode(self._get_tab_locator(course, tab))) + + self.client.ajax_post(new_location.url_reverse('tabs'), {'tabs': reverse_tabs}) + + course = module_store.get_item(course_location) + + # compare to make sure that the tabs information is in the expected order after the server call + course_tabs = [] + for tab in course.tabs: + if tab['type'] == 'static_tab': + course_tabs.append(unicode(self._get_tab_locator(course, tab))) + + self.assertEqual(reverse_tabs, course_tabs) + + def test_static_tab_deletion(self): + module_store, course_location, _ = self._create_static_tabs() + + course = module_store.get_item(course_location) + num_tabs = len(course.tabs) + last_tab = course.tabs[num_tabs - 1] + url_slug = last_tab['url_slug'] + delete_url = self._get_tab_locator(course, last_tab).url_reverse('xblock') + + self.client.delete(delete_url) + + course = module_store.get_item(course_location) + self.assertEqual(num_tabs - 1, len(course.tabs)) + + def tab_matches(tab): + """ Checks if the tab matches the one we deleted """ + return tab['type'] == 'static_tab' and tab['url_slug'] == url_slug + + tab_found = any(tab_matches(tab) for tab in course.tabs) + + self.assertFalse(tab_found, "tab should have been deleted") + + def _get_tab_locator(self, course, tab): + """ Returns the locator for a given tab. """ + tab_location = 'i4x://MITx/999/static_tab/{0}'.format(tab['url_slug']) + return loc_mapper().translate_location( + course.location.course_id, Location(tab_location), False, True + ) + + def _create_static_tabs(self): + """ Creates two static tabs in a dummy course. """ module_store = modulestore('direct') - locator = _course_factory_create_course() - course_location = loc_mapper().translate_locator_to_location(locator) + CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') + course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + new_location = loc_mapper().translate_location(course_location.course_id, course_location, False, True) ItemFactory.create( parent_location=course_location, @@ -417,25 +468,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): category="static_tab", display_name="Static_2") - course = module_store.get_item(course_location) - - # reverse the ordering - reverse_tabs = [] - for tab in course.tabs: - if tab['type'] == 'static_tab': - reverse_tabs.insert(0, get_tab_locator(tab)) - - self.client.ajax_post(reverse('reorder_static_tabs'), {'tabs': reverse_tabs}) - - course = module_store.get_item(course_location) - - # compare to make sure that the tabs information is in the expected order after the server call - course_tabs = [] - for tab in course.tabs: - if tab['type'] == 'static_tab': - course_tabs.append(get_tab_locator(tab)) - - self.assertEqual(reverse_tabs, course_tabs) + return module_store, course_location, new_location def test_import_polls(self): module_store = modulestore('direct') @@ -454,31 +487,38 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/toy/.*']) def test_module_preview_in_whitelist(self): - ''' + """ Tests the ajax callback to render an XModule - ''' - direct_store = modulestore('direct') - import_from_xml(direct_store, 'common/test/data/', ['toy']) - - # also try a custom response which will trigger the 'is this course in whitelist' logic - problem_module_location = Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None]) - url = reverse('preview_component', kwargs={'location': problem_module_location.url()}) - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) + """ + resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])) + # These are the data-ids of the xblocks contained in the vertical. + # Ultimately, these must be converted to new locators. + self.assertContains(resp, 'i4x://edX/toy/video/sample_video') + self.assertContains(resp, 'i4x://edX/toy/video/separate_file_video') + self.assertContains(resp, 'i4x://edX/toy/video/video_with_end_time') + self.assertContains(resp, 'i4x://edX/toy/poll_question/T1_changemind_poll_foo_2') def test_video_module_caption_asset_path(self): - ''' + """ This verifies that a video caption url is as we expect it to be - ''' + """ + resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None])) + self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + + def _test_preview(self, location): + """ Preview test case. """ direct_store = modulestore('direct') - import_from_xml(direct_store, 'common/test/data/', ['toy']) + _, course_items = import_from_xml(direct_store, 'common/test/data/', ['toy']) # also try a custom response which will trigger the 'is this course in whitelist' logic - video_module_location = Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None]) - url = reverse('preview_component', kwargs={'location': video_module_location.url()}) - resp = self.client.get_html(url) + locator = loc_mapper().translate_location( + course_items[0].location.course_id, location, False, True + ) + resp = self.client.get_html(locator.url_reverse('xblock')) self.assertEqual(resp.status_code, 200) - self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') + # TODO: uncomment when preview no longer has locations being returned. + # _test_no_locations(self, resp) + return resp def test_delete(self): direct_store = modulestore('direct') @@ -617,7 +657,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): thumbnail = content_store.find(thumbnail_location, throw_on_not_found=False) self.assertIsNotNone(thumbnail) - def _delete_asset_in_course (self): + def _delete_asset_in_course(self): """ Helper method for: 1) importing course from xml @@ -836,6 +876,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_bad_contentstore_request(self): resp = self.client.get_html('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png') self.assertEqual(resp.status_code, 400) + _test_no_locations(self, resp, 400) def test_rewrite_nonportable_links_on_import(self): module_store = modulestore('direct') @@ -955,7 +996,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertIn(private_location_no_draft.url(), sequential.children) - locked_asset = self.lockAnAsset(content_store, location) + locked_asset = self._lock_an_asset(content_store, location) locked_asset_attrs = content_store.get_attrs(locked_asset) # the later import will reupload del locked_asset_attrs['uploadDate'] @@ -1010,7 +1051,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): shutil.rmtree(root_dir) def check_import(self, module_store, root_dir, draft_store, content_store, stub_location, course_location, - locked_asset, locked_asset_attrs): + locked_asset, locked_asset_attrs): # reimport import_from_xml( module_store, root_dir, ['test_export'], draft_store=draft_store, @@ -1018,15 +1059,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): target_location_namespace=course_location ) + # Unit test fails in Jenkins without this. + loc_mapper().translate_location(course_location.course_id, course_location, False, True) + items = module_store.get_items(stub_location.replace(category='vertical', name=None)) - self.assertGreater(len(items), 0) - for descriptor in items: - # don't try to look at private verticals. Right now we're running - # the service in non-draft aware - if getattr(descriptor, 'is_draft', False): - print "Checking {0}....".format(descriptor.location.url()) - resp = self.client.get_html(reverse('edit_unit', kwargs={'location': descriptor.location.url()})) - self.assertEqual(resp.status_code, 200) + self._check_verticals(items, course_location.course_id) # verify that we have the content in the draft store as well vertical = draft_store.get_item( @@ -1210,7 +1247,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): handouts_locator = loc_mapper().translate_location('edX/toy/2012_Fall', handout_location) # get module info (json) - resp = self.client.get(handouts_locator.url_reverse('/xblock', '')) + resp = self.client.get(handouts_locator.url_reverse('/xblock')) # make sure we got a successful response self.assertEqual(resp.status_code, 200) @@ -1309,6 +1346,17 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): items = module_store.get_items(stub_location) self.assertEqual(len(items), 1) + def _check_verticals(self, items, course_id): + """ Test getting the editing HTML for each vertical. """ + # Assert is here to make sure that the course being tested actually has verticals (units) to check. + self.assertGreater(len(items), 0) + for descriptor in items: + unit_locator = loc_mapper().translate_location(course_id, descriptor.location, False, True) + resp = self.client.get_html(unit_locator.url_reverse('unit')) + self.assertEqual(resp.status_code, 200) + # TODO: uncomment when video transcripts no longer require IDs. + # _test_no_locations(self, resp) + @override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE, MODULESTORE=TEST_MODULESTORE) class ContentStoreTest(ModuleStoreTestCase): @@ -1387,7 +1435,7 @@ class ContentStoreTest(ModuleStoreTestCase): second_course_data = self.assert_created_course(number_suffix=uuid4().hex) # unseed the forums for the first course - course_id =_get_course_id(test_course_data) + course_id = _get_course_id(test_course_data) delete_course_and_groups(course_id, commit=True) self.assertFalse(are_permissions_roles_seeded(course_id)) @@ -1503,6 +1551,7 @@ class ContentStoreTest(ModuleStoreTestCase): status_code=200, html=True ) + _test_no_locations(self, resp) def test_course_factory(self): """Test that the course factory works correctly.""" @@ -1525,6 +1574,7 @@ class ContentStoreTest(ModuleStoreTestCase): status_code=200, html=True ) + _test_no_locations(self, resp) def test_course_overview_view_with_course(self): """Test viewing the course overview page with an existing course""" @@ -1550,12 +1600,13 @@ class ContentStoreTest(ModuleStoreTestCase): } resp = self.client.ajax_post('/xblock', section_data) + _test_no_locations(self, resp, html=False) self.assertEqual(resp.status_code, 200) data = parse_json(resp) self.assertRegexpMatches( - data['id'], - r"^i4x://MITx/999/chapter/([0-9]|[a-f]){32}$" + data['locator'], + r"^MITx.999.Robot_Super_Course/branch/draft/block/chapter([0-9]|[a-f]){3}$" ) def test_capa_module(self): @@ -1571,7 +1622,7 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertEqual(resp.status_code, 200) payload = parse_json(resp) - problem_loc = Location(payload['id']) + problem_loc = loc_mapper().translate_locator_to_location(BlockUsageLocator(payload['locator'])) problem = get_modulestore(problem_loc).get_item(problem_loc) # should be a CapaDescriptor self.assertIsInstance(problem, CapaDescriptor, "New problem is not a CapaDescriptor") @@ -1584,6 +1635,13 @@ class ContentStoreTest(ModuleStoreTestCase): Import and walk through some common URL endpoints. This just verifies non-500 and no other correct behavior, so it is not a deep test """ + def test_get_html(page): + # Helper function for getting HTML for a page in Studio and + # checking that it does not error. + resp = self.client.get_html(new_location.url_reverse(page)) + self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) + import_from_xml(modulestore('direct'), 'common/test/data/', ['simple']) loc = Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]) new_location = loc_mapper().translate_location(loc.course_id, loc, False, True) @@ -1593,55 +1651,39 @@ class ContentStoreTest(ModuleStoreTestCase): self.assertContains(resp, 'Chapter 2') # go to various pages + test_get_html('import') + test_get_html('export') + test_get_html('course_team') + test_get_html('course_info') + test_get_html('checklists') + test_get_html('assets') + test_get_html('tabs') + test_get_html('settings/details') + test_get_html('settings/grading') + test_get_html('settings/advanced') - # import page - resp = self.client.get_html(new_location.url_reverse('import/', '')) - self.assertEqual(resp.status_code, 200) - - # export page - resp = self.client.get_html(new_location.url_reverse('export/', '')) - self.assertEqual(resp.status_code, 200) - - # course team - url = new_location.url_reverse('course_team/', '') - resp = self.client.get_html(url) - self.assertEqual(resp.status_code, 200) - - # course info - resp = self.client.get(new_location.url_reverse('course_info')) - self.assertEqual(resp.status_code, 200) - - # settings_details - resp = self.client.get(reverse('settings_details', - kwargs={'org': loc.org, - 'course': loc.course, - 'name': loc.name})) - self.assertEqual(resp.status_code, 200) - - # settings_details - resp = self.client.get(reverse('settings_grading', - kwargs={'org': loc.org, - 'course': loc.course, - 'name': loc.name})) - self.assertEqual(resp.status_code, 200) - - # assets_handler (HTML for full page content) - url = new_location.url_reverse('assets/', '') - resp = self.client.get_html(url) + # textbook index + resp = self.client.get_html(reverse('textbook_index', + kwargs={'org': loc.org, + 'course': loc.course, + 'name': loc.name})) self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) # go look at a subsection page subsection_location = loc.replace(category='sequential', name='test_sequence') - resp = self.client.get_html( - reverse('edit_subsection', kwargs={'location': subsection_location.url()}) - ) + subsection_locator = loc_mapper().translate_location(loc.course_id, subsection_location, False, True) + resp = self.client.get_html(subsection_locator.url_reverse('subsection')) self.assertEqual(resp.status_code, 200) + _test_no_locations(self, resp) # go look at the Edit page unit_location = loc.replace(category='vertical', name='test_vertical') - resp = self.client.get_html( - reverse('edit_unit', kwargs={'location': unit_location.url()})) + unit_locator = loc_mapper().translate_location(loc.course_id, unit_location, False, True) + resp = self.client.get_html(unit_locator.url_reverse('unit')) self.assertEqual(resp.status_code, 200) + # TODO: uncomment when video transcripts no longer require IDs. + # _test_no_locations(self, resp) def delete_item(category, name): """ Helper method for testing the deletion of an xblock item. """ @@ -1649,6 +1691,7 @@ class ContentStoreTest(ModuleStoreTestCase): del_location = loc_mapper().translate_location(loc.course_id, del_loc, False, True) resp = self.client.delete(del_location.url_reverse('xblock')) self.assertEqual(resp.status_code, 204) + _test_no_locations(self, resp, status_code=204, html=False) # delete a component delete_item(category='html', name='test_html') @@ -1848,7 +1891,9 @@ class ContentStoreTest(ModuleStoreTestCase): Show the course overview page. """ new_location = loc_mapper().translate_location(location.course_id, location, False, True) - return self.client.get_html(new_location.url_reverse('course/', '')) + resp = self.client.get_html(new_location.url_reverse('course/', '')) + _test_no_locations(self, resp) + return resp @override_settings(MODULESTORE=TEST_MODULESTORE) @@ -1915,6 +1960,32 @@ class MetadataSaveTestCase(ModuleStoreTestCase): pass +class EntryPageTestCase(TestCase): + """ + Tests entry pages that aren't specific to a course. + """ + def setUp(self): + self.client = AjaxEnabledTestClient() + + def _test_page(self, page, status_code=200): + resp = self.client.get_html(page) + self.assertEqual(resp.status_code, status_code) + _test_no_locations(self, resp, status_code) + + def test_how_it_works(self): + self._test_page("/howitworks") + + def test_signup(self): + self._test_page("/signup") + + def test_login(self): + self._test_page("/signin") + + def test_logout(self): + # Logout redirects. + self._test_page("/logout", 302) + + def _create_course(test, course_data): """ Creates a course via an AJAX request and verifies the URL returned in the response. @@ -1926,7 +1997,7 @@ def _create_course(test, course_data): test.assertEqual(response.status_code, 200) data = parse_json(response) test.assertNotIn('ErrMsg', data) - test.assertEqual(data['url'], new_location.url_reverse("course/", "")) + test.assertEqual(data['url'], new_location.url_reverse("course")) def _course_factory_create_course(): @@ -1940,3 +2011,19 @@ def _course_factory_create_course(): def _get_course_id(test_course_data): """Returns the course ID (org/number/run).""" return "{org}/{number}/{run}".format(**test_course_data) + + +def _test_no_locations(test, resp, status_code=200, html=True): + """ + Verifies that "i4x", which appears in old locations, but not + new locators, does not appear in the HTML response output. + Used to verify that database refactoring is complete. + """ + test.assertNotContains(resp, 'i4x', status_code=status_code, html=html) + if html: + # For HTML pages, it is nice to call the method with html=True because + # it checks that the HTML properly parses. However, it won't find i4x usages + # in JavaScript blocks. + content = resp.content + hits = len(re.findall(r"(?bar" # encode - decode to convert date fields and other data which changes form self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).syllabus, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).syllabus, jsondetails.syllabus, "After set syllabus" ) jsondetails.overview = "Overview" self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).overview, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).overview, jsondetails.overview, "After set overview" ) jsondetails.intro_video = "intro_video" self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).intro_video, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).intro_video, jsondetails.intro_video, "After set intro_video" ) jsondetails.effort = "effort" self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).effort, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).effort, jsondetails.effort, "After set effort" ) jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC()) self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).start_date, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).start_date, jsondetails.start_date ) jsondetails.course_image_name = "an_image.jpg" self.assertEqual( - CourseDetails.update_from_json(jsondetails.__dict__).course_image_name, + CourseDetails.update_from_json(self.course_locator, jsondetails.__dict__).course_image_name, jsondetails.course_image_name ) @override_settings(MKTG_URLS={'ROOT': 'dummy-root'}) def test_marketing_site_fetch(self): - settings_details_url = reverse( - 'settings_details', - kwargs={ - 'org': self.course.location.org, - 'name': self.course.location.name, - 'course': self.course.location.course - } - ) + settings_details_url = self.course_locator.url_reverse('settings/details/') with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': True}): - response = self.client.get(settings_details_url) + response = self.client.get_html(settings_details_url) self.assertNotContains(response, "Course Summary Page") self.assertNotContains(response, "Send a note to students via email") self.assertContains(response, "course summary page will not be viewable") @@ -135,17 +125,10 @@ class CourseDetailsTestCase(CourseTestCase): self.assertNotContains(response, "Requirements") def test_regular_site_fetch(self): - settings_details_url = reverse( - 'settings_details', - kwargs={ - 'org': self.course.location.org, - 'name': self.course.location.name, - 'course': self.course.location.course - } - ) + settings_details_url = self.course_locator.url_reverse('settings/details/') with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_MKTG_SITE': False}): - response = self.client.get(settings_details_url) + response = self.client.get_html(settings_details_url) self.assertContains(response, "Course Summary Page") self.assertContains(response, "Send a note to students via email") self.assertNotContains(response, "course summary page will not be viewable") @@ -168,10 +151,12 @@ class CourseDetailsViewTest(CourseTestCase): Tests for modifying content on the first course settings page (course dates, overview, etc.). """ def alter_field(self, url, details, field, val): + """ + Change the one field to the given value and then invoke the update post to see if it worked. + """ setattr(details, field, val) # Need to partially serialize payload b/c the mock doesn't handle it correctly payload = copy.copy(details.__dict__) - payload['course_location'] = details.course_location.url() payload['start_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.start_date) payload['end_date'] = CourseDetailsViewTest.convert_datetime_to_iso(details.end_date) payload['enrollment_start'] = CourseDetailsViewTest.convert_datetime_to_iso(details.enrollment_start) @@ -181,16 +166,17 @@ class CourseDetailsViewTest(CourseTestCase): @staticmethod def convert_datetime_to_iso(datetime_obj): + """ + Use the xblock serializer to convert the datetime + """ return Date().to_json(datetime_obj) def test_update_and_fetch(self): - loc = self.course.location - details = CourseDetails.fetch(loc) + details = CourseDetails.fetch(self.course_locator) # resp s/b json from here on - url = reverse('course_settings', kwargs={'org': loc.org, 'course': loc.course, - 'name': loc.name, 'section': 'details'}) - resp = self.client.get(url) + url = self.course_locator.url_reverse('settings/details/') + resp = self.client.get_json(url) self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, "virgin get") utc = UTC() @@ -206,6 +192,9 @@ class CourseDetailsViewTest(CourseTestCase): self.alter_field(url, details, 'course_image_name', "course_image_name") def compare_details_with_encoding(self, encoded, details, context): + """ + compare all of the fields of the before and after dicts + """ self.compare_date_fields(details, encoded, context, 'start_date') self.compare_date_fields(details, encoded, context, 'end_date') self.compare_date_fields(details, encoded, context, 'enrollment_start') @@ -216,6 +205,9 @@ class CourseDetailsViewTest(CourseTestCase): self.assertEqual(details['course_image_name'], encoded['course_image_name'], context + " images not ==") def compare_date_fields(self, details, encoded, context, field): + """ + Compare the given date fields between the before and after doing json deserialization + """ if details[field] is not None: date = Date() if field in encoded and encoded[field] is not None: @@ -234,142 +226,191 @@ class CourseGradingTest(CourseTestCase): Tests for the course settings grading page. """ def test_initial_grader(self): - descriptor = get_modulestore(self.course.location).get_item(self.course.location) - test_grader = CourseGradingModel(descriptor) - # ??? How much should this test bake in expectations about defaults and thus fail if defaults change? - self.assertEqual(self.course.location, test_grader.course_location, "Course locations") - self.assertIsNotNone(test_grader.graders, "No graders") - self.assertIsNotNone(test_grader.grade_cutoffs, "No cutoffs") + test_grader = CourseGradingModel(self.course) + self.assertIsNotNone(test_grader.graders) + self.assertIsNotNone(test_grader.grade_cutoffs) def test_fetch_grader(self): - test_grader = CourseGradingModel.fetch(self.course.location.url()) - self.assertEqual(self.course.location, test_grader.course_location, "Course locations") - self.assertIsNotNone(test_grader.graders, "No graders") - self.assertIsNotNone(test_grader.grade_cutoffs, "No cutoffs") - - test_grader = CourseGradingModel.fetch(self.course.location) - self.assertEqual(self.course.location, test_grader.course_location, "Course locations") + test_grader = CourseGradingModel.fetch(self.course_locator) self.assertIsNotNone(test_grader.graders, "No graders") self.assertIsNotNone(test_grader.grade_cutoffs, "No cutoffs") for i, grader in enumerate(test_grader.graders): - subgrader = CourseGradingModel.fetch_grader(self.course.location, i) + subgrader = CourseGradingModel.fetch_grader(self.course_locator, i) self.assertDictEqual(grader, subgrader, str(i) + "th graders not equal") - subgrader = CourseGradingModel.fetch_grader(self.course.location.list(), 0) - self.assertDictEqual(test_grader.graders[0], subgrader, "failed with location as list") - - def test_fetch_cutoffs(self): - test_grader = CourseGradingModel.fetch_cutoffs(self.course.location) - # ??? should this check that it's at least a dict? (expected is { "pass" : 0.5 } I think) - self.assertIsNotNone(test_grader, "No cutoffs via fetch") - - test_grader = CourseGradingModel.fetch_cutoffs(self.course.location.url()) - self.assertIsNotNone(test_grader, "No cutoffs via fetch with url") - - def test_fetch_grace(self): - test_grader = CourseGradingModel.fetch_grace_period(self.course.location) - # almost a worthless test - self.assertIn('grace_period', test_grader, "No grace via fetch") - - test_grader = CourseGradingModel.fetch_grace_period(self.course.location.url()) - self.assertIn('grace_period', test_grader, "No cutoffs via fetch with url") - def test_update_from_json(self): - test_grader = CourseGradingModel.fetch(self.course.location) - altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__) + test_grader = CourseGradingModel.fetch(self.course_locator) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Noop update") test_grader.graders[0]['weight'] = test_grader.graders[0].get('weight') * 2 - altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "Weight[0] * 2") test_grader.grade_cutoffs['D'] = 0.3 - altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "cutoff add D") test_grader.grace_period = {'hours': 4, 'minutes': 5, 'seconds': 0} - altered_grader = CourseGradingModel.update_from_json(test_grader.__dict__) + altered_grader = CourseGradingModel.update_from_json(self.course_locator, test_grader.__dict__) self.assertDictEqual(test_grader.__dict__, altered_grader.__dict__, "4 hour grace period") def test_update_grader_from_json(self): - test_grader = CourseGradingModel.fetch(self.course.location) - altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1]) + test_grader = CourseGradingModel.fetch(self.course_locator) + altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) self.assertDictEqual(test_grader.graders[1], altered_grader, "Noop update") test_grader.graders[1]['min_count'] = test_grader.graders[1].get('min_count') + 2 - altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1]) + altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) self.assertDictEqual(test_grader.graders[1], altered_grader, "min_count[1] + 2") test_grader.graders[1]['drop_count'] = test_grader.graders[1].get('drop_count') + 1 - altered_grader = CourseGradingModel.update_grader_from_json(test_grader.course_location, test_grader.graders[1]) + altered_grader = CourseGradingModel.update_grader_from_json(self.course_locator, test_grader.graders[1]) self.assertDictEqual(test_grader.graders[1], altered_grader, "drop_count[1] + 2") def test_update_cutoffs_from_json(self): - test_grader = CourseGradingModel.fetch(self.course.location) - CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs) + test_grader = CourseGradingModel.fetch(self.course_locator) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) # Unlike other tests, need to actually perform a db fetch for this test since update_cutoffs_from_json # simply returns the cutoffs you send into it, rather than returning the db contents. - altered_grader = CourseGradingModel.fetch(self.course.location) + altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "Noop update") test_grader.grade_cutoffs['D'] = 0.3 - CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs) - altered_grader = CourseGradingModel.fetch(self.course.location) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) + altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff add D") test_grader.grade_cutoffs['Pass'] = 0.75 - CourseGradingModel.update_cutoffs_from_json(test_grader.course_location, test_grader.grade_cutoffs) - altered_grader = CourseGradingModel.fetch(self.course.location) + CourseGradingModel.update_cutoffs_from_json(self.course_locator, test_grader.grade_cutoffs) + altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grade_cutoffs, altered_grader.grade_cutoffs, "cutoff change 'Pass'") def test_delete_grace_period(self): - test_grader = CourseGradingModel.fetch(self.course.location) - CourseGradingModel.update_grace_period_from_json(test_grader.course_location, test_grader.grace_period) + test_grader = CourseGradingModel.fetch(self.course_locator) + CourseGradingModel.update_grace_period_from_json(self.course_locator, test_grader.grace_period) # update_grace_period_from_json doesn't return anything, so query the db for its contents. - altered_grader = CourseGradingModel.fetch(self.course.location) + altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertEqual(test_grader.grace_period, altered_grader.grace_period, "Noop update") test_grader.grace_period = {'hours': 15, 'minutes': 5, 'seconds': 30} - CourseGradingModel.update_grace_period_from_json(test_grader.course_location, test_grader.grace_period) - altered_grader = CourseGradingModel.fetch(self.course.location) + CourseGradingModel.update_grace_period_from_json(self.course_locator, test_grader.grace_period) + altered_grader = CourseGradingModel.fetch(self.course_locator) self.assertDictEqual(test_grader.grace_period, altered_grader.grace_period, "Adding in a grace period") test_grader.grace_period = {'hours': 1, 'minutes': 10, 'seconds': 0} # Now delete the grace period - CourseGradingModel.delete_grace_period(test_grader.course_location) + CourseGradingModel.delete_grace_period(self.course_locator) # update_grace_period_from_json doesn't return anything, so query the db for its contents. - altered_grader = CourseGradingModel.fetch(self.course.location) + altered_grader = CourseGradingModel.fetch(self.course_locator) # Once deleted, the grace period should simply be None self.assertEqual(None, altered_grader.grace_period, "Delete grace period") def test_update_section_grader_type(self): # Get the descriptor and the section_grader_type and assert they are the default values descriptor = get_modulestore(self.course.location).get_item(self.course.location) - section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) + section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator) self.assertEqual('Not Graded', section_grader_type['graderType']) self.assertEqual(None, descriptor.format) self.assertEqual(False, descriptor.graded) # Change the default grader type to Homework, which should also mark the section as graded - CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Homework'}) + CourseGradingModel.update_section_grader_type(self.course, 'Homework') descriptor = get_modulestore(self.course.location).get_item(self.course.location) - section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) + section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator) self.assertEqual('Homework', section_grader_type['graderType']) self.assertEqual('Homework', descriptor.format) self.assertEqual(True, descriptor.graded) # Change the grader type back to Not Graded, which should also unmark the section as graded - CourseGradingModel.update_section_grader_type(self.course.location, {'graderType': 'Not Graded'}) + CourseGradingModel.update_section_grader_type(self.course, 'Not Graded') descriptor = get_modulestore(self.course.location).get_item(self.course.location) - section_grader_type = CourseGradingModel.get_section_grader_type(self.course.location) + section_grader_type = CourseGradingModel.get_section_grader_type(self.course_locator) self.assertEqual('Not Graded', section_grader_type['graderType']) self.assertEqual(None, descriptor.format) self.assertEqual(False, descriptor.graded) + def test_get_set_grader_types_ajax(self): + """ + Test configuring the graders via ajax calls + """ + grader_type_url_base = self.course_locator.url_reverse('settings/grading') + # test get whole + response = self.client.get_json(grader_type_url_base) + whole_model = json.loads(response.content) + self.assertIn('graders', whole_model) + self.assertIn('grade_cutoffs', whole_model) + self.assertIn('grace_period', whole_model) + # test post/update whole + whole_model['grace_period'] = {'hours': 1, 'minutes': 30, 'seconds': 0} + response = self.client.ajax_post(grader_type_url_base, whole_model) + self.assertEqual(200, response.status_code) + response = self.client.get_json(grader_type_url_base) + whole_model = json.loads(response.content) + self.assertEqual(whole_model['grace_period'], {'hours': 1, 'minutes': 30, 'seconds': 0}) + # test get one grader + self.assertGreater(len(whole_model['graders']), 1) # ensure test will make sense + response = self.client.get_json(grader_type_url_base + '/1') + grader_sample = json.loads(response.content) + self.assertEqual(grader_sample, whole_model['graders'][1]) + # test add grader + new_grader = { + "type": "Extra Credit", + "min_count": 1, + "drop_count": 2, + "short_label": None, + "weight": 15, + } + response = self.client.ajax_post( + '{}/{}'.format(grader_type_url_base, len(whole_model['graders'])), + new_grader + ) + self.assertEqual(200, response.status_code) + grader_sample = json.loads(response.content) + new_grader['id'] = len(whole_model['graders']) + self.assertEqual(new_grader, grader_sample) + # test delete grader + response = self.client.delete(grader_type_url_base + '/1', HTTP_ACCEPT="application/json") + self.assertEqual(204, response.status_code) + response = self.client.get_json(grader_type_url_base) + updated_model = json.loads(response.content) + new_grader['id'] -= 1 # one fewer and the id mutates + self.assertIn(new_grader, updated_model['graders']) + self.assertNotIn(whole_model['graders'][1], updated_model['graders']) + + def setup_test_set_get_section_grader_ajax(self): + """ + Populate the course, grab a section, get the url for the assignment type access + """ + self.populateCourse() + sections = get_modulestore(self.course_location).get_items( + self.course_location.replace(category="sequential", name=None) + ) + # see if test makes sense + self.assertGreater(len(sections), 0, "No sections found") + section = sections[0] # just take the first one + section_locator = loc_mapper().translate_location(self.course_location.course_id, section.location, False, True) + return section_locator.url_reverse('xblock') + + def test_set_get_section_grader_ajax(self): + """ + Test setting and getting section grades via the grade as url + """ + grade_type_url = self.setup_test_set_get_section_grader_ajax() + response = self.client.ajax_post(grade_type_url, {'graderType': u'Homework'}) + self.assertEqual(200, response.status_code) + response = self.client.get_json(grade_type_url + '?fields=graderType') + self.assertEqual(json.loads(response.content).get('graderType'), u'Homework') + # and unset + response = self.client.ajax_post(grade_type_url, {'graderType': u'Not Graded'}) + self.assertEqual(200, response.status_code) + response = self.client.get_json(grade_type_url + '?fields=graderType') + self.assertEqual(json.loads(response.content).get('graderType'), u'Not Graded') + class CourseMetadataEditingTest(CourseTestCase): """ @@ -377,15 +418,19 @@ class CourseMetadataEditingTest(CourseTestCase): """ def setUp(self): CourseTestCase.setUp(self) - CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - self.fullcourse_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + self.fullcourse = CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') + self.course_setting_url = self.course_locator.url_reverse('settings/advanced') + self.fullcourse_setting_url = loc_mapper().translate_location( + self.fullcourse.location.course_id, + self.fullcourse.location, False, True + ).url_reverse('settings/advanced') def test_fetch_initial_fields(self): - test_model = CourseMetadata.fetch(self.course.location) + test_model = CourseMetadata.fetch(self.course) self.assertIn('display_name', test_model, 'Missing editable metadata field') self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value") - test_model = CourseMetadata.fetch(self.fullcourse_location) + test_model = CourseMetadata.fetch(self.fullcourse) self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in') self.assertIn('display_name', test_model, 'full missing editable metadata field') self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value") @@ -394,17 +439,17 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertIn('xqa_key', test_model, 'xqa_key field ') def test_update_from_json(self): - test_model = CourseMetadata.update_from_json(self.course.location, { + test_model = CourseMetadata.update_from_json(self.course, { "advertised_start": "start A", - "testcenter_info": {"c": "test"}, "days_early_for_beta": 2 }) self.update_check(test_model) # try fresh fetch to ensure persistence - test_model = CourseMetadata.fetch(self.course.location) + fresh = modulestore().get_item(self.course_location) + test_model = CourseMetadata.fetch(fresh) self.update_check(test_model) # now change some of the existing metadata - test_model = CourseMetadata.update_from_json(self.course.location, { + test_model = CourseMetadata.update_from_json(fresh, { "advertised_start": "start B", "display_name": "jolly roger"} ) @@ -418,13 +463,15 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value") self.assertIn('advertised_start', test_model, 'Missing new advertised_start metadata field') self.assertEqual(test_model['advertised_start'], 'start A', "advertised_start not expected value") - self.assertIn('testcenter_info', test_model, 'Missing testcenter_info metadata field') - self.assertDictEqual(test_model['testcenter_info'], {"c": "test"}, "testcenter_info not expected value") self.assertIn('days_early_for_beta', test_model, 'Missing days_early_for_beta metadata field') self.assertEqual(test_model['days_early_for_beta'], 2, "days_early_for_beta not expected value") def test_delete_key(self): - test_model = CourseMetadata.delete_key(self.fullcourse_location, {'deleteKeys': ['doesnt_exist', 'showanswer', 'xqa_key']}) + test_model = CourseMetadata.update_from_json( + self.fullcourse, { + "unsetKeys": ['showanswer', 'xqa_key'] + } + ) # ensure no harm self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in') self.assertIn('display_name', test_model, 'full missing editable metadata field') @@ -434,27 +481,113 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertEqual('finished', test_model['showanswer'], 'showanswer field still in') self.assertEqual(None, test_model['xqa_key'], 'xqa_key field still in') + def test_http_fetch_initial_fields(self): + response = self.client.get_json(self.course_setting_url) + test_model = json.loads(response.content) + self.assertIn('display_name', test_model, 'Missing editable metadata field') + self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value") + + response = self.client.get_json(self.fullcourse_setting_url) + test_model = json.loads(response.content) + self.assertNotIn('graceperiod', test_model, 'blacklisted field leaked in') + self.assertIn('display_name', test_model, 'full missing editable metadata field') + self.assertEqual(test_model['display_name'], 'Robot Super Course', "not expected value") + self.assertIn('rerandomize', test_model, 'Missing rerandomize metadata field') + self.assertIn('showanswer', test_model, 'showanswer field ') + self.assertIn('xqa_key', test_model, 'xqa_key field ') + + def test_http_update_from_json(self): + response = self.client.ajax_post(self.course_setting_url, { + "advertised_start": "start A", + "testcenter_info": {"c": "test"}, + "days_early_for_beta": 2, + "unsetKeys": ['showanswer', 'xqa_key'], + }) + test_model = json.loads(response.content) + self.update_check(test_model) + self.assertEqual('finished', test_model['showanswer'], 'showanswer field still in') + self.assertEqual(None, test_model['xqa_key'], 'xqa_key field still in') + + response = self.client.get_json(self.course_setting_url) + test_model = json.loads(response.content) + self.update_check(test_model) + # now change some of the existing metadata + response = self.client.ajax_post(self.course_setting_url, { + "advertised_start": "start B", + "display_name": "jolly roger" + }) + test_model = json.loads(response.content) + self.assertIn('display_name', test_model, 'Missing editable metadata field') + self.assertEqual(test_model['display_name'], 'jolly roger', "not expected value") + self.assertIn('advertised_start', test_model, 'Missing revised advertised_start metadata field') + self.assertEqual(test_model['advertised_start'], 'start B', "advertised_start not expected value") + + def test_advanced_components_munge_tabs(self): + """ + Test that adding and removing specific advanced components adds and removes tabs. + """ + self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), self.course.tabs) + self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), self.course.tabs) + self.client.ajax_post(self.course_setting_url, { + ADVANCED_COMPONENT_POLICY_KEY: ["combinedopenended"] + }) + course = modulestore().get_item(self.course_location) + self.assertIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs) + self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), course.tabs) + self.client.ajax_post(self.course_setting_url, { + ADVANCED_COMPONENT_POLICY_KEY: [] + }) + course = modulestore().get_item(self.course_location) + self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs) + class CourseGraderUpdatesTest(CourseTestCase): + """ + Test getting, deleting, adding, & updating graders + """ def setUp(self): + """Compute the url to use in tests""" super(CourseGraderUpdatesTest, self).setUp() - self.url = reverse("course_settings", kwargs={ - 'org': self.course.location.org, - 'course': self.course.location.course, - 'name': self.course.location.name, - 'grader_index': 0, - }) + self.url = self.course_locator.url_reverse('settings/grading') + self.starting_graders = CourseGradingModel(self.course).graders def test_get(self): - resp = self.client.get(self.url) + """Test getting a specific grading type record.""" + resp = self.client.get_json(self.url + '/0') self.assertEqual(resp.status_code, 200) obj = json.loads(resp.content) + self.assertEqual(self.starting_graders[0], obj) def test_delete(self): - resp = self.client.delete(self.url) + """Test deleting a specific grading type record.""" + resp = self.client.delete(self.url + '/0', HTTP_ACCEPT="application/json") self.assertEqual(resp.status_code, 204) + current_graders = CourseGradingModel.fetch(self.course_locator).graders + self.assertNotIn(self.starting_graders[0], current_graders) + self.assertEqual(len(self.starting_graders) - 1, len(current_graders)) - def test_post(self): + def test_update(self): + """Test updating a specific grading type record.""" + grader = { + "id": 0, + "type": "manual", + "min_count": 5, + "drop_count": 10, + "short_label": "yo momma", + "weight": 17.3, + } + resp = self.client.ajax_post(self.url + '/0', grader) + self.assertEqual(resp.status_code, 200) + obj = json.loads(resp.content) + self.assertEqual(obj, grader) + current_graders = CourseGradingModel.fetch(self.course_locator).graders + self.assertEqual(len(self.starting_graders), len(current_graders)) + + def test_add(self): + """Test adding a grading type record.""" + # the same url works for changing the whole grading model (graceperiod, cutoffs, and grading types) when + # the grading_index is None; thus, using None to imply adding a grading_type doesn't work; so, it uses an + # index out of bounds to imply create item. grader = { "type": "manual", "min_count": 5, @@ -462,6 +595,11 @@ class CourseGraderUpdatesTest(CourseTestCase): "short_label": "yo momma", "weight": 17.3, } - resp = self.client.ajax_post(self.url, grader) + resp = self.client.ajax_post('{}/{}'.format(self.url, len(self.starting_graders) + 1), grader) self.assertEqual(resp.status_code, 200) obj = json.loads(resp.content) + self.assertEqual(obj['id'], len(self.starting_graders)) + del obj['id'] + self.assertEqual(obj, grader) + current_graders = CourseGradingModel.fetch(self.course_locator).graders + self.assertEqual(len(self.starting_graders) + 1, len(current_graders)) diff --git a/cms/djangoapps/contentstore/tests/test_import_export.py b/cms/djangoapps/contentstore/tests/test_import_export.py index 20957a3508..85df894cd4 100644 --- a/cms/djangoapps/contentstore/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/tests/test_import_export.py @@ -263,7 +263,7 @@ class ExportTestCase(CourseTestCase): parent_location=vertical.location, category='aawefawef' ) - self._verify_export_failure('/edit/i4x://MITx/999/vertical/foo') + self._verify_export_failure(u'/unit/MITx.999.Robot_Super_Course/branch/draft/block/foo') def _verify_export_failure(self, expectedText): """ Export failure helper method. """ diff --git a/cms/djangoapps/contentstore/tests/test_item.py b/cms/djangoapps/contentstore/tests/test_item.py index b34dab0a6d..4922b4888a 100644 --- a/cms/djangoapps/contentstore/tests/test_item.py +++ b/cms/djangoapps/contentstore/tests/test_item.py @@ -9,6 +9,7 @@ from xmodule.capa_module import CapaDescriptor from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import loc_mapper from xmodule.modulestore.locator import BlockUsageLocator +from xmodule.modulestore.exceptions import ItemNotFoundError class ItemTest(CourseTestCase): @@ -30,7 +31,7 @@ class ItemTest(CourseTestCase): """ Get the item referenced by the locator from the modulestore """ - store = modulestore('draft') if draft else modulestore() + store = modulestore('draft') if draft else modulestore('direct') return store.get_item(self.get_old_id(locator)) def response_locator(self, response): @@ -251,3 +252,105 @@ class TestEditItem(ItemTest): self.assertEqual(self.get_old_id(self.problem_locator).url(), children[0]) self.assertEqual(self.get_old_id(unit1_locator).url(), children[2]) self.assertEqual(self.get_old_id(unit2_locator).url(), children[1]) + + def test_make_public(self): + """ Test making a private problem public (publishing it). """ + # When the problem is first created, it is only in draft (because of its category). + with self.assertRaises(ItemNotFoundError): + self.get_item_from_modulestore(self.problem_locator, False) + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_public'} + ) + self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False)) + + def test_make_private(self): + """ Test making a public problem private (un-publishing it). """ + # Make problem public. + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_public'} + ) + self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False)) + # Now make it private + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_private'} + ) + with self.assertRaises(ItemNotFoundError): + self.get_item_from_modulestore(self.problem_locator, False) + + def test_make_draft(self): + """ Test creating a draft version of a public problem. """ + # Make problem public. + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_public'} + ) + self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False)) + # Now make it draft, which means both versions will exist. + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'create_draft'} + ) + # Update the draft version and check that published is different. + self.client.ajax_post( + self.problem_update_url, + data={'metadata': {'due': '2077-10-10T04:00Z'}} + ) + published = self.get_item_from_modulestore(self.problem_locator, False) + self.assertIsNone(published.due) + draft = self.get_item_from_modulestore(self.problem_locator, True) + self.assertEqual(draft.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) + + def test_make_public_with_update(self): + """ Update a problem and make it public at the same time. """ + self.client.ajax_post( + self.problem_update_url, + data={ + 'metadata': {'due': '2077-10-10T04:00Z'}, + 'publish': 'make_public' + } + ) + published = self.get_item_from_modulestore(self.problem_locator, False) + self.assertEqual(published.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) + + def test_make_private_with_update(self): + """ Make a problem private and update it at the same time. """ + # Make problem public. + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_public'} + ) + self.client.ajax_post( + self.problem_update_url, + data={ + 'metadata': {'due': '2077-10-10T04:00Z'}, + 'publish': 'make_private' + } + ) + with self.assertRaises(ItemNotFoundError): + self.get_item_from_modulestore(self.problem_locator, False) + draft = self.get_item_from_modulestore(self.problem_locator, True) + self.assertEqual(draft.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) + + def test_create_draft_with_update(self): + """ Create a draft and update it at the same time. """ + # Make problem public. + self.client.ajax_post( + self.problem_update_url, + data={'publish': 'make_public'} + ) + self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False)) + # Now make it draft, which means both versions will exist. + self.client.ajax_post( + self.problem_update_url, + data={ + 'metadata': {'due': '2077-10-10T04:00Z'}, + 'publish': 'create_draft' + } + ) + published = self.get_item_from_modulestore(self.problem_locator, False) + self.assertIsNone(published.due) + draft = self.get_item_from_modulestore(self.problem_locator, True) + self.assertEqual(draft.due, datetime.datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) diff --git a/cms/djangoapps/contentstore/tests/test_transcripts.py b/cms/djangoapps/contentstore/tests/test_transcripts.py index 695fdcd09f..4c481383ab 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts.py @@ -20,6 +20,7 @@ from xmodule.contentstore.django import contentstore, _CONTENTSTORE from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError from xmodule.modulestore.django import loc_mapper +from xmodule.modulestore.locator import BlockUsageLocator from contentstore.tests.modulestore_config import TEST_MODULESTORE TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) @@ -59,7 +60,7 @@ class Basetranscripts(CourseTestCase): 'type': 'video' } resp = self.client.ajax_post('/xblock', data) - self.item_location = json.loads(resp.content).get('id') + self.item_location = self._get_location(resp) self.assertEqual(resp.status_code, 200) # hI10vDNYz4M - valid Youtube ID with transcripts. @@ -72,6 +73,11 @@ class Basetranscripts(CourseTestCase): # Remove all transcripts for current module. self.clear_subs_content() + def _get_location(self, resp): + """ Returns the location (as a string) from the response returned by a create operation. """ + locator = json.loads(resp.content).get('locator') + return loc_mapper().translate_locator_to_location(BlockUsageLocator(locator)).url() + def get_youtube_ids(self): """Return youtube speeds and ids.""" item = modulestore().get_item(self.item_location) @@ -205,7 +211,7 @@ class TestUploadtranscripts(Basetranscripts): 'type': 'non_video' } resp = self.client.ajax_post('/xblock', data) - item_location = json.loads(resp.content).get('id') + item_location = self._get_location(resp) data = '' modulestore().update_item(item_location, data) @@ -416,7 +422,7 @@ class TestDownloadtranscripts(Basetranscripts): 'type': 'videoalpha' } resp = self.client.ajax_post('/xblock', data) - item_location = json.loads(resp.content).get('id') + item_location = self._get_location(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" @@ -666,7 +672,7 @@ class TestChecktranscripts(Basetranscripts): 'type': 'not_video' } resp = self.client.ajax_post('/xblock', data) - item_location = json.loads(resp.content).get('id') + item_location = self._get_location(resp) subs_id = str(uuid4()) data = textwrap.dedent(""" diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index b59f214054..0e716cc878 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -10,8 +10,9 @@ from django.test.client import Client from django.test.utils import override_settings from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from contentstore.tests.modulestore_config import TEST_MODULESTORE +from xmodule.modulestore.django import loc_mapper def parse_json(response): @@ -41,6 +42,7 @@ class AjaxEnabledTestClient(Client): if not isinstance(data, basestring): data = json.dumps(data or {}) kwargs.setdefault("HTTP_X_REQUESTED_WITH", "XMLHttpRequest") + kwargs.setdefault("HTTP_ACCEPT", "application/json") return self.post(path=path, data=data, content_type=content_type, **kwargs) def get_html(self, path, data=None, follow=False, **extra): @@ -88,6 +90,9 @@ class CourseTestCase(ModuleStoreTestCase): display_name='Robot Super Course', ) self.course_location = self.course.location + self.course_locator = loc_mapper().translate_location( + self.course.location.course_id, self.course.location, False, True + ) def createNonStaffAuthedUserClient(self): """ @@ -106,3 +111,16 @@ class CourseTestCase(ModuleStoreTestCase): client = Client() client.login(username=uname, password=password) return client, nonstaff + + def populateCourse(self): + """ + Add 2 chapters, 4 sections, 8 verticals, 16 problems to self.course (branching 2) + """ + def descend(parent, stack): + xblock_type = stack.pop(0) + for _ in range(2): + child = ItemFactory.create(category=xblock_type, parent_location=parent.location) + if stack: + descend(child, stack) + + descend(self.course, ['chapter', 'sequential', 'vertical', 'problem']) diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 5643e5c044..61c6c672a7 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -5,7 +5,6 @@ from util.json_request import JsonResponse from django.http import HttpResponseBadRequest from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods -from django.core.urlresolvers import reverse from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response from django.http import HttpResponseNotFound @@ -22,6 +21,8 @@ from xmodule.modulestore.locator import BlockUsageLocator __all__ = ['checklists_handler'] + +# pylint: disable=unused-argument @require_http_methods(("GET", "POST", "PUT")) @login_required @ensure_csrf_cookie @@ -85,8 +86,8 @@ def checklists_handler(request, tag=None, course_id=None, branch=None, version_g return JsonResponse(expanded_checklist) else: return HttpResponseBadRequest( - ( "Could not save checklist state because the checklist index " - "was out of range or unspecified."), + ("Could not save checklist state because the checklist index " + "was out of range or unspecified."), content_type="text/plain" ) else: @@ -113,14 +114,12 @@ def expand_checklist_action_url(course_module, checklist): The method does a copy of the input checklist and does not modify the input argument. """ expanded_checklist = copy.deepcopy(checklist) - oldurlconf_map = { - "SettingsDetails": "settings_details", - "SettingsGrading": "settings_grading" - } urlconf_map = { "ManageUsers": "course_team", - "CourseOutline": "course" + "CourseOutline": "course", + "SettingsDetails": "settings/details", + "SettingsGrading": "settings/grading", } for item in expanded_checklist.get('items'): @@ -130,12 +129,5 @@ def expand_checklist_action_url(course_module, checklist): ctx_loc = course_module.location location = loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True) item['action_url'] = location.url_reverse(url_prefix, '') - elif action_url in oldurlconf_map: - urlconf_name = oldurlconf_map[action_url] - item['action_url'] = reverse(urlconf_name, kwargs={ - 'org': course_module.location.org, - 'course': course_module.location.course, - 'name': course_module.location.name, - }) return expanded_checklist diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 327e75c7f4..3742c7af20 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -2,21 +2,19 @@ import json import logging from collections import defaultdict -from django.http import (HttpResponse, HttpResponseBadRequest, - HttpResponseForbidden) +from django.http import HttpResponseBadRequest from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie from django.conf import settings -from xmodule.modulestore.exceptions import (ItemNotFoundError, - InvalidLocationError) +from xmodule.modulestore.exceptions import ItemNotFoundError from mitxmako.shortcuts import render_to_response -from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore from xmodule.util.date_utils import get_default_time_display from xmodule.modulestore.django import loc_mapper +from xmodule.modulestore.locator import BlockUsageLocator from xblock.fields import Scope from util.json_request import expect_json, JsonResponse @@ -25,7 +23,6 @@ from contentstore.utils import get_lms_link_for_item, compute_unit_state, UnitSt from models.settings.course_grading import CourseGradingModel -from .helpers import _xmodule_recurse from .access import has_access from xmodule.x_module import XModuleDescriptor from xblock.plugin import PluginMissingError @@ -33,17 +30,13 @@ from xblock.runtime import Mixologist __all__ = ['OPEN_ENDED_COMPONENT_TYPES', 'ADVANCED_COMPONENT_POLICY_KEY', - 'edit_subsection', - 'edit_unit', - 'assignment_type_update', - 'create_draft', - 'publish_draft', - 'unpublish_unit', + 'subsection_handler', + 'unit_handler' ] log = logging.getLogger(__name__) -# NOTE: edit_unit assumes this list is disjoint from ADVANCED_COMPONENT_TYPES +# NOTE: unit_handler assumes this list is disjoint from ADVANCED_COMPONENT_TYPES COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video'] OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"] @@ -58,93 +51,87 @@ ADVANCED_COMPONENT_CATEGORY = 'advanced' ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules' +@require_http_methods(["GET"]) @login_required -def edit_subsection(request, location): - "Edit the subsection of a course" - # check that we have permissions to edit this item - try: - course = get_course_for_item(location) - except InvalidLocationError: - return HttpResponseBadRequest() +def subsection_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): + """ + The restful handler for subsection-specific requests. - if not has_access(request.user, course.location): - raise PermissionDenied() + GET + html: return html page for editing a subsection + json: not currently supported + """ + if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + try: + old_location, course, item, lms_link = _get_item_in_course(request, locator) + except ItemNotFoundError: + return HttpResponseBadRequest() - try: - item = modulestore().get_item(location, depth=1) - except ItemNotFoundError: - return HttpResponseBadRequest() + preview_link = get_lms_link_for_item(old_location, course_id=course.location.course_id, preview=True) - lms_link = get_lms_link_for_item( - location, course_id=course.location.course_id - ) - preview_link = get_lms_link_for_item( - location, course_id=course.location.course_id, preview=True - ) + # make sure that location references a 'sequential', otherwise return + # BadRequest + if item.location.category != 'sequential': + return HttpResponseBadRequest() - # make sure that location references a 'sequential', otherwise return - # BadRequest - if item.location.category != 'sequential': - return HttpResponseBadRequest() + parent_locs = modulestore().get_parent_locations(old_location, None) - parent_locs = modulestore().get_parent_locations(location, None) - - # we're for now assuming a single parent - if len(parent_locs) != 1: - logging.error( + # we're for now assuming a single parent + if len(parent_locs) != 1: + logging.error( 'Multiple (or none) parents have been found for %s', - location + unicode(locator) + ) + + # this should blow up if we don't find any parents, which would be erroneous + parent = modulestore().get_item(parent_locs[0]) + + # remove all metadata from the generic dictionary that is presented in a + # more normalized UI. We only want to display the XBlocks fields, not + # the fields from any mixins that have been added + fields = getattr(item, 'unmixed_class', item.__class__).fields + + policy_metadata = dict( + (field.name, field.read_from(item)) + for field + in fields.values() + if field.name not in ['display_name', 'start', 'due', 'format'] and field.scope == Scope.settings ) - # this should blow up if we don't find any parents, which would be erroneous - parent = modulestore().get_item(parent_locs[0]) + can_view_live = False + subsection_units = item.get_children() + for unit in subsection_units: + state = compute_unit_state(unit) + if state == UnitState.public or state == UnitState.draft: + can_view_live = True + break - # remove all metadata from the generic dictionary that is presented in a - # more normalized UI. We only want to display the XBlocks fields, not - # the fields from any mixins that have been added - fields = getattr(item, 'unmixed_class', item.__class__).fields + course_locator = loc_mapper().translate_location( + course.location.course_id, course.location, False, True + ) - policy_metadata = dict( - (field.name, field.read_from(item)) - for field - in fields.values() - if field.name not in ['display_name', 'start', 'due', 'format'] - and field.scope == Scope.settings - ) - - can_view_live = False - subsection_units = item.get_children() - for unit in subsection_units: - state = compute_unit_state(unit) - if state == UnitState.public or state == UnitState.draft: - can_view_live = True - break - - locator = loc_mapper().translate_location( - course.location.course_id, item.location, False, True - ) - - return render_to_response( - 'edit_subsection.html', - { - 'subsection': item, - 'context_course': course, - 'new_unit_category': 'vertical', - 'lms_link': lms_link, - 'preview_link': preview_link, - 'course_graders': json.dumps(CourseGradingModel.fetch(course.location).graders), - # For grader, which is not yet converted - 'parent_location': course.location, - 'parent_item': parent, - 'locator': locator, - 'policy_metadata': policy_metadata, - 'subsection_units': subsection_units, - 'can_view_live': can_view_live - } - ) + return render_to_response( + 'edit_subsection.html', + { + 'subsection': item, + 'context_course': course, + 'new_unit_category': 'vertical', + 'lms_link': lms_link, + 'preview_link': preview_link, + 'course_graders': json.dumps(CourseGradingModel.fetch(course_locator).graders), + 'parent_item': parent, + 'locator': locator, + 'policy_metadata': policy_metadata, + 'subsection_units': subsection_units, + 'can_view_live': can_view_live + } + ) + else: + return HttpResponseBadRequest("Only supports html requests") -def load_mixed_class(category): +def _load_mixed_class(category): """ Load an XBlock by category name, and apply all defined mixins """ @@ -153,139 +140,125 @@ def load_mixed_class(category): return mixologist.mix(component_class) +@require_http_methods(["GET"]) @login_required -def edit_unit(request, location): +def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): """ - Display an editing page for the specified module. + The restful handler for unit-specific requests. - Expects a GET request with the parameter `id`. - - id: A Location URL + GET + html: return html page for editing a unit + json: not currently supported """ - try: - course = get_course_for_item(location) - except InvalidLocationError: - return HttpResponseBadRequest() + if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + try: + old_location, course, item, lms_link = _get_item_in_course(request, locator) + except ItemNotFoundError: + return HttpResponseBadRequest() - if not has_access(request.user, course.location): - raise PermissionDenied() - - try: - item = modulestore().get_item(location, depth=1) - except ItemNotFoundError: - return HttpResponseBadRequest() - lms_link = get_lms_link_for_item( - item.location, - course_id=course.location.course_id - ) - - # Note that the unit_state (draft, public, private) does not match up with the published value - # passed to translate_location. The two concepts are different at this point. - unit_locator = loc_mapper().translate_location( - course.location.course_id, Location(location), False, True - ) - - component_templates = defaultdict(list) - for category in COMPONENT_TYPES: - component_class = load_mixed_class(category) - # add the default template - # TODO: Once mixins are defined per-application, rather than per-runtime, - # this should use a cms mixed-in class. (cpennington) - if hasattr(component_class, 'display_name'): - display_name = component_class.display_name.default or 'Blank' - else: - display_name = 'Blank' - component_templates[category].append(( - display_name, - category, - False, # No defaults have markdown (hardcoded current default) - None # no boilerplate for overrides - )) - # add boilerplates - if hasattr(component_class, 'templates'): - for template in component_class.templates(): - filter_templates = getattr(component_class, 'filter_templates', None) - if not filter_templates or filter_templates(template, course): - component_templates[category].append(( - template['metadata'].get('display_name'), - category, - template['metadata'].get('markdown') is not None, - template.get('template_id') - )) - - # Check if there are any advanced modules specified in the course policy. - # These modules should be specified as a list of strings, where the strings - # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be - # enabled for the course. - course_advanced_keys = course.advanced_modules - - # Set component types according to course policy file - if isinstance(course_advanced_keys, list): - for category in course_advanced_keys: - if category in ADVANCED_COMPONENT_TYPES: - # Do I need to allow for boilerplates or just defaults on the - # class? i.e., can an advanced have more than one entry in the - # menu? one for default and others for prefilled boilerplates? - try: - component_class = load_mixed_class(category) - - component_templates['advanced'].append(( - component_class.display_name.default or category, - category, - False, - None # don't override default data + component_templates = defaultdict(list) + for category in COMPONENT_TYPES: + component_class = _load_mixed_class(category) + # add the default template + # TODO: Once mixins are defined per-application, rather than per-runtime, + # this should use a cms mixed-in class. (cpennington) + if hasattr(component_class, 'display_name'): + display_name = component_class.display_name.default or 'Blank' + else: + display_name = 'Blank' + component_templates[category].append(( + display_name, + category, + False, # No defaults have markdown (hardcoded current default) + None # no boilerplate for overrides + )) + # add boilerplates + if hasattr(component_class, 'templates'): + for template in component_class.templates(): + filter_templates = getattr(component_class, 'filter_templates', None) + if not filter_templates or filter_templates(template, course): + component_templates[category].append(( + template['metadata'].get('display_name'), + category, + template['metadata'].get('markdown') is not None, + template.get('template_id') )) - except PluginMissingError: - # dhm: I got this once but it can happen any time the - # course author configures an advanced component which does - # not exist on the server. This code here merely - # prevents any authors from trying to instantiate the - # non-existent component type by not showing it in the menu - pass - else: - log.error( - "Improper format for course advanced keys! %s", - course_advanced_keys - ) - components = [ - [ - component.location.url(), - loc_mapper().translate_location( - course.location.course_id, component.location, False, True + # Check if there are any advanced modules specified in the course policy. + # These modules should be specified as a list of strings, where the strings + # are the names of the modules in ADVANCED_COMPONENT_TYPES that should be + # enabled for the course. + course_advanced_keys = course.advanced_modules + + # Set component types according to course policy file + if isinstance(course_advanced_keys, list): + for category in course_advanced_keys: + if category in ADVANCED_COMPONENT_TYPES: + # Do I need to allow for boilerplates or just defaults on the + # class? i.e., can an advanced have more than one entry in the + # menu? one for default and others for prefilled boilerplates? + try: + component_class = _load_mixed_class(category) + + component_templates['advanced'].append( + ( + component_class.display_name.default or category, + category, + False, + None # don't override default data + ) + ) + except PluginMissingError: + # dhm: I got this once but it can happen any time the + # course author configures an advanced component which does + # not exist on the server. This code here merely + # prevents any authors from trying to instantiate the + # non-existent component type by not showing it in the menu + pass + else: + log.error( + "Improper format for course advanced keys! %s", + course_advanced_keys ) + + components = [ + [ + # TODO: old location needed for video transcripts. + component.location.url(), + loc_mapper().translate_location( + course.location.course_id, component.location, False, True + ) + ] + for component + in item.get_children() ] - for component - in item.get_children() - ] - # TODO (cpennington): If we share units between courses, - # this will need to change to check permissions correctly so as - # to pick the correct parent subsection + # TODO (cpennington): If we share units between courses, + # this will need to change to check permissions correctly so as + # to pick the correct parent subsection - containing_subsection_locs = modulestore().get_parent_locations( - location, None - ) - containing_subsection = modulestore().get_item(containing_subsection_locs[0]) - containing_section_locs = modulestore().get_parent_locations( + containing_subsection_locs = modulestore().get_parent_locations(old_location, None) + containing_subsection = modulestore().get_item(containing_subsection_locs[0]) + containing_section_locs = modulestore().get_parent_locations( containing_subsection.location, None - ) - containing_section = modulestore().get_item(containing_section_locs[0]) + ) + containing_section = modulestore().get_item(containing_section_locs[0]) - # cdodge hack. We're having trouble previewing drafts via jump_to redirect - # so let's generate the link url here + # cdodge hack. We're having trouble previewing drafts via jump_to redirect + # so let's generate the link url here - # need to figure out where this item is in the list of children as the - # preview will need this - index = 1 - for child in containing_subsection.get_children(): - if child.location == item.location: - break - index = index + 1 + # need to figure out where this item is in the list of children as the + # preview will need this + index = 1 + for child in containing_subsection.get_children(): + if child.location == item.location: + break + index = index + 1 - preview_lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE') + preview_lms_base = settings.MITX_FEATURES.get('PREVIEW_LMS_BASE') - preview_lms_link = ( + preview_lms_link = ( '//{preview_lms_base}/courses/{org}/{course}/' '{course_name}/courseware/{section}/{subsection}/{index}' ).format( @@ -299,102 +272,46 @@ def edit_unit(request, location): index=index ) - return render_to_response('unit.html', { - 'context_course': course, - 'unit': item, - # Still needed for creating a draft. - 'unit_location': location, - 'unit_locator': unit_locator, - 'components': components, - 'component_templates': component_templates, - 'draft_preview_link': preview_lms_link, - 'published_preview_link': lms_link, - 'subsection': containing_subsection, - 'release_date': ( - get_default_time_display(containing_subsection.start) - if containing_subsection.start is not None else None - ), - 'section': containing_section, - 'new_unit_category': 'vertical', - 'unit_state': compute_unit_state(item), - 'published_date': ( - get_default_time_display(item.published_date) - if item.published_date is not None else None - ), - }) - - -@expect_json -@login_required -@require_http_methods(("GET", "POST", "PUT")) -@ensure_csrf_cookie -def assignment_type_update(request, org, course, category, name): - """ - CRUD operations on assignment types for sections and subsections and - anything else gradable. - """ - location = Location(['i4x', org, course, category, name]) - if not has_access(request.user, location): - return HttpResponseForbidden() - - if request.method == 'GET': - rsp = CourseGradingModel.get_section_grader_type(location) - elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. - rsp = CourseGradingModel.update_section_grader_type( - location, request.json - ) - return JsonResponse(rsp) + return render_to_response('unit.html', { + 'context_course': course, + 'unit': item, + 'unit_locator': locator, + 'components': components, + 'component_templates': component_templates, + 'draft_preview_link': preview_lms_link, + 'published_preview_link': lms_link, + 'subsection': containing_subsection, + 'release_date': ( + get_default_time_display(containing_subsection.start) + if containing_subsection.start is not None else None + ), + 'section': containing_section, + 'new_unit_category': 'vertical', + 'unit_state': compute_unit_state(item), + 'published_date': ( + get_default_time_display(item.published_date) + if item.published_date is not None else None + ), + }) + else: + return HttpResponseBadRequest("Only supports html requests") @login_required -@expect_json -def create_draft(request): - "Create a draft" - location = request.json['id'] +def _get_item_in_course(request, locator): + """ + Helper method for getting the old location, containing course, + item, and lms_link for a given locator. - # check permissions for this user within this course - if not has_access(request.user, location): + Verifies that the caller has permission to access this item. + """ + if not has_access(request.user, locator): raise PermissionDenied() - # This clones the existing item location to a draft location (the draft is - # implicit, because modulestore is a Draft modulestore) - modulestore().convert_to_draft(location) + old_location = loc_mapper().translate_locator_to_location(locator) + course_location = loc_mapper().translate_locator_to_location(locator, True) + course = modulestore().get_item(course_location) + item = modulestore().get_item(old_location, depth=1) + lms_link = get_lms_link_for_item(old_location, course_id=course.location.course_id) - return HttpResponse() - - -@login_required -@expect_json -def publish_draft(request): - """ - Publish a draft - """ - location = request.json['id'] - - # check permissions for this user within this course - if not has_access(request.user, location): - raise PermissionDenied() - - item = modulestore().get_item(location) - _xmodule_recurse( - item, - lambda i: modulestore().publish(i.location, request.user.id) - ) - - return HttpResponse() - - -@login_required -@expect_json -def unpublish_unit(request): - "Unpublish a unit" - location = request.json['id'] - - # check permissions for this user within this course - if not has_access(request.user, location): - raise PermissionDenied() - - item = modulestore().get_item(location) - _xmodule_recurse(item, lambda i: modulestore().unpublish(i.location)) - - return HttpResponse() + return old_location, course, item, lms_link diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 187ee9343b..044ef79473 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -27,13 +27,11 @@ from xmodule.modulestore.exceptions import ( ItemNotFoundError, InvalidLocationError) from xmodule.modulestore import Location -from contentstore.course_info_model import ( - get_course_updates, update_course_updates, delete_course_update) +from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update from contentstore.utils import ( get_lms_link_for_item, add_extra_panel_tab, remove_extra_panel_tab, get_modulestore) -from models.settings.course_details import ( - CourseDetails, CourseSettingsEncoder) +from models.settings.course_details import CourseDetails, CourseSettingsEncoder from models.settings.course_grading import CourseGradingModel from models.settings.course_metadata import CourseMetadata @@ -53,14 +51,13 @@ from student.models import CourseEnrollment from xmodule.html_module import AboutDescriptor from xmodule.modulestore.locator import BlockUsageLocator from course_creators.views import get_course_creator_status, add_user_with_status_unrequested +from contentstore import utils __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler', - 'get_course_settings', - 'course_config_graders_page', - 'course_config_advanced_page', - 'course_settings_updates', - 'course_grader_updates', - 'course_advanced_updates', 'textbook_index', 'textbook_by_id', + 'settings_handler', + 'grading_handler', + 'advanced_settings_handler', + 'textbook_index', 'textbook_by_id', 'create_textbook'] @@ -177,7 +174,6 @@ def course_index(request, course_id, branch, version_guid, block): if not has_access(request.user, location): raise PermissionDenied() - old_location = loc_mapper().translate_locator_to_location(location) lms_link = get_lms_link_for_item(old_location) @@ -190,10 +186,8 @@ def course_index(request, course_id, branch, version_guid, block): 'lms_link': lms_link, 'sections': sections, 'course_graders': json.dumps( - CourseGradingModel.fetch(course.location).graders + CourseGradingModel.fetch(location).graders ), - # This is used by course grader, which has not yet been updated. - 'parent_location': course.location, 'parent_locator': location, 'new_section_category': 'chapter', 'new_subsection_category': 'sequential', @@ -232,14 +226,20 @@ def create_new_course(request): pass if existing_course is not None: return JsonResponse({ - 'ErrMsg': _('There is already a course defined with the same ' + 'ErrMsg': _( + 'There is already a course defined with the same ' 'organization, course number, and course run. Please ' 'change either organization or course number to be ' - 'unique.'), - 'OrgErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), - 'CourseErrMsg': _('Please change either the organization or ' - 'course number so that it is unique.'), + 'unique.' + ), + 'OrgErrMsg': _( + 'Please change either the organization or ' + 'course number so that it is unique.' + ), + 'CourseErrMsg': _( + 'Please change either the organization or ' + 'course number so that it is unique.' + ), }) # dhm: this query breaks the abstraction, but I'll fix it when I do my suspended refactoring of this @@ -254,12 +254,15 @@ def create_new_course(request): courses = modulestore().collection.find(course_search_location, fields=('_id')) if courses.count() > 0: return JsonResponse({ - 'ErrMsg': _('There is already a course defined with the same ' + 'ErrMsg': _( + 'There is already a course defined with the same ' 'organization and course number. Please ' 'change at least one field to be unique.'), - 'OrgErrMsg': _('Please change either the organization or ' + 'OrgErrMsg': _( + 'Please change either the organization or ' 'course number so that it is unique.'), - 'CourseErrMsg': _('Please change either the organization or ' + 'CourseErrMsg': _( + 'Please change either the organization or ' 'course number so that it is unique.'), }) @@ -347,9 +350,8 @@ def course_info_handler(request, tag=None, course_id=None, branch=None, version_ @ensure_csrf_cookie @require_http_methods(("GET", "POST", "PUT", "DELETE")) @expect_json -def course_info_update_handler( - request, tag=None, course_id=None, branch=None, version_guid=None, block=None, provided_id=None - ): +def course_info_update_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None, + provided_id=None): """ restful CRUD operations on course_info updates. provided_id should be none if it's new (create) and index otherwise. @@ -394,232 +396,206 @@ def course_info_update_handler( @login_required @ensure_csrf_cookie -def get_course_settings(request, org, course, name): - """ - Send models and views as well as html for editing the course settings to - the client. - - org, course, name: Attributes of the Location for the item to edit - """ - location = get_location_and_verify_access(request, org, course, name) - - course_module = modulestore().get_item(location) - - new_loc = loc_mapper().translate_location(location.course_id, location, False, True) - upload_asset_url = new_loc.url_reverse('assets/', '') - - return render_to_response('settings.html', { - 'context_course': course_module, - 'course_location': location, - 'details_url': reverse(course_settings_updates, - kwargs={"org": org, - "course": course, - "name": name, - "section": "details"}), - 'about_page_editable': not settings.MITX_FEATURES.get( - 'ENABLE_MKTG_SITE', False - ), - 'upload_asset_url': upload_asset_url - }) - - -@login_required -@ensure_csrf_cookie -def course_config_graders_page(request, org, course, name): - """ - Send models and views as well as html for editing the course settings to - the client. - - org, course, name: Attributes of the Location for the item to edit - """ - location = get_location_and_verify_access(request, org, course, name) - - course_module = modulestore().get_item(location) - course_details = CourseGradingModel.fetch(location) - - return render_to_response('settings_graders.html', { - 'context_course': course_module, - 'course_location': location, - 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder) - }) - - -@login_required -@ensure_csrf_cookie -def course_config_advanced_page(request, org, course, name): - """ - Send models and views as well as html for editing the advanced course - settings to the client. - - org, course, name: Attributes of the Location for the item to edit - """ - location = get_location_and_verify_access(request, org, course, name) - - course_module = modulestore().get_item(location) - - return render_to_response('settings_advanced.html', { - 'context_course': course_module, - 'course_location': location, - 'advanced_dict': json.dumps(CourseMetadata.fetch(location)), - }) - - +@require_http_methods(("GET", "PUT", "POST")) @expect_json +def settings_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): + """ + Course settings for dates and about pages + GET + html: get the page + json: get the CourseDetails model + PUT + json: update the Course and About xblocks through the CourseDetails model + """ + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): + raise PermissionDenied() + + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + course_old_location = loc_mapper().translate_locator_to_location(locator) + course_module = modulestore().get_item(course_old_location) + + upload_asset_url = locator.url_reverse('assets/') + + return render_to_response('settings.html', { + 'context_course': course_module, + 'course_locator': locator, + 'lms_link_for_about_page': utils.get_lms_link_for_about_page(course_old_location), + 'course_image_url': utils.course_image_url(course_module), + 'details_url': locator.url_reverse('/settings/details/'), + 'about_page_editable': not settings.MITX_FEATURES.get( + 'ENABLE_MKTG_SITE', False + ), + 'upload_asset_url': upload_asset_url + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + return JsonResponse( + CourseDetails.fetch(locator), + # encoder serializes dates, old locations, and instances + encoder=CourseSettingsEncoder + ) + else: # post or put, doesn't matter. + return JsonResponse( + CourseDetails.update_from_json(locator, request.json), + encoder=CourseSettingsEncoder + ) + + @login_required @ensure_csrf_cookie -def course_settings_updates(request, org, course, name, section): - """ - Restful CRUD operations on course settings. This differs from - get_course_settings by communicating purely through json (not rendering any - html) and handles section level operations rather than whole page. - - org, course: Attributes of the Location for the item to edit - section: one of details, faculty, grading, problems, discussions - """ - get_location_and_verify_access(request, org, course, name) - - if section == 'details': - manager = CourseDetails - elif section == 'grading': - manager = CourseGradingModel - else: - return - - if request.method == 'GET': - # Cannot just do a get w/o knowing the course name :-( - return JsonResponse( - manager.fetch(Location(['i4x', org, course, 'course', name])), - encoder=CourseSettingsEncoder - ) - elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. - return JsonResponse( - manager.update_from_json(request.json), - encoder=CourseSettingsEncoder - ) - - -@expect_json @require_http_methods(("GET", "POST", "PUT", "DELETE")) -@login_required -@ensure_csrf_cookie -def course_grader_updates(request, org, course, name, grader_index=None): - """ - Restful CRUD operations on course_info updates. This differs from - get_course_settings by communicating purely through json (not rendering any - html) and handles section level operations rather than whole page. - - org, course: Attributes of the Location for the item to edit - """ - - location = get_location_and_verify_access(request, org, course, name) - - if request.method == 'GET': - # Cannot just do a get w/o knowing the course name :-( - return JsonResponse(CourseGradingModel.fetch_grader( - Location(location), grader_index - )) - elif request.method == "DELETE": - # ??? Should this return anything? Perhaps success fail? - CourseGradingModel.delete_grader(Location(location), grader_index) - return JsonResponse() - else: # post or put, doesn't matter. - return JsonResponse(CourseGradingModel.update_grader_from_json( - Location(location), - request.json - )) - - -@require_http_methods(("GET", "POST", "PUT", "DELETE")) -@login_required -@ensure_csrf_cookie @expect_json -def course_advanced_updates(request, org, course, name): +def grading_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None, grader_index=None): """ - Restful CRUD operations on metadata. The payload is a json rep of the - metadata dicts. For delete, otoh, the payload is either a key or a list of - keys to delete. - - org, course: Attributes of the Location for the item to edit + Course Grading policy configuration + GET + html: get the page + json no grader_index: get the CourseGrading model (graceperiod, cutoffs, and graders) + json w/ grader_index: get the specific grader + PUT + json no grader_index: update the Course through the CourseGrading model + json w/ grader_index: create or update the specific grader (create if index out of range) """ - location = get_location_and_verify_access(request, org, course, name) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): + raise PermissionDenied() - if request.method == 'GET': - return JsonResponse(CourseMetadata.fetch(location)) - elif request.method == 'DELETE': - return JsonResponse(CourseMetadata.delete_key( - location, - json.loads(request.body) - )) - else: - # Whether or not to filter the tabs key out of the settings metadata - filter_tabs = True + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + course_old_location = loc_mapper().translate_locator_to_location(locator) + course_module = modulestore().get_item(course_old_location) + course_details = CourseGradingModel.fetch(locator) - # Check to see if the user instantiated any advanced components. This - # is a hack that does the following : - # 1) adds/removes the open ended panel tab to a course automatically - # if the user has indicated that they want to edit the - # combinedopendended or peergrading module - # 2) adds/removes the notes panel tab to a course automatically if - # the user has indicated that they want the notes module enabled in - # their course - # TODO refactor the above into distinct advanced policy settings - if ADVANCED_COMPONENT_POLICY_KEY in request.json: - # Get the course so that we can scrape current tabs - course_module = modulestore().get_item(location) + return render_to_response('settings_graders.html', { + 'context_course': course_module, + 'course_locator': locator, + 'course_details': json.dumps(course_details, cls=CourseSettingsEncoder), + 'grading_url': locator.url_reverse('/settings/grading/'), + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + if grader_index is None: + return JsonResponse( + CourseGradingModel.fetch(locator), + # encoder serializes dates, old locations, and instances + encoder=CourseSettingsEncoder + ) + else: + return JsonResponse(CourseGradingModel.fetch_grader(locator, grader_index)) + elif request.method in ('POST', 'PUT'): # post or put, doesn't matter. + # None implies update the whole model (cutoffs, graceperiod, and graders) not a specific grader + if grader_index is None: + return JsonResponse( + CourseGradingModel.update_from_json(locator, request.json), + encoder=CourseSettingsEncoder + ) + else: + return JsonResponse( + CourseGradingModel.update_grader_from_json(locator, request.json) + ) + elif request.method == "DELETE" and grader_index is not None: + CourseGradingModel.delete_grader(locator, grader_index) + return JsonResponse() - # Maps tab types to components - tab_component_map = { - 'open_ended': OPEN_ENDED_COMPONENT_TYPES, - 'notes': NOTE_COMPONENT_TYPES, - } - # Check to see if the user instantiated any notes or open ended - # components - for tab_type in tab_component_map.keys(): - component_types = tab_component_map.get(tab_type) - found_ac_type = False - for ac_type in component_types: - if ac_type in request.json[ADVANCED_COMPONENT_POLICY_KEY]: - # Add tab to the course if needed - changed, new_tabs = add_extra_panel_tab( - tab_type, - course_module - ) - # If a tab has been added to the course, then send the - # metadata along to CourseMetadata.update_from_json - if changed: - course_module.tabs = new_tabs - request.json.update({'tabs': new_tabs}) - # Indicate that tabs should not be filtered out of - # the metadata - filter_tabs = False - # Set this flag to avoid the tab removal code below. - found_ac_type = True - break - # If we did not find a module type in the advanced settings, - # we may need to remove the tab from the course. - if not found_ac_type: - # Remove tab from the course if needed - changed, new_tabs = remove_extra_panel_tab( - tab_type, course_module - ) +# pylint: disable=invalid-name +def _config_course_advanced_components(request, course_module): + """ + Check to see if the user instantiated any advanced components. This + is a hack that does the following : + 1) adds/removes the open ended panel tab to a course automatically + if the user has indicated that they want to edit the + combinedopendended or peergrading module + 2) adds/removes the notes panel tab to a course automatically if + the user has indicated that they want the notes module enabled in + their course + """ + # TODO refactor the above into distinct advanced policy settings + filter_tabs = True # Exceptional conditions will pull this to False + if ADVANCED_COMPONENT_POLICY_KEY in request.json: # Maps tab types to components + tab_component_map = { + 'open_ended':OPEN_ENDED_COMPONENT_TYPES, + 'notes':NOTE_COMPONENT_TYPES, + } + # Check to see if the user instantiated any notes or open ended + # components + for tab_type in tab_component_map.keys(): + component_types = tab_component_map.get(tab_type) + found_ac_type = False + for ac_type in component_types: + if ac_type in request.json[ADVANCED_COMPONENT_POLICY_KEY]: + # Add tab to the course if needed + changed, new_tabs = add_extra_panel_tab(tab_type, course_module) + # If a tab has been added to the course, then send the + # metadata along to CourseMetadata.update_from_json if changed: course_module.tabs = new_tabs request.json.update({'tabs': new_tabs}) - # Indicate that tabs should *not* be filtered out of + # Indicate that tabs should not be filtered out of # the metadata - filter_tabs = False - try: - return JsonResponse(CourseMetadata.update_from_json( - location, - request.json, - filter_tabs=filter_tabs - )) - except (TypeError, ValueError) as err: - return HttpResponseBadRequest( - "Incorrect setting format. " + str(err), - content_type="text/plain" - ) + filter_tabs = False # Set this flag to avoid the tab removal code below. + found_ac_type = True #break + + # If we did not find a module type in the advanced settings, + # we may need to remove the tab from the course. + if not found_ac_type: # Remove tab from the course if needed + changed, new_tabs = remove_extra_panel_tab(tab_type, course_module) + if changed: + course_module.tabs = new_tabs + request.json.update({'tabs':new_tabs}) + # Indicate that tabs should *not* be filtered out of + # the metadata + filter_tabs = False + + return filter_tabs + + +@login_required +@ensure_csrf_cookie +@require_http_methods(("GET", "POST", "PUT")) +@expect_json +def advanced_settings_handler(request, course_id=None, branch=None, version_guid=None, block=None, tag=None): + """ + Course settings configuration + GET + html: get the page + json: get the model + PUT, POST + json: update the Course's settings. The payload is a json rep of the + metadata dicts. The dict can include a "unsetKeys" entry which is a list + of keys whose values to unset: i.e., revert to default + """ + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): + raise PermissionDenied() + + course_old_location = loc_mapper().translate_locator_to_location(locator) + course_module = modulestore().get_item(course_old_location) + + if 'text/html' in request.META.get('HTTP_ACCEPT', '') and request.method == 'GET': + + return render_to_response('settings_advanced.html', { + 'context_course': course_module, + 'advanced_dict': json.dumps(CourseMetadata.fetch(course_module)), + 'advanced_settings_url': locator.url_reverse('settings/advanced') + }) + elif 'application/json' in request.META.get('HTTP_ACCEPT', ''): + if request.method == 'GET': + return JsonResponse(CourseMetadata.fetch(course_module)) + else: + # Whether or not to filter the tabs key out of the settings metadata + filter_tabs = _config_course_advanced_components(request, course_module) + try: + return JsonResponse(CourseMetadata.update_from_json( + course_module, + request.json, + filter_tabs=filter_tabs + )) + except (TypeError, ValueError) as err: + return HttpResponseBadRequest( + "Incorrect setting format. {}".format(err), + content_type="text/plain" + ) class TextbookValidationError(Exception): diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index 5d1b26ec3d..f740d10707 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -14,7 +14,6 @@ from django.conf import settings from django.http import HttpResponse from django.contrib.auth.decorators import login_required from django_future.csrf import ensure_csrf_cookie -from django.core.urlresolvers import reverse from django.core.servers.basehttp import FileWrapper from django.core.files.temp import NamedTemporaryFile from django.core.exceptions import SuspiciousOperation, PermissionDenied @@ -140,7 +139,7 @@ def import_handler(request, tag=None, course_id=None, branch=None, version_guid= "size": size, "deleteUrl": "", "deleteType": "", - "url": location.url_reverse('import/', ''), + "url": location.url_reverse('import'), "thumbnailUrl": "" }] }) @@ -252,8 +251,8 @@ def import_handler(request, tag=None, course_id=None, branch=None, version_guid= course_module = modulestore().get_item(old_location) return render_to_response('import.html', { 'context_course': course_module, - 'successful_import_redirect_url': location.url_reverse("course/", ""), - 'import_status_url': location.url_reverse("import_status/", "fillerName"), + 'successful_import_redirect_url': location.url_reverse("course"), + 'import_status_url': location.url_reverse("import_status", "fillerName"), }) else: return HttpResponseNotFound() @@ -313,7 +312,7 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid= # an _accept URL parameter will be preferred over HTTP_ACCEPT in the header. requested_format = request.REQUEST.get('_accept', request.META.get('HTTP_ACCEPT', 'text/html')) - export_url = location.url_reverse('export/', '') + '?_accept=application/x-tgz' + export_url = location.url_reverse('export') + '?_accept=application/x-tgz' if 'application/x-tgz' in requested_format: name = old_location.name export_file = NamedTemporaryFile(prefix=name + '.', suffix=".tar.gz") @@ -339,16 +338,16 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid= # if we have a nested exception, then we'll show the more generic error message pass + unit_locator = loc_mapper().translate_location(old_location.course_id, parent.location, False, True) + return render_to_response('export.html', { 'context_course': course_module, 'in_err': True, 'raw_err_msg': str(e), 'failed_module': failed_item, 'unit': unit, - 'edit_unit_url': reverse('edit_unit', kwargs={ - 'location': parent.location - }) if parent else '', - 'course_home_url': location.url_reverse("course/", ""), + 'edit_unit_url': unit_locator.url_reverse("unit") if parent else "", + 'course_home_url': location.url_reverse("course"), 'export_url': export_url }) @@ -359,7 +358,7 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid= 'in_err': True, 'unit': None, 'raw_err_msg': str(e), - 'course_home_url': location.url_reverse("course/", ""), + 'course_home_url': location.url_reverse("course"), 'export_url': export_url }) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 97bfde3b82..220da038a7 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -3,7 +3,9 @@ import logging from uuid import uuid4 +from functools import partial from static_replace import replace_static_urls +from xmodule_modifiers import wrap_xblock from django.core.exceptions import PermissionDenied from django.contrib.auth.decorators import login_required @@ -27,6 +29,9 @@ from xmodule.modulestore.locator import BlockUsageLocator from student.models import CourseEnrollment from django.http import HttpResponseBadRequest from xblock.fields import Scope +from preview import handler_prefix, get_preview_html +from mitxmako.shortcuts import render_to_response, render_to_string +from models.settings.course_grading import CourseGradingModel __all__ = ['orphan_handler', 'xblock_handler'] @@ -51,17 +56,21 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= all children and "all_versions" to delete from all (mongo) versions. GET json: returns representation of the xblock (locator id, data, and metadata). + if ?fields=graderType, it returns the graderType for the unit instead of the above. + html: returns HTML for rendering the xblock (which includes both the "preview" view and the "editor" view) PUT or POST - json: if xblock location is specified, update the xblock instance. The json payload can contain + json: if xblock locator is specified, update the xblock instance. The json payload can contain these fields, all optional: :data: the new value for the data. :children: the locator ids of children for this xblock. :metadata: new values for the metadata fields. Any whose values are None will be deleted not set to None! Absent ones will be left alone. :nullout: which metadata fields to set to None + :graderType: change how this unit is graded + :publish: can be one of three values, 'make_public, 'make_private', or 'create_draft' The JSON representation on the updated xblock (minus children) is returned. - if xblock location is not specified, create a new xblock instance. The json playload can contain + if xblock locator is not specified, create a new xblock instance. The json playload can contain these fields: :parent_locator: parent for new xblock, required :category: type of xblock, required @@ -70,14 +79,38 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= The locator (and old-style id) for the created xblock (minus children) is returned. """ if course_id is not None: - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) - if not has_access(request.user, location): + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): raise PermissionDenied() - old_location = loc_mapper().translate_locator_to_location(location) + old_location = loc_mapper().translate_locator_to_location(locator) if request.method == 'GET': - rsp = _get_module_info(location) - return JsonResponse(rsp) + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + fields = request.REQUEST.get('fields', '').split(',') + if 'graderType' in fields: + # right now can't combine output of this w/ output of _get_module_info, but worthy goal + return JsonResponse(CourseGradingModel.get_section_grader_type(locator)) + # TODO: pass fields to _get_module_info and only return those + rsp = _get_module_info(locator) + return JsonResponse(rsp) + else: + component = modulestore().get_item(old_location) + # Wrap the generated fragment in the xmodule_editor div so that the javascript + # can bind to it correctly + component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix)) + + try: + content = component.render('studio_view').content + # catch exceptions indiscriminately, since after this point they escape the + # dungeon and surface as uneditable, unsaveable, and undeletable + # component-goblins. + except Exception as exc: # pylint: disable=W0703 + content = render_to_string('html_error.html', {'message': str(exc)}) + + return render_to_response('component.html', { + 'preview': get_preview_html(request, component), + 'editor': content + }) elif request.method == 'DELETE': delete_children = str_to_bool(request.REQUEST.get('recurse', 'False')) delete_all_versions = str_to_bool(request.REQUEST.get('all_versions', 'False')) @@ -85,12 +118,15 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= return _delete_item_at_location(old_location, delete_children, delete_all_versions) else: # Since we have a course_id, we are updating an existing xblock. return _save_item( - location, + request, + locator, old_location, data=request.json.get('data'), children=request.json.get('children'), metadata=request.json.get('metadata'), - nullout=request.json.get('nullout') + nullout=request.json.get('nullout'), + grader_type=request.json.get('graderType'), + publish=request.json.get('publish'), ) elif request.method in ('PUT', 'POST'): return _create_item(request) @@ -101,11 +137,14 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= ) -def _save_item(usage_loc, item_location, data=None, children=None, metadata=None, nullout=None): +def _save_item(request, usage_loc, item_location, data=None, children=None, metadata=None, nullout=None, + grader_type=None, publish=None): """ - Saves certain properties (data, children, metadata, nullout) for a given xblock item. + Saves xblock w/ its fields. Has special processing for grader_type, publish, and nullout and Nones in metadata. + nullout means to truly set the field to None whereas nones in metadata mean to unset them (so they revert + to default). - The item_location is still the old-style location. + The item_location is still the old-style location whereas usage_loc is a BlockUsageLocator """ store = get_modulestore(item_location) @@ -123,6 +162,14 @@ def _save_item(usage_loc, item_location, data=None, children=None, metadata=None log.error("Can't find item by location.") return JsonResponse({"error": "Can't find item by location: " + str(item_location)}, 404) + if publish: + if publish == 'make_private': + _xmodule_recurse(existing_item, lambda i: modulestore().unpublish(i.location)) + elif publish == 'create_draft': + # This clones the existing item location to a draft location (the draft is + # implicit, because modulestore is a Draft modulestore) + modulestore().convert_to_draft(item_location) + if data: store.update_item(item_location, data) else: @@ -170,12 +217,25 @@ def _save_item(usage_loc, item_location, data=None, children=None, metadata=None if existing_item.category == 'video': manage_video_subtitles_save(existing_item, existing_item) - # Note that children aren't being returned until we have a use case. - return JsonResponse({ + result = { 'id': unicode(usage_loc), 'data': data, 'metadata': own_metadata(existing_item) - }) + } + + if grader_type is not None: + result.update(CourseGradingModel.update_section_grader_type(existing_item, grader_type)) + + # Make public after updating the xblock, in case the caller asked + # for both an update and a publish. + if publish and publish == 'make_public': + _xmodule_recurse( + existing_item, + lambda i: modulestore().publish(i.location, request.user.id) + ) + + # Note that children aren't being returned until we have a use case. + return JsonResponse(result) @login_required @@ -192,10 +252,7 @@ def _create_item(request): raise PermissionDenied() parent = get_modulestore(category).get_item(parent_location) - # Necessary to set revision=None or else metadata inheritance does not work - # (the ID with @draft will be used as the key in the inherited metadata map, - # and that is not expected by the code that later references it). - dest_location = parent_location.replace(category=category, name=uuid4().hex, revision=None) + dest_location = parent_location.replace(category=category, name=uuid4().hex) # get the metadata, display_name, and definition from the request metadata = {} @@ -224,7 +281,7 @@ def _create_item(request): course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True) locator = loc_mapper().translate_location(course_location.course_id, dest_location, False, True) - return JsonResponse({'id': dest_location.url(), "locator": unicode(locator)}) + return JsonResponse({"locator": unicode(locator)}) def _delete_item_at_location(item_location, delete_children=False, delete_all_versions=False): diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index ab1375554d..123d7fbadb 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -3,7 +3,7 @@ from functools import partial from django.conf import settings from django.core.urlresolvers import reverse -from django.http import Http404, HttpResponseBadRequest, HttpResponseForbidden +from django.http import Http404, HttpResponseBadRequest from django.contrib.auth.decorators import login_required from mitxmako.shortcuts import render_to_response, render_to_string @@ -24,10 +24,9 @@ from util.sandboxing import can_execute_unsafe_code import static_replace from .session_kv_store import SessionKeyValueStore from .helpers import render_from_lms -from .access import has_access from ..utils import get_course_for_item -__all__ = ['preview_handler', 'preview_component'] +__all__ = ['preview_handler'] log = logging.getLogger(__name__) @@ -53,13 +52,13 @@ def preview_handler(request, usage_id, handler, suffix=''): usage_id: The usage-id of the block to dispatch to, passed through `quote_slashes` handler: The handler to execute - suffix: The remaineder of the url to be passed to the handler + suffix: The remainder of the url to be passed to the handler """ location = unquote_slashes(usage_id) descriptor = modulestore().get_item(location) - instance = load_preview_module(request, descriptor) + instance = _load_preview_module(request, descriptor) # Let the module handle the AJAX req = django_to_webob_request(request) try: @@ -85,32 +84,6 @@ def preview_handler(request, usage_id, handler, suffix=''): return webob_to_django_response(resp) -@login_required -def preview_component(request, location): - "Return the HTML preview of a component" - # TODO (vshnayder): change name from id to location in coffee+html as well. - if not has_access(request.user, location): - return HttpResponseForbidden() - - component = modulestore().get_item(location) - # Wrap the generated fragment in the xmodule_editor div so that the javascript - # can bind to it correctly - component.runtime.wrappers.append(partial(wrap_xblock, handler_prefix)) - - try: - content = component.render('studio_view').content - # catch exceptions indiscriminately, since after this point they escape the - # dungeon and surface as uneditable, unsaveable, and undeletable - # component-goblins. - except Exception as exc: # pylint: disable=W0703 - content = render_to_string('html_error.html', {'message': str(exc)}) - - return render_to_response('component.html', { - 'preview': get_preview_html(request, component), - 'editor': content - }) - - class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method """ An XModule ModuleSystem for use in Studio previews @@ -119,7 +92,7 @@ class PreviewModuleSystem(ModuleSystem): # pylint: disable=abstract-method return handler_prefix(block, handler_name, suffix) + '?' + query -def preview_module_system(request, descriptor): +def _preview_module_system(request, descriptor): """ Returns a ModuleSystem for the specified descriptor that is specialized for rendering module previews. @@ -135,7 +108,7 @@ def preview_module_system(request, descriptor): # TODO (cpennington): Do we want to track how instructors are using the preview problems? track_function=lambda event_type, event: None, filestore=descriptor.runtime.resources_fs, - get_module=partial(load_preview_module, request), + get_module=partial(_load_preview_module, request), render_template=render_from_lms, debug=True, replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_id=course_id), @@ -162,7 +135,7 @@ def preview_module_system(request, descriptor): ) -def load_preview_module(request, descriptor): +def _load_preview_module(request, descriptor): """ Return a preview XModule instantiated from the supplied descriptor. @@ -171,7 +144,7 @@ def load_preview_module(request, descriptor): """ student_data = DbModel(SessionKeyValueStore(request)) descriptor.bind_for_student( - preview_module_system(request, descriptor), + _preview_module_system(request, descriptor), LmsFieldData(descriptor._field_data, student_data), # pylint: disable=protected-access ) return descriptor @@ -182,7 +155,7 @@ def get_preview_html(request, descriptor): Returns the HTML returned by the XModule's student_view, specified by the descriptor and idx. """ - module = load_preview_module(request, descriptor) + module = _load_preview_module(request, descriptor) try: content = module.render("student_view").content except Exception as exc: # pylint: disable=W0703 diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index de0a1899b3..9ab03a4093 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -10,7 +10,7 @@ from mitxmako.shortcuts import render_to_response from external_auth.views import ssl_login_shortcut -__all__ = ['signup', 'old_login_redirect', 'login_page', 'howitworks'] +__all__ = ['signup', 'login_page', 'howitworks'] @ensure_csrf_cookie @@ -22,13 +22,6 @@ def signup(request): return render_to_response('signup.html', {'csrf': csrf_token}) -def old_login_redirect(request): - ''' - Redirect to the active login url. - ''' - return redirect('login', permanent=True) - - @ssl_login_shortcut @ensure_csrf_cookie def login_page(request): diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 277445e3b9..46791ddc26 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -2,12 +2,13 @@ Views related to course tabs """ from access import has_access -from util.json_request import expect_json +from util.json_request import expect_json, JsonResponse -from django.http import HttpResponse, HttpResponseBadRequest +from django.http import HttpResponseNotFound from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie +from django.views.decorators.http import require_http_methods from mitxmako.shortcuts import render_to_response from xmodule.modulestore import Location from xmodule.modulestore.inheritance import own_metadata @@ -19,7 +20,7 @@ from ..utils import get_modulestore from django.utils.translation import ugettext as _ -__all__ = ['edit_tabs', 'reorder_static_tabs'] +__all__ = ['tabs_handler'] def initialize_course_tabs(course): @@ -43,107 +44,113 @@ def initialize_course_tabs(course): modulestore('direct').update_metadata(course.location.url(), own_metadata(course)) - -@login_required @expect_json -def reorder_static_tabs(request): - "Order the static tabs in the requested order" - def get_location_for_tab(tab): - tab_locator = BlockUsageLocator(tab) - return loc_mapper().translate_locator_to_location(tab_locator) - - tabs = request.json['tabs'] - course_location = loc_mapper().translate_locator_to_location(BlockUsageLocator(tabs[0]), get_course=True) - - if not has_access(request.user, course_location): - raise PermissionDenied() - - course = get_modulestore(course_location).get_item(course_location) - - # get list of existing static tabs in course - # make sure they are the same lengths (i.e. the number of passed in tabs equals the number - # that we know about) otherwise we can drop some! - - existing_static_tabs = [t for t in course.tabs if t['type'] == 'static_tab'] - if len(existing_static_tabs) != len(tabs): - return HttpResponseBadRequest() - - # load all reference tabs, return BadRequest if we can't find any of them - tab_items = [] - for tab in tabs: - item = modulestore('direct').get_item(get_location_for_tab(tab)) - if item is None: - return HttpResponseBadRequest() - - tab_items.append(item) - - # now just go through the existing course_tabs and re-order the static tabs - reordered_tabs = [] - static_tab_idx = 0 - for tab in course.tabs: - if tab['type'] == 'static_tab': - reordered_tabs.append({'type': 'static_tab', - 'name': tab_items[static_tab_idx].display_name, - 'url_slug': tab_items[static_tab_idx].location.name}) - static_tab_idx += 1 - else: - reordered_tabs.append(tab) - - # OK, re-assemble the static tabs in the new order - course.tabs = reordered_tabs - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - course.save() - modulestore('direct').update_metadata(course.location, own_metadata(course)) - # TODO: above two lines are used for the primitive-save case. Maybe factor them out? - return HttpResponse() - - @login_required @ensure_csrf_cookie -def edit_tabs(request, org, course, coursename): - "Edit tabs" - location = ['i4x', org, course, 'course', coursename] - store = get_modulestore(location) - course_item = store.get_item(location) +@require_http_methods(("GET", "POST", "PUT")) +def tabs_handler(request, tag=None, course_id=None, branch=None, version_guid=None, block=None): + """ + The restful handler for static tabs. - # check that logged in user has permissions to this item - if not has_access(request.user, location): + GET + html: return page for editing static tabs + json: not supported + PUT or POST + json: update the tab order. It is expected that the request body contains a JSON-encoded dict with entry "tabs". + The value for "tabs" is an array of tab locators, indicating the desired order of the tabs. + + Creating a tab, deleting a tab, or changing its contents is not supported through this method. + Instead use the general xblock URL (see item.xblock_handler). + """ + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + if not has_access(request.user, locator): raise PermissionDenied() - # see tabs have been uninitialized (e.g. supporing courses created before tab support in studio) - if course_item.tabs is None or len(course_item.tabs) == 0: - initialize_course_tabs(course_item) + old_location = loc_mapper().translate_locator_to_location(locator) + store = get_modulestore(old_location) + course_item = store.get_item(old_location) - # first get all static tabs from the tabs list - # we do this because this is also the order in which items are displayed in the LMS - static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + if request.method == 'GET': + raise NotImplementedError('coming soon') + else: + if 'tabs' in request.json: + def get_location_for_tab(tab): + """ Returns the location (old-style) for a tab. """ + return loc_mapper().translate_locator_to_location(BlockUsageLocator(tab)) - static_tabs = [] - for static_tab_ref in static_tabs_refs: - static_tab_loc = Location(location)._replace(category='static_tab', name=static_tab_ref['url_slug']) - static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + tabs = request.json['tabs'] - components = [ - [ - static_tab.location.url(), + # get list of existing static tabs in course + # make sure they are the same lengths (i.e. the number of passed in tabs equals the number + # that we know about) otherwise we will inadvertently drop some! + existing_static_tabs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + if len(existing_static_tabs) != len(tabs): + return JsonResponse( + {"error": "number of tabs must be {}".format(len(existing_static_tabs))}, status=400 + ) + + # load all reference tabs, return BadRequest if we can't find any of them + tab_items = [] + for tab in tabs: + item = modulestore('direct').get_item(get_location_for_tab(tab)) + if item is None: + return JsonResponse( + {"error": "no tab for found location {}".format(tab)}, status=400 + ) + + tab_items.append(item) + + # now just go through the existing course_tabs and re-order the static tabs + reordered_tabs = [] + static_tab_idx = 0 + for tab in course_item.tabs: + if tab['type'] == 'static_tab': + reordered_tabs.append( + {'type': 'static_tab', + 'name': tab_items[static_tab_idx].display_name, + 'url_slug': tab_items[static_tab_idx].location.name, + } + ) + static_tab_idx += 1 + else: + reordered_tabs.append(tab) + + # OK, re-assemble the static tabs in the new order + course_item.tabs = reordered_tabs + modulestore('direct').update_metadata(course_item.location, own_metadata(course_item)) + return JsonResponse() + else: + raise NotImplementedError('Creating or changing tab content is not supported.') + elif request.method == 'GET': # assume html + # see tabs have been uninitialized (e.g. supporting courses created before tab support in studio) + if course_item.tabs is None or len(course_item.tabs) == 0: + initialize_course_tabs(course_item) + + # first get all static tabs from the tabs list + # we do this because this is also the order in which items are displayed in the LMS + static_tabs_refs = [t for t in course_item.tabs if t['type'] == 'static_tab'] + + static_tabs = [] + for static_tab_ref in static_tabs_refs: + static_tab_loc = old_location.replace(category='static_tab', name=static_tab_ref['url_slug']) + static_tabs.append(modulestore('direct').get_item(static_tab_loc)) + + components = [ loc_mapper().translate_location( course_item.location.course_id, static_tab.location, False, True ) + for static_tab + in static_tabs ] - for static_tab - in static_tabs - ] - course_locator = loc_mapper().translate_location( - course_item.location.course_id, course_item.location, False, True - ) - - return render_to_response('edit-tabs.html', { - 'context_course': course_item, - 'components': components, - 'locator': course_locator - }) + return render_to_response('edit-tabs.html', { + 'context_course': course_item, + 'components': components, + 'course_locator': locator + }) + else: + return HttpResponseNotFound() # "primitive" tab edit functions driven by the command line. @@ -167,7 +174,7 @@ def primitive_delete(course, num): # Note for future implementations: if you delete a static_tab, then Chris Dodge # points out that there's other stuff to delete beyond this element. # This code happens to not delete static_tab so it doesn't come up. - primitive_save(course) + modulestore('direct').update_metadata(course.location, own_metadata(course)) def primitive_insert(course, num, tab_type, name): @@ -176,11 +183,5 @@ def primitive_insert(course, num, tab_type, name): new_tab = {u'type': unicode(tab_type), u'name': unicode(name)} tabs = course.tabs tabs.insert(num, new_tab) - primitive_save(course) - - -def primitive_save(course): - "Saves the course back to modulestore." - # This code copied from reorder_static_tabs above - course.save() modulestore('direct').update_metadata(course.location, own_metadata(course)) + diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 99ce00b891..dd8582ba76 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -1,20 +1,25 @@ +import re +import logging +import datetime +import json +from json.encoder import JSONEncoder + from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata -import json -from json.encoder import JSONEncoder from contentstore.utils import get_modulestore, course_image_url from models.settings import course_grading from contentstore.utils import update_item from xmodule.fields import Date -import re -import logging -import datetime +from xmodule.modulestore.django import loc_mapper class CourseDetails(object): - def __init__(self, location): - self.course_location = location # a Location obj + def __init__(self, org, course_id, run): + # still need these for now b/c the client's screen shows these 3 fields + self.org = org + self.course_id = course_id + self.run = run self.start_date = None # 'start' self.end_date = None # 'end' self.enrollment_start = None @@ -27,16 +32,13 @@ class CourseDetails(object): self.course_image_asset_path = "" # URL of the course image @classmethod - def fetch(cls, course_location): + def fetch(cls, course_locator): """ Fetch the course details for the given course from persistence and return a CourseDetails model. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - course = cls(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_locator) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) + course = cls(course_old_location.org, course_old_location.course, course_old_location.name) course.start_date = descriptor.start course.end_date = descriptor.end @@ -45,7 +47,7 @@ class CourseDetails(object): course.course_image_name = descriptor.course_image course.course_image_asset_path = course_image_url(descriptor) - temploc = course_location.replace(category='about', name='syllabus') + temploc = course_old_location.replace(category='about', name='syllabus') try: course.syllabus = get_modulestore(temploc).get_item(temploc).data except ItemNotFoundError: @@ -73,14 +75,12 @@ class CourseDetails(object): return course @classmethod - def update_from_json(cls, jsondict): + def update_from_json(cls, course_locator, jsondict): """ Decode the json into CourseDetails and save any changed attrs to the db """ - # TODO make it an error for this to be undefined & for it to not be retrievable from modulestore - course_location = Location(jsondict['course_location']) - # Will probably want to cache the inflight courses because every blur generates an update - descriptor = get_modulestore(course_location).get_item(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_locator) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) dirty = False @@ -134,11 +134,11 @@ class CourseDetails(object): # MongoKeyValueStore before we update the mongo datastore. descriptor.save() - get_modulestore(course_location).update_metadata(course_location, own_metadata(descriptor)) + get_modulestore(course_old_location).update_metadata(course_old_location, own_metadata(descriptor)) # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed # to make faster, could compare against db or could have client send over a list of which fields changed. - temploc = Location(course_location).replace(category='about', name='syllabus') + temploc = Location(course_old_location).replace(category='about', name='syllabus') update_item(temploc, jsondict['syllabus']) temploc = temploc.replace(name='overview') @@ -151,9 +151,9 @@ class CourseDetails(object): recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video']) update_item(temploc, recomposed_video_tag) - # Could just generate and return a course obj w/o doing any db reads, but I put the reads in as a means to confirm + # Could just return jsondict w/o doing any db reads, but I put the reads in as a means to confirm # it persisted correctly - return CourseDetails.fetch(course_location) + return CourseDetails.fetch(course_locator) @staticmethod def parse_video_tag(raw_video): @@ -188,6 +188,9 @@ class CourseDetails(object): # TODO move to a more general util? class CourseSettingsEncoder(json.JSONEncoder): + """ + Serialize CourseDetails, CourseGradingModel, datetime, and old Locations + """ def default(self, obj): if isinstance(obj, (CourseDetails, course_grading.CourseGradingModel)): return obj.__dict__ diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 578961fad6..fbbb37450c 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -1,6 +1,7 @@ -from xmodule.modulestore import Location -from contentstore.utils import get_modulestore from datetime import timedelta +from contentstore.utils import get_modulestore +from xmodule.modulestore.django import loc_mapper +from xblock.fields import Scope class CourseGradingModel(object): @@ -9,22 +10,20 @@ class CourseGradingModel(object): """ # Within this class, allow access to protected members of client classes. # This comes up when accessing kvs data and caches during kvs saves and modulestore writes. - # pylint: disable=W0212 def __init__(self, course_descriptor): - self.course_location = course_descriptor.location - self.graders = [CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader)] # weights transformed to ints [0..100] + self.graders = [ + CourseGradingModel.jsonize_grader(i, grader) for i, grader in enumerate(course_descriptor.raw_grader) + ] # weights transformed to ints [0..100] self.grade_cutoffs = course_descriptor.grade_cutoffs self.grace_period = CourseGradingModel.convert_set_grace_period(course_descriptor) @classmethod - def fetch(cls, course_location): + def fetch(cls, course_locator): """ - Fetch the course details for the given course from persistence and return a CourseDetails model. + Fetch the course grading policy for the given course from persistence and return a CourseGradingModel. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_locator) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) model = cls(descriptor) return model @@ -35,12 +34,8 @@ class CourseGradingModel(object): Fetch the course's nth grader Returns an empty dict if there's no such grader. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) - # # ??? it would be good if these had the course_location in them so that they stand alone sufficiently - # # but that would require not using CourseDescriptor's field directly. Opinions? + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) index = int(index) if len(descriptor.raw_grader) > index: @@ -57,48 +52,26 @@ class CourseGradingModel(object): } @staticmethod - def fetch_cutoffs(course_location): - """ - Fetch the course's grade cutoffs. - """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) - return descriptor.grade_cutoffs - - @staticmethod - def fetch_grace_period(course_location): - """ - Fetch the course's default grace period. - """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) - return {'grace_period': CourseGradingModel.convert_set_grace_period(descriptor)} - - @staticmethod - def update_from_json(jsondict): + def update_from_json(course_locator, jsondict): """ Decode the json into CourseGradingModel and save any changes. Returns the modified model. Probably not the usual path for updates as it's too coarse grained. """ - course_location = Location(jsondict['course_location']) - descriptor = get_modulestore(course_location).get_item(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_locator) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) + graders_parsed = [CourseGradingModel.parse_grader(jsonele) for jsonele in jsondict['graders']] descriptor.raw_grader = graders_parsed descriptor.grade_cutoffs = jsondict['grade_cutoffs'] - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor.xblock_kvs._data) + get_modulestore(course_old_location).update_item( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content) + ) - CourseGradingModel.update_grace_period_from_json(course_location, jsondict['grace_period']) + CourseGradingModel.update_grace_period_from_json(course_locator, jsondict['grace_period']) - return CourseGradingModel.fetch(course_location) + return CourseGradingModel.fetch(course_locator) @staticmethod def update_grader_from_json(course_location, grader): @@ -106,12 +79,8 @@ class CourseGradingModel(object): Create or update the grader of the given type (string key) for the given course. Returns the modified grader which is a full model on the client but not on the server (just a dict) """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) - # # ??? it would be good if these had the course_location in them so that they stand alone sufficiently - # # but that would require not using CourseDescriptor's field directly. Opinions? + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) # parse removes the id; so, grab it before parse index = int(grader.get('id', len(descriptor.raw_grader))) @@ -122,10 +91,9 @@ class CourseGradingModel(object): else: descriptor.raw_grader.append(grader) - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) + get_modulestore(course_old_location).update_item( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content) + ) return CourseGradingModel.jsonize_grader(index, descriptor.raw_grader[index]) @@ -135,16 +103,13 @@ class CourseGradingModel(object): Create or update the grade cutoffs for the given course. Returns sent in cutoffs (ie., no extra db fetch). """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - descriptor = get_modulestore(course_location).get_item(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) descriptor.grade_cutoffs = cutoffs - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) + get_modulestore(course_old_location).update_item( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content) + ) return cutoffs @@ -155,8 +120,8 @@ class CourseGradingModel(object): grace_period entry in an enclosing dict. It is also safe to call this method with a value of None for graceperiodjson. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) # Before a graceperiod has ever been created, it will be None (once it has been # created, it cannot be set back to None). @@ -164,81 +129,67 @@ class CourseGradingModel(object): if 'grace_period' in graceperiodjson: graceperiodjson = graceperiodjson['grace_period'] - # lms requires these to be in a fixed order grace_timedelta = timedelta(**graceperiodjson) - - descriptor = get_modulestore(course_location).get_item(course_location) descriptor.graceperiod = grace_timedelta - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_metadata(course_location, descriptor._field_data._kvs._metadata) + get_modulestore(course_old_location).update_metadata( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings) + ) @staticmethod def delete_grader(course_location, index): """ Delete the grader of the given type from the given course. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) - descriptor = get_modulestore(course_location).get_item(course_location) index = int(index) if index < len(descriptor.raw_grader): del descriptor.raw_grader[index] # force propagation to definition descriptor.raw_grader = descriptor.raw_grader - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_item(course_location, descriptor._field_data._kvs._data) + get_modulestore(course_old_location).update_item( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.content) + ) @staticmethod def delete_grace_period(course_location): """ - Delete the course's default grace period. + Delete the course's grace period. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) + course_old_location = loc_mapper().translate_locator_to_location(course_location) + descriptor = get_modulestore(course_old_location).get_item(course_old_location) - descriptor = get_modulestore(course_location).get_item(course_location) del descriptor.graceperiod - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_metadata(course_location, descriptor._field_data._kvs._metadata) + get_modulestore(course_old_location).update_metadata( + course_old_location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings) + ) @staticmethod def get_section_grader_type(location): - if not isinstance(location, Location): - location = Location(location) - - descriptor = get_modulestore(location).get_item(location) - return {"graderType": descriptor.format if descriptor.format is not None else 'Not Graded', - "location": location, - "id": 99 # just an arbitrary value to - } + old_location = loc_mapper().translate_locator_to_location(location) + descriptor = get_modulestore(old_location).get_item(old_location) + return { + "graderType": descriptor.format if descriptor.format is not None else 'Not Graded', + "location": unicode(location), + } @staticmethod - def update_section_grader_type(location, jsondict): - if not isinstance(location, Location): - location = Location(location) - - descriptor = get_modulestore(location).get_item(location) - if 'graderType' in jsondict and jsondict['graderType'] != u"Not Graded": - descriptor.format = jsondict.get('graderType') + def update_section_grader_type(descriptor, grader_type): + if grader_type is not None and grader_type != u"Not Graded": + descriptor.format = grader_type descriptor.graded = True else: del descriptor.format del descriptor.graded - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(location).update_metadata(location, descriptor._field_data._kvs._metadata) + get_modulestore(descriptor.location).update_metadata( + descriptor.location, descriptor.get_explicitly_set_fields_by_scope(Scope.settings) + ) + return {'graderType': grader_type} @staticmethod def convert_set_grace_period(descriptor): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 603865b884..ddb4814511 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -1,7 +1,7 @@ -from xmodule.modulestore import Location +from xblock.fields import Scope + from contentstore.utils import get_modulestore from xmodule.modulestore.inheritance import own_metadata -from xblock.fields import Scope from cms.xmodule_namespace import CmsBlockMixin @@ -20,21 +20,18 @@ class CourseMetadata(object): 'tabs', 'graceperiod', 'checklists', - 'show_timezone' + 'show_timezone', + 'format', + 'graded', ] @classmethod - def fetch(cls, course_location): + def fetch(cls, descriptor): """ Fetch the key:value editable course details for the given course from persistence and return a CourseMetadata model. """ - if not isinstance(course_location, Location): - course_location = Location(course_location) - - course = {} - - descriptor = get_modulestore(course_location).get_item(course_location) + result = {} for field in descriptor.fields.values(): if field.name in CmsBlockMixin.fields: @@ -46,19 +43,17 @@ class CourseMetadata(object): if field.name in cls.FILTERED_LIST: continue - course[field.name] = field.read_json(descriptor) + result[field.name] = field.read_json(descriptor) - return course + return result @classmethod - def update_from_json(cls, course_location, jsondict, filter_tabs=True): + def update_from_json(cls, descriptor, jsondict, filter_tabs=True): """ Decode the json into CourseMetadata and save any changed attrs to the db. Ensures none of the fields are in the blacklist. """ - descriptor = get_modulestore(course_location).get_item(course_location) - dirty = False # Copy the filtered list to avoid permanently changing the class attribute. @@ -72,39 +67,17 @@ class CourseMetadata(object): if key in filtered_list: continue + if key == "unsetKeys": + dirty = True + for unset in val: + descriptor.fields[unset].delete_from(descriptor) + if hasattr(descriptor, key) and getattr(descriptor, key) != val: dirty = True value = descriptor.fields[key].from_json(val) setattr(descriptor, key, value) if dirty: - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - get_modulestore(course_location).update_metadata(course_location, - own_metadata(descriptor)) + get_modulestore(descriptor.location).update_metadata(descriptor.location, own_metadata(descriptor)) - # Could just generate and return a course obj w/o doing any db reads, - # but I put the reads in as a means to confirm it persisted correctly - return cls.fetch(course_location) - - @classmethod - def delete_key(cls, course_location, payload): - ''' - Remove the given metadata key(s) from the course. payload can be a - single key or [key..] - ''' - descriptor = get_modulestore(course_location).get_item(course_location) - - for key in payload['deleteKeys']: - if hasattr(descriptor, key): - delattr(descriptor, key) - - # Save the data that we've just changed to the underlying - # MongoKeyValueStore before we update the mongo datastore. - descriptor.save() - - get_modulestore(course_location).update_metadata(course_location, - own_metadata(descriptor)) - - return cls.fetch(course_location) + return cls.fetch(descriptor) diff --git a/cms/envs/aws.py b/cms/envs/aws.py index 592415f61a..1b0c0ef648 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -166,9 +166,14 @@ SEGMENT_IO_KEY = AUTH_TOKENS.get('SEGMENT_IO_KEY') if SEGMENT_IO_KEY: MITX_FEATURES['SEGMENT_IO'] = ENV_TOKENS.get('SEGMENT_IO', False) - AWS_ACCESS_KEY_ID = AUTH_TOKENS["AWS_ACCESS_KEY_ID"] +if AWS_ACCESS_KEY_ID == "": + AWS_ACCESS_KEY_ID = None + AWS_SECRET_ACCESS_KEY = AUTH_TOKENS["AWS_SECRET_ACCESS_KEY"] +if AWS_SECRET_ACCESS_KEY == "": + AWS_SECRET_ACCESS_KEY = None + DATABASES = AUTH_TOKENS['DATABASES'] MODULESTORE = AUTH_TOKENS['MODULESTORE'] CONTENTSTORE = AUTH_TOKENS['CONTENTSTORE'] diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index 4c69172170..e25f092c9a 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -23,7 +23,8 @@ EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' ################################# LMS INTEGRATION ############################# -MITX_FEATURES['PREVIEW_LMS_BASE'] = "preview.localhost:8000" +LMS_BASE = "localhost:8000" +MITX_FEATURES['PREVIEW_LMS_BASE'] = "preview." + LMS_BASE ################################# CELERY ###################################### diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 2ee8d17979..c84b60be61 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -197,7 +197,8 @@ define([ "js/spec/transcripts/videolist_spec", "js/spec/transcripts/message_manager_spec", "js/spec/transcripts/file_uploader_spec", - "js/spec/utils/module_spec" + "js/spec/utils/module_spec", + "js/spec/models/explicit_url_spec" # these tests are run separate in the cms-squire suite, due to process # isolation issues with Squire.js diff --git a/cms/static/coffee/spec/views/course_info_spec.coffee b/cms/static/coffee/spec/views/course_info_spec.coffee index 3c388fa593..1e843d59fb 100644 --- a/cms/static/coffee/spec/views/course_info_spec.coffee +++ b/cms/static/coffee/spec/views/course_info_spec.coffee @@ -196,3 +196,22 @@ define ["js/views/course_info_handout", "js/views/course_info_update", "js/model @handoutsEdit.$el.find('.edit-button').click() expect(@handoutsEdit.$codeMirror.getValue().trim()).toEqual('/static/fromServer.jpg') + it "can open course handouts with bad html on edit", -> + # Enter some bad html in handouts section, verifying that the + # model/handoutform opens when "Edit" is clicked + + @model = new ModuleInfo({ + id: 'handouts-id', + data: '

[LINK TEXT]

') + expect($('.edit-handouts-form').is(':hidden')).toEqual(false) \ No newline at end of file diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 22d1052fa3..36716668d3 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -1,12 +1,9 @@ -define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) -> +define ["coffee/src/views/module_edit", "js/models/module_info", "xmodule"], (ModuleEdit, ModuleModel) -> describe "ModuleEdit", -> beforeEach -> - @stubModule = jasmine.createSpy("Module") - @stubModule.id = 'stub-id' - @stubModule.get = (param)-> - if param == 'old_id' - return 'stub-old-id' + @stubModule = new ModuleModel + id: "stub-id" setFixtures """
  • @@ -62,7 +59,7 @@ define ["coffee/src/views/module_edit", "xmodule"], (ModuleEdit) -> @moduleEdit.render() it "loads the module preview and editor via ajax on the view element", -> - expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/preview_component/#{@moduleEdit.model.get('old_id')}", jasmine.any(Function)) + expect(@moduleEdit.$el.load).toHaveBeenCalledWith("/xblock/#{@moduleEdit.model.id}", jasmine.any(Function)) @moduleEdit.$el.load.mostRecentCall.args[1]() expect(@moduleEdit.loadDisplay).toHaveBeenCalled() expect(@moduleEdit.delegateEvents).toHaveBeenCalled() diff --git a/cms/static/coffee/spec/views/overview_spec.coffee b/cms/static/coffee/spec/views/overview_spec.coffee index 9ece1b0059..cbc0821213 100644 --- a/cms/static/coffee/spec/views/overview_spec.coffee +++ b/cms/static/coffee/spec/views/overview_spec.coffee @@ -36,7 +36,7 @@ define ["js/views/overview", "js/views/feedback_notification", "sinon", "js/base appendSetFixtures """
    -
    diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index a13e572887..729a17615e 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -69,15 +69,13 @@ define ["backbone", "jquery", "underscore", "gettext", "xblock/runtime.v1", payload (data) => @model.set(id: data.locator) - @model.set(old_id: data.id) - @$el.data('id', data.id) @$el.data('locator', data.locator) @render() ) render: -> - if @model.get('old_id') - @$el.load("/preview_component/#{@model.get('old_id')}", => + if @model.id + @$el.load(@model.url(), => @loadDisplay() @delegateEvents() ) diff --git a/cms/static/coffee/src/views/tabs.coffee b/cms/static/coffee/src/views/tabs.coffee index 0f72e8bddb..83ca7dc2fe 100644 --- a/cms/static/coffee/src/views/tabs.coffee +++ b/cms/static/coffee/src/views/tabs.coffee @@ -6,8 +6,7 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views initialize: => @$('.component').each((idx, element) => model = new ModuleModel({ - id: $(element).data('locator'), - old_id:$(element).data('id') + id: $(element).data('locator') }) new ModuleEditView( @@ -38,14 +37,17 @@ define ["jquery", "jquery.ui", "backbone", "js/views/feedback_prompt", "js/views analytics.track "Reordered Static Pages", course: course_location_analytics + saving = new NotificationView.Mini({title: gettext("Saving…")}) + saving.show() + $.ajax({ type:'POST', - url: '/reorder_static_tabs', + url: @model.url(), data: JSON.stringify({ tabs : tabs }), contentType: 'application/json' - }) + }).success(=> saving.hide()) addNewTab: (event) => event.preventDefault() diff --git a/cms/static/coffee/src/views/unit.coffee b/cms/static/coffee/src/views/unit.coffee index 075b56d1b0..c4ff25c309 100644 --- a/cms/static/coffee/src/views/unit.coffee +++ b/cms/static/coffee/src/views/unit.coffee @@ -63,7 +63,6 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @$('.component').each (idx, element) => model = new ModuleModel id: $(element).data('locator') - old_id: $(element).data('id') new ModuleEditView el: element, onDelete: @deleteComponent, @@ -167,7 +166,7 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @wait(true) $.ajax({ type: 'DELETE', - url: @model.urlRoot + "/" + @$el.data('locator') + "?" + $.param({recurse: true}) + url: @model.url() + "?" + $.param({recurse: true}) }).success(=> analytics.track "Deleted Draft", @@ -180,8 +179,8 @@ define ["jquery", "jquery.ui", "gettext", "backbone", createDraft: (event) -> @wait(true) - $.postJSON('/create_draft', { - id: @$el.data('id') + $.postJSON(@model.url(), { + publish: 'create_draft' }, => analytics.track "Created Draft", course: course_location_analytics @@ -194,8 +193,8 @@ define ["jquery", "jquery.ui", "gettext", "backbone", @wait(true) @saveDraft() - $.postJSON('/publish_draft', { - id: @$el.data('id') + $.postJSON(@model.url(), { + publish: 'make_public' }, => analytics.track "Published Draft", course: course_location_analytics @@ -206,16 +205,16 @@ define ["jquery", "jquery.ui", "gettext", "backbone", setVisibility: (event) -> if @$('.visibility-select').val() == 'private' - target_url = '/unpublish_unit' + action = 'make_private' visibility = "private" else - target_url = '/publish_draft' + action = 'make_public' visibility = "public" @wait(true) - $.postJSON(target_url, { - id: @$el.data('id') + $.postJSON(@model.url(), { + publish: action }, => analytics.track "Set Unit Visibility", course: course_location_analytics diff --git a/cms/static/js/base.js b/cms/static/js/base.js index bc7260cf64..174ab10c89 100644 --- a/cms/static/js/base.js +++ b/cms/static/js/base.js @@ -237,7 +237,7 @@ function createNewUnit(e) { function(data) { // redirect to the edit page - window.location = "/edit/" + data['id']; + window.location = "/unit/" + data['locator']; }); } diff --git a/cms/static/js/collections/course_grader.js b/cms/static/js/collections/course_grader.js index c4adf64e1f..7dde698cb2 100644 --- a/cms/static/js/collections/course_grader.js +++ b/cms/static/js/collections/course_grader.js @@ -2,10 +2,6 @@ define(["backbone", "js/models/settings/course_grader"], function(Backbone, Cour var CourseGraderCollection = Backbone.Collection.extend({ model : CourseGrader, - course_location : null, // must be set to a Location object - url : function() { - return '/' + this.course_location.get('org') + "/" + this.course_location.get('course') + '/settings-grading/' + this.course_location.get('name') + '/'; - }, sumWeights : function() { return this.reduce(function(subtotal, grader) { return subtotal + grader.get('weight'); }, 0); } diff --git a/cms/static/js/models/assignment_grade.js b/cms/static/js/models/assignment_grade.js index 4c3d54b976..83f00a7d10 100644 --- a/cms/static/js/models/assignment_grade.js +++ b/cms/static/js/models/assignment_grade.js @@ -1,26 +1,14 @@ -define(["backbone", "underscore", "js/models/location"], function(Backbone, _, Location) { +define(["backbone", "underscore"], function(Backbone, _) { var AssignmentGrade = Backbone.Model.extend({ defaults : { - graderType : null, // the type label (string). May be "Not Graded" which implies None. I'd like to use id but that's ephemeral - location : null // A location object + graderType : null, // the type label (string). May be "Not Graded" which implies None. + locator : null // locator for the block }, - initialize : function(attrs) { - if (attrs['assignmentUrl']) { - this.set('location', new Location(attrs['assignmentUrl'], {parse: true})); - } - }, - parse : function(attrs) { - if (attrs && attrs['location']) { - attrs.location = new Location(attrs['location'], {parse: true}); - } - }, - urlRoot : function() { - if (this.has('location')) { - var location = this.get('location'); - return '/' + location.get('org') + "/" + location.get('course') + '/' + location.get('category') + '/' - + location.get('name') + '/gradeas/'; - } - else return ""; + idAttribute: 'locator', + urlRoot : '/xblock/', + url: function() { + // add ?fields=graderType to the request url (only needed for fetch, but innocuous for others) + return Backbone.Model.prototype.url.apply(this) + '?' + $.param({fields: 'graderType'}); } }); return AssignmentGrade; diff --git a/cms/static/js/models/course_info.js b/cms/static/js/models/course_info.js index e5a6114dff..e4c816ccf3 100644 --- a/cms/static/js/models/course_info.js +++ b/cms/static/js/models/course_info.js @@ -5,12 +5,9 @@ define(["backbone"], function(Backbone) { url: '', defaults: { - "courseId": "", // the location url "updates" : null, // UpdateCollection "handouts": null // HandoutCollection - }, - - idAttribute : "courseId" + } }); return CourseInfo; }); diff --git a/cms/static/js/models/explicit_url.js b/cms/static/js/models/explicit_url.js new file mode 100644 index 0000000000..aae69608af --- /dev/null +++ b/cms/static/js/models/explicit_url.js @@ -0,0 +1,14 @@ +/** + * A model that simply allows the update URL to be passed + * in as an argument. + */ +define(["backbone"], function(Backbone){ + return Backbone.Model.extend({ + defaults: { + "explicit_url": "" + }, + url: function() { + return this.get("explicit_url"); + } + }); +}); diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index 13cc4ce692..058cacadd7 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -1,8 +1,10 @@ -define(["backbone", "underscore", "gettext", "js/models/location"], function(Backbone, _, gettext, Location) { +define(["backbone", "underscore", "gettext"], function(Backbone, _, gettext) { var CourseDetails = Backbone.Model.extend({ defaults: { - location : null, // the course's Location model, required + org : '', + course_id: '', + run: '', start_date: null, // maps to 'start' end_date: null, // maps to 'end' enrollment_start: null, @@ -17,9 +19,6 @@ var CourseDetails = Backbone.Model.extend({ // When init'g from html script, ensure you pass {parse: true} as an option (2nd arg to reset) parse: function(attributes) { - if (attributes['course_location']) { - attributes.location = new Location(attributes.course_location, {parse:true}); - } if (attributes['start_date']) { attributes.start_date = new Date(attributes.start_date); } diff --git a/cms/static/js/models/settings/course_grading_policy.js b/cms/static/js/models/settings/course_grading_policy.js index 1e23a4ecf4..d034aa2cef 100644 --- a/cms/static/js/models/settings/course_grading_policy.js +++ b/cms/static/js/models/settings/course_grading_policy.js @@ -3,15 +3,11 @@ define(["backbone", "js/models/location", "js/collections/course_grader"], var CourseGradingPolicy = Backbone.Model.extend({ defaults : { - course_location : null, graders : null, // CourseGraderCollection grade_cutoffs : null, // CourseGradeCutoff model grace_period : null // either null or { hours: n, minutes: m, ...} }, parse: function(attributes) { - if (attributes['course_location']) { - attributes.course_location = new Location(attributes.course_location, {parse:true}); - } if (attributes['graders']) { var graderCollection; // interesting race condition: if {parse:true} when newing, then parse called before .attributes created @@ -21,7 +17,6 @@ var CourseGradingPolicy = Backbone.Model.extend({ } else { graderCollection = new CourseGraderCollection(attributes.graders, {parse:true}); - graderCollection.course_location = attributes['course_location'] || this.get('course_location'); } attributes.graders = graderCollection; } @@ -35,10 +30,6 @@ var CourseGradingPolicy = Backbone.Model.extend({ } return attributes; }, - url : function() { - var location = this.get('course_location'); - return '/' + location.get('org') + "/" + location.get('course') + '/settings-details/' + location.get('name') + '/section/grading'; - }, gracePeriodToDate : function() { var newDate = new Date(); if (this.has('grace_period') && this.get('grace_period')['hours']) diff --git a/cms/static/js/spec/models/explicit_url_spec.js b/cms/static/js/spec/models/explicit_url_spec.js new file mode 100644 index 0000000000..df70d47b63 --- /dev/null +++ b/cms/static/js/spec/models/explicit_url_spec.js @@ -0,0 +1,12 @@ +define(['js/models/explicit_url'], + function (Model) { + describe('Model ', function () { + it('allows url to be passed in constructor', function () { + expect(new Model({'explicit_url': '/fancy/url'}).url()).toBe('/fancy/url'); + }); + it('returns empty string if url not set', function () { + expect(new Model().url()).toBe(''); + }); + }); + } +); diff --git a/cms/static/js/views/course_info_handout.js b/cms/static/js/views/course_info_handout.js index f9804d03a4..9309deda1b 100644 --- a/cms/static/js/views/course_info_handout.js +++ b/cms/static/js/views/course_info_handout.js @@ -30,6 +30,7 @@ define(["backbone", "underscore", "codemirror", "js/views/feedback_notification" model: this.model })) ); + $('.handouts-content').html(this.model.get('data')); this.$preview = this.$el.find('.handouts-content'); this.$form = this.$el.find(".edit-handouts-form"); this.$editor = this.$form.find('.handouts-content-editor'); @@ -50,32 +51,43 @@ define(["backbone", "underscore", "codemirror", "js/views/feedback_notification" }, onSave: function(event) { - this.model.set('data', this.$codeMirror.getValue()); - var saving = new NotificationView.Mini({ - title: gettext('Saving…') - }); - saving.show(); - this.model.save({}, { - success: function() { - saving.hide(); - } - }); - this.render(); - this.$form.hide(); - this.closeEditor(); - - analytics.track('Saved Course Handouts', { - 'course': course_location_analytics - }); + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); + if ($('.CodeMirror-lines').find('.cm-error').length == 0){ + this.model.set('data', this.$codeMirror.getValue()); + var saving = new NotificationView.Mini({ + title: gettext('Saving…') + }); + saving.show(); + this.model.save({}, { + success: function() { + saving.hide(); + } + }); + this.render(); + this.$form.hide(); + this.closeEditor(); + analytics.track('Saved Course Handouts', { + 'course': course_location_analytics + }); + }else{ + $('#handout_error').addClass('is-shown'); + $('.save-button').addClass('is-disabled'); + event.preventDefault(); + } }, onCancel: function(event) { + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); this.$form.hide(); this.closeEditor(); }, closeEditor: function() { + $('#handout_error').removeClass('is-shown'); + $('.save-button').removeClass('is-disabled'); this.$form.hide(); ModalUtils.hideModalCover(); this.$form.find('.CodeMirror').remove(); diff --git a/cms/static/js/views/course_info_helper.js b/cms/static/js/views/course_info_helper.js index ec4a6ba550..fb3474cdb0 100644 --- a/cms/static/js/views/course_info_helper.js +++ b/cms/static/js/views/course_info_helper.js @@ -6,7 +6,10 @@ define(["codemirror", "utility"], var $codeMirror = CodeMirror.fromTextArea(textArea, { mode: "text/html", lineNumbers: true, - lineWrapping: true + lineWrapping: true, + onChange: function () { + $('.save-button').removeClass('is-disabled'); + } }); $codeMirror.setValue(content); $codeMirror.clearHistory(); diff --git a/cms/static/js/views/overview_assignment_grader.js b/cms/static/js/views/overview_assignment_grader.js index 40e9349693..b7b501f572 100644 --- a/cms/static/js/views/overview_assignment_grader.js +++ b/cms/static/js/views/overview_assignment_grader.js @@ -21,7 +21,7 @@ define(["backbone", "underscore", "gettext", "js/models/assignment_grade", "js/v '
  • Not Graded
    • ' + ''); this.assignmentGrade = new AssignmentGrade({ - assignmentUrl : this.$el.closest('.id-holder').data('id'), + locator : this.$el.closest('.id-holder').data('locator'), graderType : this.$el.data('initial-status')}); // TODO throw exception if graders is null this.graders = this.options['graders']; diff --git a/cms/static/js/views/settings/main.js b/cms/static/js/views/settings/main.js index ded2781f66..63776829c3 100644 --- a/cms/static/js/views/settings/main.js +++ b/cms/static/js/views/settings/main.js @@ -21,9 +21,9 @@ var DetailsView = ValidatingView.extend({ initialize : function() { this.fileAnchorTemplate = _.template(' <%= filename %>'); // fill in fields - this.$el.find("#course-name").val(this.model.get('location').get('name')); - this.$el.find("#course-organization").val(this.model.get('location').get('org')); - this.$el.find("#course-number").val(this.model.get('location').get('course')); + this.$el.find("#course-organization").val(this.model.get('org')); + this.$el.find("#course-number").val(this.model.get('course_id')); + this.$el.find("#course-name").val(this.model.get('run')); this.$el.find('.set-date').datepicker({ 'dateFormat': 'm/d/yy' }); // Avoid showing broken image on mistyped/nonexistent image diff --git a/cms/static/sass/contexts/_ie.scss b/cms/static/sass/contexts/_ie.scss index 4599ec3e68..ed9484f010 100644 --- a/cms/static/sass/contexts/_ie.scss +++ b/cms/static/sass/contexts/_ie.scss @@ -10,6 +10,10 @@ &.is-shown { bottom: 0; } + + &.is-hiding { + bottom: -($ui-notification-height); + } } } diff --git a/cms/static/sass/elements/_xmodules.scss b/cms/static/sass/elements/_xmodules.scss index 576fd9549b..8ffccfef2d 100644 --- a/cms/static/sass/elements/_xmodules.scss +++ b/cms/static/sass/elements/_xmodules.scss @@ -1,4 +1,15 @@ -// studio - elements - xmodules +// studio - elements - xmodules & xblocks +// ==================== + +// general - display mode (xblock-student_view or xmodule_display) +.xmodule_display, .xblock-student_view { + + // font styling + i, em { + font-style: italic; + } +} + // ==================== // Video Alpha diff --git a/cms/templates/asset_index.html b/cms/templates/asset_index.html index 5576664df7..4f6f14a466 100644 --- a/cms/templates/asset_index.html +++ b/cms/templates/asset_index.html @@ -187,7 +187,7 @@ require(["domReady", "jquery", "gettext", "js/models/asset", "js/collections/ass ${_('close')}

    ${_("Upload New File")}

    -

    +

    diff --git a/cms/templates/course_info.html b/cms/templates/course_info.html index 401d32ccb2..8cddb8a8c5 100644 --- a/cms/templates/course_info.html +++ b/cms/templates/course_info.html @@ -33,7 +33,6 @@ require(["domReady!", "jquery", "js/collections/course_update", "js/models/modul var editor = new CourseInfoEditView({ el: $('.main-wrapper'), model : new CourseInfoModel({ - courseId : '${context_course.location}', updates : course_updates, base_asset_url : '${base_asset_url}', handouts : course_handouts diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index 2dba71c279..70c3e61801 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -9,12 +9,15 @@ <%block name="jsextra"> - + But in this there should be
    Great ideas require offsetting. - + bad tests require drivel
    - + [code] Code should be nicely monospaced. [/code] @@ -322,7 +363,7 @@ describe 'MarkdownEditingDescriptor', -> expect(data).toEqual("""

    Not a header

    A header

    - +

    Multiple choice w/ parentheticals

    @@ -332,7 +373,7 @@ describe 'MarkdownEditingDescriptor', -> no space b4 close paren - +

    Choice checks

    @@ -343,25 +384,25 @@ describe 'MarkdownEditingDescriptor', -> no space - +

    Option with multiple correct ones

    - + - +

    Option with embedded parens

    - + - +

    What happens w/ empty correct options?

    - + - +

    Explanation

    @@ -379,14 +420,14 @@ describe 'MarkdownEditingDescriptor', -> console.log(two * 2); - +

    But in this there should be

    Great ideas require offsetting.

    - +

    bad tests require drivel

    - + 
    
         Code should be nicely monospaced.
    diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js index 8921e819e4..72b5e4e3b2 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js @@ -270,7 +270,7 @@ } }); - // Disabled 10/29/13 due to flakiness in master + // Disabled 11/25/13 due to flakiness in master xdescribe('multiple YT on page', function () { var state1, state2, state3; diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index 29f4567c8a..4e45d32838 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -456,7 +456,7 @@ expect(videoCaption.currentIndex).toEqual(5); }); - // Disabled 10/25/13 due to flakiness in master + // Disabled 11/25/13 due to flakiness in master xit('scroll caption to new position', function () { expect($.fn.scrollTo).toHaveBeenCalled(); }); @@ -537,7 +537,7 @@ }); }); - // Disabled 10/23/13 due to flakiness in master + // Disabled 11/25/13 due to flakiness in master xdescribe('scrollCaption', function () { beforeEach(function () { initialize(); @@ -682,7 +682,7 @@ .toHaveAttr('title', 'Turn off captions'); }); - // Test turned off due to flakiness (30.10.2013). + // Test turned off due to flakiness (11/25/13) xit('scroll the caption', function () { // After transcripts are shown, and the video plays for a // bit. diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js index 6f33d978fb..71cf4f45c4 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_focus_grabber_spec.js @@ -72,7 +72,17 @@ expect(state.focusGrabber.disableFocusGrabber).toHaveBeenCalled(); }); - it('after controls hide focus grabbers are enabled', function () { + // Disabled on 18.11.2013 due to flakiness on local dev machine. + // + // Video FocusGrabber: after controls hide focus grabbers are + // enabled [fail] + // Expected spy enableFocusGrabber to have been called. + // + // Approximately 1 in 8 times this test fails. + // + // TODO: Most likely, focusGrabber will be disabled in the future. This + // test could become unneeded in the future. + xit('after controls hide focus grabbers are enabled', function () { runs(function () { // Captions should not be "sticky" for the autohide mechanism // to work. diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js index 88da768f84..62e1475356 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js @@ -4,11 +4,21 @@ videoProgressSlider, videoSpeedControl, videoVolumeControl, oldOTBD; - function initialize(fixture) { - if (typeof fixture === 'undefined') { - loadFixtures('video_all.html'); - } else { + function initialize(fixture, params) { + if (_.isString(fixture)) { loadFixtures(fixture); + } else { + if (_.isObject(fixture)) { + params = fixture; + } + + loadFixtures('video_all.html'); + } + + if (_.isObject(params)) { + $('#example') + .find('#video_id') + .data(params); } state = new Video('#example'); @@ -532,8 +542,54 @@ }); }); - // Disabled 10/24/13 due to flakiness in master - xdescribe('updatePlayTime', function () { + describe('update with start & end time', function () { + var START_TIME = 1, END_TIME = 2; + + beforeEach(function () { + initialize({start: START_TIME, end: END_TIME}); + + spyOn(videoPlayer, 'update').andCallThrough(); + spyOn(videoPlayer, 'pause').andCallThrough(); + spyOn(videoProgressSlider, 'notifyThroughHandleEnd') + .andCallThrough(); + }); + + it('video is paused on first endTime, start & end time are reset', function () { + var checkForStartEndTimeSet = true; + + videoProgressSlider.notifyThroughHandleEnd.reset(); + videoPlayer.pause.reset(); + videoPlayer.play(); + + waitsFor(function () { + if ( + !isFinite(videoPlayer.currentTime) || + videoPlayer.currentTime <= 0 + ) { + return false; + } + + if (checkForStartEndTimeSet) { + checkForStartEndTimeSet = false; + + expect(videoPlayer.startTime).toBe(START_TIME); + expect(videoPlayer.endTime).toBe(END_TIME); + } + + return videoPlayer.pause.calls.length === 1 + }, 5000, 'pause() has been called'); + + runs(function () { + expect(videoPlayer.startTime).toBe(0); + expect(videoPlayer.endTime).toBe(null); + + expect(videoProgressSlider.notifyThroughHandleEnd) + .toHaveBeenCalledWith({end: true}); + }); + }); + }); + + describe('updatePlayTime', function () { beforeEach(function () { initialize(); @@ -548,7 +604,7 @@ duration = videoPlayer.duration(); if (duration > 0) { - return true; + return true; } return false; @@ -612,6 +668,74 @@ }); }); + describe('updatePlayTime when start & end times are defined', function () { + var START_TIME = 1, + END_TIME = 2; + + beforeEach(function () { + initialize({start: START_TIME, end: END_TIME}); + + spyOn(videoPlayer, 'updatePlayTime').andCallThrough(); + spyOn(videoPlayer.player, 'seekTo').andCallThrough(); + spyOn(videoProgressSlider, 'updateStartEndTimeRegion') + .andCallThrough(); + }); + + it('when duration becomes available, updatePlayTime() is called', function () { + var duration; + + expect(videoPlayer.initialSeekToStartTime).toBeTruthy(); + expect(videoPlayer.seekToStartTimeOldSpeed).toBe('void'); + + videoPlayer.play(); + + waitsFor(function () { + duration = videoPlayer.duration(); + + return duration > 0 && + videoPlayer.initialSeekToStartTime === false; + }, 'duration becomes available', 1000); + + runs(function () { + expect(videoPlayer.startTime).toBe(START_TIME); + expect(videoPlayer.endTime).toBe(END_TIME); + + expect(videoPlayer.player.seekTo).toHaveBeenCalledWith(START_TIME); + + expect(videoProgressSlider.updateStartEndTimeRegion) + .toHaveBeenCalledWith({duration: duration}); + + expect(videoPlayer.seekToStartTimeOldSpeed).toBe(state.speed); + }); + }); + }); + + describe('updatePlayTime with invalid endTime', function () { + beforeEach(function () { + initialize({end: 100000}); + + spyOn(videoPlayer, 'updatePlayTime').andCallThrough(); + }); + + it('invalid endTime is reset to null', function () { + var duration; + + videoPlayer.updatePlayTime.reset(); + videoPlayer.play(); + + waitsFor(function () { + duration = videoPlayer.duration(); + + return duration > 0 && + videoPlayer.initialSeekToStartTime === false; + }, 'updatePlayTime was invoked and duration is set', 5000); + + runs(function () { + expect(videoPlayer.endTime).toBe(null); + }); + }); + }); + describe('toggleFullScreen', function () { describe('when the video player is not full screen', function () { beforeEach(function () { diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js index 751506b410..a8450c39cd 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js @@ -153,7 +153,7 @@ }); }); - // Turned off test due to flakiness (30.10.2013). + // Turned off test due to flakiness (11/25/13) xit('trigger seek event', function() { runs(function () { videoProgressSlider.onSlide( @@ -219,7 +219,7 @@ }); }); - // Turned off test due to flakiness (30.10.2013). + // Turned off test due to flakiness (11/25/13) xit('trigger seek event', function() { runs(function () { videoProgressSlider.onStop( @@ -285,6 +285,55 @@ expect(params).toEqual(expectedParams); }); }); + + describe('notifyThroughHandleEnd', function () { + beforeEach(function () { + initialize(); + + spyOnEvent(videoProgressSlider.handle, 'focus'); + spyOn(videoProgressSlider, 'notifyThroughHandleEnd') + .andCallThrough(); + }); + + it('params.end = true', function () { + videoProgressSlider.notifyThroughHandleEnd({end: true}); + + expect(videoProgressSlider.handle.attr('title')) + .toBe('video ended'); + + expect('focus').toHaveBeenTriggeredOn(videoProgressSlider.handle); + }); + + it('params.end = false', function () { + videoProgressSlider.notifyThroughHandleEnd({end: false}); + + expect(videoProgressSlider.handle.attr('title')) + .toBe('video position'); + + expect('focus').not.toHaveBeenTriggeredOn(videoProgressSlider.handle); + }); + + it('is called when video plays', function () { + videoPlayer.play(); + + waitsFor(function () { + var duration = videoPlayer.duration(), + currentTime = videoPlayer.currentTime; + + return ( + isFinite(duration) && + duration > 0 && + isFinite(currentTime) && + currentTime > 0 + ); + }, 'duration is set, video is playing', 5000); + + runs(function () { + expect(videoProgressSlider.notifyThroughHandleEnd) + .toHaveBeenCalledWith({end: false}); + }); + }); + }); }); }).call(this); diff --git a/common/lib/xmodule/xmodule/js/src/collapsible.coffee b/common/lib/xmodule/xmodule/js/src/collapsible.coffee index 2f6b2c9781..837231c482 100644 --- a/common/lib/xmodule/xmodule/js/src/collapsible.coffee +++ b/common/lib/xmodule/xmodule/js/src/collapsible.coffee @@ -1,7 +1,7 @@ class @Collapsible # Set of library functions that provide a simple way to add collapsible - # functionality to elements. + # functionality to elements. # setCollapsibles: # Scan element's content for generic collapsible containers @@ -9,12 +9,15 @@ class @Collapsible ### el: container ### + linkTop = 'See full output' + linkBottom = 'See full output' + # standard longform + shortfom pattern el.find('.longform').hide() - el.find('.shortform').append('See full output') + el.find('.shortform').append(linkTop, linkBottom) # custom longform + shortform text pattern - short_custom = el.find('.shortform-custom') + short_custom = el.find('.shortform-custom') # set up each one individually short_custom.each (index, elt) => open_text = $(elt).data('open-text') @@ -31,13 +34,18 @@ class @Collapsible @toggleFull: (event, open_text, close_text) => event.preventDefault() - $(event.target).parent().siblings().slideToggle() - $(event.target).parent().parent().toggleClass('open') + parent = $(event.target).parent() + parent.siblings().slideToggle() + parent.parent().toggleClass('open') if $(event.target).text() == open_text new_text = close_text else new_text = open_text - $(event.target).text(new_text) + if $(event.target).hasClass('full') + el = parent.find('.full') + else + el = $(event.target) + el.text(new_text) @toggleHint: (event) => event.preventDefault() diff --git a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee index ba85db574b..9f5d361009 100644 --- a/common/lib/xmodule/xmodule/js/src/problem/edit.coffee +++ b/common/lib/xmodule/xmodule/js/src/problem/edit.coffee @@ -228,11 +228,13 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor }); // replace string and numerical - xml = xml.replace(/^\=\s*(.*?$)/gm, function(match, p) { - var string; - var floatValue = parseFloat(p); + xml = xml.replace(/(^\=\s*(.*?$)(\n*or\=\s*(.*?$))*)+/gm, function(match, p) { + var string, + answersList = p.replace(/^(or)?=\s*/gm, '').split('\n'), + floatValue = parseFloat(answersList[0]); + if(!isNaN(floatValue)) { - var params = /(.*?)\+\-\s*(.*?$)/.exec(p); + var params = /(.*?)\+\-\s*(.*?$)/.exec(answersList[0]); if(params) { string = '\n'; string += ' \n'; @@ -242,10 +244,16 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor string += ' \n'; string += '\n\n'; } else { - string = '\n \n\n\n'; + var answers = []; + + for(var i = 0; i < answersList.length; i++) { + answers.push(answersList[i]) + } + + string = '\n \n\n\n'; } return string; - }); + }); // replace selects xml = xml.replace(/\[\[(.+?)\]\]/g, function(match, p) { @@ -262,13 +270,13 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor selectString += '\n\n'; return selectString; }); - + // replace explanations xml = xml.replace(/\[explanation\]\n?([^\]]*)\[\/?explanation\]/gmi, function(match, p1) { var selectString = '\n
    \nExplanation\n\n' + p1 + '\n
    \n    '; return selectString; }); - + // replace code blocks xml = xml.replace(/\[code\]\n?([^\]]*)\[\/?code\]/gmi, function(match, p1) { var selectString = '
    \n' + p1 + '
    '; @@ -293,7 +301,7 @@ class @MarkdownEditingDescriptor extends XModule.Descriptor // rid white space xml = xml.replace(/\n\n\n/g, '\n'); - + // surround w/ problem tag xml = '\n' + xml + '\n'; diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js index c5dab0af85..50eff093ee 100644 --- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js @@ -3,7 +3,7 @@ // VideoPlayer module. define( 'video/03_video_player.js', -['video/02_html5_video.js', 'video/00_resizer.js' ], +['video/02_html5_video.js', 'video/00_resizer.js'], function (HTML5Video, Resizer) { var dfd = $.Deferred(); @@ -83,11 +83,9 @@ function (HTML5Video, Resizer) { state.videoPlayer.initialSeekToStartTime = true; - state.videoPlayer.oneTimePauseAtEndTime = true; - - // The initial value of the variable `seekToStartTimeOldSpeed` - // should always differ from the value returned by the duration - // function. + // At the start, the initial value of the variable + // `seekToStartTimeOldSpeed` should always differ from the value + // returned by the duration function. state.videoPlayer.seekToStartTimeOldSpeed = 'void'; state.videoPlayer.playerVars = { @@ -215,8 +213,7 @@ function (HTML5Video, Resizer) { // This function gets the video's current play position in time // (currentTime) and its duration. - // It is called at a regular interval when the video is playing (see - // below). + // It is called at a regular interval when the video is playing. function update() { this.videoPlayer.currentTime = this.videoPlayer.player .getCurrentTime(); @@ -224,22 +221,28 @@ function (HTML5Video, Resizer) { if (isFinite(this.videoPlayer.currentTime)) { this.videoPlayer.updatePlayTime(this.videoPlayer.currentTime); - // We need to pause the video is current time is smaller (or equal) - // than end time. Also, we must make sure that the end time is the - // one that was set in the configuration parameter. If it differs, - // this means that it was either reset to the end, or the duration - // changed it's value. + // We need to pause the video if current time is smaller (or equal) + // than end time. Also, we must make sure that this is only done + // once. // - // In the case of YouTube Flash mode, we must remember that the - // start and end times are rescaled based on the current speed of - // the video. + // If `endTime` is not `null`, then we are safe to pause the + // video. `endTime` will be set to `null`, and this if statement + // will not be executed on next runs. if ( - this.videoPlayer.endTime <= this.videoPlayer.currentTime && - this.videoPlayer.oneTimePauseAtEndTime + this.videoPlayer.endTime != null && + this.videoPlayer.endTime <= this.videoPlayer.currentTime ) { - this.videoPlayer.oneTimePauseAtEndTime = false; this.videoPlayer.pause(); - this.videoPlayer.endTime = this.videoPlayer.duration(); + + // After the first time the video reached the `endTime`, + // `startTime` and `endTime` are disabled. The video will play + // from start to the end on subsequent runs. + this.videoPlayer.startTime = 0; + this.videoPlayer.endTime = null; + + this.trigger('videoProgressSlider.notifyThroughHandleEnd', { + end: true + }); } } } @@ -321,8 +324,10 @@ function (HTML5Video, Resizer) { } ); + // After the user seeks, startTime and endTime are disabled. The video + // will play from start to the end on subsequent runs. this.videoPlayer.startTime = 0; - this.videoPlayer.endTime = duration; + this.videoPlayer.endTime = null; this.videoPlayer.player.seekTo(newTime, true); @@ -344,11 +349,21 @@ function (HTML5Video, Resizer) { var time = this.videoPlayer.duration(); this.trigger('videoControl.pause', null); + this.trigger('videoProgressSlider.notifyThroughHandleEnd', { + end: true + }); if (this.config.show_captions) { this.trigger('videoCaption.pause', null); } + // When only `startTime` is set, the video will play to the end + // starting at `startTime`. After the first time the video reaches the + // end, `startTime` and `endTime` are disabled. The video will play + // from start to the end on subsequent runs. + this.videoPlayer.startTime = 0; + this.videoPlayer.endTime = null; + // Sometimes `onEnded` events fires when `currentTime` not equal // `duration`. In this case, slider doesn't reach the end point of // timeline. @@ -391,6 +406,10 @@ function (HTML5Video, Resizer) { this.trigger('videoControl.play', null); + this.trigger('videoProgressSlider.notifyThroughHandleEnd', { + end: false + }); + if (this.config.show_captions) { this.trigger('videoCaption.play', null); } @@ -531,7 +550,7 @@ function (HTML5Video, Resizer) { function updatePlayTime(time) { var duration = this.videoPlayer.duration(), - durationChange; + durationChange, tempStartTime, tempEndTime; if ( duration > 0 && @@ -545,13 +564,23 @@ function (HTML5Video, Resizer) { this.videoPlayer.initialSeekToStartTime === false ) { durationChange = true; - } else { + } else { // this.videoPlayer.initialSeekToStartTime === true + this.videoPlayer.initialSeekToStartTime = false; + durationChange = false; } - this.videoPlayer.initialSeekToStartTime = false; this.videoPlayer.seekToStartTimeOldSpeed = this.speed; + // Current startTime and endTime could have already been reset. + // We will remember their current values, and reset them at the + // end. We need to perform the below calculations on start and end + // times so that the range on the slider gets correctly updated in + // the case of speed change in Flash player mode (for YouTube + // videos). + tempStartTime = this.videoPlayer.startTime; + tempEndTime = this.videoPlayer.endTime; + // We retrieve the original times. They could have been changed due // to the fact of speed change (duration change). This happens when // in YouTube Flash mode. There each speed is a different video, @@ -566,31 +595,33 @@ function (HTML5Video, Resizer) { this.videoPlayer.startTime /= Number(this.speed); } } + + // An `endTime` of `null` means that either the user didn't set + // and `endTime`, or it was set to a value greater than the + // duration of the video. + // + // If `endTime` is `null`, the video will play to the end. We do + // not set the `endTime` to the duration of the video because + // sometimes in YouTube mode the duration changes slightly during + // the course of playback. This would cause the video to pause just + // before the actual end of the video. if ( - this.videoPlayer.endTime === null || + this.videoPlayer.endTime !== null && this.videoPlayer.endTime > duration ) { - this.videoPlayer.endTime = duration; - } else { + this.videoPlayer.endTime = null; + } else if (this.videoPlayer.endTime !== null) { if (this.currentPlayerMode === 'flash') { this.videoPlayer.endTime /= Number(this.speed); } } - // If this is not a duration change (if it is, we continue playing - // from current time), then we need to seek the video to the start - // time. - // - // We seek only if start time differs from zero. - if (durationChange === false && this.videoPlayer.startTime > 0) { - this.videoPlayer.player.seekTo(this.videoPlayer.startTime); - } - // Rebuild the slider start-end range (if it doesn't take up the - // whole slider). + // whole slider). Remember that endTime === null means the end time + // is set to the end of video by default. if (!( this.videoPlayer.startTime === 0 && - this.videoPlayer.endTime === duration + this.videoPlayer.endTime === null )) { this.trigger( 'videoProgressSlider.updateStartEndTimeRegion', @@ -599,6 +630,28 @@ function (HTML5Video, Resizer) { } ); } + + // If this is not a duration change (if it is, we continue playing + // from current time), then we need to seek the video to the start + // time. + // + // We seek only if start time differs from zero, and we haven't + // performed already such a seek. + if ( + durationChange === false && + this.videoPlayer.startTime > 0 && + !(tempStartTime === 0 && tempEndTime === null) + ) { + this.videoPlayer.player.seekTo(this.videoPlayer.startTime); + } + + // Reset back the actual startTime and endTime if they have been + // already reset (a seek event happened, the video already ended + // once, or endTime has already been reached once). + if (tempStartTime === 0 && tempEndTime === null) { + this.videoPlayer.startTime = 0; + this.videoPlayer.endTime = null; + } } this.trigger( diff --git a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js index 59750a0f11..e10c4fbb24 100644 --- a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js +++ b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js @@ -41,7 +41,8 @@ function () { onSlide: onSlide, onStop: onStop, updatePlayTime: updatePlayTime, - updateStartEndTimeRegion: updateStartEndTimeRegion + updateStartEndTimeRegion: updateStartEndTimeRegion, + notifyThroughHandleEnd: notifyThroughHandleEnd }; state.bindTo(methodsDict, state.videoProgressSlider, state); @@ -111,11 +112,6 @@ function () { duration = params.duration; } - // If the range spans the entire length of video, we don't do anything. - if (!this.videoPlayer.startTime && !this.videoPlayer.endTime) { - return; - } - start = this.videoPlayer.startTime; // If end is set to null, then we set it to the end of the video. We @@ -199,8 +195,6 @@ function () { }, 200); } - // Changed for tests -- JM: Check if it is the cause of Chrome Bug Valera - // noticed function updatePlayTime(params) { var time = Math.floor(params.time), duration = Math.floor(params.duration); @@ -215,6 +209,33 @@ function () { } } + // When the video stops playing (either because the end was reached, or + // because endTime was reached), the screen reader must be notified that + // the video is no longer playing. We do this by a little trick. Setting + // the title attribute of the slider know to "video ended", and focusing + // on it. The screen reader will read the attr text. + // + // The user can then tab his way forward, landing on the next control + // element, the Play button. + // + // @param params - object with property `end`. If set to true, the + // function must set the title attribute to + // `video ended`; + // if set to false, the function must reset the attr to + // it's original state. + // + // This function will be triggered from VideoPlayer methods onEnded(), + // onPlay(), and update() (update method handles endTime). + function notifyThroughHandleEnd(params) { + if (params.end) { + this.videoProgressSlider.handle + .attr('title', 'video ended') + .focus(); + } else { + this.videoProgressSlider.handle.attr('title', 'video position'); + } + } + // Returns a string describing the current time of video in hh:mm:ss // format. function getTimeDescription(time) { diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 33b65b8b10..c7355c2f29 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -204,21 +204,16 @@ class LocMapperStore(object): self._decode_from_mongo(old_name), None) elif usage_id == locator.usage_id: - # figure out revision - # enforce the draft only if category in [..] logic - if category in draft.DIRECT_ONLY_CATEGORIES: - revision = None - elif locator.branch == candidate['draft_branch']: - revision = draft.DRAFT - else: - revision = None + # Always return revision=None because the + # old draft module store wraps locations as draft before + # trying to access things. return Location( 'i4x', candidate['_id']['org'], candidate['_id']['course'], category, self._decode_from_mongo(old_name), - revision) + None) return None def add_block_location_translator(self, location, old_course_id=None, usage_id=None): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f368032f17..2f6fabfce0 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -778,11 +778,7 @@ class MongoModuleStore(ModuleStoreWriteBase): children: A list of child item identifiers """ - # We expect the children IDs to always be the non-draft version. With view refactoring - # for split, we are now passing the draft version in some cases. - children_ids = [Location(child).replace(revision=None).url() for child in children] - - self._update_single_item(location, {'definition.children': children_ids}) + self._update_single_item(location, {'definition.children': children}) # recompute (and update) the metadata inheritance tree which is cached self.refresh_cached_metadata_inheritance_tree(Location(location)) # fire signal that we've written to DB diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 6318c5e68f..f7a0250155 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -81,7 +81,7 @@ class DraftModuleStore(MongoModuleStore): try: return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth)) except ItemNotFoundError: - return wrap_draft(super(DraftModuleStore, self).get_item(as_published(location), depth=depth)) + return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth)) def get_instance(self, course_id, location, depth=0): """ @@ -169,7 +169,7 @@ class DraftModuleStore(MongoModuleStore): try: draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.convert_to_draft(as_published(location)) + self.convert_to_draft(location) except ItemNotFoundError, e: if not allow_not_found: raise e @@ -187,7 +187,7 @@ class DraftModuleStore(MongoModuleStore): draft_loc = as_draft(location) draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.convert_to_draft(as_published(location)) + self.convert_to_draft(location) return super(DraftModuleStore, self).update_children(draft_loc, children) @@ -203,7 +203,7 @@ class DraftModuleStore(MongoModuleStore): draft_item = self.get_item(location) if not getattr(draft_item, 'is_draft', False): - self.convert_to_draft(as_published(location)) + self.convert_to_draft(location) if 'is_draft' in metadata: del metadata['is_draft'] @@ -262,7 +262,7 @@ class DraftModuleStore(MongoModuleStore): """ Turn the published version into a draft, removing the published version """ - self.convert_to_draft(as_published(location)) + self.convert_to_draft(location) super(DraftModuleStore, self).delete_item(location) def _query_children_for_cache_children(self, items): diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 27a758c083..355f5bfa62 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -88,7 +88,7 @@ class SplitMigrator(object): index_info = self.split_modulestore.get_course_index_info(course_version_locator) versions = index_info['versions'] versions['draft'] = versions['published'] - self.split_modulestore.update_course_index(course_version_locator, {'versions': versions}, update_versions=True) + self.split_modulestore.update_course_index(index_info) # clean up orphans in published version: in old mongo, parents pointed to the union of their published and draft # children which meant some pointers were to non-existent locations in 'direct' diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py index 9b2a652a95..ded67104b4 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py @@ -22,5 +22,4 @@ class DefinitionLazyLoader(object): Fetch the definition. Note, the caller should replace this lazy loader pointer with the result so as not to fetch more than once """ - return self.modulestore.definitions.find_one( - {'_id': self.definition_locator.definition_id}) + return self.modulestore.db_connection.get_definition(self.definition_locator.definition_id) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py new file mode 100644 index 0000000000..510c100048 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/mongo_connection.py @@ -0,0 +1,116 @@ +""" +Segregation of pymongo functions from the data modeling mechanisms for split modulestore. +""" +import pymongo + +class MongoConnection(object): + """ + Segregation of pymongo functions from the data modeling mechanisms for split modulestore. + """ + def __init__( + self, db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs + ): + """ + Create & open the connection, authenticate, and provide pointers to the collections + """ + self.database = pymongo.database.Database( + pymongo.MongoClient( + host=host, + port=port, + tz_aware=tz_aware, + **kwargs + ), + db + ) + + if user is not None and password is not None: + self.database.authenticate(user, password) + + self.course_index = self.database[collection + '.active_versions'] + self.structures = self.database[collection + '.structures'] + self.definitions = self.database[collection + '.definitions'] + + # every app has write access to the db (v having a flag to indicate r/o v write) + # Force mongo to report errors, at the expense of performance + # pymongo docs suck but explanation: + # http://api.mongodb.org/java/2.10.1/com/mongodb/WriteConcern.html + self.course_index.write_concern = {'w': 1} + self.structures.write_concern = {'w': 1} + self.definitions.write_concern = {'w': 1} + + def get_structure(self, key): + """ + Get the structure from the persistence mechanism whose id is the given key + """ + return self.structures.find_one({'_id': key}) + + def find_matching_structures(self, query): + """ + Find the structure matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.structures.find(query) + + def insert_structure(self, structure): + """ + Create the structure in the db + """ + self.structures.insert(structure) + + def update_structure(self, structure): + """ + Update the db record for structure + """ + self.structures.update({'_id': structure['_id']}, structure) + + def get_course_index(self, key): + """ + Get the course_index from the persistence mechanism whose id is the given key + """ + return self.course_index.find_one({'_id': key}) + + def find_matching_course_indexes(self, query): + """ + Find the course_index matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.course_index.find(query) + + def insert_course_index(self, course_index): + """ + Create the course_index in the db + """ + self.course_index.insert(course_index) + + def update_course_index(self, course_index): + """ + Update the db record for course_index + """ + self.course_index.update({'_id': course_index['_id']}, course_index) + + def delete_course_index(self, key): + """ + Delete the course_index from the persistence mechanism whose id is the given key + """ + return self.course_index.remove({'_id': key}) + + def get_definition(self, key): + """ + Get the definition from the persistence mechanism whose id is the given key + """ + return self.definitions.find_one({'_id': key}) + + def find_matching_definitions(self, query): + """ + Find the definitions matching the query. Right now the query must be a legal mongo query + :param query: a mongo-style query of {key: [value|{$in ..}|..], ..} + """ + return self.definitions.find(query) + + def insert_definition(self, definition): + """ + Create the definition in the db + """ + self.definitions.insert(definition) + + diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 6152416596..a6349e6113 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -1,7 +1,6 @@ import threading import datetime import logging -import pymongo import re from importlib import import_module from path import path @@ -21,6 +20,7 @@ from .caching_descriptor_system import CachingDescriptorSystem from xblock.fields import Scope from xblock.runtime import Mixologist from bson.objectid import ObjectId +from xmodule.modulestore.split_mongo.mongo_connection import MongoConnection log = logging.getLogger(__name__) #============================================================================== @@ -49,7 +49,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): A Mongodb backed ModuleStore supporting versions, inheritance, and sharing. """ - # pylint: disable=W0201 def __init__(self, doc_store_config, fs_root, render_template, default_class=None, error_tracker=null_error_tracker, @@ -62,44 +61,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): super(SplitMongoModuleStore, self).__init__(**kwargs) self.loc_mapper = loc_mapper - def do_connection( - db, collection, host, port=27017, tz_aware=True, user=None, password=None, **kwargs - ): - """ - Create & open the connection, authenticate, and provide pointers to the collections - """ - self.db = pymongo.database.Database( - pymongo.MongoClient( - host=host, - port=port, - tz_aware=tz_aware, - **kwargs - ), - db - ) - - if user is not None and password is not None: - self.db.authenticate(user, password) - - self.course_index = self.db[collection + '.active_versions'] - self.structures = self.db[collection + '.structures'] - self.definitions = self.db[collection + '.definitions'] - - do_connection(**doc_store_config) + self.db_connection = MongoConnection(**doc_store_config) + self.db = self.db_connection.database # Code review question: How should I expire entries? # _add_cache could use a lru mechanism to control the cache size? self.thread_cache = threading.local() - - # every app has write access to the db (v having a flag to indicate r/o v write) - # Force mongo to report errors, at the expense of performance - # pymongo docs suck but explanation: - # http://api.mongodb.org/java/2.10.1/com/mongodb/WriteConcern.html - self.course_index.write_concern = {'w': 1} - self.structures.write_concern = {'w': 1} - self.definitions.write_concern = {'w': 1} - if default_class is not None: module_path, _, class_name = default_class.rpartition('.') class_ = getattr(import_module(module_path), class_name) @@ -138,7 +106,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): block['definition'] = DefinitionLazyLoader(self, block['definition']) else: # Load all descendants by id - descendent_definitions = self.definitions.find({ + descendent_definitions = self.db_connection.find_matching_definitions({ '_id': {'$in': [block['definition'] for block in new_module_data.itervalues()]}}) # turn into a map @@ -226,7 +194,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if course_locator.course_id is not None and course_locator.branch is not None: # use the course_id - index = self.course_index.find_one({'_id': course_locator.course_id}) + index = self.db_connection.get_course_index(course_locator.course_id) if index is None: raise ItemNotFoundError(course_locator) if course_locator.branch not in index['versions']: @@ -241,7 +209,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # cast string to ObjectId if necessary version_guid = course_locator.as_object_id(version_guid) - entry = self.structures.find_one({'_id': version_guid}) + entry = self.db_connection.get_structure(version_guid) # b/c more than one course can use same structure, the 'course_id' and 'branch' are not intrinsic to structure # and the one assoc'd w/ it by another fetch may not be the one relevant to this fetch; so, @@ -269,7 +237,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if qualifiers is None: qualifiers = {} qualifiers.update({"versions.{}".format(branch): {"$exists": True}}) - matching = self.course_index.find(qualifiers) + matching = self.db_connection.find_matching_course_indexes(qualifiers) # collect ids and then query for those version_guids = [] @@ -279,7 +247,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): version_guids.append(version_guid) id_version_map[version_guid] = structure['_id'] - course_entries = self.structures.find({'_id': {'$in': version_guids}}) + course_entries = self.db_connection.find_matching_structures({'_id': {'$in': version_guids}}) # get the block for the course element (s/b the root) result = [] @@ -455,7 +423,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): """ if course_locator.course_id is None: return None - index = self.course_index.find_one({'_id': course_locator.course_id}) + index = self.db_connection.get_course_index(course_locator.course_id) return index # TODO figure out a way to make this info accessible from the course descriptor @@ -487,7 +455,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'edited_on': when the change was made } """ - definition = self.definitions.find_one({'_id': definition_locator.definition_id}) + definition = self.db_connection.get_definition(definition_locator.definition_id) if definition is None: return None return definition['edit_info'] @@ -509,14 +477,14 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # TODO if depth is significant, it may make sense to get all that have the same original_version # and reconstruct the subtree from version_guid - next_entries = self.structures.find({'previous_version' : version_guid}) + next_entries = self.db_connection.find_matching_structures({'previous_version' : version_guid}) # must only scan cursor's once next_versions = [struct for struct in next_entries] result = {version_guid: [CourseLocator(version_guid=struct['_id']) for struct in next_versions]} depth = 1 while depth < version_history_depth and len(next_versions) > 0: depth += 1 - next_entries = self.structures.find({'previous_version': + next_entries = self.db_connection.find_matching_structures({'previous_version': {'$in': [struct['_id'] for struct in next_versions]}}) next_versions = [struct for struct in next_entries] for course_structure in next_versions: @@ -537,7 +505,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): course_struct = self._lookup_course(block_locator.version_agnostic())['structure'] usage_id = block_locator.usage_id update_version_field = 'blocks.{}.edit_info.update_version'.format(usage_id) - all_versions_with_block = self.structures.find({'original_version': course_struct['original_version'], + all_versions_with_block = self.db_connection.find_matching_structures({'original_version': course_struct['original_version'], update_version_field: {'$exists': True}}) # find (all) root versions and build map previous: [successors] possible_roots = [] @@ -596,7 +564,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): "original_version": new_id, } } - self.definitions.insert(document) + self.db_connection.insert_definition(document) definition_locator = DefinitionLocator(new_id) return definition_locator @@ -618,7 +586,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # if this looks in cache rather than fresh fetches, then it will probably not detect # actual change b/c the descriptor and cache probably point to the same objects - old_definition = self.definitions.find_one({'_id': definition_locator.definition_id}) + old_definition = self.db_connection.get_definition(definition_locator.definition_id) if old_definition is None: raise ItemNotFoundError(definition_locator.url()) @@ -630,7 +598,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): old_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) # previous version id old_definition['edit_info']['previous_version'] = definition_locator.definition_id - self.definitions.insert(old_definition) + self.db_connection.insert_definition(old_definition) return DefinitionLocator(old_definition['_id']), True else: return definition_locator, False @@ -657,7 +625,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param course_blocks: the current list of blocks. :param category: """ - existing_uses = self.course_index.find({"_id": {"$regex": id_root}}) + existing_uses = self.db_connection.find_matching_course_indexes({"_id": {"$regex": id_root}}) if existing_uses.count() > 0: max_found = 0 matcher = re.compile(id_root + r'(\d+)') @@ -779,11 +747,11 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): parent['edit_info']['update_version'] = new_id if continue_version: # db update - self.structures.update({'_id': new_id}, new_structure) + self.db_connection.update_structure(new_structure) # clear cache so things get refetched and inheritance recomputed self._clear_cache(new_id) else: - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: @@ -856,7 +824,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'original_version': definition_id, } } - self.definitions.insert(definition_entry) + self.db_connection.insert_definition(definition_entry) new_id = ObjectId() draft_structure = { @@ -880,7 +848,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): } } } - self.structures.insert(draft_structure) + self.db_connection.insert_structure(draft_structure) if versions_dict is None: versions_dict = {master_branch: new_id} @@ -898,20 +866,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if block_fields is not None: root_block['fields'].update(block_fields) if definition_fields is not None: - definition = self.definitions.find_one({'_id': root_block['definition']}) + definition = self.db_connection.get_definition(root_block['definition']) definition['fields'].update(definition_fields) definition['edit_info']['previous_version'] = definition['_id'] definition['edit_info']['edited_by'] = user_id definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) definition['_id'] = ObjectId() - self.definitions.insert(definition) + self.db_connection.insert_definition(definition) root_block['definition'] = definition['_id'] root_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) root_block['edit_info']['edited_by'] = user_id root_block['edit_info']['previous_version'] = root_block['edit_info'].get('update_version') root_block['edit_info']['update_version'] = new_id - self.structures.insert(draft_structure) + self.db_connection.insert_structure(draft_structure) versions_dict[master_branch] = new_id # create the index entry @@ -926,7 +894,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), 'versions': versions_dict} - self.course_index.insert(index_entry) + self.db_connection.insert_course_index(index_entry) return self.get_course(CourseLocator(course_id=new_id, branch=master_branch)) def update_item(self, descriptor, user_id, force=False): @@ -978,7 +946,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'previous_version': block_data['edit_info']['update_version'], 'update_version': new_id, } - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: self._update_head(index_entry, descriptor.location.branch, new_id) @@ -1016,7 +984,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): is_updated = self._persist_subdag(xblock, user_id, new_structure['blocks'], new_id) if is_updated: - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) # update the index entry if appropriate if index_entry is not None: @@ -1115,31 +1083,18 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): '''Deprecated, use update_item.''' raise NotImplementedError('use update_item') - def update_course_index(self, course_locator, new_values_dict, update_versions=False): + def update_course_index(self, updated_index_entry): """ - Change the given course's index entry for the given fields. new_values_dict - should be a subset of the dict returned by get_course_index_info. - It cannot include '_id' (will raise IllegalArgument). - Provide update_versions=True if you intend this to replace the versions hash. + Change the given course's index entry. + Note, this operation can be dangerous and break running courses. - If the dict includes versions and not update_versions, it will raise an exception. - - If the dict includes edited_on or edited_by, it will raise an exception - Does not return anything useful. """ # TODO how should this log the change? edited_on and edited_by for this entry # has the semantic of who created the course and when; so, changing those will lose # that information. - if '_id' in new_values_dict: - raise ValueError("Cannot override _id") - if 'edited_on' in new_values_dict or 'edited_by' in new_values_dict: - raise ValueError("Cannot set edited_on or edited_by") - if not update_versions and 'versions' in new_values_dict: - raise ValueError("Cannot override versions without setting update_versions") - self.course_index.update({'_id': course_locator.course_id}, - {'$set': new_values_dict}) + self.db_connection.update_course_index(updated_index_entry) def delete_item(self, usage_locator, user_id, delete_children=False, force=False): """ @@ -1182,7 +1137,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): remove_subtree(usage_locator.usage_id) # update index if appropriate and structures - self.structures.insert(new_structure) + self.db_connection.insert_structure(new_structure) result = CourseLocator(version_guid=new_id) @@ -1204,11 +1159,11 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param course_id: uses course_id rather than locator to emphasize its global effect """ - index = self.course_index.find_one({'_id': course_id}) + index = self.db_connection.get_course_index(course_id) if index is None: raise ItemNotFoundError(course_id) # this is the only real delete in the system. should it do something else? - self.course_index.remove(index['_id']) + self.db_connection.delete_course_index(index['_id']) def get_errored_courses(self): """ @@ -1296,7 +1251,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): block['fields']["children"] = [ usage_id for usage_id in block['fields']["children"] if usage_id in original_structure['blocks'] ] - self.structures.update({'_id': original_structure['_id']}, original_structure) + self.db_connection.update_structure(original_structure) # clear cache again b/c inheritance may be wrong over orphans self._clear_cache(original_structure['_id']) @@ -1379,7 +1334,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): else: return None else: - index_entry = self.course_index.find_one({'_id': locator.course_id}) + index_entry = self.db_connection.get_course_index(locator.course_id) is_head = ( locator.version_guid is None or index_entry['versions'][locator.branch] == locator.version_guid @@ -1424,9 +1379,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): :param course_locator: :param new_id: """ - self.course_index.update( - {"_id": index_entry["_id"]}, - {"$set": {"versions.{}".format(branch): new_id}}) + index_entry['versions'][branch] = new_id + self.db_connection.update_course_index(index_entry) def _partition_fields_by_scope(self, category, fields): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 2a1c165993..2fbff10423 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -274,7 +274,9 @@ class TestLocationMapper(unittest.TestCase): course_id=prob_locator.course_id, branch='draft', usage_id=prob_locator.usage_id ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) - self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', 'draft')) + # Even though the problem was set as draft, we always return revision=None to work + # with old mongo/draft modulestores. + self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) prob_locator = BlockUsageLocator( course_id=new_style_course_id, usage_id='problem2', branch='production' ) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py index 049fbd2ef8..74975f2896 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_migrator.py @@ -59,9 +59,9 @@ class TestMigration(unittest.TestCase): dbref = self.loc_mapper.db dbref.drop_collection(self.loc_mapper.location_map) split_db = self.split_mongo.db - split_db.drop_collection(split_db.course_index) - split_db.drop_collection(split_db.structures) - split_db.drop_collection(split_db.definitions) + split_db.drop_collection(self.split_mongo.db_connection.course_index) + split_db.drop_collection(self.split_mongo.db_connection.structures) + split_db.drop_collection(self.split_mongo.db_connection.definitions) # old_mongo doesn't give a db attr, but all of the dbs are the same dbref.drop_collection(self.old_mongo.collection) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index aa095d8e4c..92de35e39f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -1018,41 +1018,29 @@ class TestCourseCreation(SplitModuleTest): Test changing the org, pretty id, etc of a course. Test that it doesn't allow changing the id, etc. """ locator = CourseLocator(course_id="GreekHero", branch='draft') - modulestore().update_course_index(locator, {'org': 'funkyU'}) + course_info = modulestore().get_course_index_info(locator) + course_info['org'] = 'funkyU' + modulestore().update_course_index(course_info) course_info = modulestore().get_course_index_info(locator) self.assertEqual(course_info['org'], 'funkyU') - modulestore().update_course_index(locator, {'org': 'moreFunky', 'prettyid': 'Ancient Greek Demagods'}) + course_info['org'] = 'moreFunky' + course_info['prettyid'] = 'Ancient Greek Demagods' + modulestore().update_course_index(course_info) course_info = modulestore().get_course_index_info(locator) self.assertEqual(course_info['org'], 'moreFunky') self.assertEqual(course_info['prettyid'], 'Ancient Greek Demagods') - self.assertRaises(ValueError, modulestore().update_course_index, locator, {'_id': 'funkygreeks'}) - - with self.assertRaises(ValueError): - modulestore().update_course_index( - locator, - {'edited_on': datetime.datetime.now(UTC)} - ) - with self.assertRaises(ValueError): - modulestore().update_course_index( - locator, - {'edited_by': 'sneak'} - ) - - self.assertRaises(ValueError, modulestore().update_course_index, locator, - {'versions': {'draft': self.GUID_D1}}) - # an allowed but not necessarily recommended way to revert the draft version versions = course_info['versions'] versions['draft'] = self.GUID_D1 - modulestore().update_course_index(locator, {'versions': versions}, update_versions=True) + modulestore().update_course_index(course_info) course = modulestore().get_course(locator) self.assertEqual(str(course.location.version_guid), self.GUID_D1) # an allowed but not recommended way to publish a course versions['published'] = self.GUID_D1 - modulestore().update_course_index(locator, {'versions': versions}, update_versions=True) + modulestore().update_course_index(course_info) course = modulestore().get_course(CourseLocator(course_id=locator.course_id, branch="published")) self.assertEqual(str(course.location.version_guid), self.GUID_D1) @@ -1068,9 +1056,9 @@ class TestCourseCreation(SplitModuleTest): self.assertEqual(new_course.location.usage_id, 'top') self.assertEqual(new_course.category, 'chapter') # look at db to verify - db_structure = modulestore().structures.find_one({ - '_id': new_course.location.as_object_id(new_course.location.version_guid) - }) + db_structure = modulestore().db_connection.get_structure( + new_course.location.as_object_id(new_course.location.version_guid) + ) self.assertIsNotNone(db_structure, "Didn't find course") self.assertNotIn('course', db_structure['blocks']) self.assertIn('top', db_structure['blocks']) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 5512af2143..9d055356fc 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -79,7 +79,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # tags that really need unique names--they store (or should store) state. need_uniq_names = ('problem', 'sequential', 'video', 'course', 'chapter', - 'videosequence', 'poll_question', 'timelimit') + 'videosequence', 'poll_question', 'vertical') attr = xml_data.attrib tag = xml_data.tag diff --git a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py index 6e33a54323..2b5efcb87f 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_exporter.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_exporter.py @@ -97,16 +97,15 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d if len(draft_verticals) > 0: draft_course_dir = export_fs.makeopendir('drafts') for draft_vertical in draft_verticals: - if getattr(draft_vertical, 'is_draft', False): - parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id) - # Don't try to export orphaned items. - if len(parent_locs) > 0: - logging.debug('parent_locs = {0}'.format(parent_locs)) - draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url() - sequential = modulestore.get_item(Location(parent_locs[0])) - index = sequential.children.index(draft_vertical.location.url()) - draft_vertical.xml_attributes['index_in_children_list'] = str(index) - draft_vertical.export_to_xml(draft_course_dir) + parent_locs = draft_modulestore.get_parent_locations(draft_vertical.location, course.location.course_id) + # Don't try to export orphaned items. + if len(parent_locs) > 0: + logging.debug('parent_locs = {0}'.format(parent_locs)) + draft_vertical.xml_attributes['parent_sequential_url'] = Location(parent_locs[0]).url() + sequential = modulestore.get_item(Location(parent_locs[0])) + index = sequential.children.index(draft_vertical.location.url()) + draft_vertical.xml_attributes['index_in_children_list'] = str(index) + draft_vertical.export_to_xml(draft_course_dir) def export_extra_content(export_fs, modulestore, course_id, course_location, category_type, dirname, file_suffix=''): diff --git a/common/lib/xmodule/xmodule/tests/rendering/__init__.py b/common/lib/xmodule/xmodule/tests/rendering/__init__.py index 9a3a52262e..d93c168c8c 100644 --- a/common/lib/xmodule/xmodule/tests/rendering/__init__.py +++ b/common/lib/xmodule/xmodule/tests/rendering/__init__.py @@ -1,2 +1 @@ import core -import xmodule_asserts \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/tests/rendering/xmodule_asserts.py b/common/lib/xmodule/xmodule/tests/rendering/xmodule_asserts.py deleted file mode 100644 index fa4dd66b06..0000000000 --- a/common/lib/xmodule/xmodule/tests/rendering/xmodule_asserts.py +++ /dev/null @@ -1,31 +0,0 @@ -""" -View assertion functions for XModules -""" - -from __future__ import absolute_import - -from nose.tools import assert_equals, assert_not_equals # pylint: disable=no-name-in-module - -from xmodule.timelimit_module import TimeLimitModule, TimeLimitDescriptor - -from xmodule.tests.rendering.core import assert_student_view_valid_html, assert_student_view_invalid_html - - -@assert_student_view_valid_html.register(TimeLimitModule) -@assert_student_view_valid_html.register(TimeLimitDescriptor) -def _(block, html): - """ - Assert that a TimeLimitModule renders student_view html correctly - """ - assert_not_equals(0, block.get_display_items()) - assert_student_view_valid_html(block.get_children()[0], html) - - -@assert_student_view_invalid_html.register(TimeLimitModule) -@assert_student_view_invalid_html.register(TimeLimitDescriptor) -def _(block, html): - """ - Assert that a TimeLimitModule renders student_view html correctly - """ - assert_equals(0, len(block.get_display_items())) - assert_equals(u"", html) diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 04a4e84f73..c1f4bb2ee2 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -92,6 +92,16 @@ class ImportTestCase(BaseCourseTestCase): self.assertNotEqual(descriptor1.location, descriptor2.location) + # Check that each vertical gets its very own url_name + bad_xml = '''''' + bad_xml2 = '''''' + + descriptor1 = system.process_xml(bad_xml) + descriptor2 = system.process_xml(bad_xml2) + + self.assertNotEqual(descriptor1.location, descriptor2.location) + + def test_reimport(self): '''Make sure an already-exported error xml tag loads properly''' diff --git a/common/lib/xmodule/xmodule/timelimit_module.py b/common/lib/xmodule/xmodule/timelimit_module.py deleted file mode 100644 index 73744b5e8b..0000000000 --- a/common/lib/xmodule/xmodule/timelimit_module.py +++ /dev/null @@ -1,136 +0,0 @@ -import logging - -from lxml import etree -from time import time - -from xmodule.editing_module import XMLEditingDescriptor -from xmodule.xml_module import XmlDescriptor -from xmodule.x_module import XModule -from xmodule.progress import Progress -from xmodule.exceptions import NotFoundError -from xblock.fields import Float, String, Boolean, Scope -from xblock.fragment import Fragment - - -log = logging.getLogger(__name__) - - -class TimeLimitFields(object): - has_children = True - - beginning_at = Float(help="The time this timer was started", scope=Scope.user_state) - ending_at = Float(help="The time this timer will end", scope=Scope.user_state) - accomodation_code = String(help="A code indicating accommodations to be given the student", scope=Scope.user_state) - time_expired_redirect_url = String(help="Url to redirect users to after the timelimit has expired", scope=Scope.settings) - duration = Float(help="The length of this timer", scope=Scope.settings) - suppress_toplevel_navigation = Boolean(help="Whether the toplevel navigation should be suppressed when viewing this module", scope=Scope.settings) - - -class TimeLimitModule(TimeLimitFields, XModule): - ''' - Wrapper module which imposes a time constraint for the completion of its child. - ''' - - # For a timed activity, we are only interested here - # in time-related accommodations, and these should be disjoint. - # (For proctored exams, it is possible to have multiple accommodations - # apply to an exam, so they require accommodating a multi-choice.) - TIME_ACCOMMODATION_CODES = (('NONE', 'No Time Accommodation'), - ('ADDHALFTIME', 'Extra Time - 1 1/2 Time'), - ('ADD30MIN', 'Extra Time - 30 Minutes'), - ('DOUBLE', 'Extra Time - Double Time'), - ('TESTING', 'Extra Time -- Large amount for testing purposes') - ) - - def _get_accommodated_duration(self, duration): - ''' - Get duration for activity, as adjusted for accommodations. - Input and output are expressed in seconds. - ''' - if self.accommodation_code is None or self.accommodation_code == 'NONE': - return duration - elif self.accommodation_code == 'ADDHALFTIME': - # TODO: determine what type to return - return int(duration * 1.5) - elif self.accommodation_code == 'ADD30MIN': - return (duration + (30 * 60)) - elif self.accommodation_code == 'DOUBLE': - return (duration * 2) - elif self.accommodation_code == 'TESTING': - # when testing, set timer to run for a week at a time. - return 3600 * 24 * 7 - - @property - def has_begun(self): - return self.beginning_at is not None - - @property - def has_ended(self): - if not self.ending_at: - return False - return self.ending_at < time() - - def begin(self, duration): - ''' - Sets the starting time and ending time for the activity, - based on the duration provided (in seconds). - ''' - self.beginning_at = time() - modified_duration = self._get_accommodated_duration(duration) - self.ending_at = self.beginning_at + modified_duration - - def get_remaining_time_in_ms(self): - return int((self.ending_at - time()) * 1000) - - def student_view(self, context): - # assumes there is one and only one child, so it only renders the first child - children = self.get_display_items() - if children: - child = children[0] - return child.render('student_view', context) - else: - return Fragment() - - def get_progress(self): - ''' Return the total progress, adding total done and total available. - (assumes that each submodule uses the same "units" for progress.) - ''' - # TODO: Cache progress or children array? - children = self.get_children() - progresses = [child.get_progress() for child in children] - progress = reduce(Progress.add_counts, progresses) - return progress - - def handle_ajax(self, _dispatch, _data): - raise NotFoundError('Unexpected dispatch type') - - def get_icon_class(self): - children = self.get_children() - if children: - return children[0].get_icon_class() - else: - return "other" - -class TimeLimitDescriptor(TimeLimitFields, XMLEditingDescriptor, XmlDescriptor): - - module_class = TimeLimitModule - - @classmethod - def definition_from_xml(cls, xml_object, system): - children = [] - for child in xml_object: - try: - children.append(system.process_xml(etree.tostring(child, encoding='unicode')).location.url()) - except Exception as e: - log.exception("Unable to load child when parsing TimeLimit wrapper. Continuing...") - if system.error_tracker is not None: - system.error_tracker("ERROR: " + str(e)) - continue - return {}, children - - def definition_to_xml(self, resource_fs): - xml_object = etree.Element('timelimit') - for child in self.get_children(): - xml_object.append( - etree.fromstring(child.export_to_xml(resource_fs))) - return xml_object diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index c64f1e0be3..e692dfb21c 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -3,7 +3,6 @@ import copy import logging import os import sys -from collections import namedtuple from lxml import etree from xblock.fields import Dict, Scope, ScopeIds @@ -133,15 +132,12 @@ class XmlDescriptor(XModuleDescriptor): 'ispublic', # if True, then course is listed for all users; see 'xqa_key', # for xqaa server access 'giturl', # url of git server for origin of file - # information about testcenter exams is a dict (of dicts), not a string, - # so it cannot be easily exportable as a course element's attribute. - 'testcenter_info', # VS[compat] Remove once unused. 'name', 'slug') metadata_to_strip = ('data_dir', 'tabs', 'grading_policy', 'published_by', 'published_date', - 'discussion_blackouts', 'testcenter_info', + 'discussion_blackouts', # VS[compat] -- remove the below attrs once everything is in the CMS 'course', 'org', 'url_name', 'filename', # Used for storing xml attributes between import and export, for roundtrips diff --git a/common/static/coffee/src/discussion/discussion.coffee b/common/static/coffee/src/discussion/discussion.coffee index 5a52cd4de0..954dd80129 100644 --- a/common/static/coffee/src/discussion/discussion.coffee +++ b/common/static/coffee/src/discussion/discussion.coffee @@ -25,9 +25,8 @@ if Backbone? @add model model - retrieveAnotherPage: (mode, options={}, sort_options={})-> - @current_page += 1 - data = { page: @current_page } + retrieveAnotherPage: (mode, options={}, sort_options={}, error=null)-> + data = { page: @current_page + 1 } switch mode when 'search' url = DiscussionUtil.urlFor 'search' @@ -59,6 +58,7 @@ if Backbone? @reset new_collection @pages = response.num_pages @current_page = response.page + error: error sortByDate: (thread) -> # diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 3dde9bf950..f99eba3872 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -36,12 +36,15 @@ if Backbone? event.preventDefault() @newPostForm.slideUp(300) + hideDiscussion: -> + @$("section.discussion").slideUp() + @toggleDiscussionBtn.removeClass('shown') + @toggleDiscussionBtn.find('.button-text').html("Show Discussion") + @showed = false + toggleDiscussion: (event) -> if @showed - @$("section.discussion").slideUp() - @toggleDiscussionBtn.removeClass('shown') - @toggleDiscussionBtn.find('.button-text').html("Show Discussion") - @showed = false + @hideDiscussion() else @toggleDiscussionBtn.addClass('shown') @toggleDiscussionBtn.find('.button-text').html("Hide Discussion") @@ -51,9 +54,17 @@ if Backbone? @showed = true else $elem = @toggleDiscussionBtn - @loadPage $elem + @loadPage( + $elem, + => + @hideDiscussion() + DiscussionUtil.discussionAlert( + "Sorry", + "We had some trouble loading the discussion. Please try again." + ) + ) - loadPage: ($elem)=> + loadPage: ($elem, error) => discussionId = @$el.data("discussion-id") url = DiscussionUtil.urlFor('retrieve_discussion', discussionId) + "?page=#{@page}" DiscussionUtil.safeAjax @@ -63,6 +74,7 @@ if Backbone? type: "GET" dataType: 'json' success: (response, textStatus, jqXHR) => @renderDiscussion($elem, response, textStatus, discussionId) + error: error renderDiscussion: ($elem, response, textStatus, discussionId) => window.user = new DiscussionUser(response.user_info) @@ -131,5 +143,14 @@ if Backbone? navigateToPage: (event) => event.preventDefault() window.history.pushState({}, window.document.title, event.target.href) + currPage = @page @page = $(event.target).data('page-number') - @loadPage($(event.target)) + @loadPage( + $(event.target), + => + @page = currPage + DiscussionUtil.discussionAlert( + "Sorry", + "We had some trouble loading the threads you requested. Please try again." + ) + ) diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index 73cfde8a06..89014c5f57 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -87,6 +87,13 @@ class @DiscussionUtil "notifications_status" : "/notification_prefs/status" }[name] + @makeFocusTrap: (elem) -> + elem.keydown( + (event) -> + if event.which == 9 # Tab + event.preventDefault() + ) + @discussionAlert: (header, body) -> if $("#discussion-alert").length == 0 alertDiv = $("" ) - # Capture focus - alertDiv.find("button").keydown( - (event) -> - if event.which == 9 # Tab - event.preventDefault() - ) + @makeFocusTrap(alertDiv.find("button")) alertTrigger = $("").css("display", "none") alertTrigger.leanModal({closeButton: "#discussion-alert .dismiss", overlay: 1, top: 200}) $("body").append(alertDiv).append(alertTrigger) diff --git a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee index 5a1e3fa9ae..57385c15bd 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee @@ -124,8 +124,11 @@ if Backbone? loadMorePages: (event) -> if event event.preventDefault() - @$(".more-pages").html('
    Loading more threads
    ') + @$(".more-pages").html('
    Loading more threads
    ') @$(".more-pages").addClass("loading") + loadingDiv = @$(".more-pages .loading-animation") + DiscussionUtil.makeFocusTrap(loadingDiv) + loadingDiv.focus() options = {} switch @mode when 'search' @@ -156,7 +159,11 @@ if Backbone? $(".post-list a").first()?.focus() ) - @collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy}) + error = => + @renderThreads() + DiscussionUtil.discussionAlert("Sorry", "We had some trouble loading more threads. Please try again.") + + @collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy}, error) renderThread: (thread) => content = $(_.template($("#thread-list-item-template").html())(thread.toJSON())) diff --git a/common/static/js/capa/choicetextinput.js b/common/static/js/capa/choicetextinput.js index 4d7540f938..514e3f67f5 100644 --- a/common/static/js/capa/choicetextinput.js +++ b/common/static/js/capa/choicetextinput.js @@ -1,13 +1,13 @@ (function () { var update = function () { // Whenever a value changes create a new serialized version of this - // problem's inputs and set the hidden input fields value to equal it. - var parent = $(this).closest('.problems-wrapper'); + // problem's inputs and set the hidden input field's value to equal it. + var parent = $(this).closest('section.choicetextinput'); // find the closest parent problems-wrapper and use that as the problem // grab the input id from the input // real_input is the hidden input field var real_input = $('input.choicetextvalue', parent); - var all_inputs = $('.choicetextinput .ctinput', parent); + var all_inputs = $('input.ctinput', parent); var user_inputs = {}; $(all_inputs).each(function (index, elt) { var node = $(elt); diff --git a/common/static/js/vendor/backbone-min.js b/common/static/js/vendor/backbone-min.js index fe80cd78df..bce4fbc1b1 100644 --- a/common/static/js/vendor/backbone-min.js +++ b/common/static/js/vendor/backbone-min.js @@ -1,4 +1 @@ (function(){var t=this;var e=t.Backbone;var i=[];var r=i.push;var s=i.slice;var n=i.splice;var a;if(typeof exports!=="undefined"){a=exports}else{a=t.Backbone={}}a.VERSION="1.0.0";var h=t._;if(!h&&typeof require!=="undefined")h=require("underscore");a.$=t.jQuery||t.Zepto||t.ender||t.$;a.noConflict=function(){t.Backbone=e;return this};a.emulateHTTP=false;a.emulateJSON=false;var o=a.Events={on:function(t,e,i){if(!l(this,"on",t,[e,i])||!e)return this;this._events||(this._events={});var r=this._events[t]||(this._events[t]=[]);r.push({callback:e,context:i,ctx:i||this});return this},once:function(t,e,i){if(!l(this,"once",t,[e,i])||!e)return this;var r=this;var s=h.once(function(){r.off(t,s);e.apply(this,arguments)});s._callback=e;return this.on(t,s,i)},off:function(t,e,i){var r,s,n,a,o,u,c,f;if(!this._events||!l(this,"off",t,[e,i]))return this;if(!t&&!e&&!i){this._events={};return this}a=t?[t]:h.keys(this._events);for(o=0,u=a.length;o").attr(t);this.setElement(e,false)}else{this.setElement(h.result(this,"el"),false)}}});a.sync=function(t,e,i){var r=k[t];h.defaults(i||(i={}),{emulateHTTP:a.emulateHTTP,emulateJSON:a.emulateJSON});var s={type:r,dataType:"json"};if(!i.url){s.url=h.result(e,"url")||U()}if(i.data==null&&e&&(t==="create"||t==="update"||t==="patch")){s.contentType="application/json";s.data=JSON.stringify(i.attrs||e.toJSON(i))}if(i.emulateJSON){s.contentType="application/x-www-form-urlencoded";s.data=s.data?{model:s.data}:{}}if(i.emulateHTTP&&(r==="PUT"||r==="DELETE"||r==="PATCH")){s.type="POST";if(i.emulateJSON)s.data._method=r;var n=i.beforeSend;i.beforeSend=function(t){t.setRequestHeader("X-HTTP-Method-Override",r);if(n)return n.apply(this,arguments)}}if(s.type!=="GET"&&!i.emulateJSON){s.processData=false}if(s.type==="PATCH"&&window.ActiveXObject&&!(window.external&&window.external.msActiveXFilteringEnabled)){s.xhr=function(){return new ActiveXObject("Microsoft.XMLHTTP")}}var o=i.xhr=a.ajax(h.extend(s,i));e.trigger("request",e,o,i);return o};var k={create:"POST",update:"PUT",patch:"PATCH","delete":"DELETE",read:"GET"};a.ajax=function(){return a.$.ajax.apply(a.$,arguments)};var S=a.Router=function(t){t||(t={});if(t.routes)this.routes=t.routes;this._bindRoutes();this.initialize.apply(this,arguments)};var $=/\((.*?)\)/g;var T=/(\(\?)?:\w+/g;var H=/\*\w+/g;var A=/[\-{}\[\]+?.,\\\^$|#\s]/g;h.extend(S.prototype,o,{initialize:function(){},route:function(t,e,i){if(!h.isRegExp(t))t=this._routeToRegExp(t);if(h.isFunction(e)){i=e;e=""}if(!i)i=this[e];var r=this;a.history.route(t,function(s){var n=r._extractParameters(t,s);i&&i.apply(r,n);r.trigger.apply(r,["route:"+e].concat(n));r.trigger("route",e,n);a.history.trigger("route",r,e,n)});return this},navigate:function(t,e){a.history.navigate(t,e);return this},_bindRoutes:function(){if(!this.routes)return;this.routes=h.result(this,"routes");var t,e=h.keys(this.routes);while((t=e.pop())!=null){this.route(t,this.routes[t])}},_routeToRegExp:function(t){t=t.replace(A,"\\$&").replace($,"(?:$1)?").replace(T,function(t,e){return e?t:"([^/]+)"}).replace(H,"(.*?)");return new RegExp("^"+t+"$")},_extractParameters:function(t,e){var i=t.exec(e).slice(1);return h.map(i,function(t){return t?decodeURIComponent(t):null})}});var I=a.History=function(){this.handlers=[];h.bindAll(this,"checkUrl");if(typeof window!=="undefined"){this.location=window.location;this.history=window.history}};var N=/^[#\/]|\s+$/g;var P=/^\/+|\/+$/g;var O=/msie [\w.]+/;var C=/\/$/;I.started=false;h.extend(I.prototype,o,{interval:50,getHash:function(t){var e=(t||this).location.href.match(/#(.*)$/);return e?e[1]:""},getFragment:function(t,e){if(t==null){if(this._hasPushState||!this._wantsHashChange||e){t=this.location.pathname;var i=this.root.replace(C,"");if(!t.indexOf(i))t=t.substr(i.length)}else{t=this.getHash()}}return t.replace(N,"")},start:function(t){if(I.started)throw new Error("Backbone.history has already been started");I.started=true;this.options=h.extend({},{root:"/"},this.options,t);this.root=this.options.root;this._wantsHashChange=this.options.hashChange!==false;this._wantsPushState=!!this.options.pushState;this._hasPushState=!!(this.options.pushState&&this.history&&this.history.pushState);var e=this.getFragment();var i=document.documentMode;var r=O.exec(navigator.userAgent.toLowerCase())&&(!i||i<=7);this.root=("/"+this.root+"/").replace(P,"/");if(r&&this._wantsHashChange){this.iframe=a.$('
    @@ -34,16 +61,13 @@ {% trans "Save changes" %} - + {% trans "Back to editor" %}
    - + {% include "wiki/includes/cheatsheet.html" %} {% endblock %} - - - diff --git a/lms/templates/wiki/history.html b/lms/templates/wiki/history.html index 5488abb97d..798b030c03 100644 --- a/lms/templates/wiki/history.html +++ b/lms/templates/wiki/history.html @@ -10,7 +10,7 @@ {% endaddtoblock %} @@ -81,7 +95,7 @@ {% trans "Click each revision to see a list of edited lines. Click the Preview button to see how the article looked at this stage. At the bottom of this page, you can change to a particular revision or merge an old revision with the current one." %}

    -
    +
    {% for revision in revisions %}
    @@ -107,16 +121,29 @@
    {% if not revision == article.current_revision %} - + + + {% trans "Preview this revision" %} + + + {% if article|can_write:user %} + + {% endif %} + {% endif %} - - {% if article|can_write:user %} - - {% endif %} - +
    @@ -140,84 +167,103 @@ {% endfor %} - + {% include "wiki/includes/pagination.html" %} - + {% if revisions.count > 1 %}
    {% if article|can_write:user %} - + {% else %} {% endif %} -
    - {% endif %} - + -