From 72a7ec9e070abfb8ea8c6a8b3b6cc10184287f49 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Mon, 11 Mar 2013 11:31:48 -0400 Subject: [PATCH 01/11] studio - advanced settings: removed new/delete controls from advanced policies and makde key fields readonly/disabled since we're now showing all policy metadata --- cms/static/client_templates/advanced_entry.html | 6 +----- cms/static/sass/_base.scss | 5 +++++ cms/static/sass/_settings.scss | 5 +++++ cms/templates/settings_advanced.html | 6 ------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cms/static/client_templates/advanced_entry.html b/cms/static/client_templates/advanced_entry.html index 0312fdd344..8b1fb57b18 100644 --- a/cms/static/client_templates/advanced_entry.html +++ b/cms/static/client_templates/advanced_entry.html @@ -1,7 +1,7 @@
  • - + Keys are case sensitive and cannot contain spaces or start with a number
    @@ -9,8 +9,4 @@ - -
    - Delete -
  • \ No newline at end of file diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 5d4bc7c773..0e88f89c87 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -405,6 +405,11 @@ textarea.text { @include linear-gradient($paleYellow, tint($paleYellow, 90%)); outline: 0; } + + &[disabled] { + border-color: $gray-l4; + color: $gray-l2; + } } // forms - specific diff --git a/cms/static/sass/_settings.scss b/cms/static/sass/_settings.scss index d8011dd651..5214974498 100644 --- a/cms/static/sass/_settings.scss +++ b/cms/static/sass/_settings.scss @@ -672,6 +672,11 @@ body.course.settings { .key { width: flex-grid(3, 9); margin-right: flex-gutter(); + + // disabled/uneditable style + label { + color: $gray-l2; + } } .value { diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index ceee406398..daf0776a0c 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -67,12 +67,6 @@ editor.render(); - -
    - - New Manual Policy - -
    From 603d11577f0a25cbe353499518f6b42f86291a83 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Mon, 11 Mar 2013 11:40:36 -0400 Subject: [PATCH 02/11] studio - settings: matched up styling/treatment of disabled input fields in schedule/details settings view to new advanced settings UI --- cms/static/sass/_settings.scss | 6 +----- cms/templates/settings.html | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cms/static/sass/_settings.scss b/cms/static/sass/_settings.scss index 5214974498..ca868b248c 100644 --- a/cms/static/sass/_settings.scss +++ b/cms/static/sass/_settings.scss @@ -241,11 +241,7 @@ body.course.settings { .field.is-not-editable { label, .label { - color: $gray-l3; - } - - input { - opacity: 0.25; + color: $gray-l2; } } diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 39e9b70f9d..a79fded9df 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -70,17 +70,17 @@ from contentstore import utils
    1. - +
    2. - +
    3. - +
    These are used in your course URL, and cannot be changed From 82a00104c25d16fc63264bf43ca9fadd3b8262b7 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Mon, 11 Mar 2013 12:24:43 -0400 Subject: [PATCH 03/11] studio - settings: revised markup and styling around disabled/readonly fields --- cms/static/client_templates/advanced_entry.html | 5 ++--- cms/static/sass/_base.scss | 10 ++++++++++ cms/static/sass/_settings.scss | 11 +++-------- cms/templates/settings.html | 6 +++--- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cms/static/client_templates/advanced_entry.html b/cms/static/client_templates/advanced_entry.html index 8b1fb57b18..1dbd99cc8b 100644 --- a/cms/static/client_templates/advanced_entry.html +++ b/cms/static/client_templates/advanced_entry.html @@ -1,8 +1,7 @@
  • -
    +
    - - Keys are case sensitive and cannot contain spaces or start with a number +
    diff --git a/cms/static/sass/_base.scss b/cms/static/sass/_base.scss index 0e88f89c87..a2d46c0510 100644 --- a/cms/static/sass/_base.scss +++ b/cms/static/sass/_base.scss @@ -410,6 +410,16 @@ textarea.text { border-color: $gray-l4; color: $gray-l2; } + + &[readonly] { + border-color: $gray-l4; + color: $gray-l1; + + &:focus { + @include linear-gradient($lightGrey, tint($lightGrey, 90%)); + outline: 0; + } + } } // forms - specific diff --git a/cms/static/sass/_settings.scss b/cms/static/sass/_settings.scss index ca868b248c..a42ff80bc2 100644 --- a/cms/static/sass/_settings.scss +++ b/cms/static/sass/_settings.scss @@ -239,9 +239,9 @@ body.course.settings { // not editable fields .field.is-not-editable { - - label, .label { - color: $gray-l2; + + & label.is-focused { + color: $gray-d2; } } @@ -668,11 +668,6 @@ body.course.settings { .key { width: flex-grid(3, 9); margin-right: flex-gutter(); - - // disabled/uneditable style - label { - color: $gray-l2; - } } .value { diff --git a/cms/templates/settings.html b/cms/templates/settings.html index a79fded9df..b26a17125b 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -70,17 +70,17 @@ from contentstore import utils
    1. - +
    2. - +
    3. - +
    These are used in your course URL, and cannot be changed From 40af08aa2d6d65531eeb9b1e6d0f1c2e09dba26d Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 11 Mar 2013 14:01:45 -0400 Subject: [PATCH 04/11] Saving tests in progress --- .../features/advanced-settings.feature | 64 +++------ .../features/advanced-settings.py | 136 +++++------------- 2 files changed, 60 insertions(+), 140 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 779d44e4b2..c35fe1ed13 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -2,53 +2,33 @@ Feature: Advanced (manual) course policy In order to specify course policy settings for which no custom user interface exists I want to be able to manually enter JSON key/value pairs - Scenario: A course author sees only display_name on a newly created course - Given I have opened a new course in Studio - When I select the Advanced Settings - Then I see only the display name +# Scenario: A course author sees default advanced settings +# Given I have opened a new course in Studio +# When I select the Advanced Settings +# Then I see default advanced settings - @skip-phantom - Scenario: Test if there are no policy settings without existing UI controls - Given I am on the Advanced Course Settings page in Studio - When I delete the display name - Then there are no advanced policy settings - And I reload the page - Then there are no advanced policy settings - @skip-phantom - Scenario: Test cancel editing key name - Given I am on the Advanced Course Settings page in Studio - When I edit the name of a policy key - And I press the "Cancel" notification button - Then the policy key name is unchanged +# Scenario: Test cancel editing key value +# Given I am on the Advanced Course Settings page in Studio +# When I edit the value of a policy key +# And I press the "Cancel" notification button +# Then the policy key value is unchanged +# - Scenario: Test editing key name - Given I am on the Advanced Course Settings page in Studio - When I edit the name of a policy key - And I press the "Save" notification button - Then the policy key name is changed - - Scenario: Test cancel editing key value - Given I am on the Advanced Course Settings page in Studio - When I edit the value of a policy key - And I press the "Cancel" notification button - Then the policy key value is unchanged - - @skip-phantom Scenario: Test editing key value 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 the policy key value is changed - - Scenario: Add new entries, and they appear alphabetically after save - Given I am on the Advanced Course Settings page in Studio - When I create New Entries - Then they are alphabetized - And I reload the page - Then they are alphabetized - - Scenario: Test how multi-line input appears - Given I am on the Advanced Course Settings page in Studio - When I create a JSON object - Then it is displayed as formatted +# +# Scenario: Add new entries, and they appear alphabetically after save +# Given I am on the Advanced Course Settings page in Studio +# When I create New Entries +# Then they are alphabetized +# And I reload the page +# Then they are alphabetized +# +# Scenario: Test how multi-line input appears +# Given I am on the Advanced Course Settings page in Studio +# When I create a JSON object +# Then it is displayed as formatted diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 1024579b45..ff44ecebfe 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -20,7 +20,6 @@ def i_select_advanced_settings(step): css_click(expand_icon_css) link_css = 'li.nav-course-settings-advanced a' css_click(link_css) - # world.browser.click_link_by_text('Advanced Settings') @step('I am on the Advanced Course Settings page in Studio$') @@ -35,13 +34,6 @@ def reload_the_page(step): world.browser.reload() -@step(u'I edit the name of a policy key$') -def edit_the_name_of_a_policy_key(step): - policy_key_css = 'input.policy-key' - e = css_find(policy_key_css).first - e.type('_new') - - @step(u'I press the "([^"]*)" notification button$') def press_the_notification_button(step, name): def is_visible(driver): @@ -59,6 +51,11 @@ def press_the_notification_button(step, name): css_click_at(css) wait_for(is_invisible) + if name == "Save": + css = "" + wait_for(is_visible) + + @step(u'I edit the value of a policy key$') def edit_the_value_of_a_policy_key(step): """ @@ -66,23 +63,11 @@ def edit_the_value_of_a_policy_key(step): area, so cheat and do it from the policy key field :) """ policy_key_css = 'input.policy-key' - e = css_find(policy_key_css).first + index = get_index_of("display_name") + e = css_find(policy_key_css)[index] e._element.send_keys(Keys.TAB, Keys.END, Keys.ARROW_LEFT, ' ', 'X') -@step('I delete the display name$') -def delete_the_display_name(step): - delete_entry(0) - click_save() - - -@step('create New Entries$') -def create_new_entries(step): - create_entry("z", "apple") - create_entry("a", "zebra") - click_save() - - @step('I create a JSON object$') def create_JSON_object(step): create_entry("json", '{"key": "value", "key_2": "value_2"}') @@ -90,19 +75,11 @@ def create_JSON_object(step): ############### RESULTS #################### -@step('I see only the display name$') -def i_see_only_display_name(step): - assert_policy_entries(["display_name"], ['"Robot Super Course"']) - - -@step('there are no advanced policy settings$') -def no_policy_settings(step): - keys_css = 'input.policy-key' - val_css = 'textarea.json' - k = world.browser.is_element_not_present_by_css(keys_css, 5) - v = world.browser.is_element_not_present_by_css(val_css, 5) - assert_true(k) - assert_true(v) +@step('I see default advanced settings$') +def i_see_default_advanced_settings(step): + # Test only a few of the existing properties (there are around 34 of them) + assert_policy_entries( + ["advanced_modules", "display_name", "show_calculator"], ["[]", '"Robot Super Course"', "false"], False) @step('they are alphabetized$') @@ -114,77 +91,39 @@ def they_are_alphabetized(step): def it_is_formatted(step): assert_policy_entries(["display_name", "json"], ['"Robot Super Course"', '{\n "key": "value",\n "key_2": "value_2"\n}']) - -@step(u'the policy key name is unchanged$') -def the_policy_key_name_is_unchanged(step): - policy_key_css = 'input.policy-key' - val = css_find(policy_key_css).first.value - assert_equal(val, 'display_name') - - -@step(u'the policy key name is changed$') -def the_policy_key_name_is_changed(step): - policy_key_css = 'input.policy-key' - val = css_find(policy_key_css).first.value - assert_equal(val, 'display_name_new') - - @step(u'the policy key value is unchanged$') def the_policy_key_value_is_unchanged(step): - policy_value_css = 'li.course-advanced-policy-list-item div.value textarea' - val = css_find(policy_value_css).first.value - assert_equal(val, '"Robot Super Course"') + assert_equal(get_display_name_value(), '"Robot Super Course"') @step(u'the policy key value is changed$') -def the_policy_key_value_is_unchanged(step): - policy_value_css = 'li.course-advanced-policy-list-item div.value textarea' - val = css_find(policy_value_css).first.value - assert_equal(val, '"Robot Super Course X"') +def the_policy_key_value_is_changed(step): + assert_equal(get_display_name_value(), '"Robot Super Course X"') ############# HELPERS ############### -def create_entry(key, value): - # Scroll down the page so the button is visible - world.scroll_to_bottom() - css_click_at('a.new-advanced-policy-item', 10, 10) - new_key_css = 'div#__new_advanced_key__ input' - new_key_element = css_find(new_key_css).first - new_key_element.fill(key) -# For some reason have to get the instance for each command -# (get error that it is no longer attached to the DOM) -# Have to do all this because Selenium fill does not remove existing text - new_value_css = 'div.CodeMirror textarea' - css_find(new_value_css).last.fill("") - css_find(new_value_css).last._element.send_keys(Keys.DELETE, Keys.DELETE) - css_find(new_value_css).last.fill(value) - # Add in a TAB key press because intermittently on ubuntu the - # last character of "value" above was not getting typed in - css_find(new_value_css).last._element.send_keys(Keys.TAB) +def assert_policy_entries(expected_keys, expected_values, assertLength=True): + key_css = '.key input.policy-key' + key_elements = css_find(key_css) + if assertLength: + assert_equal(len(expected_keys), len(key_elements)) + + value_css = 'textarea.json' + for counter in range(len(expected_keys)): + index = get_index_of(expected_keys[counter]) + assert_false(index == -1, "Could not find key: " + expected_keys[counter]) + assert_equal(expected_values[counter], css_find(value_css)[index].value, "value is incorrect") -def delete_entry(index): - """ - Delete the nth entry where index is 0-based - """ - css = 'a.delete-button' - assert_true(world.browser.is_element_present_by_css(css, 5)) - delete_buttons = css_find(css) - assert_true(len(delete_buttons) > index, "no delete button exists for entry " + str(index)) - delete_buttons[index].click() +def get_index_of(expected_key): + key_css = '.key input.policy-key' + for counter in range(len(css_find(key_css))): + # Sometimes get stale reference if I hold on to the array of elements + key = css_find(key_css)[counter].value + if key == expected_key: + return counter - -def assert_policy_entries(expected_keys, expected_values): - assert_entries('.key input.policy-key', expected_keys) - assert_entries('textarea.json', expected_values) - - -def assert_entries(css, expected_values): - webElements = css_find(css) - assert_equal(len(expected_values), len(webElements)) -# Sometimes get stale reference if I hold on to the array of elements - for counter in range(len(expected_values)): - assert_equal(expected_values[counter], css_find(css)[counter].value) + return -1 def click_save(): @@ -192,6 +131,7 @@ def click_save(): css_click_at(css) -def fill_last_field(value): - newValue = css_find('#__new_advanced_key__ input').first - newValue.fill(value) +def get_display_name_value(): + policy_value_css = 'textarea.json' + index = get_index_of("display_name") + return css_find(policy_value_css)[index].value From 7e8fcb85ff74136c4889cb0f18b630d8f66dc9b6 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 11 Mar 2013 16:56:24 -0400 Subject: [PATCH 05/11] Updated Selenium test, deleted dead code related to Advanced Settings. --- .../features/advanced-settings.feature | 52 ++++--- .../features/advanced-settings.py | 107 +++++++------ .../models/settings/course_metadata.py | 3 +- .../client_templates/advanced_entry.html | 2 +- cms/static/js/models/settings/advanced.js | 13 +- cms/static/js/views/settings/advanced_view.js | 146 +----------------- cms/templates/settings_advanced.html | 2 +- 7 files changed, 96 insertions(+), 229 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index c35fe1ed13..af97709ad0 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -2,33 +2,41 @@ Feature: Advanced (manual) course policy In order to specify course policy settings for which no custom user interface exists I want to be able to manually enter JSON key/value pairs -# Scenario: A course author sees default advanced settings -# Given I have opened a new course in Studio -# When I select the Advanced Settings -# Then I see default advanced settings + Scenario: A course author sees default advanced settings + Given I have opened a new course in Studio + When I select the Advanced Settings + Then I see default advanced settings + Scenario: Add new entries, and they appear alphabetically after save + Given I am on the Advanced Course Settings page in Studio + Then the settings are alphabetized -# Scenario: Test cancel editing key value -# Given I am on the Advanced Course Settings page in Studio -# When I edit the value of a policy key -# And I press the "Cancel" notification button -# Then the policy key value is unchanged -# + Scenario: Test cancel editing key value + Given I am on the Advanced Course Settings page in Studio + When I edit the value of a policy key + And I press the "Cancel" notification button + Then the policy key value is unchanged + And I reload the page + Then the policy key value is unchanged Scenario: Test editing key value 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 the policy key value is changed -# -# Scenario: Add new entries, and they appear alphabetically after save -# Given I am on the Advanced Course Settings page in Studio -# When I create New Entries -# Then they are alphabetized -# And I reload the page -# Then they are alphabetized -# -# Scenario: Test how multi-line input appears -# Given I am on the Advanced Course Settings page in Studio -# When I create a JSON object -# Then it is displayed as formatted + And I reload the page + Then the policy key value is changed + + Scenario: Test how multi-line input appears + Given I am on the Advanced Course Settings page in Studio + When I create a JSON object as a value + Then it is displayed as formatted + And I reload the page + Then it is displayed as formatted + + Scenario: Test automatic quoting of non-JSON values + Given I am on the Advanced Course Settings page in Studio + When I create a non-JSON value not in quotes + Then it is displayed as a string + And I reload the page + Then it is displayed as a string diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index ff44ecebfe..7e86e94a31 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -1,6 +1,7 @@ from lettuce import world, step from common import * import time +from terrain.steps import reload_the_page from selenium.common.exceptions import WebDriverException from selenium.webdriver.support import expected_conditions as EC @@ -11,6 +12,10 @@ http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdrive """ from selenium.webdriver.common.keys import Keys +KEY_CSS = '.key input.policy-key' +VALUE_CSS = 'textarea.json' +DISPLAY_NAME_KEY = "display_name" +DISPLAY_NAME_VALUE = '"Robot Super Course"' ############### ACTIONS #################### @step('I select the Advanced Settings$') @@ -28,32 +33,26 @@ def i_am_on_advanced_course_settings(step): step.given('I select the Advanced Settings') -# TODO: this is copied from terrain's step.py. Need to figure out how to share that code. -@step('I reload the page$') -def reload_the_page(step): - world.browser.reload() - - @step(u'I press the "([^"]*)" notification button$') def press_the_notification_button(step, name): def is_visible(driver): - return EC.visibility_of_element_located((By.CSS_SELECTOR,css,)) - def is_invisible(driver): - return EC.invisibility_of_element_located((By.CSS_SELECTOR,css,)) + return EC.visibility_of_element_located((By.CSS_SELECTOR, css,)) + + # def is_invisible(driver): + # return EC.invisibility_of_element_located((By.CSS_SELECTOR,css,)) css = 'a.%s-button' % name.lower() wait_for(is_visible) + time.sleep(float(1)) + css_click_at(css) - try: - css_click_at(css) - wait_for(is_invisible) - except WebDriverException, e: - css_click_at(css) - wait_for(is_invisible) - - if name == "Save": - css = "" - wait_for(is_visible) +# is_invisible is not returning a boolean, not working +# try: +# css_click_at(css) +# wait_for(is_invisible) +# except WebDriverException, e: +# css_click_at(css) +# wait_for(is_invisible) @step(u'I edit the value of a policy key$') @@ -62,16 +61,18 @@ def edit_the_value_of_a_policy_key(step): It is hard to figure out how to get into the CodeMirror area, so cheat and do it from the policy key field :) """ - policy_key_css = 'input.policy-key' - index = get_index_of("display_name") - e = css_find(policy_key_css)[index] + e = css_find(KEY_CSS)[get_index_of(DISPLAY_NAME_KEY)] e._element.send_keys(Keys.TAB, Keys.END, Keys.ARROW_LEFT, ' ', 'X') -@step('I create a JSON object$') +@step('I create a JSON object as a value$') def create_JSON_object(step): - create_entry("json", '{"key": "value", "key_2": "value_2"}') - click_save() + change_display_name_value(step, '{"key": "value", "key_2": "value_2"}') + + +@step('I create a non-JSON value not in quotes$') +def create_value_not_in_quotes(step): + change_display_name_value(step, 'quote me') ############### RESULTS #################### @@ -79,21 +80,32 @@ def create_JSON_object(step): def i_see_default_advanced_settings(step): # Test only a few of the existing properties (there are around 34 of them) assert_policy_entries( - ["advanced_modules", "display_name", "show_calculator"], ["[]", '"Robot Super Course"', "false"], False) + ["advanced_modules", DISPLAY_NAME_KEY, "show_calculator"], ["[]", DISPLAY_NAME_VALUE, "false"]) -@step('they are alphabetized$') +@step('the settings are alphabetized$') def they_are_alphabetized(step): - assert_policy_entries(["a", "display_name", "z"], ['"zebra"', '"Robot Super Course"', '"apple"']) + key_elements = css_find(KEY_CSS) + all_keys = [] + for key in key_elements: + all_keys.append(key.value) + + assert_equal(sorted(all_keys), all_keys, "policy keys were not sorted") @step('it is displayed as formatted$') def it_is_formatted(step): - assert_policy_entries(["display_name", "json"], ['"Robot Super Course"', '{\n "key": "value",\n "key_2": "value_2"\n}']) + assert_policy_entries([DISPLAY_NAME_KEY], ['{\n "key": "value",\n "key_2": "value_2"\n}']) + + +@step('it is displayed as a string') +def it_is_formatted(step): + assert_policy_entries([DISPLAY_NAME_KEY], ['"quote me"']) + @step(u'the policy key value is unchanged$') def the_policy_key_value_is_unchanged(step): - assert_equal(get_display_name_value(), '"Robot Super Course"') + assert_equal(get_display_name_value(), DISPLAY_NAME_VALUE) @step(u'the policy key value is changed$') @@ -102,36 +114,33 @@ def the_policy_key_value_is_changed(step): ############# HELPERS ############### -def assert_policy_entries(expected_keys, expected_values, assertLength=True): - key_css = '.key input.policy-key' - key_elements = css_find(key_css) - if assertLength: - assert_equal(len(expected_keys), len(key_elements)) - - value_css = 'textarea.json' +def assert_policy_entries(expected_keys, expected_values): for counter in range(len(expected_keys)): index = get_index_of(expected_keys[counter]) assert_false(index == -1, "Could not find key: " + expected_keys[counter]) - assert_equal(expected_values[counter], css_find(value_css)[index].value, "value is incorrect") + assert_equal(expected_values[counter], css_find(VALUE_CSS)[index].value, "value is incorrect") def get_index_of(expected_key): - key_css = '.key input.policy-key' - for counter in range(len(css_find(key_css))): + for counter in range(len(css_find(KEY_CSS))): # Sometimes get stale reference if I hold on to the array of elements - key = css_find(key_css)[counter].value + key = css_find(KEY_CSS)[counter].value if key == expected_key: return counter return -1 -def click_save(): - css = "a.save-button" - css_click_at(css) - - def get_display_name_value(): - policy_value_css = 'textarea.json' - index = get_index_of("display_name") - return css_find(policy_value_css)[index].value + index = get_index_of(DISPLAY_NAME_KEY) + return css_find(VALUE_CSS)[index].value + + +def change_display_name_value(step, new_value): + e = css_find(KEY_CSS)[get_index_of(DISPLAY_NAME_KEY)] + display_name = get_display_name_value() + for count in range(len(display_name)): + e._element.send_keys(Keys.TAB, Keys.END, Keys.BACK_SPACE) + # Must delete "" before typing the JSON value + e._element.send_keys(Keys.TAB, Keys.END, Keys.BACK_SPACE, Keys.BACK_SPACE, new_value) + press_the_notification_button(step, "Save") \ No newline at end of file diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 3972751082..ed11a6d7a4 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -10,8 +10,7 @@ class CourseMetadata(object): For CRUD operations on metadata fields which do not have specific editors on the other pages including any user generated ones. The objects have no predefined attrs but instead are obj encodings of the editable metadata. ''' - # __new_advanced_key__ is used by client not server; so, could argue against it being here - FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', '__new_advanced_key__'] + FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod'] @classmethod def fetch(cls, course_location): diff --git a/cms/static/client_templates/advanced_entry.html b/cms/static/client_templates/advanced_entry.html index 1dbd99cc8b..6be22e2116 100644 --- a/cms/static/client_templates/advanced_entry.html +++ b/cms/static/client_templates/advanced_entry.html @@ -1,5 +1,5 @@
  • -
    +
    diff --git a/cms/static/js/models/settings/advanced.js b/cms/static/js/models/settings/advanced.js index 5741eb88dd..c1707220df 100644 --- a/cms/static/js/models/settings/advanced.js +++ b/cms/static/js/models/settings/advanced.js @@ -1,8 +1,6 @@ if (!CMS.Models['Settings']) CMS.Models.Settings = {}; CMS.Models.Settings.Advanced = Backbone.Model.extend({ - // the key for a newly added policy-- before the user has entered a key value - new_key : "__new_advanced_key__", defaults: { // the properties are whatever the user types in (in addition to whatever comes originally from the server) @@ -12,16 +10,7 @@ CMS.Models.Settings.Advanced = Backbone.Model.extend({ blacklistKeys : [], // an array which the controller should populate directly for now [static not instance based] validate: function (attrs) { - var errors = {}; - for (var key in attrs) { - if (key === this.new_key || _.isEmpty(key)) { - errors[key] = "A key must be entered."; - } - else if (_.contains(this.blacklistKeys, key)) { - errors[key] = key + " is a reserved keyword or can be edited on another screen"; - } - } - if (!_.isEmpty(errors)) return errors; + // Keys can no longer be edited. We are currently not validating values. }, save : function (attrs, options) { diff --git a/cms/static/js/views/settings/advanced_view.js b/cms/static/js/views/settings/advanced_view.js index a933bbdb9b..e3ff098efb 100644 --- a/cms/static/js/views/settings/advanced_view.js +++ b/cms/static/js/views/settings/advanced_view.js @@ -6,14 +6,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ // Model class is CMS.Models.Settings.Advanced events : { - 'click .delete-button' : "deleteEntry", - 'click .new-button' : "addEntry", - // update model on changes - 'change .policy-key' : "updateKey", - // keypress to catch alpha keys and backspace/delete on some browsers - 'keypress .policy-key' : "showSaveCancelButtons", - // keyup to catch backspace/delete reliably - 'keyup .policy-key' : "showSaveCancelButtons", 'focus :input' : "focusInput", 'blur :input' : "blurInput" // TODO enable/disable save based on validation (currently enabled whenever there are changes) @@ -95,16 +87,11 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ mirror.setValue(stringValue); } catch(quotedE) { // TODO: validation error - console.log("Error with JSON, even after converting to String."); - console.log(quotedE); + // console.log("Error with JSON, even after converting to String."); + // console.log(quotedE); JSONValue = undefined; } } - else { - // TODO: validation error - console.log("Error with JSON, but will not convert to String."); - console.log(e); - } } if (JSONValue !== undefined) { self.clearValidationErrors(); @@ -113,7 +100,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ } }); }, - showMessage: function (type) { this.$el.find(".message-status").removeClass("is-shown"); if (type) { @@ -128,56 +114,19 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ else { // This is the case of the page first rendering, or when Cancel is pressed. this.hideSaveCancelButtons(); - this.toggleNewButton(true); } }, - showSaveCancelButtons: function(event) { if (!this.buttonsVisible) { - if (event && (event.type === 'keypress' || event.type === 'keyup')) { - // check whether it's really an altering event: note, String.fromCharCode(keyCode) will - // give positive values for control/command/option-letter combos; so, don't use it - if (!((event.charCode && String.fromCharCode(event.charCode) !== "") || - // 8 = backspace, 46 = delete - event.keyCode === 8 || event.keyCode === 46)) return; - } this.$el.find(".message-status").removeClass("is-shown"); $('.wrapper-notification').addClass('is-shown'); this.buttonsVisible = true; } }, - hideSaveCancelButtons: function() { $('.wrapper-notification').removeClass('is-shown'); this.buttonsVisible = false; }, - - toggleNewButton: function (enable) { - var newButton = this.$el.find(".new-button"); - if (enable) { - newButton.removeClass('disabled'); - } - else { - newButton.addClass('disabled'); - } - }, - - deleteEntry : function(event) { - event.preventDefault(); - // find out which entry - var li$ = $(event.currentTarget).closest('li'); - // Not data b/c the validation view uses it for a selector - var key = $('.key', li$).attr('id'); - - delete this.selectorToField[this.fieldToSelectorMap[key]]; - delete this.fieldToSelectorMap[key]; - if (key !== this.model.new_key) { - this.model.deleteKeys.push(key); - this.model.unset(key); - } - li$.remove(); - this.showSaveCancelButtons(); - }, saveView : function(event) { // TODO one last verification scan: // call validateKey on each to ensure proper format @@ -201,102 +150,15 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ error : CMS.ServerError }); }, - addEntry : function() { - var listEle$ = this.$el.find('.course-advanced-policy-list'); - var newEle = this.renderTemplate("", ""); - listEle$.append(newEle); - // need to re-find b/c replaceWith seems to copy rather than use the specific ele instance - var policyValueDivs = this.$el.find('#' + this.model.new_key).closest('li').find('.json'); - // only 1 but hey, let's take advantage of the context mechanism - _.each(policyValueDivs, this.attachJSONEditor, this); - this.toggleNewButton(false); - }, - updateKey : function(event) { - var parentElement = $(event.currentTarget).closest('.key'); - // old key: either the key as in the model or new_key. - // That is, it doesn't change as the val changes until val is accepted. - var oldKey = parentElement.attr('id'); - // TODO: validation of keys with spaces. For now at least trim strings to remove initial and - // trailing whitespace - var newKey = $.trim($(event.currentTarget).val()); - if (oldKey !== newKey) { - // TODO: is it OK to erase other validation messages? - this.clearValidationErrors(); - - if (!this.validateKey(oldKey, newKey)) return; - - if (this.model.has(newKey)) { - var error = {}; - error[oldKey] = 'You have already defined "' + newKey + '" in the manual policy definitions.'; - error[newKey] = "You tried to enter a duplicate of this key."; - this.model.trigger("invalid", this.model, error); - return false; - } - - // explicitly call validate to determine whether to proceed (relying on triggered error means putting continuation in the success - // method which is uglier I think?) - var newEntryModel = {}; - // set the new key's value to the old one's - newEntryModel[newKey] = (oldKey === this.model.new_key ? '' : this.model.get(oldKey)); - - var validation = this.model.validate(newEntryModel); - if (validation) { - if (_.has(validation, newKey)) { - // swap to the key which the map knows about - validation[oldKey] = validation[newKey]; - } - this.model.trigger("invalid", this.model, validation); - // abandon update - return; - } - - // Now safe to actually do the update - this.model.set(newEntryModel); - - // update maps - var selector = this.fieldToSelectorMap[oldKey]; - this.selectorToField[selector] = newKey; - this.fieldToSelectorMap[newKey] = selector; - delete this.fieldToSelectorMap[oldKey]; - - if (oldKey !== this.model.new_key) { - // mark the old key for deletion and delete from field maps - this.model.deleteKeys.push(oldKey); - this.model.unset(oldKey) ; - } - else { - // id for the new entry will now be the key value. Enable new entry button. - this.toggleNewButton(true); - } - - // check for newkey being the name of one which was previously deleted in this session - var wasDeleting = this.model.deleteKeys.indexOf(newKey); - if (wasDeleting >= 0) { - this.model.deleteKeys.splice(wasDeleting, 1); - } - - // Update the ID to the new value. - parentElement.attr('id', newKey); - - } - }, - validateKey : function(oldKey, newKey) { - // model validation can't handle malformed keys nor notice if 2 fields have same key; so, need to add that chk here - // TODO ensure there's no spaces or illegal chars (note some checking for spaces currently done in model's - // validate method. - return true; - }, - renderTemplate: function (key, value) { var newKeyId = _.uniqueId('policy_key_'), newEle = this.template({ key : key, value : JSON.stringify(value, null, 4), keyUniqueId: newKeyId, valueUniqueId: _.uniqueId('policy_value_')}); - this.fieldToSelectorMap[(_.isEmpty(key) ? this.model.new_key : key)] = newKeyId; - this.selectorToField[newKeyId] = (_.isEmpty(key) ? this.model.new_key : key); + this.fieldToSelectorMap[key] = newKeyId; + this.selectorToField[newKeyId] = key; return newEle; }, - focusInput : function(event) { $(event.target).prev().addClass("is-focused"); }, diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index daf0776a0c..ada5205aae 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -104,7 +104,7 @@ editor.render();

    Note: Your changes will not take effect until you save your - progress. Take care with key and value formatting, as validation is not implemented.

    + progress. Take care with policy value formatting, as validation is not implemented.

    From 99f55266f58dd89e5df1ca9ab82e90c774715ce5 Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 12 Mar 2013 09:27:46 -0400 Subject: [PATCH 06/11] Do not need to pass blacklist keys to client. --- cms/djangoapps/contentstore/views.py | 1 - cms/static/js/models/settings/advanced.js | 1 - cms/static/js/template_loader.js | 2 +- cms/templates/settings_advanced.html | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 358c8e201c..59b08594ba 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -1200,7 +1200,6 @@ def course_config_advanced_page(request, org, course, name): return render_to_response('settings_advanced.html', { 'context_course': course_module, 'course_location' : location, - 'advanced_blacklist' : json.dumps(CourseMetadata.FILTERED_LIST), 'advanced_dict' : json.dumps(CourseMetadata.fetch(location)), }) diff --git a/cms/static/js/models/settings/advanced.js b/cms/static/js/models/settings/advanced.js index c1707220df..adc259239d 100644 --- a/cms/static/js/models/settings/advanced.js +++ b/cms/static/js/models/settings/advanced.js @@ -7,7 +7,6 @@ CMS.Models.Settings.Advanced = Backbone.Model.extend({ }, // which keys to send as the deleted keys on next save deleteKeys : [], - blacklistKeys : [], // an array which the controller should populate directly for now [static not instance based] validate: function (attrs) { // Keys can no longer be edited. We are currently not validating values. diff --git a/cms/static/js/template_loader.js b/cms/static/js/template_loader.js index 3492ca677a..f64c685364 100644 --- a/cms/static/js/template_loader.js +++ b/cms/static/js/template_loader.js @@ -5,7 +5,7 @@ if (typeof window.templateLoader == 'function') return; var templateLoader = { - templateVersion: "0.0.15", + templateVersion: "0.0.16", templates: {}, loadRemoteTemplate: function(templateName, filename, callback) { if (!this.templates[templateName]) { diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index ada5205aae..f592baee89 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -21,7 +21,6 @@ $(document).ready(function () { // proactively populate advanced b/c it has the filtered list and doesn't really follow the model pattern var advancedModel = new CMS.Models.Settings.Advanced(${advanced_dict | n}, {parse: true}); - advancedModel.blacklistKeys = ${advanced_blacklist | n}; advancedModel.url = "${reverse('course_advanced_settings_updates', kwargs=dict(org=context_course.location.org, course=context_course.location.course, name=context_course.location.name))}"; var editor = new CMS.Views.Settings.Advanced({ From eea59005918848603ededf7bc89e6b4fb4da77db Mon Sep 17 00:00:00 2001 From: cahrens Date: Tue, 12 Mar 2013 11:15:50 -0400 Subject: [PATCH 07/11] Add in Mark's copy changes. --- cms/templates/settings_advanced.html | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index f592baee89..838af5ada9 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -60,8 +60,7 @@ editor.render(); Manually Edit Course Policy Values (JSON Key / Value pairs) -

    Warning: Add only manual policy data that you are familiar - with.

    +

    Warning: Do not modify these policies unless you are familiar with their purpose.

      @@ -73,9 +72,9 @@ editor.render();