Commit 27013e74 by polesye

BLD-1049: Remove Group Configurations.

parent 9c52e5d7
...@@ -982,7 +982,7 @@ class GroupConfiguration(object): ...@@ -982,7 +982,7 @@ class GroupConfiguration(object):
) )
@staticmethod @staticmethod
def _get_usage_info(course, modulestore): def get_usage_info(course, store):
""" """
Get all units names and their urls that have experiments and associated Get all units names and their urls that have experiments and associated
with configurations. with configurations.
...@@ -996,18 +996,18 @@ class GroupConfiguration(object): ...@@ -996,18 +996,18 @@ class GroupConfiguration(object):
} }
""" """
usage_info = {} usage_info = {}
descriptors = modulestore.get_items(course.id, category='split_test') descriptors = store.get_items(course.id, category='split_test')
for split_test in descriptors: for split_test in descriptors:
if split_test.user_partition_id not in usage_info: if split_test.user_partition_id not in usage_info:
usage_info[split_test.user_partition_id] = [] usage_info[split_test.user_partition_id] = []
unit_location = modulestore.get_parent_location(split_test.location) unit_location = store.get_parent_location(split_test.location)
if not unit_location: if not unit_location:
log.warning("Parent location of split_test module not found: %s", split_test.location) log.warning("Parent location of split_test module not found: %s", split_test.location)
continue continue
try: try:
unit = modulestore.get_item(unit_location) unit = store.get_item(unit_location)
except ItemNotFoundError: except ItemNotFoundError:
log.warning("Unit not found: %s", unit_location) log.warning("Unit not found: %s", unit_location)
continue continue
...@@ -1023,13 +1023,13 @@ class GroupConfiguration(object): ...@@ -1023,13 +1023,13 @@ class GroupConfiguration(object):
return usage_info return usage_info
@staticmethod @staticmethod
def add_usage_info(course, modulestore): def add_usage_info(course, store):
""" """
Add usage information to group configurations json. Add usage information to group configurations json.
Returns json of group configurations updated with usage information. Returns json of group configurations updated with usage information.
""" """
usage_info = GroupConfiguration._get_usage_info(course, modulestore) usage_info = GroupConfiguration.get_usage_info(course, store)
configurations = [] configurations = []
for partition in course.user_partitions: for partition in course.user_partitions:
configuration = partition.to_json() configuration = partition.to_json()
...@@ -1091,7 +1091,7 @@ def group_configurations_list_handler(request, course_key_string): ...@@ -1091,7 +1091,7 @@ def group_configurations_list_handler(request, course_key_string):
@login_required @login_required
@ensure_csrf_cookie @ensure_csrf_cookie
@require_http_methods(("POST", "PUT")) @require_http_methods(("POST", "PUT", "DELETE"))
def group_configurations_detail_handler(request, course_key_string, group_configuration_id): def group_configurations_detail_handler(request, course_key_string, group_configuration_id):
""" """
JSON API endpoint for manipulating a group configuration via its internal ID. JSON API endpoint for manipulating a group configuration via its internal ID.
...@@ -1124,6 +1124,22 @@ def group_configurations_detail_handler(request, course_key_string, group_config ...@@ -1124,6 +1124,22 @@ def group_configurations_detail_handler(request, course_key_string, group_config
course.user_partitions.append(new_configuration) course.user_partitions.append(new_configuration)
store.update_item(course, request.user.id) store.update_item(course, request.user.id)
return JsonResponse(new_configuration.to_json(), status=201) return JsonResponse(new_configuration.to_json(), status=201)
elif request.method == "DELETE":
if not configuration:
return JsonResponse(status=404)
# Verify that group configuration is not already in use.
usages = GroupConfiguration.get_usage_info(course, store)
if usages.get(int(group_configuration_id)):
return JsonResponse(
{"error": _("This Group Configuration is already in use and cannot be removed.")},
status=400
)
index = course.user_partitions.index(configuration)
course.user_partitions.pop(index)
store.update_item(course, request.user.id)
return JsonResponse(status=204)
def _get_course_creator_status(user): def _get_course_creator_status(user):
......
...@@ -22,6 +22,7 @@ GROUP_CONFIGURATION_JSON = { ...@@ -22,6 +22,7 @@ GROUP_CONFIGURATION_JSON = {
} }
# pylint: disable=no-member
class HelperMethods(object): class HelperMethods(object):
""" """
Mixin that provides useful methods for Group Configuration tests. Mixin that provides useful methods for Group Configuration tests.
...@@ -137,7 +138,7 @@ class GroupConfigurationsBaseTestCase(object): ...@@ -137,7 +138,7 @@ class GroupConfigurationsBaseTestCase(object):
# pylint: disable=no-member # pylint: disable=no-member
@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') @skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase): class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
""" """
Test cases for group_configurations_list_handler. Test cases for group_configurations_list_handler.
""" """
...@@ -233,7 +234,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations ...@@ -233,7 +234,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
# pylint: disable=no-member # pylint: disable=no-member
@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') @skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase): class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
""" """
Test cases for group_configurations_detail_handler. Test cases for group_configurations_detail_handler.
""" """
...@@ -294,9 +295,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -294,9 +295,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
""" """
Edit group configuration and check its id and modified fields. Edit group configuration and check its id and modified fields.
""" """
self.course.user_partitions = [ self._add_user_partitions()
UserPartition(self.ID, 'First name', 'First description', [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')]),
]
self.save_course() self.save_course()
expected = { expected = {
...@@ -327,6 +326,65 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -327,6 +326,65 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name') self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups[1].name, u'Group C') self.assertEqual(user_partititons[0].groups[1].name, u'Group C')
def test_can_delete_group_configuration(self):
"""
Delete group configuration and check user partitions.
"""
self._add_user_partitions(count=2)
self.save_course()
response = self.client.delete(
self._url(cid=0),
content_type="application/json",
HTTP_ACCEPT="application/json",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 204)
self.reload_course()
# Verify that user_partitions is properly updated in the course.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'Name 1')
def test_cannot_delete_used_group_configuration(self):
"""
Cannot delete group configuration if it is in use.
"""
self._add_user_partitions(count=2)
self._create_content_experiment(cid=0)
response = self.client.delete(
self._url(cid=0),
content_type="application/json",
HTTP_ACCEPT="application/json",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 400)
content = json.loads(response.content)
self.assertTrue(content['error'])
self.reload_course()
# Verify that user_partitions is still the same.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 2)
self.assertEqual(user_partititons[0].name, u'Name 0')
def test_cannot_delete_non_existent_group_configuration(self):
"""
Cannot delete group configuration if it is doesn't exist.
"""
self._add_user_partitions(count=2)
response = self.client.delete(
self._url(cid=999),
content_type="application/json",
HTTP_ACCEPT="application/json",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 404)
# Verify that user_partitions is still the same.
user_partititons = self.course.user_partitions
self.assertEqual(len(user_partititons), 2)
self.assertEqual(user_partititons[0].name, u'Name 0')
# pylint: disable=no-member # pylint: disable=no-member
@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') @skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
...@@ -335,6 +393,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -335,6 +393,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
Tests for usage information of configurations. Tests for usage information of configurations.
""" """
def setUp(self): def setUp(self):
"""
Set up group configurations and split test module.
"""
super(GroupConfigurationsUsageInfoTestCase, self).setUp() super(GroupConfigurationsUsageInfoTestCase, self).setUp()
def test_group_configuration_not_used(self): def test_group_configuration_not_used(self):
...@@ -439,5 +500,5 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -439,5 +500,5 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
display_name='Test Content Experiment' display_name='Test Content Experiment'
) )
self.save_course() self.save_course()
actual = GroupConfiguration._get_usage_info(self.course, self.store) actual = GroupConfiguration.get_usage_info(self.course, self.store)
self.assertEqual(actual, {0: []}) self.assertEqual(actual, {0: []})
define([ define([
'js/models/course', 'js/models/group_configuration', 'underscore', 'js/models/course', 'js/models/group_configuration',
'js/collections/group_configuration', 'js/collections/group_configuration',
'js/views/group_configuration_details', 'js/views/group_configuration_details',
'js/views/group_configurations_list', 'js/views/group_configuration_edit', 'js/views/group_configurations_list', 'js/views/group_configuration_edit',
...@@ -8,7 +8,7 @@ define([ ...@@ -8,7 +8,7 @@ define([
'js/views/feedback_notification', 'js/spec_helpers/create_sinon', 'js/views/feedback_notification', 'js/spec_helpers/create_sinon',
'js/spec_helpers/edit_helpers', 'jasmine-stealth' 'js/spec_helpers/edit_helpers', 'jasmine-stealth'
], function( ], function(
Course, GroupConfigurationModel, GroupConfigurationCollection, _, Course, GroupConfigurationModel, GroupConfigurationCollection,
GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit, GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit,
GroupConfigurationItem, GroupModel, GroupCollection, GroupEdit, GroupConfigurationItem, GroupModel, GroupCollection, GroupEdit,
Notification, create_sinon, view_helpers Notification, create_sinon, view_helpers
...@@ -33,7 +33,8 @@ define([ ...@@ -33,7 +33,8 @@ define([
usageText: '.group-configuration-usage-text', usageText: '.group-configuration-usage-text',
usageTextAnchor: '.group-configuration-usage-text > a', usageTextAnchor: '.group-configuration-usage-text > a',
usageUnit: '.group-configuration-usage-unit', usageUnit: '.group-configuration-usage-unit',
usageUnitAnchor: '.group-configuration-usage-unit > a' usageUnitAnchor: '.group-configuration-usage-unit > a',
note: '.wrapper-delete-button'
}; };
beforeEach(function() { beforeEach(function() {
...@@ -105,6 +106,7 @@ define([ ...@@ -105,6 +106,7 @@ define([
it('should render properly', function() { it('should render properly', function() {
expect(this.view.$el).toContainText('Configuration'); expect(this.view.$el).toContainText('Configuration');
expect(this.view.$el).toContainText('ID: 0'); expect(this.view.$el).toContainText('ID: 0');
expect(this.view.$('.delete')).toExist();
}); });
it('should show groups appropriately', function() { it('should show groups appropriately', function() {
...@@ -171,6 +173,10 @@ define([ ...@@ -171,6 +173,10 @@ define([
usageUnitAnchors = this.view.$(SELECTORS.usageUnitAnchor); usageUnitAnchors = this.view.$(SELECTORS.usageUnitAnchor);
expect(this.view.$(SELECTORS.note)).toHaveAttr(
'data-tooltip', 'Cannot delete when in use by an experiment'
);
expect(this.view.$('.delete')).toHaveClass('is-disabled');
expect(this.view.$(SELECTORS.usageCount)).not.toExist(); expect(this.view.$(SELECTORS.usageCount)).not.toExist();
expect(this.view.$(SELECTORS.usageText)) expect(this.view.$(SELECTORS.usageText))
.toContainText('This Group Configuration is used in:'); .toContainText('This Group Configuration is used in:');
...@@ -183,15 +189,17 @@ define([ ...@@ -183,15 +189,17 @@ define([
}); });
it('should hide non-empty usage appropriately', function() { it('should hide non-empty usage appropriately', function() {
this.model.set('usage', this.model.set('usage', [
[ {'label': 'label1', 'url': 'url1'},
{'label': 'label1', 'url': 'url1'}, {'label': 'label2', 'url': 'url2'}
{'label': 'label2', 'url': 'url2'} ]);
]
);
this.model.set('showGroups', true); this.model.set('showGroups', true);
this.view.$('.hide-groups').click(); this.view.$('.hide-groups').click();
expect(this.view.$(SELECTORS.note)).toHaveAttr(
'data-tooltip', 'Cannot delete when in use by an experiment'
);
expect(this.view.$('.delete')).toHaveClass('is-disabled');
expect(this.view.$(SELECTORS.usageText)).not.toExist(); expect(this.view.$(SELECTORS.usageText)).not.toExist();
expect(this.view.$(SELECTORS.usageUnit)).not.toExist(); expect(this.view.$(SELECTORS.usageUnit)).not.toExist();
expect(this.view.$(SELECTORS.usageCount)) expect(this.view.$(SELECTORS.usageCount))
...@@ -234,6 +242,7 @@ define([ ...@@ -234,6 +242,7 @@ define([
name: 'Configuration', name: 'Configuration',
description: 'Configuration Description' description: 'Configuration Description'
}); });
expect(this.view.$('.delete')).toExist();
}); });
it ('should allow you to create new groups', function() { it ('should allow you to create new groups', function() {
...@@ -372,7 +381,7 @@ define([ ...@@ -372,7 +381,7 @@ define([
}); });
}); });
it('groups have correct default names and placeholders', function () { it('groups have correct default names', function () {
var group1 = new GroupModel({ name: 'Group A' }), var group1 = new GroupModel({ name: 'Group A' }),
group2 = new GroupModel({ name: 'Group B' }), group2 = new GroupModel({ name: 'Group B' }),
collection = this.model.get('groups'); collection = this.model.get('groups');
...@@ -400,12 +409,24 @@ define([ ...@@ -400,12 +409,24 @@ define([
'Group A', 'Group C', 'Group D', 'Group E', 'Group F', 'Group G' 'Group A', 'Group C', 'Group D', 'Group E', 'Group F', 'Group G'
]); ]);
}); });
it('cannot be deleted if it is in use', function () {
this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]);
this.view.render();
expect(this.view.$(SELECTORS.note)).toHaveAttr(
'data-tooltip', 'Cannot delete when in use by an experiment'
);
expect(this.view.$('.delete')).toHaveClass('is-disabled');
});
}); });
describe('GroupConfigurationsList', function() { describe('GroupConfigurationsList', function() {
var emptyMessage = 'You haven\'t created any group configurations yet.';
beforeEach(function() { beforeEach(function() {
view_helpers.installTemplate('no-group-configurations', true); view_helpers.installTemplate('no-group-configurations', true);
this.model = new GroupConfigurationModel({ id: 0 });
this.collection = new GroupConfigurationCollection(); this.collection = new GroupConfigurationCollection();
this.view = new GroupConfigurationsList({ this.view = new GroupConfigurationsList({
collection: this.collection collection: this.collection
...@@ -415,39 +436,52 @@ define([ ...@@ -415,39 +436,52 @@ define([
describe('empty template', function () { describe('empty template', function () {
it('should be rendered if no group configurations', function() { it('should be rendered if no group configurations', function() {
expect(this.view.$el).toContainText( expect(this.view.$el).toContainText(emptyMessage);
'You haven\'t created any group configurations yet.'
);
expect(this.view.$('.new-button')).toExist(); expect(this.view.$('.new-button')).toExist();
expect(this.view.$(SELECTORS.itemView)).not.toExist(); expect(this.view.$(SELECTORS.itemView)).not.toExist();
}); });
it('should disappear if group configuration is added', function() { it('should disappear if group configuration is added', function() {
var emptyMessage = 'You haven\'t created any group ' +
'configurations yet.';
expect(this.view.$el).toContainText(emptyMessage); expect(this.view.$el).toContainText(emptyMessage);
expect(this.view.$(SELECTORS.itemView)).not.toExist(); expect(this.view.$(SELECTORS.itemView)).not.toExist();
this.collection.add([{}]); this.collection.add(this.model);
expect(this.view.$el).not.toContainText(emptyMessage); expect(this.view.$el).not.toContainText(emptyMessage);
expect(this.view.$(SELECTORS.itemView)).toExist(); expect(this.view.$(SELECTORS.itemView)).toExist();
}); });
it('should appear if configurations were removed', function() {
this.collection.add(this.model);
expect(this.view.$(SELECTORS.itemView)).toExist();
this.collection.remove(this.model);
expect(this.view.$el).toContainText(emptyMessage);
expect(this.view.$(SELECTORS.itemView)).not.toExist();
});
}); });
}); });
describe('GroupConfigurationItem', function() { describe('GroupConfigurationItem', function() {
var clickDeleteItem;
beforeEach(function() { beforeEach(function() {
view_helpers.installTemplates([ view_helpers.installTemplates([
'group-configuration-edit', 'group-configuration-details' 'group-configuration-edit', 'group-configuration-details'
], true); ], true);
this.model = new GroupConfigurationModel({ id: 0 }); this.model = new GroupConfigurationModel({ id: 0 });
this.collection = new GroupConfigurationCollection([ this.model ]); this.collection = new GroupConfigurationCollection([ this.model ]);
this.collection.url = '/group_configurations';
this.view = new GroupConfigurationItem({ this.view = new GroupConfigurationItem({
model: this.model model: this.model
}); });
appendSetFixtures(this.view.render().el); appendSetFixtures(this.view.render().el);
}); });
clickDeleteItem = function (view, promptSpy) {
view.$('.delete').click();
view_helpers.verifyPromptShowing(promptSpy, /Delete this Group Configuration/);
view_helpers.confirmPrompt(promptSpy);
view_helpers.verifyPromptHidden(promptSpy);
};
it('should render properly', function() { it('should render properly', function() {
// Details view by default // Details view by default
expect(this.view.$(SELECTORS.detailsView)).toExist(); expect(this.view.$(SELECTORS.detailsView)).toExist();
...@@ -458,6 +492,40 @@ define([ ...@@ -458,6 +492,40 @@ define([
expect(this.view.$(SELECTORS.detailsView)).toExist(); expect(this.view.$(SELECTORS.detailsView)).toExist();
expect(this.view.$(SELECTORS.editView)).not.toExist(); expect(this.view.$(SELECTORS.editView)).not.toExist();
}); });
it('should destroy itself on confirmation of deleting', function () {
var requests = create_sinon.requests(this),
promptSpy = view_helpers.createPromptSpy(),
notificationSpy = view_helpers.createNotificationSpy();
clickDeleteItem(this.view, promptSpy);
// Backbone.emulateHTTP is enabled in our system, so setting this
// option will fake PUT, PATCH and DELETE requests with a HTTP POST,
// setting the X-HTTP-Method-Override header with the true method.
create_sinon.expectJsonRequest(requests, 'POST', '/group_configurations/0');
expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE');
view_helpers.verifyNotificationShowing(notificationSpy, /Deleting/);
create_sinon.respondToDelete(requests);
view_helpers.verifyNotificationHidden(notificationSpy);
expect($(SELECTORS.itemView)).not.toExist();
});
it('does not hide deleting message if failure', function() {
var requests = create_sinon.requests(this),
promptSpy = view_helpers.createPromptSpy(),
notificationSpy = view_helpers.createNotificationSpy();
clickDeleteItem(this.view, promptSpy);
// Backbone.emulateHTTP is enabled in our system, so setting this
// option will fake PUT, PATCH and DELETE requests with a HTTP POST,
// setting the X-HTTP-Method-Override header with the true method.
create_sinon.expectJsonRequest(requests, 'POST', '/group_configurations/0');
expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE');
view_helpers.verifyNotificationShowing(notificationSpy, /Deleting/);
create_sinon.respondWithError(requests);
view_helpers.verifyNotificationShowing(notificationSpy, /Deleting/);
expect($(SELECTORS.itemView)).toExist();
});
}); });
describe('GroupEdit', function() { describe('GroupEdit', function() {
......
define(["sinon", "underscore"], function(sinon, _) { define(["sinon", "underscore"], function(sinon, _) {
var fakeServer, fakeRequests, respondWithJson, respondWithError; var fakeServer, fakeRequests, expectJsonRequest, respondWithJson, respondWithError, respondToDelete;
/* These utility methods are used by Jasmine tests to create a mock server or /* These utility methods are used by Jasmine tests to create a mock server or
* get reference to mock requests. In either case, the cleanup (restore) is done with * get reference to mock requests. In either case, the cleanup (restore) is done with
...@@ -45,6 +45,17 @@ define(["sinon", "underscore"], function(sinon, _) { ...@@ -45,6 +45,17 @@ define(["sinon", "underscore"], function(sinon, _) {
return requests; return requests;
}; };
expectJsonRequest = function(requests, method, url, jsonRequest, requestIndex) {
var request;
if (_.isUndefined(requestIndex)) {
requestIndex = requests.length - 1;
}
request = requests[requestIndex];
expect(request.url).toEqual(url);
expect(request.method).toEqual(method);
expect(JSON.parse(request.requestBody)).toEqual(jsonRequest);
};
respondWithJson = function(requests, jsonResponse, requestIndex) { respondWithJson = function(requests, jsonResponse, requestIndex) {
if (_.isUndefined(requestIndex)) { if (_.isUndefined(requestIndex)) {
requestIndex = requests.length - 1; requestIndex = requests.length - 1;
...@@ -63,10 +74,20 @@ define(["sinon", "underscore"], function(sinon, _) { ...@@ -63,10 +74,20 @@ define(["sinon", "underscore"], function(sinon, _) {
JSON.stringify({ })); JSON.stringify({ }));
}; };
respondToDelete = function(requests, requestIndex) {
if (_.isUndefined(requestIndex)) {
requestIndex = requests.length - 1;
}
requests[requestIndex].respond(204,
{ "Content-Type": "application/json" });
};
return { return {
"server": fakeServer, "server": fakeServer,
"requests": fakeRequests, "requests": fakeRequests,
"expectJsonRequest": expectJsonRequest,
"respondWithJson": respondWithJson, "respondWithJson": respondWithJson,
"respondWithError": respondWithError "respondWithError": respondWithError,
"respondToDelete": respondToDelete
}; };
}); });
/** /**
* Provides helper methods for invoking Studio modal windows in Jasmine tests. * Provides helper methods for invoking Studio modal windows in Jasmine tests.
*/ */
define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sinon"], define(['jquery', 'js/views/feedback_notification', 'js/views/feedback_prompt'],
function($, NotificationView, create_sinon) { function($, NotificationView, Prompt) {
var installTemplate, installTemplates, installViewTemplates, createNotificationSpy, 'use strict';
verifyNotificationShowing, verifyNotificationHidden; var installTemplate, installTemplates, installViewTemplates, createFeedbackSpy, verifyFeedbackShowing,
verifyFeedbackHidden, createNotificationSpy, verifyNotificationShowing,
verifyNotificationHidden, createPromptSpy, confirmPrompt, verifyPromptShowing,
verifyPromptHidden;
installTemplate = function(templateName, isFirst) { installTemplate = function(templateName, isFirst) {
var template = readFixtures(templateName + '.underscore'), var template = readFixtures(templateName + '.underscore'),
templateId = templateName + '-tpl'; templateId = templateName + '-tpl';
if (isFirst) { if (isFirst) {
setFixtures($("<script>", { id: templateId, type: "text/template" }).text(template)); setFixtures($('<script>', { id: templateId, type: 'text/template' }).text(template));
} else { } else {
appendSetFixtures($("<script>", { id: templateId, type: "text/template" }).text(template)); appendSetFixtures($('<script>', { id: templateId, type: 'text/template' }).text(template));
} }
}; };
...@@ -35,22 +38,56 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino ...@@ -35,22 +38,56 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino
appendSetFixtures('<div id="page-notification"></div>'); appendSetFixtures('<div id="page-notification"></div>');
}; };
createFeedbackSpy = function(type, intent) {
var feedbackSpy = spyOnConstructor(type, intent, ['show', 'hide']);
feedbackSpy.show.andReturn(feedbackSpy);
return feedbackSpy;
};
verifyFeedbackShowing = function(feedbackSpy, text) {
var options;
expect(feedbackSpy.constructor).toHaveBeenCalled();
expect(feedbackSpy.show).toHaveBeenCalled();
expect(feedbackSpy.hide).not.toHaveBeenCalled();
options = feedbackSpy.constructor.mostRecentCall.args[0];
expect(options.title).toMatch(text);
};
verifyFeedbackHidden = function(feedbackSpy) {
expect(feedbackSpy.hide).toHaveBeenCalled();
};
createNotificationSpy = function(type) { createNotificationSpy = function(type) {
var notificationSpy = spyOnConstructor(NotificationView, type || "Mini", ["show", "hide"]); return createFeedbackSpy(NotificationView, type || 'Mini');
notificationSpy.show.andReturn(notificationSpy);
return notificationSpy;
}; };
verifyNotificationShowing = function(notificationSpy, text) { verifyNotificationShowing = function(notificationSpy, text) {
expect(notificationSpy.constructor).toHaveBeenCalled(); verifyFeedbackShowing.apply(this, arguments);
expect(notificationSpy.show).toHaveBeenCalled();
expect(notificationSpy.hide).not.toHaveBeenCalled();
var options = notificationSpy.constructor.mostRecentCall.args[0];
expect(options.title).toMatch(text);
}; };
verifyNotificationHidden = function(notificationSpy) { verifyNotificationHidden = function(notificationSpy) {
expect(notificationSpy.hide).toHaveBeenCalled(); verifyFeedbackHidden.apply(this, arguments);
};
createPromptSpy = function(type) {
return createFeedbackSpy(Prompt, type || 'Warning');
};
confirmPrompt = function(promptSpy, pressSecondaryButton) {
expect(promptSpy.constructor).toHaveBeenCalled();
if (pressSecondaryButton) {
promptSpy.constructor.mostRecentCall.args[0].actions.secondary.click(promptSpy);
} else {
promptSpy.constructor.mostRecentCall.args[0].actions.primary.click(promptSpy);
}
};
verifyPromptShowing = function(promptSpy, text) {
verifyFeedbackShowing.apply(this, arguments);
};
verifyPromptHidden = function(promptSpy) {
verifyFeedbackHidden.apply(this, arguments);
}; };
return { return {
...@@ -59,6 +96,10 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino ...@@ -59,6 +96,10 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino
'installViewTemplates': installViewTemplates, 'installViewTemplates': installViewTemplates,
'createNotificationSpy': createNotificationSpy, 'createNotificationSpy': createNotificationSpy,
'verifyNotificationShowing': verifyNotificationShowing, 'verifyNotificationShowing': verifyNotificationShowing,
'verifyNotificationHidden': verifyNotificationHidden 'verifyNotificationHidden': verifyNotificationHidden,
'confirmPrompt': confirmPrompt,
'createPromptSpy': createPromptSpy,
'verifyPromptShowing': verifyPromptShowing,
'verifyPromptHidden': verifyPromptHidden
}; };
}); });
...@@ -42,11 +42,11 @@ function(BaseView, _, $, gettext, GroupEdit) { ...@@ -42,11 +42,11 @@ function(BaseView, _, $, gettext, GroupEdit) {
uniqueId: _.uniqueId(), uniqueId: _.uniqueId(),
name: this.model.escape('name'), name: this.model.escape('name'),
description: this.model.escape('description'), description: this.model.escape('description'),
usage: this.model.get('usage'),
isNew: this.model.isNew(), isNew: this.model.isNew(),
error: this.model.validationError error: this.model.validationError
})); }));
this.addAll(); this.addAll();
return this; return this;
}, },
......
...@@ -10,6 +10,9 @@ define([ ...@@ -10,6 +10,9 @@ define([
attributes: { attributes: {
'tabindex': -1 'tabindex': -1
}, },
events: {
'click .delete': 'deleteConfiguration'
},
className: function () { className: function () {
var index = this.model.collection.indexOf(this.model); var index = this.model.collection.indexOf(this.model);
...@@ -26,6 +29,24 @@ define([ ...@@ -26,6 +29,24 @@ define([
this.listenTo(this.model, 'remove', this.remove); this.listenTo(this.model, 'remove', this.remove);
}, },
deleteConfiguration: function(event) {
if(event && event.preventDefault) { event.preventDefault(); }
var self = this;
this.confirmThenRunOperation(
gettext('Delete this Group Configuration?'),
gettext('Deleting this Group Configuration is permanent and cannot be undone.'),
gettext('Delete'),
function() {
return self.runOperationShowingMessage(
gettext('Deleting') + '&hellip;',
function () {
return self.model.destroy({ wait: true });
}
);
}
);
},
render: function() { render: function() {
// Removes a view from the DOM, and calls stopListening to remove // Removes a view from the DOM, and calls stopListening to remove
// any bound events that the view has listened to. // any bound events that the view has listened to.
......
...@@ -14,6 +14,7 @@ define([ ...@@ -14,6 +14,7 @@ define([
initialize: function() { initialize: function() {
this.emptyTemplate = this.loadTemplate('no-group-configurations'); this.emptyTemplate = this.loadTemplate('no-group-configurations');
this.listenTo(this.collection, 'add', this.addNewItemView); this.listenTo(this.collection, 'add', this.addNewItemView);
this.listenTo(this.collection, 'remove', this.handleDestory);
}, },
render: function() { render: function() {
...@@ -57,6 +58,12 @@ define([ ...@@ -57,6 +58,12 @@ define([
addOne: function(event) { addOne: function(event) {
if(event && event.preventDefault) { event.preventDefault(); } if(event && event.preventDefault) { event.preventDefault(); }
this.collection.add([{ editing: true }]); this.collection.add([{ editing: true }]);
},
handleDestory: function () {
if(this.collection.length === 0) {
this.$el.html(this.emptyTemplate());
}
} }
}); });
......
...@@ -81,6 +81,7 @@ ...@@ -81,6 +81,7 @@
"draggabilly": "js/vendor/draggabilly.pkgd", "draggabilly": "js/vendor/draggabilly.pkgd",
"URI": "js/vendor/URI.min", "URI": "js/vendor/URI.min",
"ieshim": "js/src/ie_shim", "ieshim": "js/src/ie_shim",
"tooltip_manager": "coffee/src/discussion/tooltip_manager",
// externally hosted files // externally hosted files
"tender": [ "tender": [
...@@ -234,7 +235,7 @@ ...@@ -234,7 +235,7 @@
deps: ["jquery", "gettext"], deps: ["jquery", "gettext"],
callback: function() { callback: function() {
// load other scripts on every page, after jquery loads // load other scripts on every page, after jquery loads
require(["js/base", "coffee/src/main", "coffee/src/logger", "datepair", "accessibility", "ieshim"]); require(["js/base", "coffee/src/main", "coffee/src/logger", "datepair", "accessibility", "ieshim", "tooltip_manager"]);
// we need "datepair" because it dynamically modifies the page // we need "datepair" because it dynamically modifies the page
// when it is loaded -- yuck! // when it is loaded -- yuck!
} }
......
...@@ -72,11 +72,13 @@ function(doc, GroupConfigurationCollection, GroupConfigurationsPage) { ...@@ -72,11 +72,13 @@ function(doc, GroupConfigurationCollection, GroupConfigurationsPage) {
<aside class="content-supplementary" role="complimentary"> <aside class="content-supplementary" role="complimentary">
<div class="bit"> <div class="bit">
<h3 class="title-3">${_("What can I do on this page?")}</h3> <h3 class="title-3">${_("What can I do on this page?")}</h3>
<p>${_("You can create and edit group configurations.")}</p> <p>${_("You can create, edit, and delete group configurations.")}</p>
<p>${_("A group configuration defines how many groups of students are in an experiment. When you create a content experiment, you select the group configuration to use.")}</p> <p>${_("A group configuration defines how many groups of students are in an experiment. When you create an experiment, you select the group configuration to use.")}</p>
<p>${_("Click {em_start}New Group Configuration{em_end} to add a new configuration. To edit an existing configuration, hover over its box and click {em_start}Edit{em_end}.").format(em_start='<strong>', em_end="</strong>")}</p> <p>${_("Click {em_start}New Group Configuration{em_end} to add a new configuration. To edit a configuration, hover over its box and click {em_start}Edit{em_end}.").format(em_start='<strong>', em_end="</strong>")}</p>
<p>${_("You can delete a group configuration only if it is not in use in an experiment. To delete a configuration, hover over its box and click the delete icon.")}</p>
<p><a href="${get_online_help_info(online_help_token())['doc_url']}" target="_blank">${_("Learn More")}</a></p> <p><a href="${get_online_help_info(online_help_token())['doc_url']}" target="_blank">${_("Learn More")}</a></p>
</div> </div>
......
...@@ -44,6 +44,15 @@ ...@@ -44,6 +44,15 @@
<li class="action action-edit"> <li class="action action-edit">
<button class="edit"><%= gettext("Edit") %></button> <button class="edit"><%= gettext("Edit") %></button>
</li> </li>
<% if (_.isEmpty(usage)) { %>
<li class="action action-delete wrapper-delete-button">
<button class="delete action-icon"><i class="icon-trash"></i><span><%= gettext("Delete") %></span></button>
</li>
<% } else { %>
<li class="action action-delete wrapper-delete-button" data-tooltip="<%= gettext('Cannot delete when in use by an experiment') %>">
<button class="delete action-icon is-disabled"><i class="icon-trash"></i><span><%= gettext("Delete") %></span></button>
</li>
<% } %>
</ul> </ul>
</div> </div>
<% if(showGroups) { %> <% if(showGroups) { %>
......
...@@ -36,5 +36,16 @@ ...@@ -36,5 +36,16 @@
<div class="actions"> <div class="actions">
<button class="action action-primary" type="submit"><% if (isNew) { print(gettext("Create")) } else { print(gettext("Save")) } %></button> <button class="action action-primary" type="submit"><% if (isNew) { print(gettext("Create")) } else { print(gettext("Save")) } %></button>
<button class="action action-secondary action-cancel"><%= gettext("Cancel") %></button> <button class="action action-secondary action-cancel"><%= gettext("Cancel") %></button>
<% if (!isNew) { %>
<% if (_.isEmpty(usage)) { %>
<span class="wrapper-delete-button">
<a class="button action-delete delete" href="#"><%= gettext("Delete") %></a>
</span>
<% } else { %>
<span class="wrapper-delete-button" data-tooltip="<%= gettext('Cannot delete when in use by an experiment') %>">
<a class="button action-delete delete is-disabled" href="#"><%= gettext("Delete") %></a>
</span>
<% } %>
<% } %>
</div> </div>
</form> </form>
$ ->
new TooltipManager
class @TooltipManager class @TooltipManager
constructor: () -> constructor: () ->
@$body = $('body') @$body = $('body')
...@@ -45,3 +42,8 @@ class @TooltipManager ...@@ -45,3 +42,8 @@ class @TooltipManager
hideTooltip: (e) => hideTooltip: (e) =>
@$tooltip.hide().css('opacity', 0) @$tooltip.hide().css('opacity', 0)
clearTimeout(@tooltipTimer) clearTimeout(@tooltipTimer)
# Move initialization at the bottom to make sure that TooltipManager is already
# assigned to the Global object.
$ ->
new TooltipManager
...@@ -499,7 +499,6 @@ class CourseFixture(StudioApiFixture): ...@@ -499,7 +499,6 @@ class CourseFixture(StudioApiFixture):
try: try:
loc = response.json().get('locator') loc = response.json().get('locator')
xblock_desc.locator = loc xblock_desc.locator = loc
except ValueError: except ValueError:
raise CourseFixtureError("Could not decode JSON from '{0}'".format(response.content)) raise CourseFixtureError("Could not decode JSON from '{0}'".format(response.content))
......
...@@ -3,6 +3,7 @@ Course Group Configurations page. ...@@ -3,6 +3,7 @@ Course Group Configurations page.
""" """
from .course_page import CoursePage from .course_page import CoursePage
from .utils import confirm_prompt
class GroupConfigurationsPage(CoursePage): class GroupConfigurationsPage(CoursePage):
...@@ -53,15 +54,13 @@ class GroupConfiguration(object): ...@@ -53,15 +54,13 @@ class GroupConfiguration(object):
""" """
Expand/collapse group configuration. Expand/collapse group configuration.
""" """
css = 'a.group-toggle' self.find_css('a.group-toggle').first.click()
self.find_css(css).first.click()
def add_group(self): def add_group(self):
""" """
Add new group. Add new group.
""" """
css = 'button.action-add-group' self.find_css('button.action-add-group').first.click()
self.find_css(css).first.click()
def get_text(self, css): def get_text(self, css):
""" """
...@@ -73,37 +72,47 @@ class GroupConfiguration(object): ...@@ -73,37 +72,47 @@ class GroupConfiguration(object):
""" """
Click on the `Course Outline` link. Click on the `Course Outline` link.
""" """
css = 'p.group-configuration-usage-text a' self.find_css('p.group-configuration-usage-text a').first.click()
self.find_css(css).first.click()
def click_unit_anchor(self, index=0): def click_unit_anchor(self, index=0):
""" """
Click on the link to the unit. Click on the link to the unit.
""" """
css = 'li.group-configuration-usage-unit a' self.find_css('li.group-configuration-usage-unit a').nth(index).click()
self.find_css(css).nth(index).click()
def edit(self): def edit(self):
""" """
Open editing view for the group configuration. Open editing view for the group configuration.
""" """
css = '.action-edit .edit' self.find_css('.action-edit .edit').first.click()
self.find_css(css).first.click()
@property
def delete_button_is_disabled(self):
return self.find_css('.actions .delete.is-disabled').present
@property
def delete_button_is_absent(self):
return not self.find_css('.actions .delete').present
def delete(self):
"""
Delete the group configuration.
"""
self.find_css('.actions .delete').first.click()
confirm_prompt(self.page)
def save(self): def save(self):
""" """
Save group configuration. Save group configuration.
""" """
css = '.action-primary' self.find_css('.action-primary').first.click()
self.find_css(css).first.click()
self.page.wait_for_ajax() self.page.wait_for_ajax()
def cancel(self): def cancel(self):
""" """
Cancel group configuration. Cancel group configuration.
""" """
css = '.action-secondary' self.find_css('.action-secondary').first.click()
self.find_css(css).first.click()
@property @property
def mode(self): def mode(self):
...@@ -149,8 +158,7 @@ class GroupConfiguration(object): ...@@ -149,8 +158,7 @@ class GroupConfiguration(object):
""" """
Set group configuration name. Set group configuration name.
""" """
css = '.group-configuration-name-input' self.find_css('.group-configuration-name-input').first.fill(value)
self.find_css(css).first.fill(value)
@property @property
def description(self): def description(self):
...@@ -164,20 +172,24 @@ class GroupConfiguration(object): ...@@ -164,20 +172,24 @@ class GroupConfiguration(object):
""" """
Set group configuration description. Set group configuration description.
""" """
css = '.group-configuration-description-input' self.find_css('.group-configuration-description-input').first.fill(value)
self.find_css(css).first.fill(value)
@property @property
def groups(self): def groups(self):
""" """
Return list of groups. Return list of groups.
""" """
css = '.group'
def group_selector(group_index): def group_selector(group_index):
return self.get_selector('.group-{} '.format(group_index)) return self.get_selector('.group-{} '.format(group_index))
return [Group(self.page, group_selector(index)) for index, element in enumerate(self.find_css(css))] return [Group(self.page, group_selector(index)) for index, element in enumerate(self.find_css('.group'))]
@property
def delete_note(self):
"""
Return delete note for the group configuration.
"""
return self.find_css('.wrapper-delete-button').first.attrs('data-tooltip')[0]
def __repr__(self): def __repr__(self):
return "<{}:{}>".format(self.__class__.__name__, self.name) return "<{}:{}>".format(self.__class__.__name__, self.name)
......
...@@ -111,3 +111,14 @@ def get_codemirror_value(page, index=0, find_prefix="$"): ...@@ -111,3 +111,14 @@ def get_codemirror_value(page, index=0, find_prefix="$"):
return {find_prefix}('div.CodeMirror:eq({index})').get(0).CodeMirror.getValue(); return {find_prefix}('div.CodeMirror:eq({index})').get(0).CodeMirror.getValue();
""".format(index=index, find_prefix=find_prefix) """.format(index=index, find_prefix=find_prefix)
) )
def confirm_prompt(page, cancel=False):
"""
Ensures that a modal prompt and confirmation button are visible, then clicks the button. The prompt is canceled iff
cancel is True.
"""
page.wait_for_element_visibility('.prompt', 'Prompt is visible')
confirmation_button_css = '.prompt .action-' + ('secondary' if cancel else 'primary')
page.wait_for_element_visibility(confirmation_button_css, 'Confirmation button is visible')
click_css(page, confirmation_button_css, require_notification=(not cancel))
...@@ -375,6 +375,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): ...@@ -375,6 +375,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
# Save the configuration # Save the configuration
self.assertEqual(config.get_text('.action-primary'), "CREATE") self.assertEqual(config.get_text('.action-primary'), "CREATE")
self.assertTrue(config.delete_button_is_absent)
config.save() config.save()
self._assert_fields( self._assert_fields(
...@@ -652,6 +653,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): ...@@ -652,6 +653,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
usage = config.usages[0] usage = config.usages[0]
config.click_unit_anchor() config.click_unit_anchor()
unit = UnitPage(self.browser, vertical.locator)
# Waiting for the page load and verify that we've landed on the unit page # Waiting for the page load and verify that we've landed on the unit page
EmptyPromise( EmptyPromise(
lambda: unit.is_browser_on_page(), "loaded page {!r}".format(unit), lambda: unit.is_browser_on_page(), "loaded page {!r}".format(unit),
...@@ -659,3 +661,71 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): ...@@ -659,3 +661,71 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
).fulfill() ).fulfill()
self.assertIn(unit.name, usage) self.assertIn(unit.name, usage)
def test_can_delete_unused_group_configuration(self):
"""
Scenario: Ensure that the user can delete unused group configuration.
Given I have a course with 2 group configurations
And I go to the Group Configuration page
When I delete the Group Configuration with name "Configuration 1"
Then I see that there is one Group Configuration
When I edit the Group Configuration with name "Configuration 2"
And I delete the Group Configuration with name "Configuration 2"
Then I see that the are no Group Configurations
"""
self.course_fixture._update_xblock(self.course_fixture._course_location, {
"metadata": {
u"user_partitions": [
UserPartition(0, 'Configuration 1', 'Description of the group configuration.', [Group("0", 'Group 0'), Group("1", 'Group 1')]).to_json(),
UserPartition(1, 'Configuration 2', 'Second group configuration.', [Group("0", 'Alpha'), Group("1", 'Beta'), Group("2", 'Gamma')]).to_json()
],
},
})
self.page.visit()
self.assertEqual(len(self.page.group_configurations), 2)
config = self.page.group_configurations[1]
# Delete first group configuration via detail view
config.delete()
self.assertEqual(len(self.page.group_configurations), 1)
config = self.page.group_configurations[0]
config.edit()
self.assertFalse(config.delete_button_is_disabled)
# Delete first group configuration via edit view
config.delete()
self.assertEqual(len(self.page.group_configurations), 0)
def test_cannot_delete_used_group_configuration(self):
"""
Scenario: Ensure that the user cannot delete unused group configuration.
Given I have a course with group configuration that is used in the Content Experiment
When I go to the Group Configuration page
Then I do not see delete button and I see a note about that
When I edit the Group Configuration
Then I do not see delete button and I see the note about that
"""
# Create a new group configurations
self.course_fixture._update_xblock(self.course_fixture._course_location, {
"metadata": {
u"user_partitions": [
UserPartition(0, "Name", "Description.", [Group("0", "Group A"), Group("1", "Group B")]).to_json()
],
},
})
vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0]
self.course_fixture.create_xblock(
vertical.locator,
XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata={'user_partition_id': 0})
)
# Go to the Group Configuration Page and click unit anchor
self.page.visit()
config = self.page.group_configurations[0]
self.assertTrue(config.delete_button_is_disabled)
self.assertIn('Cannot delete when in use by an experiment', config.delete_note)
config.edit()
self.assertTrue(config.delete_button_is_disabled)
self.assertIn('Cannot delete when in use by an experiment', config.delete_note)
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