Commit fda3f5ac by polesye

BLD-1110: Create, edit, delete groups.

parent 602be5e9
...@@ -901,12 +901,11 @@ class GroupConfiguration(object): ...@@ -901,12 +901,11 @@ class GroupConfiguration(object):
if len(self.configuration.get('groups', [])) < 2: if len(self.configuration.get('groups', [])) < 2:
raise GroupConfigurationsValidationError(_("must have at least two groups")) raise GroupConfigurationsValidationError(_("must have at least two groups"))
def generate_id(self): def generate_id(self, used_ids):
""" """
Generate unique id for the group configuration. Generate unique id for the group configuration.
If this id is already used, we generate new one. If this id is already used, we generate new one.
""" """
used_ids = self.get_used_ids()
cid = random.randint(100, 10 ** 12) cid = random.randint(100, 10 ** 12)
while cid in used_ids: while cid in used_ids:
...@@ -918,21 +917,18 @@ class GroupConfiguration(object): ...@@ -918,21 +917,18 @@ class GroupConfiguration(object):
""" """
Assign id for the json representation of group configuration. Assign id for the json representation of group configuration.
""" """
self.configuration['id'] = int(configuration_id) if configuration_id else self.generate_id() self.configuration['id'] = int(configuration_id) if configuration_id else self.generate_id(self.get_used_ids())
def assign_group_ids(self): def assign_group_ids(self):
""" """
Assign ids for the group_configuration's groups. Assign ids for the group_configuration's groups.
""" """
# this is temporary logic, we are going to build default groups on front-end used_ids = [g.id for p in self.course.user_partitions for g in p.groups]
if not self.configuration.get('groups'):
self.configuration['groups'] = [
{'name': 'Group A'}, {'name': 'Group B'},
]
# Assign ids to every group in configuration. # Assign ids to every group in configuration.
for index, group in enumerate(self.configuration.get('groups', [])): for group in self.configuration.get('groups', []):
group['id'] = index if group.get('id') is None:
group["id"] = self.generate_id(used_ids)
used_ids.append(group["id"])
def get_used_ids(self): def get_used_ids(self):
""" """
......
...@@ -5,6 +5,7 @@ import json ...@@ -5,6 +5,7 @@ import json
from unittest import skipUnless from unittest import skipUnless
from django.conf import settings from django.conf import settings
from contentstore.utils import reverse_course_url from contentstore.utils import reverse_course_url
from contentstore.views.component import SPLIT_TEST_COMPONENT_TYPE
from contentstore.tests.utils import CourseTestCase from contentstore.tests.utils import CourseTestCase
from xmodule.partitions.partitions import Group, UserPartition from xmodule.partitions.partitions import Group, UserPartition
...@@ -12,6 +13,10 @@ from xmodule.partitions.partitions import Group, UserPartition ...@@ -12,6 +13,10 @@ from xmodule.partitions.partitions import Group, UserPartition
GROUP_CONFIGURATION_JSON = { GROUP_CONFIGURATION_JSON = {
u'name': u'Test name', u'name': u'Test name',
u'description': u'Test description', u'description': u'Test description',
u'groups': [
{u'name': u'Group A'},
{u'name': u'Group B'},
],
} }
...@@ -96,7 +101,6 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations ...@@ -96,7 +101,6 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
""" """
Test cases for group_configurations_list_handler. Test cases for group_configurations_list_handler.
""" """
def setUp(self): def setUp(self):
""" """
Set up GroupConfigurationsListHandlerTestCase. Set up GroupConfigurationsListHandlerTestCase.
...@@ -109,13 +113,35 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations ...@@ -109,13 +113,35 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
""" """
return reverse_course_url('group_configurations_list_handler', self.course.id) return reverse_course_url('group_configurations_list_handler', self.course.id)
def test_can_retrieve_html(self): def test_view_index_ok(self):
""" """
Check that the group configuration index page responds correctly. Basic check that the groups configuration page responds correctly.
""" """
self.course.user_partitions = [
UserPartition(0, 'First name', 'First description', [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')]),
]
self.save_course()
if SPLIT_TEST_COMPONENT_TYPE not in self.course.advanced_modules:
self.course.advanced_modules.append(SPLIT_TEST_COMPONENT_TYPE)
self.store.update_item(self.course, self.user.id)
response = self.client.get(self._url()) response = self.client.get(self._url())
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertIn('New Group Configuration', response.content) self.assertContains(response, 'First name')
self.assertContains(response, 'Group C')
def test_view_index_disabled(self):
"""
Check that group configuration page is not displayed when turned off.
"""
if SPLIT_TEST_COMPONENT_TYPE in self.course.advanced_modules:
self.course.advanced_modules.remove(SPLIT_TEST_COMPONENT_TYPE)
self.store.update_item(self.course, self.user.id)
resp = self.client.get(self._url())
self.assertContains(resp, "module is disabled")
def test_unsupported_http_accept_header(self): def test_unsupported_http_accept_header(self):
""" """
...@@ -157,8 +183,12 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations ...@@ -157,8 +183,12 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
self.assertEqual(len(group_ids), 2) self.assertEqual(len(group_ids), 2)
self.reload_course() self.reload_course()
# Verify that user_partitions in the course contains the new group configuration. # Verify that user_partitions in the course contains the new group configuration.
self.assertEqual(len(self.course.user_partitions), 1) user_partititons = self.course.user_partitions
self.assertEqual(self.course.user_partitions[0].name, u'Test name') self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'Group A')
self.assertEqual(user_partititons[0].groups[1].name, u'Group B')
# pylint: disable=no-member # pylint: disable=no-member
...@@ -204,7 +234,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -204,7 +234,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
response = self.client.put( response = self.client.put(
self._url(cid=999), self._url(cid=999),
data=json.dumps(GROUP_CONFIGURATION_JSON), data=json.dumps(expected),
content_type="application/json", content_type="application/json",
HTTP_ACCEPT="application/json", HTTP_ACCEPT="application/json",
HTTP_X_REQUESTED_WITH="XMLHttpRequest", HTTP_X_REQUESTED_WITH="XMLHttpRequest",
...@@ -213,8 +243,12 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -213,8 +243,12 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
self.assertEqual(content, expected) self.assertEqual(content, expected)
self.reload_course() self.reload_course()
# Verify that user_partitions in the course contains the new group configuration. # Verify that user_partitions in the course contains the new group configuration.
self.assertEqual(len(self.course.user_partitions), 1) user_partititons = self.course.user_partitions
self.assertEqual(self.course.user_partitions[0].name, u'Test name') self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'Group A')
self.assertEqual(user_partititons[0].groups[1].name, u'Group B')
def test_can_edit_group_configuration(self): def test_can_edit_group_configuration(self):
""" """
...@@ -231,8 +265,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -231,8 +265,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
u'description': u'New Test description', u'description': u'New Test description',
u'version': 1, u'version': 1,
u'groups': [ u'groups': [
{u'id': 0, u'name': u'Group A', u'version': 1}, {u'id': 0, u'name': u'New Group Name', u'version': 1},
{u'id': 1, u'name': u'Group B', u'version': 1}, {u'id': 2, u'name': u'Group C', u'version': 1},
], ],
} }
response = self.client.put( response = self.client.put(
...@@ -246,5 +280,9 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -246,5 +280,9 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
self.assertEqual(content, expected) self.assertEqual(content, expected)
self.reload_course() self.reload_course()
# Verify that user_partitions is properly updated in the course. # Verify that user_partitions is properly updated in the course.
self.assertEqual(len(self.course.user_partitions), 1) user_partititons = self.course.user_partitions
self.assertEqual(self.course.user_partitions[0].name, u'New Test name') self.assertEqual(len(user_partititons), 1)
self.assertEqual(user_partititons[0].name, u'New Test name')
self.assertEqual(len(user_partititons[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name')
self.assertEqual(user_partititons[0].groups[1].name, u'Group C')
...@@ -6,19 +6,17 @@ import ddt ...@@ -6,19 +6,17 @@ import ddt
from mock import patch from mock import patch
from pytz import UTC from pytz import UTC
from unittest import skipUnless
from webob import Response from webob import Response
from django.conf import settings
from django.http import Http404 from django.http import Http404
from django.test import TestCase from django.test import TestCase
from django.test.client import RequestFactory from django.test.client import RequestFactory
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from contentstore.utils import reverse_usage_url, reverse_course_url from contentstore.utils import reverse_usage_url
from contentstore.views.preview import StudioUserService
from contentstore.views.component import ( from contentstore.views.component import (
component_handler, get_component_templates, component_handler, get_component_templates
SPLIT_TEST_COMPONENT_TYPE
) )
from contentstore.tests.utils import CourseTestCase from contentstore.tests.utils import CourseTestCase
...@@ -776,7 +774,6 @@ class TestEditItem(ItemTest): ...@@ -776,7 +774,6 @@ class TestEditItem(ItemTest):
self.verify_publish_state(html_usage_key, PublishState.draft) self.verify_publish_state(html_usage_key, PublishState.draft)
@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class TestEditSplitModule(ItemTest): class TestEditSplitModule(ItemTest):
""" """
Tests around editing instances of the split_test module. Tests around editing instances of the split_test module.
...@@ -977,6 +974,12 @@ class TestEditSplitModule(ItemTest): ...@@ -977,6 +974,12 @@ class TestEditSplitModule(ItemTest):
group_id_to_child = split_test.group_id_to_child group_id_to_child = split_test.group_id_to_child
self.assertEqual(2, len(group_id_to_child)) self.assertEqual(2, len(group_id_to_child))
# Test environment and Studio use different module systems
# (CachingDescriptorSystem is used in tests, PreviewModuleSystem in Studio).
# CachingDescriptorSystem doesn't have user service, that's needed for
# SplitTestModule. So, in this line of code we add this service manually.
split_test.runtime._services['user'] = StudioUserService(self.request) # pylint: disable=protected-access
# Call add_missing_groups method to add the missing group. # Call add_missing_groups method to add the missing group.
split_test.add_missing_groups(self.request) split_test.add_missing_groups(self.request)
split_test = self._assert_children(3) split_test = self._assert_children(3)
...@@ -989,34 +992,6 @@ class TestEditSplitModule(ItemTest): ...@@ -989,34 +992,6 @@ class TestEditSplitModule(ItemTest):
split_test = self._assert_children(3) split_test = self._assert_children(3)
self.assertEqual(group_id_to_child, split_test.group_id_to_child) self.assertEqual(group_id_to_child, split_test.group_id_to_child)
def test_view_index_ok(self):
"""
Basic check that the groups configuration page responds correctly.
"""
if SPLIT_TEST_COMPONENT_TYPE not in self.course.advanced_modules:
self.course.advanced_modules.append(SPLIT_TEST_COMPONENT_TYPE)
self.store.update_item(self.course, self.user.id)
url = reverse_course_url('group_configurations_list_handler', self.course.id)
resp = self.client.get(url)
self.assertContains(resp, self.course.display_name)
self.assertContains(resp, 'First Partition')
self.assertContains(resp, 'alpha')
self.assertContains(resp, 'Second Partition')
self.assertContains(resp, 'Group 1')
def test_view_index_disabled(self):
"""
Check that group configuration page is not displayed when turned off.
"""
if SPLIT_TEST_COMPONENT_TYPE in self.course.advanced_modules:
self.course.advanced_modules.remove(SPLIT_TEST_COMPONENT_TYPE)
self.store.update_item(self.course, self.user.id)
url = reverse_course_url('group_configurations_list_handler', self.course.id)
resp = self.client.get(url)
self.assertContains(resp, "module is disabled")
@ddt.ddt @ddt.ddt
class TestComponentHandler(TestCase): class TestComponentHandler(TestCase):
......
...@@ -214,7 +214,6 @@ define([ ...@@ -214,7 +214,6 @@ define([
"js/spec/models/component_template_spec", "js/spec/models/component_template_spec",
"js/spec/models/explicit_url_spec", "js/spec/models/explicit_url_spec",
"js/spec/models/group_configuration_spec",
"js/spec/utils/drag_and_drop_spec", "js/spec/utils/drag_and_drop_spec",
"js/spec/utils/handle_iframe_binding_spec", "js/spec/utils/handle_iframe_binding_spec",
...@@ -223,6 +222,7 @@ define([ ...@@ -223,6 +222,7 @@ define([
"js/spec/views/baseview_spec", "js/spec/views/baseview_spec",
"js/spec/views/paging_spec", "js/spec/views/paging_spec",
"js/spec/views/assets_spec", "js/spec/views/assets_spec",
"js/spec/views/group_configuration_spec",
"js/spec/views/container_spec", "js/spec/views/container_spec",
"js/spec/views/unit_spec", "js/spec/views/unit_spec",
...@@ -235,8 +235,6 @@ define([ ...@@ -235,8 +235,6 @@ define([
"js/spec/views/modals/base_modal_spec", "js/spec/views/modals/base_modal_spec",
"js/spec/views/modals/edit_xblock_spec", "js/spec/views/modals/edit_xblock_spec",
"js/spec/views/group_configuration_spec",
"js/spec/xblock/cms.runtime.v1_spec", "js/spec/xblock/cms.runtime.v1_spec",
# these tests are run separately in the cms-squire suite, due to process # these tests are run separately in the cms-squire suite, due to process
......
...@@ -176,5 +176,6 @@ jasmine.getFixtures().fixturesPath += 'coffee/fixtures' ...@@ -176,5 +176,6 @@ jasmine.getFixtures().fixturesPath += 'coffee/fixtures'
define([ define([
"coffee/spec/views/assets_spec", "coffee/spec/views/assets_spec",
"js/spec/video/translations_editor_spec", "js/spec/video/translations_editor_spec",
"js/spec/video/file_uploader_editor_spec" "js/spec/video/file_uploader_editor_spec",
"js/spec/models/group_configuration_spec"
]) ])
define([ define([
'backbone', 'js/models/group' 'underscore', 'underscore.string', 'backbone', 'gettext', 'js/models/group'
], ],
function (Backbone, GroupModel) { function (_, str, Backbone, gettext, GroupModel) {
'use strict'; 'use strict';
var GroupCollection = Backbone.Collection.extend({ var GroupCollection = Backbone.Collection.extend({
model: GroupModel, model: GroupModel,
comparator: 'order',
/*
* Return next index for the model.
* @return {Number}
*/
nextOrder: function() {
if(!this.length) {
return 0;
}
return this.last().get('order') + 1;
},
/** /**
* Indicates if the collection is empty when all the models are empty * Indicates if the collection is empty when all the models are empty
* or the collection does not include any models. * or the collection does not include any models.
...@@ -13,7 +25,86 @@ function (Backbone, GroupModel) { ...@@ -13,7 +25,86 @@ function (Backbone, GroupModel) {
return this.length === 0 || this.every(function(m) { return this.length === 0 || this.every(function(m) {
return m.isEmpty(); return m.isEmpty();
}); });
} },
/*
* Return default name for the group.
* @return {String}
* @examples
* Group A, Group B, Group AA, Group ZZZ etc.
*/
getNextDefaultGroupName: function () {
var index = this.nextOrder(),
usedNames = _.pluck(this.toJSON(), 'name'),
name = '';
do {
name = str.sprintf(gettext('Group %s'), this.getGroupId(index));
index ++;
} while (_.contains(usedNames, name));
return name;
},
/*
* Return group id for the default name of the group.
* @param {Number} number Current index of the model in the collection.
* @return {String}
* @examples
* A, B, AA in Group A, Group B, ..., Group AA, etc.
*/
getGroupId: (function () {
/*
Translators: Dictionary used for creation ids that are used in
default group names. For example: A, B, AA in Group A,
Group B, ..., Group AA, etc.
*/
var dict = gettext('ABCDEFGHIJKLMNOPQRSTUVWXYZ').split(''),
len = dict.length,
divide;
divide = function(numerator, denominator) {
if (!_.isNumber(numerator) || !denominator) {
return null;
}
return {
quotient: numerator / denominator,
remainder: numerator % denominator
};
};
return function getId(number) {
var accumulatedValues = '',
result = divide(number, len),
index;
if (result) {
// subtract 1 to start the count with 0.
index = Math.floor(result.quotient) - 1;
// Proceed by dividing the non-remainder part of the
// dividend by the desired base until the result is less
// than one.
if (index < len) {
// if index < 0, we do not need an additional power.
if (index > -1) {
// Get value for the next power.
accumulatedValues += dict[index];
}
} else {
// If we need more than 1 additional power.
// Get value for the next powers.
accumulatedValues += getId(index);
}
// Accumulated values + the current reminder
return accumulatedValues + dict[result.remainder];
}
return String(number);
};
}())
}); });
return GroupCollection; return GroupCollection;
......
define([ define([
'backbone', 'gettext', 'backbone.associations' 'backbone', 'underscore', 'underscore.string', 'gettext',
], function(Backbone, gettext) { 'backbone.associations'
], function(Backbone, _, str, gettext) {
'use strict'; 'use strict';
var Group = Backbone.AssociatedModel.extend({ var Group = Backbone.AssociatedModel.extend({
defaults: function() { defaults: function() {
return { return {
name: '', name: '',
version: null version: null,
}; order: null
};
}, },
isEmpty: function() { isEmpty: function() {
...@@ -16,13 +18,14 @@ define([ ...@@ -16,13 +18,14 @@ define([
toJSON: function() { toJSON: function() {
return { return {
id: this.get('id'),
name: this.get('name'), name: this.get('name'),
version: this.get('version') version: this.get('version')
}; };
}, },
validate: function(attrs) { validate: function(attrs) {
if (!attrs.name) { if (!str.trim(attrs.name)) {
return { return {
message: gettext('Group name is required'), message: gettext('Group name is required'),
attributes: { name: true } attributes: { name: true }
......
...@@ -11,7 +11,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -11,7 +11,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
name: '', name: '',
description: '', description: '',
version: null, version: null,
groups: new GroupCollection([]), groups: new GroupCollection([
{
name: gettext('Group A'),
order: 0
},
{
name: gettext('Group B'),
order: 1
}
]),
showGroups: false, showGroups: false,
editing: false editing: false
}; };
...@@ -30,16 +39,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -30,16 +39,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
}, },
setOriginalAttributes: function() { setOriginalAttributes: function() {
this._originalAttributes = this.toJSON(); this._originalAttributes = this.parse(this.toJSON());
}, },
reset: function() { reset: function() {
this.set(this._originalAttributes); this.set(this._originalAttributes, { parse: true });
}, },
isDirty: function() { isDirty: function() {
return !_.isEqual( return !_.isEqual(
this._originalAttributes, this.toJSON() this._originalAttributes, this.parse(this.toJSON())
); );
}, },
...@@ -47,6 +56,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -47,6 +56,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
return !this.get('name') && this.get('groups').isEmpty(); return !this.get('name') && this.get('groups').isEmpty();
}, },
parse: function(response) {
var attrs = $.extend(true, {}, response);
_.each(attrs.groups, function(group, index) {
group.order = group.order || index;
});
return attrs;
},
toJSON: function() { toJSON: function() {
return { return {
id: this.get('id'), id: this.get('id'),
...@@ -64,7 +83,29 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -64,7 +83,29 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
attributes: {name: true} attributes: {name: true}
}; };
} }
if (attrs.groups.length < 2) {
return {
message: gettext('There must be at least two groups'),
attributes: { groups: true }
};
} else {
// validate all groups
var invalidGroups = [];
attrs.groups.each(function(group) {
if(!group.isValid()) {
invalidGroups.push(group);
}
});
if (!_.isEmpty(invalidGroups)) {
return {
message: gettext('All groups must have a name'),
attributes: { groups: invalidGroups }
};
}
}
} }
}); });
return GroupConfiguration; return GroupConfiguration;
}); });
define([ define([
'backbone', 'coffee/src/main', 'js/models/group_configuration', 'backbone', 'coffee/src/main', 'js/models/group_configuration',
'js/models/group', 'js/collections/group' 'js/models/group', 'js/collections/group', 'squire'
], function( ], function(
Backbone, main, GroupConfigurationModel, GroupModel, GroupCollection Backbone, main, GroupConfigurationModel, GroupModel, GroupCollection, Squire
) { ) {
'use strict'; 'use strict';
beforeEach(function() { beforeEach(function() {
...@@ -32,11 +32,12 @@ define([ ...@@ -32,11 +32,12 @@ define([
expect(this.model.get('showGroups')).toBeFalsy(); expect(this.model.get('showGroups')).toBeFalsy();
}); });
it('should be empty by default', function() { it('should have a collection with 2 groups by default', function() {
var groups = this.model.get('groups'); var groups = this.model.get('groups');
expect(groups).toBeInstanceOf(GroupCollection); expect(groups).toBeInstanceOf(GroupCollection);
expect(this.model.isEmpty()).toBeTruthy(); expect(groups.at(0).get('name')).toBe('Group A');
expect(groups.at(1).get('name')).toBe('Group B');
}); });
it('should be able to reset itself', function() { it('should be able to reset itself', function() {
...@@ -72,8 +73,6 @@ define([ ...@@ -72,8 +73,6 @@ define([
return deepAttributes(obj.attributes); return deepAttributes(obj.attributes);
} else if (obj instanceof Backbone.Collection) { } else if (obj instanceof Backbone.Collection) {
return obj.map(deepAttributes); return obj.map(deepAttributes);
} else if (_.isArray(obj)) {
return _.map(obj, deepAttributes);
} else if (_.isObject(obj)) { } else if (_.isObject(obj)) {
var attributes = {}; var attributes = {};
...@@ -114,14 +113,18 @@ define([ ...@@ -114,14 +113,18 @@ define([
'groups': [ 'groups': [
{ {
'version': 1, 'version': 1,
'order': 0,
'name': 'Group 1' 'name': 'Group 1'
}, { }, {
'version': 1, 'version': 1,
'order': 1,
'name': 'Group 2' 'name': 'Group 2'
} }
] ]
}, },
model = new GroupConfigurationModel(serverModelSpec); model = new GroupConfigurationModel(
serverModelSpec, { parse: true }
);
expect(deepAttributes(model)).toEqual(clientModelSpec); expect(deepAttributes(model)).toEqual(clientModelSpec);
expect(model.toJSON()).toEqual(serverModelSpec); expect(model.toJSON()).toEqual(serverModelSpec);
...@@ -145,11 +148,12 @@ define([ ...@@ -145,11 +148,12 @@ define([
describe('GroupModel', function() { describe('GroupModel', function() {
beforeEach(function() { beforeEach(function() {
this.model = new GroupModel(); this.collection = new GroupCollection([{}]);
this.model = this.collection.at(0);
}); });
describe('Basic', function() { describe('Basic', function() {
it('should have a name by default', function() { it('should have an empty name by default', function() {
expect(this.model.get('name')).toEqual(''); expect(this.model.get('name')).toEqual('');
}); });
...@@ -166,10 +170,41 @@ define([ ...@@ -166,10 +170,41 @@ define([
}); });
it('can pass validation', function() { it('can pass validation', function() {
var model = new GroupModel({ name: 'a' }); var model = new GroupConfigurationModel({ name: 'foo' });
expect(model.isValid()).toBeTruthy(); expect(model.isValid()).toBeTruthy();
}); });
it('requires at least two groups', function() {
var group1 = new GroupModel({ name: 'Group A' }),
group2 = new GroupModel({ name: 'Group B' }),
model = new GroupConfigurationModel({ name: 'foo' });
model.get('groups').reset([group1]);
expect(model.isValid()).toBeFalsy();
model.get('groups').add(group2);
expect(model.isValid()).toBeTruthy();
});
it('requires a valid group', function() {
var group = new GroupModel(),
model = new GroupConfigurationModel({ name: 'foo' });
model.get('groups').reset([group]);
expect(model.isValid()).toBeFalsy();
});
it('requires all groups to be valid', function() {
var group1 = new GroupModel({ name: 'Group A' }),
group2 = new GroupModel(),
model = new GroupConfigurationModel({ name: 'foo' });
model.get('groups').reset([group1, group2]);
expect(model.isValid()).toBeFalsy();
});
}); });
}); });
...@@ -183,15 +218,94 @@ define([ ...@@ -183,15 +218,94 @@ define([
}); });
it('is empty if all groups are empty', function() { it('is empty if all groups are empty', function() {
this.collection.add([{}, {}, {}]); this.collection.add([{ name: '' }, { name: '' }, { name: '' }]);
expect(this.collection.isEmpty()).toBeTruthy(); expect(this.collection.isEmpty()).toBeTruthy();
}); });
it('is not empty if a group is not empty', function() { it('is not empty if a group is not empty', function() {
this.collection.add([{}, { name: 'full' }, {} ]); this.collection.add([
{ name: '' }, { name: 'full' }, { name: '' }
]);
expect(this.collection.isEmpty()).toBeFalsy(); expect(this.collection.isEmpty()).toBeFalsy();
}); });
describe('getGroupId', function () {
var collection, injector, mockGettext, initializeGroupModel;
mockGettext = function (returnedValue) {
var injector = new Squire();
injector.mock('gettext', function () {
return function () { return returnedValue; };
});
return injector;
};
initializeGroupModel = function (dict, that) {
runs(function() {
injector = mockGettext(dict);
injector.require(['js/collections/group'],
function(GroupCollection) {
collection = new GroupCollection();
});
});
waitsFor(function() {
return collection;
}, 'GroupModel was not instantiated', 500);
that.after(function () {
collection = null;
injector.clean();
injector.remove();
});
};
it('returns correct ids', function () {
var collection = new GroupCollection();
expect(collection.getGroupId(0)).toBe('A');
expect(collection.getGroupId(1)).toBe('B');
expect(collection.getGroupId(25)).toBe('Z');
expect(collection.getGroupId(702)).toBe('AAA');
expect(collection.getGroupId(704)).toBe('AAC');
expect(collection.getGroupId(475253)).toBe('ZZZZ');
expect(collection.getGroupId(475254)).toBe('AAAAA');
expect(collection.getGroupId(475279)).toBe('AAAAZ');
});
it('just 1 character in the dictionary', function () {
initializeGroupModel('1', this);
runs(function() {
expect(collection.getGroupId(0)).toBe('1');
expect(collection.getGroupId(1)).toBe('11');
expect(collection.getGroupId(5)).toBe('111111');
});
});
it('allow to use unicode characters in the dict', function () {
initializeGroupModel('ö诶úeœ', this);
runs(function() {
expect(collection.getGroupId(0)).toBe('ö');
expect(collection.getGroupId(1)).toBe('诶');
expect(collection.getGroupId(5)).toBe('öö');
expect(collection.getGroupId(29)).toBe('œœ');
expect(collection.getGroupId(30)).toBe('ööö');
expect(collection.getGroupId(43)).toBe('öúe');
});
});
it('return initial value if dictionary is empty', function () {
initializeGroupModel('', this);
runs(function() {
expect(collection.getGroupId(0)).toBe('0');
expect(collection.getGroupId(5)).toBe('5');
expect(collection.getGroupId(30)).toBe('30');
});
});
});
}); });
}); });
...@@ -3,13 +3,15 @@ define([ ...@@ -3,13 +3,15 @@ define([
'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',
'js/views/group_configuration_item', 'js/views/feedback_notification', 'js/views/group_configuration_item', 'js/models/group',
'js/spec_helpers/create_sinon', 'js/spec_helpers/edit_helpers', 'js/collections/group', 'js/views/group_edit',
'jasmine-stealth' 'js/views/feedback_notification', 'js/spec_helpers/create_sinon',
'js/spec_helpers/edit_helpers', 'jasmine-stealth'
], function( ], function(
Course, GroupConfigurationModel, GroupConfigurationCollection, Course, GroupConfigurationModel, GroupConfigurationCollection,
GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit, GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit,
GroupConfigurationItem, Notification, create_sinon, view_helpers GroupConfigurationItem, GroupModel, GroupCollection, GroupEdit,
Notification, create_sinon, view_helpers
) { ) {
'use strict'; 'use strict';
var SELECTORS = { var SELECTORS = {
...@@ -17,13 +19,15 @@ define([ ...@@ -17,13 +19,15 @@ define([
editView: '.group-configuration-edit', editView: '.group-configuration-edit',
itemView: '.group-configurations-list-item', itemView: '.group-configurations-list-item',
group: '.group', group: '.group',
groupFields: '.groups-fields',
name: '.group-configuration-name', name: '.group-configuration-name',
description: '.group-configuration-description', description: '.group-configuration-description',
groupsCount: '.group-configuration-groups-count', groupsCount: '.group-configuration-groups-count',
groupsAllocation: '.group-allocation', groupsAllocation: '.group-allocation',
errorMessage: '.group-configuration-edit-error', errorMessage: '.group-configuration-edit-error',
inputGroupName: '.group-name',
inputName: '.group-configuration-name-input', inputName: '.group-configuration-name-input',
inputDescription: '.group-configuration-description-input' inputDescription: '.group-configuration-description-input',
}; };
beforeEach(function() { beforeEach(function() {
...@@ -59,6 +63,13 @@ define([ ...@@ -59,6 +63,13 @@ define([
return _.every(values, function (value, key) { return _.every(values, function (value, key) {
return this.actual.get(key) === value; return this.actual.get(key) === value;
}.bind(this)); }.bind(this));
},
toHaveDefaultNames: function (values) {
var actualValues = $.map(this.actual, function (item) {
return $(item).val();
});
return _.isEqual(actualValues, values);
} }
}); });
}); });
...@@ -90,7 +101,7 @@ define([ ...@@ -90,7 +101,7 @@ define([
}); });
it('should show groups appropriately', function() { it('should show groups appropriately', function() {
this.model.get('groups').add([{}, {}, {}]); this.model.get('groups').add([{}]);
this.model.set('showGroups', false); this.model.set('showGroups', false);
this.view.$('.show-groups').click(); this.view.$('.show-groups').click();
...@@ -104,7 +115,7 @@ define([ ...@@ -104,7 +115,7 @@ define([
}); });
it('should hide groups appropriately', function() { it('should hide groups appropriately', function() {
this.model.get('groups').add([{}, {}, {}]); this.model.get('groups').add([{}]);
this.model.set('showGroups', true); this.model.set('showGroups', true);
this.view.$('.hide-groups').click(); this.view.$('.hide-groups').click();
...@@ -129,7 +140,9 @@ define([ ...@@ -129,7 +140,9 @@ define([
beforeEach(function() { beforeEach(function() {
view_helpers.installViewTemplates(); view_helpers.installViewTemplates();
view_helpers.installTemplate('group-configuration-edit'); view_helpers.installTemplates([
'group-configuration-edit', 'group-edit'
]);
this.model = new GroupConfigurationModel({ this.model = new GroupConfigurationModel({
name: 'Configuration', name: 'Configuration',
...@@ -152,10 +165,18 @@ define([ ...@@ -152,10 +165,18 @@ define([
}); });
}); });
it ('should allow you to create new groups', function() {
var numGroups = this.model.get('groups').length;
this.view.$('.action-add-group').click();
expect(this.model.get('groups').length).toEqual(numGroups + 1);
});
it('should save properly', function() { it('should save properly', function() {
var requests = create_sinon.requests(this), var requests = create_sinon.requests(this),
notificationSpy = view_helpers.createNotificationSpy(); notificationSpy = view_helpers.createNotificationSpy(),
groups;
this.view.$('.action-add-group').click();
setValuesToInputs(this.view, { setValuesToInputs(this.view, {
inputName: 'New Configuration', inputName: 'New Configuration',
inputDescription: 'New Description' inputDescription: 'New Description'
...@@ -170,6 +191,10 @@ define([ ...@@ -170,6 +191,10 @@ define([
name: 'New Configuration', name: 'New Configuration',
description: 'New Description' description: 'New Description'
}); });
groups = this.model.get('groups');
expect(groups.length).toBe(3);
expect(groups.at(2).get('name')).toBe('Group C');
expect(this.view.$el).not.toExist(); expect(this.view.$el).not.toExist();
}); });
...@@ -185,10 +210,14 @@ define([ ...@@ -185,10 +210,14 @@ define([
}); });
it('does not save on cancel', function() { it('does not save on cancel', function() {
this.view.$('.action-add-group').click();
setValuesToInputs(this.view, { setValuesToInputs(this.view, {
inputName: 'New Configuration', inputName: 'New Configuration',
inputDescription: 'New Description' inputDescription: 'New Description'
}); });
expect(this.model.get('groups').length).toBe(3);
this.view.$('.action-cancel').click(); this.view.$('.action-cancel').click();
expect(this.model).toBeCorrectValuesInModel({ expect(this.model).toBeCorrectValuesInModel({
name: 'Configuration', name: 'Configuration',
...@@ -197,6 +226,7 @@ define([ ...@@ -197,6 +226,7 @@ define([
// Model is still exist in the collection // Model is still exist in the collection
expect(this.collection.indexOf(this.model)).toBeGreaterThan(-1); expect(this.collection.indexOf(this.model)).toBeGreaterThan(-1);
expect(this.collection.length).toBe(1); expect(this.collection.length).toBe(1);
expect(this.model.get('groups').length).toBe(2);
}); });
it('should be removed on cancel if it is a new item', function() { it('should be removed on cancel if it is a new item', function() {
...@@ -236,6 +266,69 @@ define([ ...@@ -236,6 +266,69 @@ define([
expect(this.view.$(SELECTORS.errorMessage)).not.toExist(); expect(this.view.$(SELECTORS.errorMessage)).not.toExist();
expect(requests.length).toBe(1); expect(requests.length).toBe(1);
}); });
it('should have appropriate class names on focus/blur', function () {
var groupInput = this.view.$(SELECTORS.inputGroupName).first(),
groupFields = this.view.$(SELECTORS.groupFields);
groupInput.focus();
expect(groupFields).toHaveClass('is-focused');
groupInput.blur();
expect(groupFields).not.toHaveClass('is-focused');
});
describe('removes all newly created groups on cancel', function () {
it('if the model has a non-empty groups', function() {
var groups = this.model.get('groups');
this.view.render();
groups.add([{ name: 'non-empty' }]);
expect(groups.length).toEqual(3);
this.view.$('.action-cancel').click();
// Restore to default state (2 groups by default).
expect(groups.length).toEqual(2);
});
it('if the model has no non-empty groups', function() {
var groups = this.model.get('groups');
this.view.render();
groups.add([{}, {}, {}]);
expect(groups.length).toEqual(5);
this.view.$('.action-cancel').click();
// Restore to default state (2 groups by default).
expect(groups.length).toEqual(2);
});
});
it('groups have correct default names and placeholders', function () {
var group1 = new GroupModel({ name: 'Group A' }),
group2 = new GroupModel({ name: 'Group B' }),
collection = this.model.get('groups');
collection.reset([group1, group2]); // Group A, Group B
this.view.$('.action-add-group').click(); // Add Group C
this.view.$('.action-add-group').click(); // Add Group D
this.view.$('.action-add-group').click(); // Add Group E
expect(this.view.$(SELECTORS.inputGroupName)).toHaveDefaultNames([
'Group A', 'Group B', 'Group C', 'Group D', 'Group E'
]);
// Remove Group B
this.view.$('.group-1 .action-close').click();
expect(this.view.$(SELECTORS.inputGroupName)).toHaveDefaultNames([
'Group A', 'Group C', 'Group D', 'Group E'
]);
this.view.$('.action-add-group').click(); // Add Group F
this.view.$('.action-add-group').click(); // Add Group G
expect(this.view.$(SELECTORS.inputGroupName)).toHaveDefaultNames([
'Group A', 'Group C', 'Group D', 'Group E', 'Group F', 'Group G'
]);
});
}); });
describe('GroupConfigurationsList', function() { describe('GroupConfigurationsList', function() {
...@@ -254,7 +347,7 @@ define([ ...@@ -254,7 +347,7 @@ define([
expect(this.view.$el).toContainText( expect(this.view.$el).toContainText(
'You haven\'t created any group configurations yet.' 'You haven\'t created any group configurations yet.'
); );
expect(this.view.$el).toContain('.new-button'); expect(this.view.$('.new-button')).toExist();
expect(this.view.$(SELECTORS.itemView)).not.toExist(); expect(this.view.$(SELECTORS.itemView)).not.toExist();
}); });
...@@ -273,8 +366,9 @@ define([ ...@@ -273,8 +366,9 @@ define([
describe('GroupConfigurationItem', function() { describe('GroupConfigurationItem', function() {
beforeEach(function() { beforeEach(function() {
view_helpers.installTemplate('group-configuration-edit', true); view_helpers.installTemplates([
view_helpers.installTemplate('group-configuration-details'); 'group-configuration-edit', 'group-configuration-details'
], 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.view = new GroupConfigurationItem({ this.view = new GroupConfigurationItem({
...@@ -294,6 +388,35 @@ define([ ...@@ -294,6 +388,35 @@ define([
expect(this.view.$(SELECTORS.editView)).not.toExist(); expect(this.view.$(SELECTORS.editView)).not.toExist();
}); });
}); });
describe('GroupEdit', function() {
beforeEach(function() {
view_helpers.installTemplate('group-edit', true);
this.model = new GroupModel({
name: 'Group A'
});
this.collection = new GroupCollection([this.model]);
this.view = new GroupEdit({
model: this.model
});
});
describe('Basic', function () {
it('can render properly', function() {
this.view.render();
expect(this.view.$('.group-name').val()).toBe('Group A');
expect(this.view.$('.group-allocation')).toContainText('100%');
});
it ('can delete itself', function() {
this.view.render().$('.action-close').click();
expect(this.collection.length).toEqual(0);
});
});
});
}); });
...@@ -3,12 +3,13 @@ ...@@ -3,12 +3,13 @@
*/ */
define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sinon"], define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sinon"],
function($, NotificationView, create_sinon) { function($, NotificationView, create_sinon) {
var installTemplate, installViewTemplates, createNotificationSpy, verifyNotificationShowing, var installTemplate, installTemplates, installViewTemplates, createNotificationSpy,
verifyNotificationHidden; verifyNotificationShowing, verifyNotificationHidden;
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 {
...@@ -16,6 +17,19 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino ...@@ -16,6 +17,19 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino
} }
}; };
installTemplates = function(templateNames, isFirst) {
if (!$.isArray(templateNames)) {
templateNames = [templateNames];
}
$.each(templateNames, function(index, templateName) {
installTemplate(templateName, isFirst);
if (isFirst) {
isFirst = false;
}
});
};
installViewTemplates = function(append) { installViewTemplates = function(append) {
installTemplate('system-feedback', !append); installTemplate('system-feedback', !append);
appendSetFixtures('<div id="page-notification"></div>'); appendSetFixtures('<div id="page-notification"></div>');
...@@ -41,6 +55,7 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino ...@@ -41,6 +55,7 @@ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sino
return { return {
'installTemplate': installTemplate, 'installTemplate': installTemplate,
'installTemplates': installTemplates,
'installViewTemplates': installViewTemplates, 'installViewTemplates': installViewTemplates,
'createNotificationSpy': createNotificationSpy, 'createNotificationSpy': createNotificationSpy,
'verifyNotificationShowing': verifyNotificationShowing, 'verifyNotificationShowing': verifyNotificationShowing,
......
...@@ -42,13 +42,13 @@ function(BaseView, _, gettext) { ...@@ -42,13 +42,13 @@ function(BaseView, _, gettext) {
this.model.set('editing', true); this.model.set('editing', true);
}, },
showGroups: function(e) { showGroups: function(event) {
if(e && e.preventDefault) { e.preventDefault(); } if(event && event.preventDefault) { event.preventDefault(); }
this.model.set('showGroups', true); this.model.set('showGroups', true);
}, },
hideGroups: function(e) { hideGroups: function(event) {
if(e && e.preventDefault) { e.preventDefault(); } if(event && event.preventDefault) { event.preventDefault(); }
this.model.set('showGroups', false); this.model.set('showGroups', false);
}, },
......
define([ define([
'js/views/baseview', 'underscore', 'jquery', 'gettext' 'js/views/baseview', 'underscore', 'jquery', 'gettext',
'js/views/group_edit'
], ],
function(BaseView, _, $, gettext) { function(BaseView, _, $, gettext, GroupEdit) {
'use strict'; 'use strict';
var GroupConfigurationEdit = BaseView.extend({ var GroupConfigurationEdit = BaseView.extend({
tagName: 'div', tagName: 'div',
events: { events: {
'change .group-configuration-name-input': 'setName', 'change .group-configuration-name-input': 'setName',
'change .group-configuration-description-input': 'setDescription', 'change .group-configuration-description-input': 'setDescription',
"click .action-add-group": "createGroup",
'focus .input-text': 'onFocus', 'focus .input-text': 'onFocus',
'blur .input-text': 'onBlur', 'blur .input-text': 'onBlur',
'submit': 'setAndClose', 'submit': 'setAndClose',
...@@ -24,8 +26,14 @@ function(BaseView, _, $, gettext) { ...@@ -24,8 +26,14 @@ function(BaseView, _, $, gettext) {
}, },
initialize: function() { initialize: function() {
var groups;
this.template = this.loadTemplate('group-configuration-edit'); this.template = this.loadTemplate('group-configuration-edit');
this.listenTo(this.model, 'invalid', this.render); this.listenTo(this.model, 'invalid', this.render);
groups = this.model.get('groups');
this.listenTo(groups, 'add', this.addOne);
this.listenTo(groups, 'reset', this.addAll);
this.listenTo(groups, 'all', this.render);
}, },
render: function() { render: function() {
...@@ -37,10 +45,31 @@ function(BaseView, _, $, gettext) { ...@@ -37,10 +45,31 @@ function(BaseView, _, $, gettext) {
isNew: this.model.isNew(), isNew: this.model.isNew(),
error: this.model.validationError error: this.model.validationError
})); }));
this.addAll();
return this; return this;
}, },
addOne: function(group) {
var view = new GroupEdit({ model: group });
this.$('ol.groups').append(view.render().el);
return this;
},
addAll: function() {
this.model.get('groups').each(this.addOne, this);
},
createGroup: function(event) {
if(event && event.preventDefault) { event.preventDefault(); }
var collection = this.model.get('groups');
collection.add([{
name: collection.getNextDefaultGroupName(),
order: collection.nextOrder()
}]);
},
setName: function(event) { setName: function(event) {
if(event && event.preventDefault) { event.preventDefault(); } if(event && event.preventDefault) { event.preventDefault(); }
this.model.set( this.model.set(
...@@ -62,6 +91,16 @@ function(BaseView, _, $, gettext) { ...@@ -62,6 +91,16 @@ function(BaseView, _, $, gettext) {
this.setName(); this.setName();
this.setDescription(); this.setDescription();
_.each(this.$('.groups li'), function(li, i) {
var group = this.model.get('groups').at(i);
if(group) {
group.set({
'name': $('.group-name', li).val()
});
}
}, this);
return this; return this;
}, },
......
...@@ -56,7 +56,7 @@ define([ ...@@ -56,7 +56,7 @@ 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 }]);
} }
}); });
......
define([
'js/views/baseview', 'underscore', 'underscore.string', 'jquery', 'gettext'
],
function(BaseView, _, str, $, gettext) {
'use strict';
_.str = str; // used in template
var GroupEdit = BaseView.extend({
tagName: 'li',
events: {
'click .action-close': 'removeGroup',
'change .group-name': 'changeName',
'focus .groups-fields input': 'onFocus',
'blur .groups-fields input': 'onBlur'
},
className: function() {
var index = this.model.collection.indexOf(this.model);
return 'field-group group group-' + index;
},
initialize: function() {
this.template = this.loadTemplate('group-edit');
this.listenTo(this.model, 'change', this.render);
},
render: function() {
var collection = this.model.collection,
index = collection.indexOf(this.model);
this.$el.html(this.template({
name: this.model.escape('name'),
allocation: this.getAllocation(),
index: index,
error: this.model.validationError
}));
return this;
},
changeName: function(event) {
if(event && event.preventDefault) { event.preventDefault(); }
this.model.set({
name: this.$('.group-name').val()
}, { silent: true });
return this;
},
removeGroup: function(event) {
if(event && event.preventDefault) { event.preventDefault(); }
this.model.collection.remove(this.model);
return this.remove();
},
getAllocation: function() {
return Math.floor(100 / this.model.collection.length);
},
onFocus: function () {
this.$el.closest('.groups-fields').addClass('is-focused');
},
onBlur: function () {
this.$el.closest('.groups-fields').removeClass('is-focused');
}
});
return GroupEdit;
});
...@@ -50,6 +50,7 @@ lib_paths: ...@@ -50,6 +50,7 @@ lib_paths:
- xmodule_js/common_static/js/vendor/jasmine-imagediff.js - xmodule_js/common_static/js/vendor/jasmine-imagediff.js
- xmodule_js/common_static/js/vendor/jasmine.async.js - xmodule_js/common_static/js/vendor/jasmine.async.js
- xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js - xmodule_js/common_static/js/vendor/CodeMirror/codemirror.js
- xmodule_js/common_static/js/vendor/domReady.js
- xmodule_js/common_static/js/vendor/URI.min.js - xmodule_js/common_static/js/vendor/URI.min.js
- xmodule_js/src/xmodule.js - xmodule_js/src/xmodule.js
- xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js - xmodule_js/common_static/coffee/src/jquery.immediateDescendents.js
......
...@@ -250,6 +250,7 @@ ...@@ -250,6 +250,7 @@
} }
} }
.groups-fields,
.group-configuration-fields { .group-configuration-fields {
@extend %cont-no-list; @extend %cont-no-list;
...@@ -361,6 +362,10 @@ ...@@ -361,6 +362,10 @@
margin-left: ($baseline*0.5); margin-left: ($baseline*0.5);
} }
} }
&.group-allocation {
color: $gray-l1;
}
} }
label.required { label.required {
...@@ -371,11 +376,67 @@ ...@@ -371,11 +376,67 @@
content: "*"; content: "*";
} }
} }
.field-group {
@include clearfix();
margin: 0 0 ($baseline/2) 0;
padding: ($baseline/4) 0 0 0;
.group-allocation,
.field {
display: inline-block;
vertical-align: middle;
margin: 0 3% 0 0;
}
.group-allocation {
max-width: 10%;
min-width: 5%;
color: $gray-l1;
}
.field {
position: relative;
&.long {
width: 80%;
}
&.short {
width: 10%;
}
}
.action-close {
@include transition(color 0.25s ease-in-out);
@include font-size(22);
display: inline-block;
border: 0;
padding: 0;
background: transparent;
color: $blue-l3;
vertical-align: middle;
&:hover {
color: $blue;
}
}
}
} }
.group-configuration-fields { .group-configuration-fields {
margin-bottom: $baseline; margin-bottom: $baseline;
} }
.action-add-group {
@extend %ui-btn-flat-outline;
@include font-size(16);
display: block;
width: 100%;
margin: ($baseline*1.5) 0 0 0;
padding: ($baseline/2);
font-weight: 600;
}
} }
} }
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
<%block name="bodyclass">is-signedin course view-group-configurations</%block> <%block name="bodyclass">is-signedin course view-group-configurations</%block>
<%block name="header_extras"> <%block name="header_extras">
% for template_name in ["group-configuration-details", "group-configuration-edit", "no-group-configurations", "basic-modal", "modal-button"]: % for template_name in ["group-configuration-details", "group-configuration-edit", "no-group-configurations", "group-edit", "basic-modal", "modal-button"]:
<script type="text/template" id="${template_name}-tpl"> <script type="text/template" id="${template_name}-tpl">
<%static:include path="js/${template_name}.underscore" /> <%static:include path="js/${template_name}.underscore" />
</script> </script>
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
require(["domReady!", "js/collections/group_configuration", "js/views/pages/group_configurations"], require(["domReady!", "js/collections/group_configuration", "js/views/pages/group_configurations"],
function(doc, GroupConfigurationCollection, GroupConfigurationsPage) { function(doc, GroupConfigurationCollection, GroupConfigurationsPage) {
% if configurations is not None: % if configurations is not None:
var collection = new GroupConfigurationCollection(${json.dumps(configurations)}); var collection = new GroupConfigurationCollection(${json.dumps(configurations)}, { parse: true });
collection.url = "${group_configuration_url}"; collection.url = "${group_configuration_url}";
new GroupConfigurationsPage({ new GroupConfigurationsPage({
......
...@@ -25,6 +25,13 @@ ...@@ -25,6 +25,13 @@
<span class="tip tip-stacked"><%= gettext("Optional long description") %></span> <span class="tip tip-stacked"><%= gettext("Optional long description") %></span>
</div> </div>
</fieldset> </fieldset>
<fieldset class="groups-fields">
<legend class="sr"><%= gettext("Group information") %></legend>
<label class="groups-fields-label required"><%= gettext("Groups") %></label>
<span class="tip tip-stacked"><%= gettext("Name of the groups that students will be assigned to, for example, Control, Video, Problems. You must have two or more groups.") %></span>
<ol class="groups list-input enum"></ol>
<button class="action action-add-group"><i class="icon-plus"></i> <%= gettext("Add another group") %></button>
</fieldset>
</div> </div>
<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>
......
<div class="input-wrap field long text required field-add-group-name group-<%= index %>-name
<% if (error && error.attributes && error.attributes.name) { print('error'); } %>"><input name="group-<%= index %>-name" class="group-name long" value="<%= name %>" type="text">
</div><div class="group-allocation"><%= allocation %>%</div>
<a href="" class="action action-close"><i class="icon-remove-sign"></i> <span class="sr"><%= gettext("delete group") %></span></a>
...@@ -126,7 +126,7 @@ class ContainerPage(PageObject): ...@@ -126,7 +126,7 @@ class ContainerPage(PageObject):
""" """
Click the "add missing groups" link. Click the "add missing groups" link.
""" """
click_css(self, '.add-missing-groups-button') self.q(css='.add-missing-groups-button').first.click()
def missing_groups_button_present(self): def missing_groups_button_present(self):
""" """
......
...@@ -55,6 +55,13 @@ class GroupConfiguration(object): ...@@ -55,6 +55,13 @@ class GroupConfiguration(object):
css = 'a.group-toggle' css = 'a.group-toggle'
self.find_css(css).first.click() self.find_css(css).first.click()
def add_group(self):
"""
Add new group.
"""
css = 'button.action-add-group'
self.find_css(css).first.click()
def get_text(self, css): def get_text(self, css):
""" """
Return text for the defined by css locator. Return text for the defined by css locator.
...@@ -144,10 +151,10 @@ class GroupConfiguration(object): ...@@ -144,10 +151,10 @@ class GroupConfiguration(object):
""" """
css = '.group' css = '.group'
def group_selector(config_index, group_index): def group_selector(group_index):
return self.get_selector('.groups-{} .group-{} '.format(config_index, group_index)) return self.get_selector('.group-{} '.format(group_index))
return [Group(self.page, group_selector(self.index, 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(css))]
def __repr__(self): def __repr__(self):
return "<{}:{}>".format(self.__class__.__name__, self.name) return "<{}:{}>".format(self.__class__.__name__, self.name)
...@@ -170,11 +177,19 @@ class Group(object): ...@@ -170,11 +177,19 @@ class Group(object):
@property @property
def name(self): def name(self):
""" """
Return group name. Return the name of the group .
""" """
css = '.group-name' css = '.group-name'
return self.find_css(css).first.text[0] return self.find_css(css).first.text[0]
@name.setter
def name(self, value):
"""
Set the name for the group.
"""
css = '.group-name'
self.find_css(css).first.fill(value)
@property @property
def allocation(self): def allocation(self):
""" """
...@@ -183,5 +198,12 @@ class Group(object): ...@@ -183,5 +198,12 @@ class Group(object):
css = '.group-allocation' css = '.group-allocation'
return self.find_css(css).first.text[0] return self.find_css(css).first.text[0]
def remove(self):
"""
Remove the group.
"""
css = '.action-close'
return self.find_css(css).first.click()
def __repr__(self): def __repr__(self):
return "<{}:{}>".format(self.__class__.__name__, self.name) return "<{}:{}>".format(self.__class__.__name__, self.name)
...@@ -11,7 +11,7 @@ from ..pages.studio.component_editor import ComponentEditorView ...@@ -11,7 +11,7 @@ from ..pages.studio.component_editor import ComponentEditorView
from ..pages.studio.utils import add_discussion from ..pages.studio.utils import add_discussion
from unittest import skip from unittest import skip
from bok_choy.promise import Promise
class ContainerBase(UniqueCourseTest): class ContainerBase(UniqueCourseTest):
""" """
...@@ -93,30 +93,6 @@ class ContainerBase(UniqueCourseTest): ...@@ -93,30 +93,6 @@ class ContainerBase(UniqueCourseTest):
break break
self.assertEqual(len(blocks_checked), len(xblocks)) self.assertEqual(len(blocks_checked), len(xblocks))
def verify_groups(self, container, active_groups, inactive_groups):
"""
Check that the groups appear and are correctly categorized as to active and inactive.
Also checks that the "add missing groups" button/link is not present unless a value of False is passed
for verify_missing_groups_not_present.
"""
def wait_for_xblocks_to_render():
# First xblock is the container for the page, subtract 1.
return (len(active_groups) + len(inactive_groups) == len(container.xblocks) - 1, len(active_groups))
Promise(wait_for_xblocks_to_render, "Number of xblocks on the page are incorrect").fulfill()
def check_xblock_names(expected_groups, actual_blocks):
self.assertEqual(len(expected_groups), len(actual_blocks))
for idx, expected in enumerate(expected_groups):
self.assertEqual('Expand or Collapse\n{}'.format(expected), actual_blocks[idx].name)
check_xblock_names(active_groups, container.active_xblocks)
check_xblock_names(inactive_groups, container.inactive_xblocks)
# Verify inactive xblocks appear after active xblocks
check_xblock_names(active_groups + inactive_groups, container.xblocks[1:])
def do_action_and_verify(self, action, expected_ordering): def do_action_and_verify(self, action, expected_ordering):
""" """
Perform the supplied action and then verify the resulting ordering. Perform the supplied action and then verify the resulting ordering.
......
...@@ -18,11 +18,51 @@ from ..pages.studio.auto_auth import AutoAuthPage ...@@ -18,11 +18,51 @@ from ..pages.studio.auto_auth import AutoAuthPage
from ..pages.studio.utils import add_advanced_component from ..pages.studio.utils import add_advanced_component
from ..pages.xblock.utils import wait_for_xblock_initialization from ..pages.xblock.utils import wait_for_xblock_initialization
from .helpers import UniqueCourseTest from .helpers import UniqueCourseTest
from test_studio_container import ContainerBase from test_studio_container import ContainerBase
class SplitTest(ContainerBase): class SplitTestMixin(object):
"""
Mixin that contains useful methods for split_test module testing.
"""
def verify_groups(self, container, active_groups, inactive_groups, verify_missing_groups_not_present=True):
"""
Check that the groups appear and are correctly categorized as to active and inactive.
Also checks that the "add missing groups" button/link is not present unless a value of False is passed
for verify_missing_groups_not_present.
"""
def wait_for_xblocks_to_render():
# First xblock is the container for the page, subtract 1.
return (len(active_groups) + len(inactive_groups) == len(container.xblocks) - 1, len(active_groups))
Promise(wait_for_xblocks_to_render, "Number of xblocks on the page are incorrect").fulfill()
def check_xblock_names(expected_groups, actual_blocks):
self.assertEqual(len(expected_groups), len(actual_blocks))
for idx, expected in enumerate(expected_groups):
self.assertEqual('Expand or Collapse\n{}'.format(expected), actual_blocks[idx].name)
check_xblock_names(active_groups, container.active_xblocks)
check_xblock_names(inactive_groups, container.inactive_xblocks)
# Verify inactive xblocks appear after active xblocks
check_xblock_names(active_groups + inactive_groups, container.xblocks[1:])
if verify_missing_groups_not_present:
self.verify_add_missing_groups_button_not_present(container)
def verify_add_missing_groups_button_not_present(self, container):
"""
Checks that the "add missing gorups" button/link is not present.
"""
def missing_groups_button_not_present():
button_present = container.missing_groups_button_present()
return (not button_present, not button_present)
Promise(missing_groups_button_not_present, "Add missing groups button should not be showing.").fulfill()
class SplitTest(ContainerBase, SplitTestMixin):
""" """
Tests for creating and editing split test instances in Studio. Tests for creating and editing split test instances in Studio.
""" """
...@@ -58,21 +98,6 @@ class SplitTest(ContainerBase): ...@@ -58,21 +98,6 @@ class SplitTest(ContainerBase):
self.user = course_fix.user self.user = course_fix.user
def verify_groups(self, container, active_groups, inactive_groups, verify_missing_groups_not_present=True):
super(SplitTest, self).verify_groups(container, active_groups, inactive_groups)
if verify_missing_groups_not_present:
self.verify_add_missing_groups_button_not_present(container)
def verify_add_missing_groups_button_not_present(self, container):
"""
Checks that the "add missing gorups" button/link is not present.
"""
def missing_groups_button_not_present():
button_present = container.missing_groups_button_present()
return (not button_present, not button_present)
Promise(missing_groups_button_not_present, "Add missing groups button should not be showing.").fulfill()
def create_poorly_configured_split_instance(self): def create_poorly_configured_split_instance(self):
""" """
Creates a split test instance with a missing group and an inactive group. Creates a split test instance with a missing group and an inactive group.
...@@ -210,7 +235,7 @@ class SettingsMenuTest(UniqueCourseTest): ...@@ -210,7 +235,7 @@ class SettingsMenuTest(UniqueCourseTest):
@skipUnless(os.environ.get('FEATURE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') @skipUnless(os.environ.get('FEATURE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature')
class GroupConfigurationsTest(ContainerBase): class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
""" """
Tests that Group Configurations page works correctly with previously Tests that Group Configurations page works correctly with previously
added configurations in Studio added configurations in Studio
...@@ -266,9 +291,10 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -266,9 +291,10 @@ class GroupConfigurationsTest(ContainerBase):
if groups: if groups:
allocation = int(math.floor(100 / len(groups))) allocation = int(math.floor(100 / len(groups)))
for index, group in enumerate(groups): self.assertEqual(groups, [group.name for group in config.groups])
self.assertEqual(group, config.groups[index].name) for group in config.groups:
self.assertEqual(str(allocation) + "%", config.groups[index].allocation) self.assertEqual(str(allocation) + "%", group.allocation)
# Collapse the configuration # Collapse the configuration
config.toggle() config.toggle()
...@@ -336,11 +362,11 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -336,11 +362,11 @@ class GroupConfigurationsTest(ContainerBase):
Scenario: Ensure that the group configuration can be created and edited correctly. Scenario: Ensure that the group configuration can be created and edited correctly.
Given I have a course without group configurations Given I have a course without group configurations
When I click button 'Create new Group Configuration' When I click button 'Create new Group Configuration'
And I set new name and description And I set new name and description, change name for the 2nd default group, add one new group
And I click button 'Create' And I click button 'Create'
Then I see the new group configuration is added Then I see the new group configuration is added and has correct data
When I edit the group group_configuration When I edit the group group_configuration
And I change the name and description And I change the name and description, add new group, remove old one and change name for the Group A
And I click button 'Save' And I click button 'Save'
Then I see the group configuration is saved successfully and has the new data Then I see the group configuration is saved successfully and has the new data
""" """
...@@ -351,15 +377,19 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -351,15 +377,19 @@ class GroupConfigurationsTest(ContainerBase):
config = self.page.group_configurations()[0] config = self.page.group_configurations()[0]
config.name = "New Group Configuration Name" config.name = "New Group Configuration Name"
config.description = "New Description of the group configuration." config.description = "New Description of the group configuration."
self.assertEqual(config.get_text('.action-primary'), "CREATE") config.groups[1].name = "New Group Name"
# Add new group
config.add_group() # Group C
# Save the configuration # Save the configuration
self.assertEqual(config.get_text('.action-primary'), "CREATE")
config.save() config.save()
self._assert_fields( self._assert_fields(
config, config,
name="New Group Configuration Name", name="New Group Configuration Name",
description="New Description of the group configuration.", description="New Description of the group configuration.",
groups=["Group A", "Group B"] groups=["Group A", "New Group Name", "Group C"]
) )
# Edit the group configuration # Edit the group configuration
...@@ -369,13 +399,20 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -369,13 +399,20 @@ class GroupConfigurationsTest(ContainerBase):
config.name = "Second Group Configuration Name" config.name = "Second Group Configuration Name"
config.description = "Second Description of the group configuration." config.description = "Second Description of the group configuration."
self.assertEqual(config.get_text('.action-primary'), "SAVE") self.assertEqual(config.get_text('.action-primary'), "SAVE")
# Add new group
config.add_group() # Group D
# Remove group with name "New Group Name"
config.groups[1].remove()
# Rename Group A
config.groups[0].name = "First Group"
# Save the configuration # Save the configuration
config.save() config.save()
self._assert_fields( self._assert_fields(
config, config,
name="Second Group Configuration Name", name="Second Group Configuration Name",
description="Second Description of the group configuration." description="Second Description of the group configuration.",
groups=["First Group", "Group C", "Group D"]
) )
def test_use_group_configuration(self): def test_use_group_configuration(self):
...@@ -383,23 +420,30 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -383,23 +420,30 @@ class GroupConfigurationsTest(ContainerBase):
Scenario: Ensure that the group configuration can be used by split_module correctly Scenario: Ensure that the group configuration can be used by split_module correctly
Given I have a course without group configurations Given I have a course without group configurations
When I create new group configuration When I create new group configuration
And I set new name, save the group configuration And I set new name and add a new group, save the group configuration
And I go to the unit page in Studio And I go to the unit page in Studio
And I add new advanced module "Content Experiment" And I add new advanced module "Content Experiment"
When I assign created group configuration to the module When I assign created group configuration to the module
Then I see the module has correct groups Then I see the module has correct groups
And I go to the Group Configuration page in Studio And I go to the Group Configuration page in Studio
And I edit the name of the group configuration And I edit the name of the group configuration, add new group and remove old one
And I go to the unit page in Studio And I go to the unit page in Studio
And I edit the unit And I edit the unit
Then I see the group configuration name is changed in `Group Configuration` dropdown Then I see the group configuration name is changed in `Group Configuration` dropdown
And the group configuration name is changed on container page And the group configuration name is changed on container page
And I see the module has 2 active groups and one inactive
And I see "Add missing groups" link exists
When I click on "Add missing groups" link
The I see the module has 3 active groups and one inactive
""" """
self.page.visit() self.page.visit()
# Create new group configuration # Create new group configuration
self.page.create() self.page.create()
config = self.page.group_configurations()[0] config = self.page.group_configurations()[0]
config.name = "New Group Configuration Name" config.name = "New Group Configuration Name"
# Add new group
config.add_group()
config.groups[2].name = "New group"
# Save the configuration # Save the configuration
config.save() config.save()
...@@ -409,12 +453,16 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -409,12 +453,16 @@ class GroupConfigurationsTest(ContainerBase):
container.edit() container.edit()
component_editor = ComponentEditorView(self.browser, container.locator) component_editor = ComponentEditorView(self.browser, container.locator)
component_editor.set_select_value_and_save('Group Configuration', 'New Group Configuration Name') component_editor.set_select_value_and_save('Group Configuration', 'New Group Configuration Name')
self.verify_groups(container, ['Group A', 'Group B'], []) self.verify_groups(container, ['Group A', 'Group B', 'New group'], [])
self.page.visit() self.page.visit()
config = self.page.group_configurations()[0] config = self.page.group_configurations()[0]
config.edit() config.edit()
config.name = "Second Group Configuration Name" config.name = "Second Group Configuration Name"
# Add new group
config.add_group() # Group D
# Remove Group A
config.groups[0].remove()
# Save the configuration # Save the configuration
config.save() config.save()
...@@ -430,13 +478,20 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -430,13 +478,20 @@ class GroupConfigurationsTest(ContainerBase):
"Second Group Configuration Name", "Second Group Configuration Name",
container.get_xblock_information_message() container.get_xblock_information_message()
) )
self.verify_groups(
container, ['Group B', 'New group'], ['Group A'],
verify_missing_groups_not_present=False
)
# Click the add button and verify that the groups were added on the page
container.add_missing_groups()
self.verify_groups(container, ['Group B', 'New group', 'Group D'], ['Group A'])
def test_can_cancel_creation_of_group_configuration(self): def test_can_cancel_creation_of_group_configuration(self):
""" """
Scenario: Ensure that creation of the group configuration can be canceled correctly. Scenario: Ensure that creation of the group configuration can be canceled correctly.
Given I have a course without group configurations Given I have a course without group configurations
When I click button 'Create new Group Configuration' When I click button 'Create new Group Configuration'
And I set new name and description And I set new name and description, add 1 additional group
And I click button 'Cancel' And I click button 'Cancel'
Then I see that there is no new group configurations in the course Then I see that there is no new group configurations in the course
""" """
...@@ -449,6 +504,8 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -449,6 +504,8 @@ class GroupConfigurationsTest(ContainerBase):
config = self.page.group_configurations()[0] config = self.page.group_configurations()[0]
config.name = "Name of the Group Configuration" config.name = "Name of the Group Configuration"
config.description = "Description of the group configuration." config.description = "Description of the group configuration."
# Add new group
config.add_group() # Group C
# Cancel the configuration # Cancel the configuration
config.cancel() config.cancel()
...@@ -459,7 +516,7 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -459,7 +516,7 @@ class GroupConfigurationsTest(ContainerBase):
Scenario: Ensure that editing of the group configuration can be canceled correctly. Scenario: Ensure that editing of the group configuration can be canceled correctly.
Given I have a course with group configuration Given I have a course with group configuration
When I go to the edit mode of the group configuration When I go to the edit mode of the group configuration
And I set new name and description And I set new name and description, add 2 additional groups
And I click button 'Cancel' And I click button 'Cancel'
Then I see that new changes were discarded Then I see that new changes were discarded
""" """
...@@ -478,6 +535,9 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -478,6 +535,9 @@ class GroupConfigurationsTest(ContainerBase):
config.name = "New Group Configuration Name" config.name = "New Group Configuration Name"
config.description = "New Description of the group configuration." config.description = "New Description of the group configuration."
# Add 2 new groups
config.add_group() # Group C
config.add_group() # Group D
# Cancel the configuration # Cancel the configuration
config.cancel() config.cancel()
...@@ -495,27 +555,39 @@ class GroupConfigurationsTest(ContainerBase): ...@@ -495,27 +555,39 @@ class GroupConfigurationsTest(ContainerBase):
And I create new group configuration with 2 default groups And I create new group configuration with 2 default groups
When I set only description and try to save When I set only description and try to save
Then I see error message "Group Configuration name is required" Then I see error message "Group Configuration name is required"
When I set new name and try to save When I set a name
And I delete the name of one of the groups and try to save
Then I see error message "All groups must have a name"
When I delete the group without name and try to save
Then I see error message "Please add at least two groups"
When I add new group and try to save
Then I see the group configuration is saved successfully Then I see the group configuration is saved successfully
""" """
self.page.visit() def try_to_save_and_verify_error_message(message):
# Try to save
config.save()
# Verify that configuration is still in editing mode
self.assertEqual(config.mode, 'edit')
# Verify error message
self.assertEqual(message, config.validation_message)
self.page.visit()
# Create new group configuration # Create new group configuration
self.page.create() self.page.create()
# Leave empty required field # Leave empty required field
config = self.page.group_configurations()[0] config = self.page.group_configurations()[0]
config.description = "Description of the group configuration." config.description = "Description of the group configuration."
# Try to save
config.save() try_to_save_and_verify_error_message("Group Configuration name is required")
# Verify that configuration is still in editing mode
self.assertEqual(config.mode, 'edit')
# Verify error message
self.assertEqual(
"Group Configuration name is required",
config.validation_message
)
# Set required field # Set required field
config.name = "Name of the Group Configuration" config.name = "Name of the Group Configuration"
config.groups[1].name = ''
try_to_save_and_verify_error_message("All groups must have a name")
config.groups[1].remove()
try_to_save_and_verify_error_message("There must be at least two groups")
config.add_group()
# Save the configuration # Save the configuration
config.save() config.save()
......
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