diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 41c171c52c..4d117a9c73 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -48,6 +48,8 @@ moved to be edited as metadata. XModule: Only write out assets files if the contents have changed. +Studio: Course settings are now saved explicitly. + XModule: Don't delete generated xmodule asset files when compiling (for instance, when XModule provides a coffeescript file, don't delete the associated javascript) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 13600f2086..514eb8898e 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -46,3 +46,9 @@ Feature: Advanced (manual) course policy Then it is displayed as a string And I reload the page Then it is displayed as a string + + Scenario: Confirmation is shown on save + Given I am on the Advanced Course Settings page in Studio + When I edit the value of a policy key + And I press the "Save" notification button + Then I see a confirmation that my changes have been saved diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 37d5c12ecc..c08216c8e6 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -2,8 +2,8 @@ #pylint: disable=W0621 from lettuce import world, step -from nose.tools import assert_false, assert_equal, assert_regexp_matches -from common import type_in_codemirror +from nose.tools import assert_false, assert_equal, assert_regexp_matches, assert_true +from common import type_in_codemirror, press_the_notification_button KEY_CSS = '.key input.policy-key' VALUE_CSS = 'textarea.json' @@ -25,20 +25,6 @@ def i_am_on_advanced_course_settings(step): step.given('I select the Advanced Settings') -@step(u'I press the "([^"]*)" notification button$') -def press_the_notification_button(step, name): - css = 'a.action-%s' % name.lower() - - # Save was clicked if either the save notification bar is gone, or we have a error notification - # overlaying it (expected in the case of typing Object into display_name). - def save_clicked(): - confirmation_dismissed = world.is_css_not_present('.is-shown.wrapper-notification-warning') - error_showing = world.is_css_present('.is-shown.wrapper-notification-error') - return confirmation_dismissed or error_showing - - world.css_click(css, success_condition=save_clicked) - - @step(u'I edit the value of a policy key$') def edit_the_value_of_a_policy_key(step): type_in_codemirror(get_index_of(DISPLAY_NAME_KEY), 'X') diff --git a/cms/djangoapps/contentstore/features/common.py b/cms/djangoapps/contentstore/features/common.py index bdf07fc5ae..5a8e02f168 100644 --- a/cms/djangoapps/contentstore/features/common.py +++ b/cms/djangoapps/contentstore/features/common.py @@ -12,6 +12,8 @@ import time from logging import getLogger logger = getLogger(__name__) +from terrain.browser import reset_data + _COURSE_NAME = 'Robot Super Course' _COURSE_NUM = '999' _COURSE_ORG = 'MITx' @@ -55,6 +57,48 @@ def i_have_opened_a_new_course(_step): open_new_course() +@step(u'I press the "([^"]*)" notification button$') +def press_the_notification_button(_step, name): + css = 'a.action-%s' % name.lower() + + # The button was clicked if either the notification bar is gone, + # or we see an error overlaying it (expected for invalid inputs). + def button_clicked(): + confirmation_dismissed = world.is_css_not_present('.is-shown.wrapper-notification-warning') + error_showing = world.is_css_present('.is-shown.wrapper-notification-error') + return confirmation_dismissed or error_showing + + world.css_click(css, success_condition=button_clicked), '%s button not clicked after 5 attempts.' % name + + +@step('I change the "(.*)" field to "(.*)"$') +def i_change_field_to_value(_step, field, value): + field_css = '#%s' % '-'.join([s.lower() for s in field.split()]) + ele = world.css_find(field_css).first + ele.fill(value) + ele._element.send_keys(Keys.ENTER) + + +@step('I reset the database') +def reset_the_db(_step): + """ + When running Lettuce tests using examples (i.e. "Confirmation is + shown on save" in course-settings.feature), the normal hooks + aren't called between examples. reset_data should run before each + scenario to flush the test database. When this doesn't happen we + get errors due to trying to insert a non-unique entry. So instead, + we delete the database manually. This has the effect of removing + any users and courses that have been created during the test run. + """ + reset_data(None) + + +@step('I see a confirmation that my changes have been saved') +def i_see_a_confirmation(step): + confirmation_css = '#alert-confirmation' + assert world.is_css_present(confirmation_css) + + ####### HELPER FUNCTIONS ############## def open_new_course(): world.clear_courses() @@ -184,6 +228,13 @@ def shows_captions(step, show_captions): assert world.is_css_not_present('.video.closed') +@step('the save button is disabled$') +def save_button_disabled(step): + button_css = '.action-save' + disabled = 'is-disabled' + assert world.css_find(button_css)[0].has_class(disabled) + + def type_in_codemirror(index, text): world.css_click(".CodeMirror", index=index) g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea") diff --git a/cms/djangoapps/contentstore/features/course-settings.feature b/cms/djangoapps/contentstore/features/course-settings.feature index e869bfe47a..5c79dc7ee3 100644 --- a/cms/djangoapps/contentstore/features/course-settings.feature +++ b/cms/djangoapps/contentstore/features/course-settings.feature @@ -5,15 +5,18 @@ Feature: Course Settings Given I have opened a new course in Studio When I select Schedule and Details And I set course dates + And I press the "Save" notification button Then I see the set dates on refresh Scenario: User can clear previously set course dates (except start date) Given I have set course dates And I clear all the dates except start + And I press the "Save" notification button Then I see cleared dates on refresh Scenario: User cannot clear the course start date Given I have set course dates + And I press the "Save" notification button And I clear the course start date Then I receive a warning about course start date And The previously set start date is shown on refresh @@ -21,5 +24,50 @@ Feature: Course Settings Scenario: User can correct the course start date warning Given I have tried to clear the course start And I have entered a new course start date + And I press the "Save" notification button Then The warning about course start date goes away And My new course start date is shown on refresh + + Scenario: Settings are only persisted when saved + Given I have set course dates + And I press the "Save" notification button + When I change fields + Then I do not see the new changes persisted on refresh + + Scenario: Settings are reset on cancel + Given I have set course dates + And I press the "Save" notification button + When I change fields + And I press the "Cancel" notification button + Then I do not see the changes + + Scenario: Confirmation is shown on save + Given I have opened a new course in Studio + When I select Schedule and Details + And I change the "" field to "" + And I press the "Save" notification button + Then I see a confirmation that my changes have been saved + # Lettuce hooks don't get called between each example, so we need + # to run the before.each_scenario hook manually to avoid database + # errors. + And I reset the database + + Examples: + | field | value | + | Course Start Time | 11:00 | + | Course Introduction Video | 4r7wHMg5Yjg | + | Course Effort | 200:00 | + + # Special case because we have to type in code mirror + Scenario: Changes in Course Overview show a confirmation + Given I have opened a new course in Studio + When I select Schedule and Details + And I change the course overview + And I press the "Save" notification button + Then I see a confirmation that my changes have been saved + + Scenario: User cannot save invalid settings + Given I have opened a new course in Studio + When I select Schedule and Details + And I change the "Course Start Date" field to "" + Then the save button is disabled diff --git a/cms/djangoapps/contentstore/features/course-settings.py b/cms/djangoapps/contentstore/features/course-settings.py index bd86fff9b7..53a3fa7870 100644 --- a/cms/djangoapps/contentstore/features/course-settings.py +++ b/cms/djangoapps/contentstore/features/course-settings.py @@ -4,7 +4,7 @@ from lettuce import world, step from terrain.steps import reload_the_page from selenium.webdriver.common.keys import Keys -import time +from common import type_in_codemirror from nose.tools import assert_true, assert_false, assert_equal @@ -47,22 +47,11 @@ def test_and_i_set_course_dates(step): set_date_or_time(COURSE_START_TIME_CSS, DUMMY_TIME) set_date_or_time(ENROLLMENT_END_TIME_CSS, DUMMY_TIME) - pause() - @step('Then I see the set dates on refresh$') def test_then_i_see_the_set_dates_on_refresh(step): reload_the_page(step) - verify_date_or_time(COURSE_START_DATE_CSS, '12/20/2013') - verify_date_or_time(COURSE_END_DATE_CSS, '12/26/2013') - verify_date_or_time(ENROLLMENT_START_DATE_CSS, '12/01/2013') - verify_date_or_time(ENROLLMENT_END_DATE_CSS, '12/10/2013') - - verify_date_or_time(COURSE_START_TIME_CSS, DUMMY_TIME) - # Unset times get set to 12 AM once the corresponding date has been set. - verify_date_or_time(COURSE_END_TIME_CSS, DEFAULT_TIME) - verify_date_or_time(ENROLLMENT_START_TIME_CSS, DEFAULT_TIME) - verify_date_or_time(ENROLLMENT_END_TIME_CSS, DUMMY_TIME) + i_see_the_set_dates() @step('And I clear all the dates except start$') @@ -71,8 +60,6 @@ def test_and_i_clear_all_the_dates_except_start(step): set_date_or_time(ENROLLMENT_START_DATE_CSS, '') set_date_or_time(ENROLLMENT_END_DATE_CSS, '') - pause() - @step('Then I see cleared dates on refresh$') def test_then_i_see_cleared_dates_on_refresh(step): @@ -119,7 +106,6 @@ def test_i_have_tried_to_clear_the_course_start(step): @step('I have entered a new course start date$') def test_i_have_entered_a_new_course_start_date(step): set_date_or_time(COURSE_START_DATE_CSS, '12/22/2013') - pause() @step('The warning about course start date goes away$') @@ -137,6 +123,30 @@ def test_my_new_course_start_date_is_shown_on_refresh(step): verify_date_or_time(COURSE_START_TIME_CSS, DUMMY_TIME) +@step('I change fields$') +def test_i_change_fields(step): + set_date_or_time(COURSE_START_DATE_CSS, '7/7/7777') + set_date_or_time(COURSE_END_DATE_CSS, '7/7/7777') + set_date_or_time(ENROLLMENT_START_DATE_CSS, '7/7/7777') + set_date_or_time(ENROLLMENT_END_DATE_CSS, '7/7/7777') + + +@step('I do not see the new changes persisted on refresh$') +def test_changes_not_shown_on_refresh(step): + step.then('Then I see the set dates on refresh') + + +@step('I do not see the changes') +def test_i_do_not_see_changes(_step): + i_see_the_set_dates() + + +@step('I change the course overview') +def test_change_course_overview(_step): + type_in_codemirror(0, "

