From d6475f4558a1959aca05f9c814bac2ff2b896ad9 Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 2 Jul 2014 13:06:34 -0400 Subject: [PATCH] New publishing controls on unit page (continued) STUD-1707 --- cms/djangoapps/contentstore/views/helpers.py | 11 +- .../contentstore/views/tests/test_item.py | 6 +- .../views/pages/container_subviews_spec.js | 57 ++++++---- cms/static/js/views/pages/container.js | 4 +- .../js/views/pages/container_subviews.js | 19 +++- cms/templates/container.html | 16 +-- cms/templates/js/publish-xblock.underscore | 6 +- .../test/acceptance/pages/lms/courseware.py | 31 ++++++ .../test/acceptance/pages/studio/container.py | 58 ++++++++++ common/test/acceptance/pages/studio/utils.py | 2 +- .../acceptance/tests/test_studio_container.py | 102 ++++++++++++++++++ 11 files changed, 268 insertions(+), 44 deletions(-) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 6b83c2de08..cefe82d4d4 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -1,6 +1,8 @@ -from __future__ import absolute_import +""" +Helper methods for Studio views. +""" -import logging +from __future__ import absolute_import from django.conf import settings from django.http import HttpResponse @@ -13,11 +15,6 @@ from contentstore.utils import reverse_course_url, reverse_usage_url __all__ = ['edge', 'event', 'landing'] -EDITING_TEMPLATES = [ - "basic-modal", "modal-button", "edit-xblock-modal", "editor-mode-button", "upload-dialog", "image-modal", - "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", - "add-xblock-component-menu-problem", "xblock-string-field-editor", -] # points to the temporary course landing page with log in and sign up def landing(request, org, course, coursename): diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 2cbcca7f93..aed94865cc 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -23,6 +23,8 @@ from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory from xmodule.capa_module import CapaDescriptor from xmodule.modulestore import PublishState +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore from xmodule.x_module import STUDIO_VIEW, STUDENT_VIEW from xblock.exceptions import NoSuchHandlerError from opaque_keys.edx.keys import UsageKey, CourseKey @@ -567,7 +569,7 @@ class TestEditItem(ItemTest): self.assertEqual(updated_draft.due, datetime(2077, 10, 10, 4, 0, tzinfo=UTC)) self.assertIsNone(published.due) # Fetch the published version again to make sure the due date is still unset. - published = modulestore().get_item(published.location, revision=REVISION_OPTION_PUBLISHED_ONLY) + published = modulestore().get_item(published.location, revision=ModuleStoreEnum.RevisionOption.published_only) self.assertIsNone(published.due) def test_make_public_with_update(self): @@ -620,7 +622,7 @@ class TestEditItem(ItemTest): draft = self.get_item_from_modulestore(self.problem_usage_key, verify_is_draft=True) self.assertNotEqual(draft.data, published.data) # Fetch the published version again to make sure the data is correct. - published = modulestore().get_item(published.location, revision=REVISION_OPTION_PUBLISHED_ONLY) + published = modulestore().get_item(published.location, revision=ModuleStoreEnum.RevisionOption.published_only) self.assertNotEqual(draft.data, published.data) def test_publish_states_of_nested_xblocks(self): diff --git a/cms/static/js/spec/views/pages/container_subviews_spec.js b/cms/static/js/spec/views/pages/container_subviews_spec.js index ecfde1559f..6170abee73 100644 --- a/cms/static/js/spec/views/pages/container_subviews_spec.js +++ b/cms/static/js/spec/views/pages/container_subviews_spec.js @@ -44,8 +44,7 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin ); }; - respondWithJson = function(json) { - var requestIndex = requests.length - 1; + respondWithJson = function(json, requestIndex) { create_sinon.respondWithJson( requests, json, @@ -131,10 +130,29 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin draftBit = "draft", publishButtonCss = ".action-publish", discardChangesButtonCss = ".action-discard", - request, lastRequest, promptSpies; + request, lastRequest, promptSpies, sendDiscardChangesToServer; lastRequest = function() { return requests[requests.length - 1]; }; + sendDiscardChangesToServer = function(test) { + // Helper function to do the discard operation, up until the server response. + renderContainerPage(mockContainerXBlockHtml, test); + fetch({"id": "locator-container", "published": true, "has_changes": true}); + expect(containerPage.$(discardChangesButtonCss)).not.toHaveClass('is-disabled'); + expect(containerPage.$(bitPublishingCss)).toHaveClass(draftBit); + // Click discard changes + containerPage.$(discardChangesButtonCss).click(); + + // Confirm the discard. + expect(promptSpies.constructor).toHaveBeenCalled(); + promptSpies.constructor.mostRecentCall.args[0].actions.primary.click(promptSpies); + + request = lastRequest(); + expect(request.url).toEqual("/xblock/locator-container"); + expect(request.method).toEqual("DELETE"); + expect(request.requestBody).toEqual(null); + }; + beforeEach(function() { promptSpies = spyOnConstructor(Prompt, "Warning", ["show", "hide"]); promptSpies.show.andReturn(this.promptSpies); @@ -199,6 +217,8 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin // Verify updates displayed expect(containerPage.$(bitPublishingCss)).toHaveClass(publishedBit); + // Verify that the "published" value has been cleared out of the model. + expect(containerPage.model.get("publish")).toBeNull(); }); it('can does not fetch if publish fails', function () { @@ -217,27 +237,26 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin // Verify still in draft state. expect(containerPage.$(bitPublishingCss)).toHaveClass(draftBit); + // Verify that the "published" value has been cleared out of the model. + expect(containerPage.model.get("publish")).toBeNull(); }); + /* STUD-1860 it('can discard changes', function () { - renderContainerPage(mockContainerXBlockHtml, this); - fetch({"id": "locator-container", "published": true, "has_changes": true}); - expect(containerPage.$(discardChangesButtonCss)).not.toHaveClass('is-disabled'); - expect(containerPage.$(bitPublishingCss)).toHaveClass(draftBit); - // Click discard changes - containerPage.$(discardChangesButtonCss).click(); + sendDiscardChangesToServer(this); - // Confirm the discard. - expect(promptSpies.constructor).toHaveBeenCalled(); - promptSpies.constructor.mostRecentCall.args[0].actions.primary.click(promptSpies); + var numRequests = requests.length; + create_sinon.respondToDelete(requests); + // Response to fetch, specifying the very next request (as multiple requests will be sent to server) + respondWithJson({"id": "locator-container", "published": true, "has_changes": false}, numRequests); + expect(containerPage.$(discardChangesButtonCss)).toHaveClass('is-disabled'); + expect(containerPage.$(bitPublishingCss)).toHaveClass(publishedBit); + }); + */ - request = lastRequest(); - expect(request.url).toEqual("/xblock/locator-container"); - expect(request.method).toEqual("DELETE"); - expect(request.requestBody).toEqual(null); + it('does not fetch if discard changes fails', function () { + sendDiscardChangesToServer(this); - // Respond with failure because code does window.location.reload (which will - // put tests into an infinite loop) on success. var numRequests = requests.length; // Respond with failure create_sinon.respondWithError(requests); @@ -249,8 +268,6 @@ define(["jquery", "underscore", "underscore.string", "js/spec_helpers/create_sin it('does not discard changes on cancel', function () { renderContainerPage(mockContainerXBlockHtml, this); fetch({"id": "locator-container", "published": true, "has_changes": true}); - expect(containerPage.$(discardChangesButtonCss)).not.toHaveClass('is-disabled'); - expect(containerPage.$(bitPublishingCss)).toHaveClass(draftBit); var numRequests = requests.length; // Click discard changes diff --git a/cms/static/js/views/pages/container.js b/cms/static/js/views/pages/container.js index af437f2d13..40cf5dbcc6 100644 --- a/cms/static/js/views/pages/container.js +++ b/cms/static/js/views/pages/container.js @@ -28,7 +28,9 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/contai if (this.isUnitPage) { this.xblockPublisher = new ContainerSubviews.Publisher({ el: this.$('#publish-unit'), - model: this.model + model: this.model, + // When "Discard Changes" is clicked, the whole page must be re-rendered. + renderPage: this.render }); this.xblockPublisher.render(); diff --git a/cms/static/js/views/pages/container_subviews.js b/cms/static/js/views/pages/container_subviews.js index df347ae667..742f505dbf 100644 --- a/cms/static/js/views/pages/container_subviews.js +++ b/cms/static/js/views/pages/container_subviews.js @@ -92,6 +92,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/feedba BaseView.prototype.initialize.call(this); this.template = this.loadTemplate('publish-xblock'); this.model.on('sync', this.onSync, this); + this.renderPage = this.options.renderPage; }, onSync: function(e) { @@ -121,6 +122,8 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/feedba this.runOperationShowingMessage(gettext('Publishing…'), function () { return xblockInfo.save({publish: 'make_public'}); + }).always(function() { + xblockInfo.set("publish", null); }).done(function () { xblockInfo.fetch(); }); @@ -130,7 +133,7 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/feedba if (e && e.preventDefault) { e.preventDefault(); } - var xblockInfo = this.model, view; + var xblockInfo = this.model, view, renderPage = this.renderPage; view = new PromptView.Warning({ title: gettext("Discard Changes"), @@ -144,7 +147,19 @@ define(["jquery", "underscore", "gettext", "js/views/baseview", "js/views/feedba type: 'DELETE', url: xblockInfo.url() }).success(function () { - return window.location.reload(); + window.alert("Refresh the page to see that changes were discarded. " + + "Auto-refresh will be implemented in a later story."); + /* Fetch is never returning on sandbox-- try + doing a PUT instead of a DELETE with publish option + to work around, or contact dev ops. + STUD-1860 + window.crazyAjaxHandler = xblockInfo.fetch({ + complete: function(a, b, c) { + debugger; + } + }); + renderPage(); + */ }); } }, diff --git a/cms/templates/container.html b/cms/templates/container.html index 00e620abfc..c4d8f37893 100644 --- a/cms/templates/container.html +++ b/cms/templates/container.html @@ -11,7 +11,7 @@ else: import json from xmodule.modulestore import PublishState -from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name, EDITING_TEMPLATES +from contentstore.views.helpers import xblock_studio_url, xblock_type_display_name from django.utils.translation import ugettext as _ %> <%block name="title">${xblock.display_name_with_default} ${xblock_type_display_name(xblock)} @@ -20,16 +20,17 @@ from django.utils.translation import ugettext as _ <%namespace name='static' file='static_content.html'/> <%namespace name="units" file="widgets/units.html" /> - +<%! +templates = ["basic-modal", "modal-button", "edit-xblock-modal", + "editor-mode-button", "upload-dialog", "image-modal", + "add-xblock-component", "add-xblock-component-button", "add-xblock-component-menu", + "add-xblock-component-menu-problem", "xblock-string-field-editor", "publish-xblock"] +%> <%block name="header_extras"> -% for template_name in EDITING_TEMPLATES: +% for template_name in templates: - - % endfor @@ -40,7 +41,6 @@ from django.utils.translation import ugettext as _ "js/collections/component_template", "xmodule", "coffee/src/main", "xblock/cms.runtime.v1"], function(doc, $, XBlockInfo, ContainerPage, ComponentTemplates, xmoduleLoader) { var templates = new ComponentTemplates(${component_templates | n}, {parse: true}); - // TODO: can go back to dumping on server side if easier. var mainXBlockInfo = new XBlockInfo(${json.dumps(xblock_info) | n}); var isUnitPage = ${json.dumps(is_unit_page)} diff --git a/cms/templates/js/publish-xblock.underscore b/cms/templates/js/publish-xblock.underscore index bec304783d..a0f0d2c7f3 100644 --- a/cms/templates/js/publish-xblock.underscore +++ b/cms/templates/js/publish-xblock.underscore @@ -7,14 +7,14 @@ <% } %> - + - + @@ -23,7 +23,7 @@ - + diff --git a/common/test/acceptance/pages/lms/courseware.py b/common/test/acceptance/pages/lms/courseware.py index 33ec207d43..38cf30badf 100644 --- a/common/test/acceptance/pages/lms/courseware.py +++ b/common/test/acceptance/pages/lms/courseware.py @@ -11,6 +11,37 @@ class CoursewarePage(CoursePage): """ url_path = "courseware/" + xblock_component_selector = '.vert .xblock' def is_browser_on_page(self): return self.q(css='body.courseware').present + + @property + def num_xblock_components(self): + """ + Return the number of rendered xblocks within the unit on the page + """ + return len(self.q(css=self.xblock_component_selector)) + + def xblock_component_type(self, index=0): + """ + Extract rendered xblock component type. + + Returns: + str: xblock module type + index: which xblock to query, where the index is the vertical display within the page + (default is 0) + """ + return self.q(css=self.xblock_component_selector).attrs('data-block-type')[index] + + def xblock_component_html_content(self, index=0): + """ + Extract rendered xblock component html content. + + Returns: + str: xblock module html content + index: which xblock to query, where the index is the vertical display within the page + (default is 0) + + """ + return self.q(css=self.xblock_component_selector).attrs('innerHTML')[index].strip() diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 8463fd09db..5d966cfac6 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -74,6 +74,64 @@ class ContainerPage(PageObject): """ return self._get_xblocks(".is-active ") + @property + def publish_title(self): + """ + Returns the title as displayed on the publishing sidebar component. + """ + return self.q(css='.pub-status').first.text[0] + + @property + def publish_action(self): + """ + Returns the link for publishing a unit. + """ + return self.q(css='.action-publish').first + + # def discard_changes(self): + # """ + # Discards draft changes and reloads the page. + # NOT YET IMPLEMENTED-- part of future story + # """ + # + # self.q(css='.action-discard').first.click() + # + # # TODO: work with Jay/Christine on this. I can't find something to wait on + # # that guarantees the button will be clickable. + # time.sleep(2) + # + # self.q(css='a.button.action-primary').first.click() + # self.wait_for_ajax() + # + # return ContainerPage(self.browser, self.locator).visit() + + def view_published_version(self): + """ + Clicks "View Published Version", which will open the published version of the unit page in the LMS. + + Switches the browser to the newly opened LMS window. + """ + self.q(css='.view-button').first.click() + self._switch_to_lms() + + def preview(self): + """ + Clicks "Preview Changes", which will open the draft version of the unit page in the LMS. + + Switches the browser to the newly opened LMS window. + """ + self.q(css='.preview-button').first.click() + self._switch_to_lms() + + def _switch_to_lms(self): + """ + Assumes LMS has opened-- switches to that window. + """ + browser_window_handles = self.browser.window_handles + # Switch to browser window that shows HTML Unit in LMS + # The last handle represents the latest windows opened + self.browser.switch_to_window(browser_window_handles[-1]) + def _get_xblocks(self, prefix=""): return self.q(css=prefix + XBlockWrapper.BODY_SELECTOR).map( lambda el: XBlockWrapper(self.browser, el.get_attribute('data-locator'))).results diff --git a/common/test/acceptance/pages/studio/utils.py b/common/test/acceptance/pages/studio/utils.py index 8bb444ce9b..8075ce386b 100644 --- a/common/test/acceptance/pages/studio/utils.py +++ b/common/test/acceptance/pages/studio/utils.py @@ -59,7 +59,7 @@ def press_the_notification_button(page, name): page.wait_for_ajax() -def add_discussion(page, menu_index): +def add_discussion(page, menu_index=0): """ Add a new instance of the discussion category. diff --git a/common/test/acceptance/tests/test_studio_container.py b/common/test/acceptance/tests/test_studio_container.py index febbe6eb5b..18276cbe67 100644 --- a/common/test/acceptance/tests/test_studio_container.py +++ b/common/test/acceptance/tests/test_studio_container.py @@ -1,5 +1,7 @@ """ Acceptance tests for Studio related to the container page. +The container page is used both for display units, and for +displaying containers within units. """ from nose.plugins.attrib import attr @@ -8,6 +10,7 @@ from ..fixtures.course import XBlockFixtureDesc from ..pages.studio.component_editor import ComponentEditorView from ..pages.studio.utils import add_discussion +from ..pages.lms.courseware import CoursewarePage from unittest import skip from acceptance.tests.base_studio_test import StudioCourseTest @@ -366,3 +369,102 @@ class EditContainerTest(NestedVerticalTest): """ container = self.go_to_nested_container_page() self.modify_display_name_and_verify(container) + + +class UnitPublishingTest(ContainerBase): + """ + Tests of the publishing control and related widgets on the Unit page. + """ + + def setup_fixtures(self): + """ + Sets up a course structure with a unit and a single HTML child. + """ + self.html_content = '

