Skip to content

E
edx-platform
Commit 7e8fcb85 by cahrens
Options

Updated Selenium test, deleted dead code related to Advanced Settings.

parent 40af08aa
Showing with 97 additions and 229 deletions
cms/djangoapps/contentstore/features/advanced-settings.feature
...@@ -2,33 +2,41 @@ Feature: Advanced (manual) course policy ...@@ -2,33 +2,41 @@ Feature: Advanced (manual) course policy
In order to specify course policy settings for which no custom user interface exists 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 I want to be able to manually enter JSON key/value pairs
# Scenario: A course author sees default advanced settings Scenario: A course author sees default advanced settings
# Given I have opened a new course in Studio Given I have opened a new course in Studio
# When I select the Advanced Settings When I select the Advanced Settings
# Then I see default 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 Scenario: Test cancel editing key value
# Given I am on the Advanced Course Settings page in Studio Given I am on the Advanced Course Settings page in Studio
# When I edit the value of a policy key When I edit the value of a policy key
# And I press the "Cancel" notification button And I press the "Cancel" notification button
# Then the policy key value is unchanged Then the policy key value is unchanged
# And I reload the page
Then the policy key value is unchanged
Scenario: Test editing key value Scenario: Test editing key value
Given I am on the Advanced Course Settings page in Studio Given I am on the Advanced Course Settings page in Studio
When I edit the value of a policy key When I edit the value of a policy key
And I press the "Save" notification button And I press the "Save" notification button
Then the policy key value is changed Then the policy key value is changed
# And I reload the page
# Scenario: Add new entries, and they appear alphabetically after save Then the policy key value is changed
# Given I am on the Advanced Course Settings page in Studio
# When I create New Entries Scenario: Test how multi-line input appears
# Then they are alphabetized Given I am on the Advanced Course Settings page in Studio
# And I reload the page When I create a JSON object as a value
# Then they are alphabetized Then it is displayed as formatted
# And I reload the page
# Scenario: Test how multi-line input appears Then it is displayed as formatted
# Given I am on the Advanced Course Settings page in Studio
# When I create a JSON object Scenario: Test automatic quoting of non-JSON values
# Then it is displayed as formatted 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
cms/djangoapps/contentstore/features/advanced-settings.py
from lettuce import world, step from lettuce import world, step
from common import * from common import *
import time import time
from terrain.steps import reload_the_page
from selenium.common.exceptions import WebDriverException from selenium.common.exceptions import WebDriverException
from selenium.webdriver.support import expected_conditions as EC 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 ...@@ -11,6 +12,10 @@ http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdrive
""" """
from selenium.webdriver.common.keys import Keys 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 #################### ############### ACTIONS ####################
@step('I select the Advanced Settings$') @step('I select the Advanced Settings$')
...@@ -28,32 +33,26 @@ def i_am_on_advanced_course_settings(step): ...@@ -28,32 +33,26 @@ def i_am_on_advanced_course_settings(step):
step.given('I select the Advanced Settings') 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$') @step(u'I press the "([^"]*)" notification button$')
def press_the_notification_button(step, name): def press_the_notification_button(step, name):
def is_visible(driver): def is_visible(driver):
return EC.visibility_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,)) # def is_invisible(driver):
# return EC.invisibility_of_element_located((By.CSS_SELECTOR,css,))
css = 'a.%s-button' % name.lower() css = 'a.%s-button' % name.lower()
wait_for(is_visible) wait_for(is_visible)
time.sleep(float(1))
css_click_at(css)
try: # is_invisible is not returning a boolean, not working
css_click_at(css) # try:
wait_for(is_invisible) # css_click_at(css)
except WebDriverException, e: # wait_for(is_invisible)
css_click_at(css) # except WebDriverException, e:
wait_for(is_invisible) # 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$') @step(u'I edit the value of a policy key$')
...@@ -62,16 +61,18 @@ def edit_the_value_of_a_policy_key(step): ...@@ -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 It is hard to figure out how to get into the CodeMirror
area, so cheat and do it from the policy key field :) area, so cheat and do it from the policy key field :)
""" """
policy_key_css = 'input.policy-key' e = css_find(KEY_CSS)[get_index_of(DISPLAY_NAME_KEY)]
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') 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): def create_JSON_object(step):
create_entry("json", '{"key": "value", "key_2": "value_2"}') change_display_name_value(step, '{"key": "value", "key_2": "value_2"}')
click_save()
@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 #################### ############### RESULTS ####################
...@@ -79,21 +80,32 @@ def create_JSON_object(step): ...@@ -79,21 +80,32 @@ def create_JSON_object(step):
def i_see_default_advanced_settings(step): def i_see_default_advanced_settings(step):
# Test only a few of the existing properties (there are around 34 of them) # Test only a few of the existing properties (there are around 34 of them)
assert_policy_entries( 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): 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$') @step('it is displayed as formatted$')
def it_is_formatted(step): 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$') @step(u'the policy key value is unchanged$')
def the_policy_key_value_is_unchanged(step): 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$') @step(u'the policy key value is changed$')
...@@ -102,36 +114,33 @@ def the_policy_key_value_is_changed(step): ...@@ -102,36 +114,33 @@ def the_policy_key_value_is_changed(step):
############# HELPERS ############### ############# HELPERS ###############
def assert_policy_entries(expected_keys, expected_values, assertLength=True): def assert_policy_entries(expected_keys, expected_values):
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)): for counter in range(len(expected_keys)):
index = get_index_of(expected_keys[counter]) index = get_index_of(expected_keys[counter])
assert_false(index == -1, "Could not find key: " + 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): 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 # 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: if key == expected_key:
return counter return counter
return -1 return -1
def click_save():
css = "a.save-button"
css_click_at(css)
def get_display_name_value(): def get_display_name_value():
policy_value_css = 'textarea.json' index = get_index_of(DISPLAY_NAME_KEY)
index = get_index_of("display_name") return css_find(VALUE_CSS)[index].value
return css_find(policy_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
cms/djangoapps/models/settings/course_metadata.py
...@@ -10,8 +10,7 @@ class CourseMetadata(object): ...@@ -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. 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. 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']
FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', '__new_advanced_key__']
@classmethod @classmethod
def fetch(cls, course_location): def fetch(cls, course_location):
......
cms/static/client_templates/advanced_entry.html
<li class="field-group course-advanced-policy-list-item"> <li class="field-group course-advanced-policy-list-item">
<div class="field is-not-editable text key" id="<%= (_.isEmpty(key) ? '__new_advanced_key__' : key) %>"> <div class="field is-not-editable text key" id="<%= key %>">
<label for="<%= keyUniqueId %>">Policy Key:</label> <label for="<%= keyUniqueId %>">Policy Key:</label>
<input readonly title="This field is disabled: policy keys cannot be edited." type="text" class="short policy-key" id="<%= keyUniqueId %>" value="<%= key %>" /> <input readonly title="This field is disabled: policy keys cannot be edited." type="text" class="short policy-key" id="<%= keyUniqueId %>" value="<%= key %>" />
</div> </div>
......
cms/static/js/models/settings/advanced.js
if (!CMS.Models['Settings']) CMS.Models.Settings = {}; if (!CMS.Models['Settings']) CMS.Models.Settings = {};
CMS.Models.Settings.Advanced = Backbone.Model.extend({ 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: { defaults: {
// the properties are whatever the user types in (in addition to whatever comes originally from the server) // 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({ ...@@ -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] blacklistKeys : [], // an array which the controller should populate directly for now [static not instance based]
validate: function (attrs) { validate: function (attrs) {
var errors = {}; // Keys can no longer be edited. We are currently not validating values.
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;
}, },
save : function (attrs, options) { save : function (attrs, options) {
......
cms/static/js/views/settings/advanced_view.js
...@@ -6,14 +6,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ ...@@ -6,14 +6,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
// Model class is CMS.Models.Settings.Advanced // Model class is CMS.Models.Settings.Advanced
events : { 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", 'focus :input' : "focusInput",
'blur :input' : "blurInput" 'blur :input' : "blurInput"
// TODO enable/disable save based on validation (currently enabled whenever there are changes) // 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({ ...@@ -95,16 +87,11 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
mirror.setValue(stringValue); mirror.setValue(stringValue);
} catch(quotedE) { } catch(quotedE) {
// TODO: validation error // TODO: validation error
console.log("Error with JSON, even after converting to String."); // console.log("Error with JSON, even after converting to String.");
console.log(quotedE); // console.log(quotedE);
JSONValue = undefined; JSONValue = undefined;
} }
} }
else {
// TODO: validation error
console.log("Error with JSON, but will not convert to String.");
console.log(e);
}
} }
if (JSONValue !== undefined) { if (JSONValue !== undefined) {
self.clearValidationErrors(); self.clearValidationErrors();
...@@ -113,7 +100,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ ...@@ -113,7 +100,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
} }
}); });
}, },
showMessage: function (type) { showMessage: function (type) {
this.$el.find(".message-status").removeClass("is-shown"); this.$el.find(".message-status").removeClass("is-shown");
if (type) { if (type) {
...@@ -128,56 +114,19 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ ...@@ -128,56 +114,19 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
else { else {
// This is the case of the page first rendering, or when Cancel is pressed. // This is the case of the page first rendering, or when Cancel is pressed.
this.hideSaveCancelButtons(); this.hideSaveCancelButtons();
this.toggleNewButton(true);
} }
}, },
showSaveCancelButtons: function(event) { showSaveCancelButtons: function(event) {
if (!this.buttonsVisible) { 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"); this.$el.find(".message-status").removeClass("is-shown");
$('.wrapper-notification').addClass('is-shown'); $('.wrapper-notification').addClass('is-shown');
this.buttonsVisible = true; this.buttonsVisible = true;
} }
}, },
hideSaveCancelButtons: function() { hideSaveCancelButtons: function() {
$('.wrapper-notification').removeClass('is-shown'); $('.wrapper-notification').removeClass('is-shown');
this.buttonsVisible = false; 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) { saveView : function(event) {
// TODO one last verification scan: // TODO one last verification scan:
// call validateKey on each to ensure proper format // call validateKey on each to ensure proper format
...@@ -201,102 +150,15 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({ ...@@ -201,102 +150,15 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
error : CMS.ServerError 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) { renderTemplate: function (key, value) {
var newKeyId = _.uniqueId('policy_key_'), var newKeyId = _.uniqueId('policy_key_'),
newEle = this.template({ key : key, value : JSON.stringify(value, null, 4), newEle = this.template({ key : key, value : JSON.stringify(value, null, 4),
keyUniqueId: newKeyId, valueUniqueId: _.uniqueId('policy_value_')}); keyUniqueId: newKeyId, valueUniqueId: _.uniqueId('policy_value_')});
this.fieldToSelectorMap[(_.isEmpty(key) ? this.model.new_key : key)] = newKeyId; this.fieldToSelectorMap[key] = newKeyId;
this.selectorToField[newKeyId] = (_.isEmpty(key) ? this.model.new_key : key); this.selectorToField[newKeyId] = key;
return newEle; return newEle;
}, },
focusInput : function(event) { focusInput : function(event) {
$(event.target).prev().addClass("is-focused"); $(event.target).prev().addClass("is-focused");
}, },
......
cms/templates/settings_advanced.html
...@@ -104,7 +104,7 @@ editor.render(); ...@@ -104,7 +104,7 @@ editor.render();
<i class="ss-icon ss-symbolicons-block icon icon-warning">&#x26A0;</i> <i class="ss-icon ss-symbolicons-block icon icon-warning">&#x26A0;</i>
<p><strong>Note: </strong>Your changes will not take effect until you <strong>save your <p><strong>Note: </strong>Your changes will not take effect until you <strong>save your
progress</strong>. Take care with key and value formatting, as validation is <strong>not implemented</strong>.</p> progress</strong>. Take care with policy value formatting, as validation is <strong>not implemented</strong>.</p>
</div> </div>
<div class="actions"> <div class="actions">
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment