Skip to content

E
edx-platform
Commit 42c40718 by Andy Armstrong
Options

Implement better select handling and tests

parent 54a8a4fc
Showing with 126 additions and 57 deletions
common/test/acceptance/pages/lms/instructor_dashboard.py
...@@ -210,7 +210,7 @@ class MembershipPageCohortManagementSection(PageObject): ...@@ -210,7 +210,7 @@ class MembershipPageCohortManagementSection(PageObject):
""" """
selector_query = self.q(css=self._bounded_selector(self.content_group_selector_css)) selector_query = self.q(css=self._bounded_selector(self.content_group_selector_css))
return [ return [
option.text for option in get_options(selector_query) if option.text != "" option.text for option in get_options(selector_query) if option.text != "Not selected"
] ]
def get_cohort_associated_content_group(self): def get_cohort_associated_content_group(self):
......
lms/static/js/groups/views/cohort_form.js
...@@ -55,10 +55,14 @@ var edx = edx || {}; ...@@ -55,10 +55,14 @@ var edx = edx || {};
this.$('.input-cohort-group-association').prop('disabled', !groupsEnabled); this.$('.input-cohort-group-association').prop('disabled', !groupsEnabled);
}, },
hasAssociatedContentGroup: function() {
return this.$('.radio-yes').prop('checked');
},
getSelectedContentGroup: function() { getSelectedContentGroup: function() {
var selectValue = this.$('.input-cohort-group-association').val(), var selectValue = this.$('.input-cohort-group-association').val(),
ids, groupId, userPartitionId, i, contentGroup; ids, groupId, userPartitionId, i, contentGroup;
if (!this.$('.radio-yes').prop('checked') || selectValue === 'None') { if (!this.hasAssociatedContentGroup() || selectValue === 'None') {
return null; return null;
} }
ids = selectValue.split(':'); ids = selectValue.split(':');
...@@ -78,45 +82,61 @@ var edx = edx || {}; ...@@ -78,45 +82,61 @@ var edx = edx || {};
return cohortName ? cohortName.trim() : this.model.get('name'); return cohortName ? cohortName.trim() : this.model.get('name');
}, },
showMessage: function(message, type) { showMessage: function(message, type, details) {
this.showNotification( this.showNotification(
{type: type || 'confirmation', title: message}, {type: type || 'confirmation', title: message, details: details},
this.$('.form-fields') this.$('.form-fields')
); );
}, },
validate: function(fieldData) {
var errorMessages;
errorMessages = [];
if (!fieldData.name) {
errorMessages.push(gettext('You must specify a name for the cohort group'));
}
if (this.hasAssociatedContentGroup() && fieldData.group_id === null) {
if (this.$('.input-cohort-group-association').val() === 'None') {
errorMessages.push(gettext('You did not select a cohorted content group'));
} else {
// If a value was selected, then it must be for a non-existent/deleted content group
errorMessages.push(gettext('The selected cohorted content group does not exist'));
}
}
return errorMessages;
},
saveForm: function() { saveForm: function() {
var self = this, var self = this,
cohort = this.model, cohort = this.model,
saveOperation = $.Deferred(), saveOperation = $.Deferred(),
isUpdate = this.model.id !== null, isUpdate = !_.isUndefined(this.model.id),
cohortName, selectedContentGroup, showErrorMessage; fieldData, selectedContentGroup, errorMessages, showErrorMessage;
showErrorMessage = function(message, details) {
self.showMessage(message, 'error', details);
};
this.removeNotification(); this.removeNotification();
showErrorMessage = function(message) { selectedContentGroup = this.getSelectedContentGroup();
self.showMessage(message, 'error'); fieldData = {
name: this.getUpdatedCohortName(),
group_id: selectedContentGroup ? selectedContentGroup.id : null,
user_partition_id: selectedContentGroup ? selectedContentGroup.get('user_partition_id') : null
}; };
cohortName = this.getUpdatedCohortName(); errorMessages = this.validate(fieldData);
if (cohortName.length === 0) { if (errorMessages.length > 0) {
showErrorMessage(gettext('Enter a name for your cohort group.')); showErrorMessage(
isUpdate ? gettext("The cohort group cannot be saved")
: gettext("The cohort group cannot be added"),
errorMessages
);
saveOperation.reject(); saveOperation.reject();
} else { } else {
selectedContentGroup = this.getSelectedContentGroup();
cohort.save( cohort.save(
{ fieldData, {patch: isUpdate}
name: cohortName,
group_id: selectedContentGroup ? selectedContentGroup.id : null,
user_partition_id: selectedContentGroup ? selectedContentGroup.get('user_partition_id') : null
},
{patch: isUpdate}
).done(function(result) { ).done(function(result) {
if (!result.error) { cohort.id = result.id;
cohort.id = result.id; self.render(); // re-render to remove any now invalid error messages
self.render(); // re-render to remove any now invalid error messages saveOperation.resolve();
saveOperation.resolve();
} else {
showErrorMessage(result.error);
saveOperation.reject();
}
}).fail(function(result) { }).fail(function(result) {
var errorMessage = null; var errorMessage = null;
try { try {
......
lms/static/js/spec/groups/views/cohorts_spec.js
...@@ -7,7 +7,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -7,7 +7,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
var catLoversInitialCount = 123, dogLoversInitialCount = 456, unknownUserMessage, var catLoversInitialCount = 123, dogLoversInitialCount = 456, unknownUserMessage,
createMockCohort, createMockCohorts, createMockContentGroups, createCohortsView, cohortsView, createMockCohort, createMockCohorts, createMockContentGroups, createCohortsView, cohortsView,
requests, respondToRefresh, verifyMessage, verifyNoMessage, verifyDetailedMessage, verifyHeader, requests, respondToRefresh, verifyMessage, verifyNoMessage, verifyDetailedMessage, verifyHeader,
expectCohortAddRequest, getAddModal, selectContentGroup, clearContentGroup, expectCohortAddRequest, getAddModal, selectContentGroup, clearContentGroup, saveFormAndExpectErrors,
MOCK_COHORTED_USER_PARTITION_ID, MOCK_UPLOAD_COHORTS_CSV_URL, MOCK_STUDIO_ADVANCED_SETTINGS_URL, MOCK_COHORTED_USER_PARTITION_ID, MOCK_UPLOAD_COHORTS_CSV_URL, MOCK_STUDIO_ADVANCED_SETTINGS_URL,
MOCK_STUDIO_GROUP_CONFIGURATIONS_URL; MOCK_STUDIO_GROUP_CONFIGURATIONS_URL;
...@@ -149,6 +149,21 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -149,6 +149,21 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
); );
}; };
saveFormAndExpectErrors = function(action, errors) {
var requestCount = requests.length,
form, expectedTitle;
if (action === 'add') {
expectedTitle = 'The cohort group cannot be added';
form = getAddModal();
} else {
expectedTitle = 'The cohort group cannot be saved';
form = cohortsView.$('.cohort-management-settings-form');
}
form.find('.action-save').click();
expect(requests.length).toBe(requestCount);
verifyDetailedMessage(expectedTitle, 'error', errors);
};
unknownUserMessage = function (name) { unknownUserMessage = function (name) {
return "Unknown user: " + name; return "Unknown user: " + name;
}; };
...@@ -300,12 +315,27 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -300,12 +315,27 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
it("does not allow a blank cohort name to be submitted", function() { it("does not allow a blank cohort name to be submitted", function() {
createCohortsView(this, {selectCohort: 1}); createCohortsView(this, {selectCohort: 1});
cohortsView.$('.action-create').click(); cohortsView.$('.action-create').click();
expect(getAddModal().find('.cohort-management-settings-form').length).toBe(1); cohortsView.$('.cohort-name').val(' ');
saveFormAndExpectErrors('add', ['You must specify a name for the cohort group']);
});
it("shows a message saving when choosing to have content groups but not selecting one", function() {
createCohortsView(this, {selectCohort: 1});
cohortsView.$('.action-create').click();
cohortsView.$('.cohort-name').val('New Cohort');
cohortsView.$('.radio-yes').prop('checked', true).change();
saveFormAndExpectErrors('add', ['You did not select a cohorted content group']);
});
it("shows two message when both fields have problems", function() {
createCohortsView(this, {selectCohort: 1});
cohortsView.$('.action-create').click();
cohortsView.$('.cohort-name').val(''); cohortsView.$('.cohort-name').val('');
expect(cohortsView.$('.cohort-management-nav')).toHaveClass('is-disabled'); cohortsView.$('.radio-yes').prop('checked', true).change();
getAddModal().find('.action-save').click(); saveFormAndExpectErrors('add', [
expect(requests.length).toBe(0); 'You must specify a name for the cohort group',
verifyMessage('Enter a name for your cohort group.', 'error'); 'You did not select a cohorted content group'
]);
}); });
it("shows a message when adding a cohort returns a server error", function() { it("shows a message when adding a cohort returns a server error", function() {
...@@ -355,8 +385,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -355,8 +385,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
// First try to save a blank name to create a message // First try to save a blank name to create a message
cohortsView.$('.action-create').click(); cohortsView.$('.action-create').click();
cohortsView.$('.cohort-name').val(''); cohortsView.$('.cohort-name').val('');
cohortsView.$('.action-save').click(); saveFormAndExpectErrors('add', ['You must specify a name for the cohort group']);
verifyMessage('Enter a name for your cohort group.', 'error');
// Now switch to a different cohort // Now switch to a different cohort
cohortsView.$('.cohort-select').val('2').change(); cohortsView.$('.cohort-select').val('2').change();
...@@ -370,8 +399,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -370,8 +399,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
// First try to save a blank name to create a message // First try to save a blank name to create a message
cohortsView.$('.action-create').click(); cohortsView.$('.action-create').click();
cohortsView.$('.cohort-name').val(''); cohortsView.$('.cohort-name').val('');
cohortsView.$('.action-save').click(); saveFormAndExpectErrors('add', ['You must specify a name for the cohort group']);
verifyMessage('Enter a name for your cohort group.', 'error');
// Now cancel the form // Now cancel the form
cohortsView.$('.action-cancel').click(); cohortsView.$('.action-cancel').click();
...@@ -524,6 +552,22 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -524,6 +552,22 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
describe("Cohort Settings", function() { describe("Cohort Settings", function() {
describe("Content Group Setting", function() { describe("Content Group Setting", function() {
var createCohortsViewWithDeletedContentGroup;
createCohortsViewWithDeletedContentGroup = function(test) {
createCohortsView(test, {
cohorts: [
{
id: 1,
name: 'Cat Lovers',
group_id: 999,
user_partition_id: MOCK_COHORTED_USER_PARTITION_ID
}
],
selectCohort: 1
});
};
it("shows a select element with an option for each content group", function () { it("shows a select element with an option for each content group", function () {
var options; var options;
createCohortsView(this, {selectCohort: 1}); createCohortsView(this, {selectCohort: 1});
...@@ -531,7 +575,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -531,7 +575,7 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
expect(cohortsView.$('.input-cohort-group-association').prop('disabled')).toBeTruthy(); expect(cohortsView.$('.input-cohort-group-association').prop('disabled')).toBeTruthy();
options = cohortsView.$('.input-cohort-group-association option'); options = cohortsView.$('.input-cohort-group-association option');
expect(options.length).toBe(3); expect(options.length).toBe(3);
expect($(options[0]).text().trim()).toBe(''); expect($(options[0]).text().trim()).toBe('Not selected');
expect($(options[1]).text().trim()).toBe('Cat Content'); expect($(options[1]).text().trim()).toBe('Cat Content');
expect($(options[2]).text().trim()).toBe('Dog Content'); expect($(options[2]).text().trim()).toBe('Dog Content');
}); });
...@@ -589,13 +633,25 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -589,13 +633,25 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
verifyMessage('Saved cohort group.', 'confirmation'); verifyMessage('Saved cohort group.', 'confirmation');
}); });
it("shows a message saving when choosing to have content groups but not selecting one", function() {
createCohortsView(this, {selectCohort: 1});
cohortsView.$('.tab-settings a').click();
cohortsView.$('.cohort-name').val('New Cohort');
cohortsView.$('.radio-yes').prop('checked', true).change();
saveFormAndExpectErrors('update', ['You did not select a cohorted content group']);
});
it("shows a message when the selected content group does not exist", function () {
createCohortsViewWithDeletedContentGroup(this);
cohortsView.$('.tab-settings a').click();
expect(cohortsView.$('option.option-unavailable').text().trim()).toBe('Deleted Content Group');
expect(cohortsView.$('.copy-error').text().trim()).toBe(
'The previously selected content group was deleted. Select another content group.'
);
});
it("can clear a selected content group which had been deleted", function () { it("can clear a selected content group which had been deleted", function () {
createCohortsView(this, { createCohortsViewWithDeletedContentGroup(this);
cohorts: [
{id: 1, name: 'Cat Lovers', group_id: 999}
],
selectCohort: 1
});
cohortsView.$('.tab-settings a').click(); cohortsView.$('.tab-settings a').click();
expect(cohortsView.$('.radio-yes').prop('checked')).toBeTruthy(); expect(cohortsView.$('.radio-yes').prop('checked')).toBeTruthy();
clearContentGroup(); clearContentGroup();
...@@ -613,18 +669,10 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe ...@@ -613,18 +669,10 @@ define(['backbone', 'jquery', 'js/common_helpers/ajax_helpers', 'js/common_helpe
expect(cohortsView.$('.copy-error').text().trim()).toBe(''); expect(cohortsView.$('.copy-error').text().trim()).toBe('');
}); });
it("shows a message when the selected content group does not exist", function () { it("shows an error when saving with a deleted content group", function () {
createCohortsView(this, { createCohortsViewWithDeletedContentGroup(this);
cohorts: [
{id: 1, name: 'Cat Lovers', group_id: 999}
],
selectCohort: 1
});
cohortsView.$('.tab-settings a').click(); cohortsView.$('.tab-settings a').click();
expect(cohortsView.$('option.option-unavailable').text().trim()).toBe('Deleted Content Group'); saveFormAndExpectErrors('save', ['The selected cohorted content group does not exist']);
expect(cohortsView.$('.copy-error').text().trim()).toBe(
'The previously selected content group was deleted. Select another content group.'
);
}); });
it("shows an error when the save fails", function () { it("shows an error when the save fails", function () {
......
lms/templates/instructor/instructor_dashboard_2/cohort-form.underscore
...@@ -27,13 +27,14 @@ ...@@ -27,13 +27,14 @@
<% <%
var foundSelected = false; var foundSelected = false;
var selectedContentGroupId = cohort.get('group_id'); var selectedContentGroupId = cohort.get('group_id');
var selectedUserPartitionId = cohort.get('user_partition_id');
var hasSelectedContentGroup = selectedContentGroupId != null; var hasSelectedContentGroup = selectedContentGroupId != null;
var hasContentGroups = contentGroups.length > 0; var hasContentGroups = contentGroups.length > 0;
%> %>
<div class="form-field"> <div class="form-field">
<div class="cohort-management-details-association-course field field-radio"> <div class="cohort-management-details-association-course field field-radio">
<h4 class="form-label"> <h4 class="form-label">
<%- gettext('Associate this cohort group with a content group') %> <%- gettext('Associated Cohorted Content Group') %>
</h4> </h4>
<label><input type="radio" class="radio-no" name="cohort-association-course" value="no" <%- !hasSelectedContentGroup ? 'checked="checked"' : '' %>/> <%- gettext("No Content Group") %></label> <label><input type="radio" class="radio-no" name="cohort-association-course" value="no" <%- !hasSelectedContentGroup ? 'checked="checked"' : '' %>/> <%- gettext("No Content Group") %></label>
<div class="input-group has-other-input-text"> <div class="input-group has-other-input-text">
...@@ -42,7 +43,7 @@ ...@@ -42,7 +43,7 @@
<div class="input-group-other"> <div class="input-group-other">
<label class="sr" for="cohort-group-association"><%- gettext("Choose a content group to associate") %></label> <label class="sr" for="cohort-group-association"><%- gettext("Choose a content group to associate") %></label>
<select name="cohort-group-association" class="input input-lg has-option-unavailable input-cohort-group-association" <%- !hasSelectedContentGroup ? 'disabled="disabled"' : '' %>> <select name="cohort-group-association" class="input input-lg has-option-unavailable input-cohort-group-association" <%- !hasSelectedContentGroup ? 'disabled="disabled"' : '' %>>
<option value="None"></option> <option value="None" <%- !hasSelectedContentGroup ? 'selected="selected"' : '' %> disabled="disabled"><%- gettext("Not selected") %></option>
<% <%
var orderedContentGroups = _.sortBy( var orderedContentGroups = _.sortBy(
...@@ -64,7 +65,7 @@ ...@@ -64,7 +65,7 @@
%> %>
<% if (hasSelectedContentGroup && !foundSelected) { %> <% if (hasSelectedContentGroup && !foundSelected) { %>
<option value="<%- contentGroupId %>:<%- contentGroupUserPartitionId %>" class="option-unavailable" selected="selected"><%- gettext("Deleted Content Group") %></option> <option value="<%- selectedContentGroupId %>:<%- selectedUserPartitionId %>" class="option-unavailable" selected="selected"><%- gettext("Deleted Content Group") %></option>
<% } %> <% } %>
</select> </select>
......
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