Overview

") + + + ############### HELPER METHODS #################### def set_date_or_time(css, date_or_time): """ @@ -155,9 +165,17 @@ def verify_date_or_time(css, date_or_time): assert_equal(date_or_time, world.css_find(css).first.value) -def pause(): +def i_see_the_set_dates(): """ - Must sleep briefly to allow last time save to finish, - else refresh of browser will fail. + Ensure that each field has the value set in `test_and_i_set_course_dates`. """ - time.sleep(float(1)) + verify_date_or_time(COURSE_START_DATE_CSS, '12/20/2013') + verify_date_or_time(COURSE_END_DATE_CSS, '12/26/2013') + verify_date_or_time(ENROLLMENT_START_DATE_CSS, '12/01/2013') + verify_date_or_time(ENROLLMENT_END_DATE_CSS, '12/10/2013') + + verify_date_or_time(COURSE_START_TIME_CSS, DUMMY_TIME) + # Unset times get set to 12 AM once the corresponding date has been set. + verify_date_or_time(COURSE_END_TIME_CSS, DEFAULT_TIME) + verify_date_or_time(ENROLLMENT_START_TIME_CSS, DEFAULT_TIME) + verify_date_or_time(ENROLLMENT_END_TIME_CSS, DUMMY_TIME) diff --git a/cms/djangoapps/contentstore/features/grading.feature b/cms/djangoapps/contentstore/features/grading.feature index 78634cb964..b01d762d73 100644 --- a/cms/djangoapps/contentstore/features/grading.feature +++ b/cms/djangoapps/contentstore/features/grading.feature @@ -32,6 +32,7 @@ Feature: Course Grading And I have populated the course And I am viewing the grading settings When I change assignment type "Homework" to "New Type" + And I press the "Save" notification button And I go back to the main course page Then I do see the assignment name "New Type" And I do not see the assignment name "Homework" @@ -41,6 +42,7 @@ Feature: Course Grading And I have populated the course And I am viewing the grading settings When I delete the assignment type "Homework" + And I press the "Save" notification button And I go back to the main course page Then I do not see the assignment name "Homework" @@ -49,5 +51,36 @@ Feature: Course Grading And I have populated the course And I am viewing the grading settings When I add a new assignment type "New Type" + And I press the "Save" notification button And I go back to the main course page Then I do see the assignment name "New Type" + + Scenario: Settings are only persisted when saved + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I change assignment type "Homework" to "New Type" + Then I do not see the changes persisted on refresh + + Scenario: Settings are reset on cancel + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I change assignment type "Homework" to "New Type" + And I press the "Cancel" notification button + Then I see the assignment type "Homework" + + Scenario: Confirmation is shown on save + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I change assignment type "Homework" to "New Type" + And I press the "Save" notification button + Then I see a confirmation that my changes have been saved + + Scenario: User cannot save invalid settings + Given I have opened a new course in Studio + And I have populated the course + And I am viewing the grading settings + When I change assignment type "Homework" to "" + Then the save button is disabled diff --git a/cms/djangoapps/contentstore/features/grading.py b/cms/djangoapps/contentstore/features/grading.py index dc41cda30f..e75d8f23ad 100644 --- a/cms/djangoapps/contentstore/features/grading.py +++ b/cms/djangoapps/contentstore/features/grading.py @@ -3,6 +3,7 @@ from lettuce import world, step from common import * +from terrain.steps import reload_the_page @step(u'I am viewing the grading settings') @@ -99,6 +100,22 @@ def populate_course(step): step.given('I have added a new subsection') +@step(u'I do not see the changes persisted on refresh$') +def changes_not_persisted(step): + reload_the_page(step) + name_id = '#course-grading-assignment-name' + ele = world.css_find(name_id)[0] + assert(ele.value == 'Homework') + + +@step(u'I see the assignment type "(.*)"$') +def i_see_the_assignment_type(_step, name): + assignment_css = '#course-grading-assignment-name' + assignments = world.css_find(assignment_css) + types = [ele['value'] for ele in assignments] + assert name in types + + def get_type_index(name): name_id = '#course-grading-assignment-name' f = world.css_find(name_id) diff --git a/cms/static/coffee/spec/views/feedback_spec.coffee b/cms/static/coffee/spec/views/feedback_spec.coffee index 059dd48ef7..1925f890e0 100644 --- a/cms/static/coffee/spec/views/feedback_spec.coffee +++ b/cms/static/coffee/spec/views/feedback_spec.coffee @@ -37,6 +37,10 @@ describe "CMS.Views.SystemFeedback", -> @renderSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'render').andCallThrough() @showSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'show').andCallThrough() @hideSpy = spyOn(CMS.Views.Alert.Confirmation.prototype, 'hide').andCallThrough() + @clock = sinon.useFakeTimers() + + afterEach -> + @clock.restore() it "requires a type and an intent", -> neither = => @@ -80,8 +84,8 @@ describe "CMS.Views.SystemFeedback", -> it "close button sends a .hide() message", -> view = new CMS.Views.Alert.Confirmation(@options).show() view.$(".action-close").click() - expect(@hideSpy).toHaveBeenCalled() + @clock.tick(900) expect(view.$('.wrapper')).toBeHiding() describe "CMS.Views.Prompt", -> diff --git a/cms/static/js/models/settings/advanced.js b/cms/static/js/models/settings/advanced.js index fee89296a6..95fc64f2a0 100644 --- a/cms/static/js/models/settings/advanced.js +++ b/cms/static/js/models/settings/advanced.js @@ -5,8 +5,6 @@ CMS.Models.Settings.Advanced = Backbone.Model.extend({ defaults: { // the properties are whatever the user types in (in addition to whatever comes originally from the server) }, - // which keys to send as the deleted keys on next save - deleteKeys : [], validate: function (attrs) { // Keys can no longer be edited. We are currently not validating values. @@ -18,32 +16,8 @@ CMS.Models.Settings.Advanced = Backbone.Model.extend({ // add saveSuccess to the success var success = options.success; options.success = function(model, resp, options) { - model.afterSave(model); if (success) success(model, resp, options); }; Backbone.Model.prototype.save.call(this, attrs, options); - }, - - afterSave : function(self) { - // remove deleted attrs - if (!_.isEmpty(self.deleteKeys)) { - // remove the to be deleted keys from the returned model - _.each(self.deleteKeys, function(key) { self.unset(key); }); - // not able to do via backbone since we're not destroying the model - $.ajax({ - url : self.url, - // json to and fro - contentType : "application/json", - dataType : "json", - // delete - type : 'DELETE', - // data - data : JSON.stringify({ deleteKeys : self.deleteKeys}) - }) - .done(function(data, status, error) { - // clear deleteKeys on success - self.deleteKeys = []; - }); - } } }); diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js index b71b4e2ab2..993832f830 100644 --- a/cms/static/js/models/settings/course_details.js +++ b/cms/static/js/models/settings/course_details.js @@ -63,13 +63,13 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({ }, _videokey_illegal_chars : /[^a-zA-Z0-9_-]/g, - save_videosource: function(newsource) { + set_videosource: function(newsource) { // newsource either is