Body of HTML Unit.

' + self.courseware = CoursewarePage(self.browser, self.course_id) + + course_fix = CourseFixture( + self.course_info['org'], + self.course_info['number'], + self.course_info['run'], + self.course_info['display_name'] + ) + + course_fix.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('vertical', 'Test Unit').add_children( + XBlockFixtureDesc('html', 'Test html', data=self.html_content) + ) + ) + ) + ).install() + + self.user = course_fix.user + + def test_publishing(self): + """ + Test the state changes when a published unit has draft changes. + """ + unit = self.go_to_unit_page() + self.assertEqual("Publishing Status\nPublished", unit.publish_title) + # Should not be able to click on Publish action -- but I don't know how to test that it is not clickable. + # TODO: continue discussion with Muhammad and Jay about this. + + # Add a component to the page so it will have unpublished changes. + add_discussion(unit) + self.assertEqual("Publishing Status\nDraft (Unpublished changes)", unit.publish_title) + unit.publish_action.click() + unit.wait_for_ajax() + self.assertEqual("Publishing Status\nPublished", unit.publish_title) + + # TODO: part of future story. + # def test_discard_changes(self): + # """ + # Test the state after discard changes. + # """ + # unit = self.go_to_unit_page() + # add_discussion(unit) + # unit = unit.discard_changes() + # self.assertEqual("Publishing Status\nPublished", unit.publish_title) + + def test_view_live_no_changes(self): + """ + Tests viewing of live with initial published content. + """ + unit = self.go_to_unit_page() + unit.view_published_version() + self.assertEqual(1, self.courseware.num_xblock_components) + self.assertEqual('html', self.courseware.xblock_component_type(0)) + + def test_view_live_changes(self): + """ + Tests that viewing of live with draft content does not show the draft content. + """ + unit = self.go_to_unit_page() + add_discussion(unit) + unit.view_published_version() + self.assertEqual(1, self.courseware.num_xblock_components) + self.assertEqual('html', self.courseware.xblock_component_type(0)) + self.assertEqual(self.html_content, self.courseware.xblock_component_html_content(0)) + + def test_view_live_after_publish(self): + """ + Tests viewing of live after creating draft and publishing it. + """ + unit = self.go_to_unit_page() + add_discussion(unit) + unit.publish_action.click() + unit.view_published_version() + self.assertEqual(2, self.courseware.num_xblock_components) + self.assertEqual('html', self.courseware.xblock_component_type(0)) + self.assertEqual('discussion', self.courseware.xblock_component_type(1)) + + # TODO: need to work with Jay/Christine to get testing of "Preview" working. + # def test_preview(self): + # unit = self.go_to_unit_page() + # add_discussion(unit) + # unit.preview() + # self.assertEqual(2, self.courseware.num_xblock_components) + # self.assertEqual('html', self.courseware.xblock_component_type(0)) + # self.assertEqual('discussion', self.courseware.xblock_component_type(1))