diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index bf4d176f10..b4bd61f34b 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -901,12 +901,11 @@ class GroupConfiguration(object): if len(self.configuration.get('groups', [])) < 2: 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. If this id is already used, we generate new one. """ - used_ids = self.get_used_ids() cid = random.randint(100, 10 ** 12) while cid in used_ids: @@ -918,21 +917,18 @@ class GroupConfiguration(object): """ 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): """ Assign ids for the group_configuration's groups. """ - # this is temporary logic, we are going to build default groups on front-end - if not self.configuration.get('groups'): - self.configuration['groups'] = [ - {'name': 'Group A'}, {'name': 'Group B'}, - ] - + used_ids = [g.id for p in self.course.user_partitions for g in p.groups] # Assign ids to every group in configuration. - for index, group in enumerate(self.configuration.get('groups', [])): - group['id'] = index + for group in self.configuration.get('groups', []): + if group.get('id') is None: + group["id"] = self.generate_id(used_ids) + used_ids.append(group["id"]) def get_used_ids(self): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index ef6a34ebda..318806dbff 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -5,6 +5,7 @@ import json from unittest import skipUnless from django.conf import settings from contentstore.utils import reverse_course_url +from contentstore.views.component import SPLIT_TEST_COMPONENT_TYPE from contentstore.tests.utils import CourseTestCase from xmodule.partitions.partitions import Group, UserPartition @@ -12,6 +13,10 @@ from xmodule.partitions.partitions import Group, UserPartition GROUP_CONFIGURATION_JSON = { u'name': u'Test name', 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 """ Test cases for group_configurations_list_handler. """ - def setUp(self): """ Set up GroupConfigurationsListHandlerTestCase. @@ -109,13 +113,35 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations """ 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()) 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): """ @@ -157,8 +183,12 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations self.assertEqual(len(group_ids), 2) self.reload_course() # Verify that user_partitions in the course contains the new group configuration. - self.assertEqual(len(self.course.user_partitions), 1) - self.assertEqual(self.course.user_partitions[0].name, u'Test name') + user_partititons = self.course.user_partitions + 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 @@ -204,7 +234,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio response = self.client.put( self._url(cid=999), - data=json.dumps(GROUP_CONFIGURATION_JSON), + data=json.dumps(expected), content_type="application/json", HTTP_ACCEPT="application/json", HTTP_X_REQUESTED_WITH="XMLHttpRequest", @@ -213,8 +243,12 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(content, expected) self.reload_course() # Verify that user_partitions in the course contains the new group configuration. - self.assertEqual(len(self.course.user_partitions), 1) - self.assertEqual(self.course.user_partitions[0].name, u'Test name') + user_partititons = self.course.user_partitions + 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): """ @@ -231,8 +265,8 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio u'description': u'New Test description', u'version': 1, u'groups': [ - {u'id': 0, u'name': u'Group A', u'version': 1}, - {u'id': 1, u'name': u'Group B', u'version': 1}, + {u'id': 0, u'name': u'New Group Name', u'version': 1}, + {u'id': 2, u'name': u'Group C', u'version': 1}, ], } response = self.client.put( @@ -246,5 +280,9 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(content, expected) self.reload_course() # Verify that user_partitions is properly updated in the course. - self.assertEqual(len(self.course.user_partitions), 1) - self.assertEqual(self.course.user_partitions[0].name, u'New Test name') + user_partititons = self.course.user_partitions + 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') diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 4ddfc9558d..6bb95416e0 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -6,19 +6,17 @@ import ddt from mock import patch from pytz import UTC -from unittest import skipUnless from webob import Response -from django.conf import settings from django.http import Http404 from django.test import TestCase from django.test.client import RequestFactory 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 ( - component_handler, get_component_templates, - SPLIT_TEST_COMPONENT_TYPE + component_handler, get_component_templates ) from contentstore.tests.utils import CourseTestCase @@ -776,7 +774,6 @@ class TestEditItem(ItemTest): self.verify_publish_state(html_usage_key, PublishState.draft) -@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') class TestEditSplitModule(ItemTest): """ Tests around editing instances of the split_test module. @@ -977,6 +974,12 @@ class TestEditSplitModule(ItemTest): group_id_to_child = split_test.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. split_test.add_missing_groups(self.request) split_test = self._assert_children(3) @@ -989,34 +992,6 @@ class TestEditSplitModule(ItemTest): split_test = self._assert_children(3) 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 class TestComponentHandler(TestCase): diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee index 4ed431b066..46b039956c 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -214,7 +214,6 @@ define([ "js/spec/models/component_template_spec", "js/spec/models/explicit_url_spec", - "js/spec/models/group_configuration_spec", "js/spec/utils/drag_and_drop_spec", "js/spec/utils/handle_iframe_binding_spec", @@ -223,6 +222,7 @@ define([ "js/spec/views/baseview_spec", "js/spec/views/paging_spec", "js/spec/views/assets_spec", + "js/spec/views/group_configuration_spec", "js/spec/views/container_spec", "js/spec/views/unit_spec", @@ -235,8 +235,6 @@ define([ "js/spec/views/modals/base_modal_spec", "js/spec/views/modals/edit_xblock_spec", - "js/spec/views/group_configuration_spec", - "js/spec/xblock/cms.runtime.v1_spec", # these tests are run separately in the cms-squire suite, due to process diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee index 7c85125c7c..baba65804f 100644 --- a/cms/static/coffee/spec/main_squire.coffee +++ b/cms/static/coffee/spec/main_squire.coffee @@ -176,5 +176,6 @@ jasmine.getFixtures().fixturesPath += 'coffee/fixtures' define([ "coffee/spec/views/assets_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" ]) diff --git a/cms/static/js/collections/group.js b/cms/static/js/collections/group.js index 5ebcdc6edb..83de360918 100644 --- a/cms/static/js/collections/group.js +++ b/cms/static/js/collections/group.js @@ -1,10 +1,22 @@ define([ - 'backbone', 'js/models/group' + 'underscore', 'underscore.string', 'backbone', 'gettext', 'js/models/group' ], -function (Backbone, GroupModel) { +function (_, str, Backbone, gettext, GroupModel) { 'use strict'; var GroupCollection = Backbone.Collection.extend({ 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 * or the collection does not include any models. @@ -13,7 +25,86 @@ function (Backbone, GroupModel) { return this.length === 0 || this.every(function(m) { 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; diff --git a/cms/static/js/models/group.js b/cms/static/js/models/group.js index d1fe22f326..96548f7e8e 100644 --- a/cms/static/js/models/group.js +++ b/cms/static/js/models/group.js @@ -1,13 +1,15 @@ define([ - 'backbone', 'gettext', 'backbone.associations' -], function(Backbone, gettext) { + 'backbone', 'underscore', 'underscore.string', 'gettext', + 'backbone.associations' +], function(Backbone, _, str, gettext) { 'use strict'; var Group = Backbone.AssociatedModel.extend({ defaults: function() { return { name: '', - version: null - }; + version: null, + order: null + }; }, isEmpty: function() { @@ -16,13 +18,14 @@ define([ toJSON: function() { return { + id: this.get('id'), name: this.get('name'), version: this.get('version') }; }, validate: function(attrs) { - if (!attrs.name) { + if (!str.trim(attrs.name)) { return { message: gettext('Group name is required'), attributes: { name: true } diff --git a/cms/static/js/models/group_configuration.js b/cms/static/js/models/group_configuration.js index 07462aba7d..54c209dd4f 100644 --- a/cms/static/js/models/group_configuration.js +++ b/cms/static/js/models/group_configuration.js @@ -11,7 +11,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { name: '', description: '', version: null, - groups: new GroupCollection([]), + groups: new GroupCollection([ + { + name: gettext('Group A'), + order: 0 + }, + { + name: gettext('Group B'), + order: 1 + } + ]), showGroups: false, editing: false }; @@ -30,16 +39,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { }, setOriginalAttributes: function() { - this._originalAttributes = this.toJSON(); + this._originalAttributes = this.parse(this.toJSON()); }, reset: function() { - this.set(this._originalAttributes); + this.set(this._originalAttributes, { parse: true }); }, isDirty: function() { return !_.isEqual( - this._originalAttributes, this.toJSON() + this._originalAttributes, this.parse(this.toJSON()) ); }, @@ -47,6 +56,16 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { 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() { return { id: this.get('id'), @@ -64,7 +83,29 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { 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; }); diff --git a/cms/static/js/spec/models/group_configuration_spec.js b/cms/static/js/spec/models/group_configuration_spec.js index a3478dd42c..e57a12d363 100644 --- a/cms/static/js/spec/models/group_configuration_spec.js +++ b/cms/static/js/spec/models/group_configuration_spec.js @@ -1,8 +1,8 @@ define([ 'backbone', 'coffee/src/main', 'js/models/group_configuration', - 'js/models/group', 'js/collections/group' + 'js/models/group', 'js/collections/group', 'squire' ], function( - Backbone, main, GroupConfigurationModel, GroupModel, GroupCollection + Backbone, main, GroupConfigurationModel, GroupModel, GroupCollection, Squire ) { 'use strict'; beforeEach(function() { @@ -32,11 +32,12 @@ define([ 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'); 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() { @@ -72,8 +73,6 @@ define([ return deepAttributes(obj.attributes); } else if (obj instanceof Backbone.Collection) { return obj.map(deepAttributes); - } else if (_.isArray(obj)) { - return _.map(obj, deepAttributes); } else if (_.isObject(obj)) { var attributes = {}; @@ -114,14 +113,18 @@ define([ 'groups': [ { 'version': 1, + 'order': 0, 'name': 'Group 1' }, { 'version': 1, + 'order': 1, 'name': 'Group 2' } ] }, - model = new GroupConfigurationModel(serverModelSpec); + model = new GroupConfigurationModel( + serverModelSpec, { parse: true } + ); expect(deepAttributes(model)).toEqual(clientModelSpec); expect(model.toJSON()).toEqual(serverModelSpec); @@ -145,11 +148,12 @@ define([ describe('GroupModel', function() { beforeEach(function() { - this.model = new GroupModel(); + this.collection = new GroupCollection([{}]); + this.model = this.collection.at(0); }); 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(''); }); @@ -166,10 +170,41 @@ define([ }); it('can pass validation', function() { - var model = new GroupModel({ name: 'a' }); + var model = new GroupConfigurationModel({ name: 'foo' }); 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([ }); it('is empty if all groups are empty', function() { - this.collection.add([{}, {}, {}]); + this.collection.add([{ name: '' }, { name: '' }, { name: '' }]); expect(this.collection.isEmpty()).toBeTruthy(); }); 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(); }); + + 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'); + }); + }); + }); }); }); diff --git a/cms/static/js/spec/views/group_configuration_spec.js b/cms/static/js/spec/views/group_configuration_spec.js index 40d966d083..4aaa6f335d 100644 --- a/cms/static/js/spec/views/group_configuration_spec.js +++ b/cms/static/js/spec/views/group_configuration_spec.js @@ -3,13 +3,15 @@ define([ 'js/collections/group_configuration', 'js/views/group_configuration_details', 'js/views/group_configurations_list', 'js/views/group_configuration_edit', - 'js/views/group_configuration_item', 'js/views/feedback_notification', - 'js/spec_helpers/create_sinon', 'js/spec_helpers/edit_helpers', - 'jasmine-stealth' + 'js/views/group_configuration_item', 'js/models/group', + 'js/collections/group', 'js/views/group_edit', + 'js/views/feedback_notification', 'js/spec_helpers/create_sinon', + 'js/spec_helpers/edit_helpers', 'jasmine-stealth' ], function( Course, GroupConfigurationModel, GroupConfigurationCollection, GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit, - GroupConfigurationItem, Notification, create_sinon, view_helpers + GroupConfigurationItem, GroupModel, GroupCollection, GroupEdit, + Notification, create_sinon, view_helpers ) { 'use strict'; var SELECTORS = { @@ -17,13 +19,15 @@ define([ editView: '.group-configuration-edit', itemView: '.group-configurations-list-item', group: '.group', + groupFields: '.groups-fields', name: '.group-configuration-name', description: '.group-configuration-description', groupsCount: '.group-configuration-groups-count', groupsAllocation: '.group-allocation', errorMessage: '.group-configuration-edit-error', + inputGroupName: '.group-name', inputName: '.group-configuration-name-input', - inputDescription: '.group-configuration-description-input' + inputDescription: '.group-configuration-description-input', }; beforeEach(function() { @@ -59,6 +63,13 @@ define([ return _.every(values, function (value, key) { return this.actual.get(key) === value; }.bind(this)); + }, + toHaveDefaultNames: function (values) { + var actualValues = $.map(this.actual, function (item) { + return $(item).val(); + }); + + return _.isEqual(actualValues, values); } }); }); @@ -90,7 +101,7 @@ define([ }); it('should show groups appropriately', function() { - this.model.get('groups').add([{}, {}, {}]); + this.model.get('groups').add([{}]); this.model.set('showGroups', false); this.view.$('.show-groups').click(); @@ -104,7 +115,7 @@ define([ }); it('should hide groups appropriately', function() { - this.model.get('groups').add([{}, {}, {}]); + this.model.get('groups').add([{}]); this.model.set('showGroups', true); this.view.$('.hide-groups').click(); @@ -129,7 +140,9 @@ define([ beforeEach(function() { view_helpers.installViewTemplates(); - view_helpers.installTemplate('group-configuration-edit'); + view_helpers.installTemplates([ + 'group-configuration-edit', 'group-edit' + ]); this.model = new GroupConfigurationModel({ name: 'Configuration', @@ -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() { var requests = create_sinon.requests(this), - notificationSpy = view_helpers.createNotificationSpy(); + notificationSpy = view_helpers.createNotificationSpy(), + groups; + this.view.$('.action-add-group').click(); setValuesToInputs(this.view, { inputName: 'New Configuration', inputDescription: 'New Description' @@ -170,6 +191,10 @@ define([ name: 'New Configuration', 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(); }); @@ -185,10 +210,14 @@ define([ }); it('does not save on cancel', function() { + this.view.$('.action-add-group').click(); setValuesToInputs(this.view, { inputName: 'New Configuration', inputDescription: 'New Description' }); + + expect(this.model.get('groups').length).toBe(3); + this.view.$('.action-cancel').click(); expect(this.model).toBeCorrectValuesInModel({ name: 'Configuration', @@ -197,6 +226,7 @@ define([ // Model is still exist in the collection expect(this.collection.indexOf(this.model)).toBeGreaterThan(-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() { @@ -236,6 +266,69 @@ define([ expect(this.view.$(SELECTORS.errorMessage)).not.toExist(); 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() { @@ -254,7 +347,7 @@ define([ expect(this.view.$el).toContainText( '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(); }); @@ -273,8 +366,9 @@ define([ describe('GroupConfigurationItem', function() { beforeEach(function() { - view_helpers.installTemplate('group-configuration-edit', true); - view_helpers.installTemplate('group-configuration-details'); + view_helpers.installTemplates([ + 'group-configuration-edit', 'group-configuration-details' + ], true); this.model = new GroupConfigurationModel({ id: 0 }); this.collection = new GroupConfigurationCollection([ this.model ]); this.view = new GroupConfigurationItem({ @@ -294,6 +388,35 @@ define([ 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); + }); + }); + }); }); diff --git a/cms/static/js/spec_helpers/view_helpers.js b/cms/static/js/spec_helpers/view_helpers.js index 1cbbd1e5d4..9baa72fe27 100644 --- a/cms/static/js/spec_helpers/view_helpers.js +++ b/cms/static/js/spec_helpers/view_helpers.js @@ -3,12 +3,13 @@ */ define(["jquery", "js/views/feedback_notification", "js/spec_helpers/create_sinon"], function($, NotificationView, create_sinon) { - var installTemplate, installViewTemplates, createNotificationSpy, verifyNotificationShowing, - verifyNotificationHidden; + var installTemplate, installTemplates, installViewTemplates, createNotificationSpy, + verifyNotificationShowing, verifyNotificationHidden; installTemplate = function(templateName, isFirst) { var template = readFixtures(templateName + '.underscore'), templateId = templateName + '-tpl'; + if (isFirst) { setFixtures($(" @@ -23,7 +23,7 @@ require(["domReady!", "js/collections/group_configuration", "js/views/pages/group_configurations"], function(doc, GroupConfigurationCollection, GroupConfigurationsPage) { % 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}"; new GroupConfigurationsPage({ diff --git a/cms/templates/js/group-configuration-edit.underscore b/cms/templates/js/group-configuration-edit.underscore index 36feb2fa87..d507a42a04 100644 --- a/cms/templates/js/group-configuration-edit.underscore +++ b/cms/templates/js/group-configuration-edit.underscore @@ -25,6 +25,13 @@ <%= gettext("Optional long description") %> +
+ <%= gettext("Group information") %> + + <%= gettext("Name of the groups that students will be assigned to, for example, Control, Video, Problems. You must have two or more groups.") %> +
    + +
    diff --git a/cms/templates/js/group-edit.underscore b/cms/templates/js/group-edit.underscore new file mode 100644 index 0000000000..dbb9adb33e --- /dev/null +++ b/cms/templates/js/group-edit.underscore @@ -0,0 +1,4 @@ +
    +
    <%= allocation %>%
    + <%= gettext("delete group") %> diff --git a/common/test/acceptance/pages/studio/container.py b/common/test/acceptance/pages/studio/container.py index 175d3429d0..9da474ee79 100644 --- a/common/test/acceptance/pages/studio/container.py +++ b/common/test/acceptance/pages/studio/container.py @@ -126,7 +126,7 @@ class ContainerPage(PageObject): """ 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): """ diff --git a/common/test/acceptance/pages/studio/settings_group_configurations.py b/common/test/acceptance/pages/studio/settings_group_configurations.py index d00bc1abcd..478805a423 100644 --- a/common/test/acceptance/pages/studio/settings_group_configurations.py +++ b/common/test/acceptance/pages/studio/settings_group_configurations.py @@ -55,6 +55,13 @@ class GroupConfiguration(object): css = 'a.group-toggle' 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): """ Return text for the defined by css locator. @@ -144,10 +151,10 @@ class GroupConfiguration(object): """ css = '.group' - def group_selector(config_index, group_index): - return self.get_selector('.groups-{} .group-{} '.format(config_index, group_index)) + def group_selector(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): return "<{}:{}>".format(self.__class__.__name__, self.name) @@ -170,11 +177,19 @@ class Group(object): @property def name(self): """ - Return group name. + Return the name of the group . """ css = '.group-name' 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 def allocation(self): """ @@ -183,5 +198,12 @@ class Group(object): css = '.group-allocation' 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): return "<{}:{}>".format(self.__class__.__name__, self.name) diff --git a/common/test/acceptance/tests/test_studio_container.py b/common/test/acceptance/tests/test_studio_container.py index 7b6ad3d824..66878b95e1 100644 --- a/common/test/acceptance/tests/test_studio_container.py +++ b/common/test/acceptance/tests/test_studio_container.py @@ -11,7 +11,7 @@ from ..pages.studio.component_editor import ComponentEditorView from ..pages.studio.utils import add_discussion from unittest import skip -from bok_choy.promise import Promise + class ContainerBase(UniqueCourseTest): """ @@ -93,30 +93,6 @@ class ContainerBase(UniqueCourseTest): break 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): """ Perform the supplied action and then verify the resulting ordering. diff --git a/common/test/acceptance/tests/test_studio_split_test.py b/common/test/acceptance/tests/test_studio_split_test.py index 41d90ea0ed..7254bdec33 100644 --- a/common/test/acceptance/tests/test_studio_split_test.py +++ b/common/test/acceptance/tests/test_studio_split_test.py @@ -18,11 +18,51 @@ from ..pages.studio.auto_auth import AutoAuthPage from ..pages.studio.utils import add_advanced_component from ..pages.xblock.utils import wait_for_xblock_initialization from .helpers import UniqueCourseTest - 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. """ @@ -58,21 +98,6 @@ class SplitTest(ContainerBase): 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): """ Creates a split test instance with a missing group and an inactive group. @@ -210,7 +235,7 @@ class SettingsMenuTest(UniqueCourseTest): @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 added configurations in Studio @@ -266,9 +291,10 @@ class GroupConfigurationsTest(ContainerBase): if groups: allocation = int(math.floor(100 / len(groups))) - for index, group in enumerate(groups): - self.assertEqual(group, config.groups[index].name) - self.assertEqual(str(allocation) + "%", config.groups[index].allocation) + self.assertEqual(groups, [group.name for group in config.groups]) + for group in config.groups: + self.assertEqual(str(allocation) + "%", group.allocation) + # Collapse the configuration config.toggle() @@ -336,11 +362,11 @@ class GroupConfigurationsTest(ContainerBase): Scenario: Ensure that the group configuration can be created and edited correctly. Given I have a course without group configurations 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' - 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 - 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' Then I see the group configuration is saved successfully and has the new data """ @@ -351,15 +377,19 @@ class GroupConfigurationsTest(ContainerBase): config = self.page.group_configurations()[0] config.name = "New Group Configuration Name" 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 + self.assertEqual(config.get_text('.action-primary'), "CREATE") config.save() self._assert_fields( config, name="New Group Configuration Name", description="New Description of the group configuration.", - groups=["Group A", "Group B"] + groups=["Group A", "New Group Name", "Group C"] ) # Edit the group configuration @@ -369,13 +399,20 @@ class GroupConfigurationsTest(ContainerBase): config.name = "Second Group Configuration Name" config.description = "Second Description of the group configuration." 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 config.save() self._assert_fields( config, 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): @@ -383,23 +420,30 @@ class GroupConfigurationsTest(ContainerBase): Scenario: Ensure that the group configuration can be used by split_module correctly Given I have a course without group configurations 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 add new advanced module "Content Experiment" When I assign created group configuration to the module Then I see the module has correct groups 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 edit the unit Then I see the group configuration name is changed in `Group Configuration` dropdown 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() # Create new group configuration self.page.create() config = self.page.group_configurations()[0] config.name = "New Group Configuration Name" + # Add new group + config.add_group() + config.groups[2].name = "New group" # Save the configuration config.save() @@ -409,12 +453,16 @@ class GroupConfigurationsTest(ContainerBase): container.edit() component_editor = ComponentEditorView(self.browser, container.locator) 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() config = self.page.group_configurations()[0] config.edit() config.name = "Second Group Configuration Name" + # Add new group + config.add_group() # Group D + # Remove Group A + config.groups[0].remove() # Save the configuration config.save() @@ -430,13 +478,20 @@ class GroupConfigurationsTest(ContainerBase): "Second Group Configuration Name", 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): """ Scenario: Ensure that creation of the group configuration can be canceled correctly. Given I have a course without group configurations 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' Then I see that there is no new group configurations in the course """ @@ -449,6 +504,8 @@ class GroupConfigurationsTest(ContainerBase): config = self.page.group_configurations()[0] config.name = "Name of the Group Configuration" config.description = "Description of the group configuration." + # Add new group + config.add_group() # Group C # Cancel the configuration config.cancel() @@ -459,7 +516,7 @@ class GroupConfigurationsTest(ContainerBase): Scenario: Ensure that editing of the group configuration can be canceled correctly. Given I have a course with 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' Then I see that new changes were discarded """ @@ -478,6 +535,9 @@ class GroupConfigurationsTest(ContainerBase): config.name = "New Group Configuration Name" 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 config.cancel() @@ -495,27 +555,39 @@ class GroupConfigurationsTest(ContainerBase): And I create new group configuration with 2 default groups When I set only description and try to save 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 """ - 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 self.page.create() # Leave empty required field config = self.page.group_configurations()[0] config.description = "Description of the group configuration." - # Try to save - config.save() - # 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 - ) + + try_to_save_and_verify_error_message("Group Configuration name is required") + # Set required field 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 config.save()