diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 7133a5571d..b25cf6895f 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -16,6 +16,7 @@ from django.core.urlresolvers import reverse from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse, Http404 from util.json_request import JsonResponse, JsonResponseBadRequest from util.date_utils import get_default_time_display +from util.db import generate_int_id, MYSQL_MAX_INT from edxmako.shortcuts import render_to_response from xmodule.course_module import DEFAULT_START_DATE @@ -29,6 +30,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseErr from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import Location from opaque_keys.edx.keys import CourseKey +from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition from django_future.csrf import ensure_csrf_cookie from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update @@ -70,7 +72,15 @@ from course_action_state.models import CourseRerunState, CourseRerunUIStateManag from course_action_state.managers import CourseActionStateItemNotFoundError from microsite_configuration import microsite from xmodule.course_module import CourseFields +from xmodule.split_test_module import get_split_user_partitions +MINIMUM_GROUP_ID = 100 + +# Note: the following content group configuration strings are not +# translated since they are not visible to users. +CONTENT_GROUP_CONFIGURATION_DESCRIPTION = 'The groups in this configuration can be mapped to cohort groups in the LMS.' + +CONTENT_GROUP_CONFIGURATION_NAME = 'Content Group Configuration' __all__ = ['course_info_handler', 'course_handler', 'course_listing', 'course_info_update_handler', @@ -1252,23 +1262,16 @@ class GroupConfiguration(object): if len(self.configuration.get('groups', [])) < 1: raise GroupConfigurationsValidationError(_("must have at least one group")) - def generate_id(self, used_ids): - """ - Generate unique id for the group configuration. - If this id is already used, we generate new one. - """ - cid = random.randint(100, 10 ** 12) - - while cid in used_ids: - cid = random.randint(100, 10 ** 12) - - return cid - def assign_id(self, configuration_id=None): """ Assign id for the json representation of group configuration. """ - self.configuration['id'] = int(configuration_id) if configuration_id else self.generate_id(self.get_used_ids()) + if configuration_id: + self.configuration['id'] = int(configuration_id) + else: + self.configuration['id'] = generate_int_id( + MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(self.course) + ) def assign_group_ids(self): """ @@ -1278,14 +1281,15 @@ class GroupConfiguration(object): # Assign ids to every group in configuration. for group in self.configuration.get('groups', []): if group.get('id') is None: - group["id"] = self.generate_id(used_ids) + group["id"] = generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, used_ids) used_ids.append(group["id"]) - def get_used_ids(self): + @staticmethod + def get_used_ids(course): """ Return a list of IDs that already in use. """ - return set([p.id for p in self.course.user_partitions]) + return set([p.id for p in course.user_partitions]) def get_user_partition(self): """ @@ -1296,21 +1300,19 @@ class GroupConfiguration(object): @staticmethod def get_usage_info(course, store): """ - Get usage information for all Group Configurations. + Get usage information for all Group Configurations currently referenced by a split_test instance. """ split_tests = store.get_items(course.id, qualifiers={'category': 'split_test'}) return GroupConfiguration._get_usage_info(store, course, split_tests) @staticmethod - def add_usage_info(course, store): + def get_split_test_partitions_with_usage(course, store): """ - Add usage information to group configurations jsons in course. - - Returns json of group configurations updated with usage information. + Returns json split_test group configurations updated with usage information. """ usage_info = GroupConfiguration.get_usage_info(course, store) configurations = [] - for partition in course.user_partitions: + for partition in get_split_user_partitions(course.user_partitions): configuration = partition.to_json() configuration['usage'] = usage_info.get(partition.id, []) configurations.append(configuration) @@ -1384,6 +1386,26 @@ class GroupConfiguration(object): configuration_json['usage'] = usage_information.get(configuration.id, []) return configuration_json + @staticmethod + def get_or_create_content_group_configuration(course): + """ + Returns the first user partition from the course which uses the + CohortPartitionScheme, or generates one if no such partition is + found. The created partition is not saved to the course until + the client explicitly creates a group within the partition and + POSTs back. + """ + content_group_configuration = get_cohorted_user_partition(course.id) + if content_group_configuration is None: + content_group_configuration = UserPartition( + id=generate_int_id(MINIMUM_GROUP_ID, MYSQL_MAX_INT, GroupConfiguration.get_used_ids(course)), + name=CONTENT_GROUP_CONFIGURATION_NAME, + description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION, + groups=[], + scheme_id='cohort' + ) + return content_group_configuration + @require_http_methods(("GET", "POST")) @login_required @@ -1405,12 +1427,21 @@ def group_configurations_list_handler(request, course_key_string): if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) course_outline_url = reverse_course_url('course_handler', course_key) - configurations = GroupConfiguration.add_usage_info(course, store) + should_show_experiment_groups = are_content_experiments_enabled(course) + if should_show_experiment_groups: + experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(course, store) + else: + experiment_group_configurations = None + content_group_configuration = GroupConfiguration.get_or_create_content_group_configuration( + course + ).to_json() return render_to_response('group_configurations.html', { 'context_course': course, 'group_configuration_url': group_configuration_url, 'course_outline_url': course_outline_url, - 'configurations': configurations, + 'experiment_group_configurations': experiment_group_configurations, + 'should_show_experiment_groups': should_show_experiment_groups, + 'content_group_configuration': content_group_configuration }) elif "application/json" in request.META.get('HTTP_ACCEPT'): if request.method == 'POST': @@ -1489,6 +1520,16 @@ def group_configurations_detail_handler(request, course_key_string, group_config return JsonResponse(status=204) +def are_content_experiments_enabled(course): + """ + Returns True if content experiments have been enabled for the course. + """ + return ( + SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and + SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules + ) + + def _get_course_creator_status(user): """ Helper method for returning the course creator status for a particular user, diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index 411c17625a..9845d78d47 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -207,6 +207,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations self.assertEqual(response.status_code, 200) self.assertContains(response, 'First name') self.assertContains(response, 'Group C') + self.assertContains(response, 'Content Group Configuration') def test_unsupported_http_accept_header(self): """ @@ -232,12 +233,9 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations {u'name': u'Group B', u'version': 1}, ], } - response = self.client.post( + response = self.client.ajax_post( self._url(), - data=json.dumps(GROUP_CONFIGURATION_JSON), - content_type="application/json", - HTTP_ACCEPT="application/json", - HTTP_X_REQUESTED_WITH="XMLHttpRequest", + data=GROUP_CONFIGURATION_JSON ) self.assertEqual(response.status_code, 201) self.assertIn("Location", response) @@ -256,6 +254,16 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations self.assertEqual(user_partititons[0].groups[0].name, u'Group A') self.assertEqual(user_partititons[0].groups[1].name, u'Group B') + def test_lazily_creates_cohort_configuration(self): + """ + Test that a cohort schemed user partition is NOT created by + default for the user. + """ + self.assertEqual(len(self.course.user_partitions), 0) + self.client.get(self._url()) + self.reload_course() + self.assertEqual(len(self.course.user_partitions), 0) + # pylint: disable=no-member class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods): @@ -425,7 +433,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): Test that right data structure will be created if group configuration is not used. """ self._add_user_partitions() - actual = GroupConfiguration.add_usage_info(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store) expected = [{ 'id': 0, 'name': 'Name 0', @@ -449,7 +457,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): vertical, __ = self._create_content_experiment(cid=0, name_suffix='0') self._create_content_experiment(name_suffix='1') - actual = GroupConfiguration.add_usage_info(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store) expected = [{ 'id': 0, @@ -492,7 +500,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): vertical, __ = self._create_content_experiment(cid=0, name_suffix='0') vertical1, __ = self._create_content_experiment(cid=0, name_suffix='1') - actual = GroupConfiguration.add_usage_info(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store) expected = [{ 'id': 0, @@ -556,7 +564,7 @@ class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods): validation.add(mocked_message) mocked_validation_messages.return_value = validation - group_configuration = GroupConfiguration.add_usage_info(self.course, self.store)[0] + group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)[0] self.assertEqual(expected_result.to_json(), group_configuration['usage'][0]['validation']) def test_error_message_present(self): diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 1e3a3082f7..23320d1580 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -28,6 +28,7 @@ class CourseMetadata(object): 'graded', 'hide_from_toc', 'pdf_textbooks', + 'user_partitions', 'name', # from xblock 'tags', # from xblock 'visible_to_staff_only', diff --git a/cms/lib/xblock/test/test_authoring_mixin.py b/cms/lib/xblock/test/test_authoring_mixin.py index 563f605a66..326b8cc1b8 100644 --- a/cms/lib/xblock/test/test_authoring_mixin.py +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -1,4 +1,7 @@ -from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme +""" +Tests for the Studio authoring XBlock mixin. +""" + from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.partitions.partitions import Group, UserPartition @@ -40,6 +43,7 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): """ Create a cohorted user partition with the specified content groups. """ + # pylint: disable=attribute-defined-outside-init self.content_partition = UserPartition( 1, 'Content Groups', @@ -60,7 +64,7 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): Set group_access for the specified item to the specified group ids within the content partition. """ - item.group_access[self.content_partition.id] = group_ids + item.group_access[self.content_partition.id] = group_ids # pylint: disable=no-member self.store.update_item(item, self.user.id) def verify_visibility_view_contains(self, item, substrings): diff --git a/cms/static/js/factories/group_configurations.js b/cms/static/js/factories/group_configurations.js index 559c4344ee..df892363e5 100644 --- a/cms/static/js/factories/group_configurations.js +++ b/cms/static/js/factories/group_configurations.js @@ -1,16 +1,22 @@ define([ - 'js/collections/group_configuration', 'js/views/pages/group_configurations' -], function(GroupConfigurationCollection, GroupConfigurationsPage) { + 'js/collections/group_configuration', 'js/models/group_configuration', 'js/views/pages/group_configurations' +], function(GroupConfigurationCollection, GroupConfigurationModel, GroupConfigurationsPage) { 'use strict'; - return function (configurations, groupConfigurationUrl, courseOutlineUrl) { - var collection = new GroupConfigurationCollection(configurations, { parse: true }), - configurationsPage; + return function (experimentsEnabled, experimentGroupConfigurationsJson, contentGroupConfigurationJson, + groupConfigurationUrl, courseOutlineUrl) { + var experimentGroupConfigurations = new GroupConfigurationCollection( + experimentGroupConfigurationsJson, {parse: true} + ), + contentGroupConfiguration = new GroupConfigurationModel(contentGroupConfigurationJson, {parse: true}); - collection.url = groupConfigurationUrl; - collection.outlineUrl = courseOutlineUrl; - configurationsPage = new GroupConfigurationsPage({ + experimentGroupConfigurations.url = groupConfigurationUrl; + experimentGroupConfigurations.outlineUrl = courseOutlineUrl; + contentGroupConfiguration.urlRoot = groupConfigurationUrl; + new GroupConfigurationsPage({ el: $('#content'), - collection: collection + experimentsEnabled: experimentsEnabled, + experimentGroupConfigurations: experimentGroupConfigurations, + contentGroupConfiguration: contentGroupConfiguration }).render(); }; }); diff --git a/cms/static/js/spec/views/group_configuration_spec.js b/cms/static/js/spec/views/group_configuration_spec.js index 5a23bd1c7b..6177d96c31 100644 --- a/cms/static/js/spec/views/group_configuration_spec.js +++ b/cms/static/js/spec/views/group_configuration_spec.js @@ -1,17 +1,15 @@ define([ - 'underscore', 'js/models/course', 'js/models/group_configuration', - '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/models/group', - 'js/collections/group', 'js/views/group_edit', + 'underscore', 'js/models/course', 'js/models/group_configuration', 'js/models/group', + 'js/collections/group_configuration', 'js/collections/group', + 'js/views/group_configuration_details', 'js/views/group_configurations_list', 'js/views/group_configuration_editor', + 'js/views/group_configuration_item', 'js/views/experiment_group_edit', 'js/views/content_group_list', 'js/views/feedback_notification', 'js/common_helpers/ajax_helpers', 'js/common_helpers/template_helpers', 'js/spec_helpers/view_helpers', 'jasmine-stealth' ], function( - _, Course, GroupConfigurationModel, GroupConfigurationCollection, - GroupConfigurationDetails, GroupConfigurationsList, GroupConfigurationEdit, - GroupConfigurationItem, GroupModel, GroupCollection, GroupEdit, - Notification, AjaxHelpers, TemplateHelpers, ViewHelpers + _, Course, GroupConfigurationModel, GroupModel, GroupConfigurationCollection, GroupCollection, + GroupConfigurationDetailsView, GroupConfigurationsListView, GroupConfigurationEditorView, + GroupConfigurationItemView, ExperimentGroupEditView, GroupList, Notification, AjaxHelpers, TemplateHelpers, + ViewHelpers ) { 'use strict'; var SELECTORS = { @@ -90,7 +88,7 @@ define([ delete window.course; }); - describe('GroupConfigurationDetails', function() { + describe('Experiment group configurations details view', function() { beforeEach(function() { TemplateHelpers.installTemplate('group-configuration-details', true); @@ -102,7 +100,7 @@ define([ this.collection = new GroupConfigurationCollection([ this.model ]); this.collection.outlineUrl = '/outline'; - this.view = new GroupConfigurationDetails({ + this.view = new GroupConfigurationDetailsView({ model: this.model }); appendSetFixtures(this.view.render().el); @@ -259,7 +257,7 @@ define([ }); }); - describe('GroupConfigurationEdit', function() { + describe('Experiment group configurations editor view', function() { var setValuesToInputs = function (view, values) { _.each(values, function (value, selector) { @@ -272,7 +270,7 @@ define([ beforeEach(function() { ViewHelpers.installViewTemplates(); TemplateHelpers.installTemplates([ - 'group-configuration-edit', 'group-edit' + 'group-configuration-editor', 'group-edit' ]); this.model = new GroupConfigurationModel({ @@ -283,7 +281,7 @@ define([ }); this.collection = new GroupConfigurationCollection([this.model]); this.collection.url = '/group_configurations'; - this.view = new GroupConfigurationEdit({ + this.view = new GroupConfigurationEditorView({ model: this.model }); appendSetFixtures(this.view.render().el); @@ -490,15 +488,17 @@ define([ }); }); - describe('GroupConfigurationsList', function() { - var emptyMessage = 'You haven\'t created any group configurations yet.'; + describe('Experiment group configurations list view', function() { + var emptyMessage = 'You have not created any group configurations yet.'; beforeEach(function() { - TemplateHelpers.installTemplate('no-group-configurations', true); + TemplateHelpers.installTemplates( + ['group-configuration-editor', 'group-edit', 'list'] + ); this.model = new GroupConfigurationModel({ id: 0 }); this.collection = new GroupConfigurationCollection(); - this.view = new GroupConfigurationsList({ + this.view = new GroupConfigurationsListView({ collection: this.collection }); appendSetFixtures(this.view.render().el); @@ -526,20 +526,25 @@ define([ expect(this.view.$el).toContainText(emptyMessage); expect(this.view.$(SELECTORS.itemView)).not.toExist(); }); + + it('can create a new group configuration', function () { + this.view.$('.new-button').click(); + expect($('.group-configuration-edit').length).toBeGreaterThan(0); + }); }); }); - describe('GroupConfigurationItem', function() { + describe('Experiment group configurations controller view', function() { var clickDeleteItem; beforeEach(function() { TemplateHelpers.installTemplates([ - 'group-configuration-edit', 'group-configuration-details' + 'group-configuration-editor', 'group-configuration-details' ], true); this.model = new GroupConfigurationModel({ id: 0 }); this.collection = new GroupConfigurationCollection([ this.model ]); this.collection.url = '/group_configurations'; - this.view = new GroupConfigurationItem({ + this.view = new GroupConfigurationItemView({ model: this.model }); appendSetFixtures(this.view.render().el); @@ -547,7 +552,7 @@ define([ clickDeleteItem = function (view, promptSpy) { view.$('.delete').click(); - ViewHelpers.verifyPromptShowing(promptSpy, /Delete this Group Configuration/); + ViewHelpers.verifyPromptShowing(promptSpy, /Delete this group configuration/); ViewHelpers.confirmPrompt(promptSpy); ViewHelpers.verifyPromptHidden(promptSpy); }; @@ -598,7 +603,7 @@ define([ }); }); - describe('GroupEdit', function() { + describe('Experiment group configurations group editor view', function() { beforeEach(function() { TemplateHelpers.installTemplate('group-edit', true); @@ -608,7 +613,7 @@ define([ this.collection = new GroupCollection([this.model]); - this.view = new GroupEdit({ + this.view = new ExperimentGroupEditView({ model: this.model }); }); @@ -626,4 +631,211 @@ define([ }); }); }); + + describe('Content groups list view', function() { + var newGroupCss = '.new-button', + addGroupCss = '.action-add', + inputCss = '.group-configuration-name-input', + saveButtonCss = '.action-primary', + cancelButtonCss = '.action-cancel', + validationErrorCss = '.group-configuration-edit-error', + scopedGroupSelector, createGroups, renderView, saveOrCancel, editNewGroup, editExistingGroup, + verifyEditingGroup, respondToSave, expectGroupsVisible, correctValidationError; + + scopedGroupSelector = function(groupIndex, additionalSelectors) { + var groupSelector = '.group-configurations-list-item-' + groupIndex; + if (additionalSelectors) { + return groupSelector + ' ' + additionalSelectors; + } else { + return groupSelector; + } + }; + + createGroups = function (groupNames) { + var groups = new GroupCollection(_.map(groupNames, function (groupName) { + return {name: groupName}; + })), + groupConfiguration = new GroupConfigurationModel({ + id: 0, + name: 'Content Group Configuration', + groups: groups + }); + groupConfiguration.urlRoot = '/mock_url'; + return groups; + }; + + renderView = function(groupNames) { + var view = new GroupList({collection: createGroups(groupNames || [])}).render(); + appendSetFixtures(view.el); + return view; + }; + + saveOrCancel = function(view, options, groupIndex) { + if (options.save) { + view.$(scopedGroupSelector(groupIndex, saveButtonCss)).click(); + } else if (options.cancel) { + view.$(scopedGroupSelector(groupIndex, cancelButtonCss)).click(); + } + }; + + editNewGroup = function(view, options) { + var newGroupIndex; + if (view.collection.length === 0) { + view.$(newGroupCss).click(); + } else { + view.$(addGroupCss).click(); + } + newGroupIndex = view.collection.length - 1; + view.$(inputCss).val(options.newName); + verifyEditingGroup(view, true, newGroupIndex); + saveOrCancel(view, options, newGroupIndex); + }; + + editExistingGroup = function(view, options) { + var groupIndex = options.groupIndex || 0; + view.$(scopedGroupSelector(groupIndex, '.edit')).click(); + view.$(scopedGroupSelector(groupIndex, inputCss)).val(options.newName); + saveOrCancel(view, options, groupIndex); + }; + + verifyEditingGroup = function(view, expectEditing, index) { + // Should prevent the user from opening more than one edit + // form at a time by removing the add button(s) when + // editing a group. + index = index || 0; + if (expectEditing) { + expect(view.$(scopedGroupSelector(index, '.group-configuration-edit'))).toExist(); + expect(view.$(newGroupCss)).not.toExist(); + expect(view.$(addGroupCss)).toHaveClass('is-hidden'); + } else { + expect(view.$('.group-configuration-edit')).not.toExist(); + if (view.collection.length === 0) { + expect(view.$(newGroupCss)).toExist(); + expect(view.$(addGroupCss)).not.toExist(); + } else { + expect(view.$(newGroupCss)).not.toExist(); + expect(view.$(addGroupCss)).not.toHaveClass('is-hidden'); + } + } + }; + + respondToSave = function(requests, view) { + expect(requests.length).toBe(1); + expect(requests[0].method).toBe('POST'); + expect(requests[0].url).toBe('/mock_url/0'); + AjaxHelpers.respondWithJson(requests, { + name: 'Content Group Configuration', + groups: view.collection.map(function(groupModel, index) { + return _.extend(groupModel.toJSON(), {id: index}); + }) + }); + }; + + correctValidationError = function(view, requests, newGroupName) { + expect(view.$(validationErrorCss)).toExist(); + verifyEditingGroup(view, true); + view.$(inputCss).val(newGroupName); + view.$(saveButtonCss).click(); + respondToSave(requests, view); + expect(view.$(validationErrorCss)).not.toExist(); + }; + + expectGroupsVisible = function(view, groupNames) { + _.each(groupNames, function(groupName) { + expect(view.$('.group-configurations-list-item')).toContainText(groupName); + }); + }; + + beforeEach(function() { + TemplateHelpers.installTemplates( + ['content-group-editor', 'content-group-details', 'list'] + ); + }); + + it('shows a message when no groups are present', function() { + expect(renderView().$('.no-group-configurations-content')) + .toContainText('You have not created any content groups yet.'); + }); + + it('can render groups', function() { + var groupNames = ['Group 1', 'Group 2', 'Group 3']; + renderView(groupNames).$('.group-configuration-details').each(function(index) { + expect($(this)).toContainText(groupNames[index]); + }); + }); + + it('can create an initial group and save', function() { + var requests = AjaxHelpers.requests(this), + newGroupName = 'New Group Name', + view = renderView(); + editNewGroup(view, {newName: newGroupName, save: true}); + respondToSave(requests, view); + verifyEditingGroup(view, false); + expectGroupsVisible(view, [newGroupName]); + }); + + it('can add another group and save', function() { + var requests = AjaxHelpers.requests(this), + oldGroupName = 'Old Group Name', + newGroupName = 'New Group Name', + view = renderView([oldGroupName]); + editNewGroup(view, {newName: newGroupName, save: true}); + respondToSave(requests, view); + verifyEditingGroup(view, false, 1); + expectGroupsVisible(view, [oldGroupName, newGroupName]); + }); + + it('can cancel adding a group', function() { + var requests = AjaxHelpers.requests(this), + newGroupName = 'New Group Name', + view = renderView(); + editNewGroup(view, {newName: newGroupName, cancel: true}); + expect(requests.length).toBe(0); + verifyEditingGroup(view, false); + expect(view.$()).not.toContainText(newGroupName); + }); + + it('can cancel editing a group', function() { + var requests = AjaxHelpers.requests(this), + originalGroupName = 'Original Group Name', + view = renderView([originalGroupName]); + editExistingGroup(view, {newName: 'New Group Name', cancel: true}); + verifyEditingGroup(view, false); + expect(requests.length).toBe(0); + expect(view.collection.at(0).get('name')).toBe(originalGroupName); + }); + + it('can show and correct a validation error', function() { + var requests = AjaxHelpers.requests(this), + newGroupName = 'New Group Name', + view = renderView(); + editNewGroup(view, {newName: '', save: true}); + expect(requests.length).toBe(0); + correctValidationError(view, requests, newGroupName); + }); + + it('can not invalidate an existing content group', function() { + var requests = AjaxHelpers.requests(this), + oldGroupName = 'Old Group Name', + view = renderView([oldGroupName]); + editExistingGroup(view, {newName: '', save: true}); + expect(requests.length).toBe(0); + correctValidationError(view, requests, oldGroupName); + }); + + it('trims whitespace', function() { + var requests = AjaxHelpers.requests(this), + newGroupName = 'New Group Name', + view = renderView(); + editNewGroup(view, {newName: ' ' + newGroupName + ' ', save: true}); + respondToSave(requests, view); + expect(view.collection.at(0).get('name')).toBe(newGroupName); + }); + + it('only edits one form at a time', function() { + var view = renderView(); + view.collection.add({name: 'Editing Group', editing: true}); + verifyEditingGroup(view, true); + }); + }); }); diff --git a/cms/static/js/spec/views/pages/group_configurations_spec.js b/cms/static/js/spec/views/pages/group_configurations_spec.js index f5eec76c6e..a2e63bc4af 100644 --- a/cms/static/js/spec/views/pages/group_configurations_spec.js +++ b/cms/static/js/spec/views/pages/group_configurations_spec.js @@ -1,21 +1,24 @@ define([ 'jquery', 'underscore', 'js/views/pages/group_configurations', - 'js/collections/group_configuration', 'js/common_helpers/template_helpers' -], function ($, _, GroupConfigurationsPage, GroupConfigurationCollection, TemplateHelpers) { + 'js/models/group_configuration', 'js/collections/group_configuration', + 'js/common_helpers/template_helpers' +], function ($, _, GroupConfigurationsPage, GroupConfigurationModel, GroupConfigurationCollection, TemplateHelpers) { 'use strict'; describe('GroupConfigurationsPage', function() { var mockGroupConfigurationsPage = readFixtures( 'mock/mock-group-configuration-page.underscore' ), - itemClassName = '.group-configurations-list-item'; + groupConfigItemClassName = '.group-configurations-list-item'; var initializePage = function (disableSpy) { var view = new GroupConfigurationsPage({ el: $('#content'), - collection: new GroupConfigurationCollection({ + experimentsEnabled: true, + experimentGroupConfigurations: new GroupConfigurationCollection({ id: 0, name: 'Configuration 1' - }) + }), + contentGroupConfiguration: new GroupConfigurationModel({groups: []}) }); if (!disableSpy) { @@ -29,15 +32,11 @@ define([ return initializePage().render(); }; - var clickNewConfiguration = function (view) { - view.$('.nav-actions .new-button').click(); - }; - beforeEach(function () { setFixtures(mockGroupConfigurationsPage); TemplateHelpers.installTemplates([ - 'no-group-configurations', 'group-configuration-edit', - 'group-configuration-details' + 'group-configuration-editor', 'group-configuration-details', 'content-group-details', + 'content-group-editor', 'group-edit', 'list' ]); this.addMatchers({ @@ -52,69 +51,67 @@ define([ var view = initializePage(); expect(view.$('.ui-loading')).toBeVisible(); view.render(); - expect(view.$(itemClassName)).toExist(); + expect(view.$(groupConfigItemClassName)).toExist(); + expect(view.$('.cohort-groups .no-group-configurations-content')).toExist(); expect(view.$('.ui-loading')).toHaveClass('is-hidden'); }); }); - describe('on page close/change', function() { - it('I see notification message if the model is changed', - function() { - var view = initializePage(true), - message; - - view.render(); - message = view.onBeforeUnload(); - expect(message).toBeUndefined(); - }); - - it('I do not see notification message if the model is not changed', - function() { - var expectedMessage ='You have unsaved changes. Do you really want to leave this page?', - view = renderPage(), - message; - - view.collection.at(0).set('name', 'Configuration 2'); - message = view.onBeforeUnload(); - expect(message).toBe(expectedMessage); - }); - }); - - describe('Check that Group Configuration will focus and expand depending on content of url hash', function() { + describe('Experiment group configurations', function() { beforeEach(function () { spyOn($.fn, 'focus'); TemplateHelpers.installTemplate('group-configuration-details'); this.view = initializePage(true); }); - it('should focus and expand group configuration if its id is part of url hash', function() { + it('should focus and expand if its id is part of url hash', function() { spyOn(this.view, 'getLocationHash').andReturn('#0'); this.view.render(); // We cannot use .toBeFocused due to flakiness. expect($.fn.focus).toHaveBeenCalled(); - expect(this.view.$(itemClassName)).toBeExpanded(); + expect(this.view.$(groupConfigItemClassName)).toBeExpanded(); }); - it('should not focus on any group configuration if url hash is empty', function() { + it('should not focus on any experiment configuration if url hash is empty', function() { spyOn(this.view, 'getLocationHash').andReturn(''); this.view.render(); expect($.fn.focus).not.toHaveBeenCalled(); - expect(this.view.$(itemClassName)).not.toBeExpanded(); + expect(this.view.$(groupConfigItemClassName)).not.toBeExpanded(); }); - it('should not focus on any group configuration if url hash contains wrong id', function() { + it('should not focus on any experiment configuration if url hash contains wrong id', function() { spyOn(this.view, 'getLocationHash').andReturn('#1'); this.view.render(); expect($.fn.focus).not.toHaveBeenCalled(); - expect(this.view.$(itemClassName)).not.toBeExpanded(); + expect(this.view.$(groupConfigItemClassName)).not.toBeExpanded(); + }); + + it('should not show a notification message if an experiment configuration is not changed', function () { + this.view.render(); + expect(this.view.onBeforeUnload()).toBeUndefined(); + }); + + it('should show a notification message if an experiment configuration is changed', function () { + this.view.experimentGroupConfigurations.at(0).set('name', 'Configuration 2'); + expect(this.view.onBeforeUnload()) + .toBe('You have unsaved changes. Do you really want to leave this page?'); }); }); - it('create new group configuration', function () { - var view = renderPage(); + describe('Content groups', function() { + beforeEach(function() { + this.view = renderPage(); + }); - clickNewConfiguration(view); - expect($('.group-configuration-edit').length).toBeGreaterThan(0); + it('should not show a notification message if a content group is not changed', function () { + expect(this.view.onBeforeUnload()).toBeUndefined(); + }); + + it('should show a notification message if a content group is changed', function () { + this.view.contentGroupConfiguration.get('groups').add({name: 'Content Group'}); + expect(this.view.onBeforeUnload()) + .toBe('You have unsaved changes. Do you really want to leave this page?'); + }); }); }); }); diff --git a/cms/static/js/views/content_group_details.js b/cms/static/js/views/content_group_details.js new file mode 100644 index 0000000000..7c0b5b86a2 --- /dev/null +++ b/cms/static/js/views/content_group_details.js @@ -0,0 +1,32 @@ +/** + * This class defines a simple display view for a content group. + * It is expected to be backed by a Group model. + */ +define([ + 'js/views/baseview' +], function(BaseView) { + 'use strict'; + + var ContentGroupDetailsView = BaseView.extend({ + tagName: 'div', + className: 'group-configuration-details', + + events: { + 'click .edit': 'editGroup' + }, + + editGroup: function() { + this.model.set({'editing': true}); + }, + + initialize: function() { + this.template = this.loadTemplate('content-group-details'); + }, + + render: function() { + this.$el.html(this.template(this.model.toJSON())); + } + }); + + return ContentGroupDetailsView; +}); diff --git a/cms/static/js/views/content_group_editor.js b/cms/static/js/views/content_group_editor.js new file mode 100644 index 0000000000..798f0ae659 --- /dev/null +++ b/cms/static/js/views/content_group_editor.js @@ -0,0 +1,44 @@ +/** + * This class defines an editing view for content groups. + * It is expected to be backed by a Group model. + */ +define([ + 'js/views/list_item_editor', 'underscore' +], +function(ListItemEditorView, _) { + 'use strict'; + + var ContentGroupEditorView = ListItemEditorView.extend({ + tagName: 'div', + className: 'group-configuration-edit', + events: { + 'submit': 'setAndClose', + 'click .action-cancel': 'cancel' + }, + + initialize: function() { + ListItemEditorView.prototype.initialize.call(this); + this.template = this.loadTemplate('content-group-editor'); + }, + + getTemplateOptions: function() { + return { + name: this.model.escape('name'), + index: this.model.collection.indexOf(this.model), + isNew: this.model.isNew(), + uniqueId: _.uniqueId() + }; + }, + + setValues: function() { + this.model.set({name: this.$('input').val().trim()}); + return this; + }, + + getSaveableModel: function() { + return this.model.collection.parents[0]; + } + }); + + return ContentGroupEditorView; +}); diff --git a/cms/static/js/views/content_group_item.js b/cms/static/js/views/content_group_item.js new file mode 100644 index 0000000000..e949c1c9d4 --- /dev/null +++ b/cms/static/js/views/content_group_item.js @@ -0,0 +1,25 @@ +/** + * This class defines an controller view for content groups. + * It renders an editor view or a details view depending on the state + * of the underlying model. + * It is expected to be backed by a Group model. + */ +define([ + 'js/views/list_item', 'js/views/content_group_editor', 'js/views/content_group_details' +], function(ListItemView, ContentGroupEditorView, ContentGroupDetailsView) { + 'use strict'; + + var ContentGroupItemView = ListItemView.extend({ + tagName: 'section', + + createEditView: function() { + return new ContentGroupEditorView({model: this.model}); + }, + + createDetailsView: function() { + return new ContentGroupDetailsView({model: this.model}); + } + }); + + return ContentGroupItemView; +}); diff --git a/cms/static/js/views/content_group_list.js b/cms/static/js/views/content_group_list.js new file mode 100644 index 0000000000..e7131beff4 --- /dev/null +++ b/cms/static/js/views/content_group_list.js @@ -0,0 +1,24 @@ +/** + * This class defines a list view for content groups. + * It is expected to be backed by a Group collection. + */ +define([ + 'js/views/list', 'js/views/content_group_item', 'gettext' +], function(ListView, ContentGroupItemView, gettext) { + 'use strict'; + + var ContentGroupListView = ListView.extend({ + tagName: 'div', + + className: 'content-group-list', + + // Translators: This refers to a content group that can be linked to a student cohort. + itemCategoryDisplayName: gettext('content group'), + + createItemView: function(options) { + return new ContentGroupItemView(options); + } + }); + + return ContentGroupListView; +}); diff --git a/cms/static/js/views/group_edit.js b/cms/static/js/views/experiment_group_edit.js similarity index 77% rename from cms/static/js/views/group_edit.js rename to cms/static/js/views/experiment_group_edit.js index 9b990d4195..8e3dccea0f 100644 --- a/cms/static/js/views/group_edit.js +++ b/cms/static/js/views/experiment_group_edit.js @@ -1,10 +1,14 @@ +/** + * This class defines an edit view for groups within content experiment group configurations. + * It is expected to be backed by a Group model. + */ define([ - 'js/views/baseview', 'underscore', 'underscore.string', 'jquery', 'gettext' + 'js/views/baseview', 'underscore', 'underscore.string', 'gettext' ], -function(BaseView, _, str, $, gettext) { +function(BaseView, _, str, gettext) { 'use strict'; _.str = str; // used in template - var GroupEdit = BaseView.extend({ + var ExperimentGroupEditView = BaseView.extend({ tagName: 'li', events: { 'click .action-close': 'removeGroup', @@ -38,7 +42,7 @@ function(BaseView, _, str, $, gettext) { }, changeName: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } + if (event && event.preventDefault) { event.preventDefault(); } this.model.set({ name: this.$('.group-name').val() }, { silent: true }); @@ -47,7 +51,7 @@ function(BaseView, _, str, $, gettext) { }, removeGroup: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } + if (event && event.preventDefault) { event.preventDefault(); } this.model.collection.remove(this.model); return this.remove(); }, @@ -65,5 +69,5 @@ function(BaseView, _, str, $, gettext) { } }); - return GroupEdit; + return ExperimentGroupEditView; }); diff --git a/cms/static/js/views/group_configuration_details.js b/cms/static/js/views/group_configuration_details.js index ac73a52610..8dcb48cf2a 100644 --- a/cms/static/js/views/group_configuration_details.js +++ b/cms/static/js/views/group_configuration_details.js @@ -1,9 +1,13 @@ +/** + * This class defines a details view for content experiment group configurations. + * It is expected to be instantiated with a GroupConfiguration model. + */ define([ 'js/views/baseview', 'underscore', 'gettext', 'underscore.string' ], function(BaseView, _, gettext, str) { 'use strict'; - var GroupConfigurationDetails = BaseView.extend({ + var GroupConfigurationDetailsView = BaseView.extend({ tagName: 'div', events: { 'click .edit': 'editConfiguration', @@ -40,17 +44,17 @@ function(BaseView, _, gettext, str) { }, editConfiguration: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } + if (event && event.preventDefault) { event.preventDefault(); } this.model.set('editing', true); }, showGroups: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } + if (event && event.preventDefault) { event.preventDefault(); } this.model.set('showGroups', true); }, hideGroups: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } + if (event && event.preventDefault) { event.preventDefault(); } this.model.set('showGroups', false); }, @@ -107,5 +111,5 @@ function(BaseView, _, gettext, str) { } }); - return GroupConfigurationDetails; + return GroupConfigurationDetailsView; }); diff --git a/cms/static/js/views/group_configuration_edit.js b/cms/static/js/views/group_configuration_edit.js deleted file mode 100644 index 3c5c59591f..0000000000 --- a/cms/static/js/views/group_configuration_edit.js +++ /dev/null @@ -1,157 +0,0 @@ -define([ - 'js/views/baseview', 'underscore', 'jquery', 'gettext', - 'js/views/group_edit', 'js/views/utils/view_utils' -], -function(BaseView, _, $, gettext, GroupEdit, ViewUtils) { - 'use strict'; - var GroupConfigurationEdit = BaseView.extend({ - tagName: 'div', - events: { - 'change .group-configuration-name-input': 'setName', - 'change .group-configuration-description-input': 'setDescription', - "click .action-add-group": "createGroup", - 'focus .input-text': 'onFocus', - 'blur .input-text': 'onBlur', - 'submit': 'setAndClose', - 'click .action-cancel': 'cancel' - }, - - className: function () { - var index = this.model.collection.indexOf(this.model); - - return [ - 'group-configuration-edit', - 'group-configuration-edit-' + index - ].join(' '); - }, - - initialize: function() { - var groups; - - this.template = this.loadTemplate('group-configuration-edit'); - 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() { - this.$el.html(this.template({ - id: this.model.get('id'), - uniqueId: _.uniqueId(), - name: this.model.escape('name'), - description: this.model.escape('description'), - usage: this.model.get('usage'), - isNew: this.model.isNew(), - error: this.model.validationError - })); - this.addAll(); - 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) { - if(event && event.preventDefault) { event.preventDefault(); } - this.model.set( - 'name', this.$('.group-configuration-name-input').val(), - { silent: true } - ); - }, - - setDescription: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } - this.model.set( - 'description', - this.$('.group-configuration-description-input').val(), - { silent: true } - ); - }, - - setValues: function() { - this.setName(); - 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; - }, - - setAndClose: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } - - this.setValues(); - if(!this.model.isValid()) { - return false; - } - - ViewUtils.runOperationShowingMessage( - gettext('Saving'), - function () { - var dfd = $.Deferred(); - - this.model.save({}, { - success: function() { - this.model.setOriginalAttributes(); - this.close(); - dfd.resolve(); - }.bind(this) - }); - - return dfd; - }.bind(this) - ); - }, - - cancel: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } - - this.model.reset(); - return this.close(); - }, - - close: function() { - var groupConfigurations = this.model.collection; - - this.remove(); - if(this.model.isNew()) { - // if the group configuration has never been saved, remove it - groupConfigurations.remove(this.model); - } else { - // tell the model that it's no longer being edited - this.model.set('editing', false); - } - - return this; - } - }); - - return GroupConfigurationEdit; -}); diff --git a/cms/static/js/views/group_configuration_editor.js b/cms/static/js/views/group_configuration_editor.js new file mode 100644 index 0000000000..41484cd707 --- /dev/null +++ b/cms/static/js/views/group_configuration_editor.js @@ -0,0 +1,120 @@ +/** + * This class defines an editing view for content experiment group configurations. + * It is expected to be backed by a GroupConfiguration model. + */ +define([ + 'js/views/list_item_editor', 'underscore', 'jquery', 'gettext', + 'js/views/experiment_group_edit' +], +function(ListItemEditorView, _, $, gettext, ExperimentGroupEditView) { + 'use strict'; + var GroupConfigurationEditorView = ListItemEditorView.extend({ + tagName: 'div', + events: { + 'change .group-configuration-name-input': 'setName', + 'change .group-configuration-description-input': 'setDescription', + 'click .action-add-group': 'createGroup', + 'focus .input-text': 'onFocus', + 'blur .input-text': 'onBlur', + 'submit': 'setAndClose', + 'click .action-cancel': 'cancel' + }, + + className: function () { + var index = this.model.collection.indexOf(this.model); + + return [ + 'group-configuration-edit', + 'group-configuration-edit-' + index + ].join(' '); + }, + + initialize: function() { + var groups = this.model.get('groups'); + + ListItemEditorView.prototype.initialize.call(this); + + this.template = this.loadTemplate('group-configuration-editor'); + this.listenTo(groups, 'add', this.onAddItem); + this.listenTo(groups, 'reset', this.addAll); + this.listenTo(groups, 'all', this.render); + }, + + render: function() { + ListItemEditorView.prototype.render.call(this); + this.addAll(); + return this; + }, + + getTemplateOptions: function() { + return { + id: this.model.get('id'), + uniqueId: _.uniqueId(), + name: this.model.escape('name'), + description: this.model.escape('description'), + usage: this.model.get('usage'), + isNew: this.model.isNew() + }; + }, + + getSaveableModel: function() { + return this.model; + }, + + onAddItem: function(group) { + var view = new ExperimentGroupEditView({ model: group }); + this.$('ol.groups').append(view.render().el); + + return this; + }, + + addAll: function() { + this.model.get('groups').each(this.onAddItem, 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) { + if (event && event.preventDefault) { event.preventDefault(); } + this.model.set( + 'name', this.$('.group-configuration-name-input').val(), + { silent: true } + ); + }, + + setDescription: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + this.model.set( + 'description', + this.$('.group-configuration-description-input').val(), + { silent: true } + ); + }, + + setValues: function() { + this.setName(); + 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 GroupConfigurationEditorView; +}); diff --git a/cms/static/js/views/group_configuration_item.js b/cms/static/js/views/group_configuration_item.js index 2b2e192d1d..badf262768 100644 --- a/cms/static/js/views/group_configuration_item.js +++ b/cms/static/js/views/group_configuration_item.js @@ -1,77 +1,43 @@ +/** + * This class defines an controller view for content experiment group configurations. + * It renders an editor view or a details view depending on the state + * of the underlying model. + * It is expected to be backed by a Group model. + */ define([ - 'js/views/baseview', 'jquery', "gettext", 'js/views/group_configuration_details', - 'js/views/group_configuration_edit', "js/views/utils/view_utils" + 'js/views/list_item', 'js/views/group_configuration_details', 'js/views/group_configuration_editor', 'gettext' ], function( - BaseView, $, gettext, GroupConfigurationDetails, GroupConfigurationEdit, ViewUtils + ListItemView, GroupConfigurationDetailsView, GroupConfigurationEditorView, gettext ) { 'use strict'; - var GroupConfigurationsItem = BaseView.extend({ + + var GroupConfigurationItemView = ListItemView.extend({ + events: { + 'click .delete': 'deleteItem' + }, + tagName: 'section', + attributes: function () { return { 'id': this.model.get('id'), 'tabindex': -1 }; }, - events: { - 'click .delete': 'deleteConfiguration' + + // Translators: this refers to a collection of groups. + itemDisplayName: gettext('group configuration'), + + canDelete: true, + + createEditView: function() { + return new GroupConfigurationEditorView({model: this.model}); }, - className: function () { - var index = this.model.collection.indexOf(this.model); - - return [ - 'group-configuration', - 'group-configurations-list-item', - 'group-configurations-list-item-' + index - ].join(' '); - }, - - initialize: function() { - this.listenTo(this.model, 'change:editing', this.render); - this.listenTo(this.model, 'remove', this.remove); - }, - - deleteConfiguration: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } - var self = this; - ViewUtils.confirmThenRunOperation( - gettext('Delete this Group Configuration?'), - gettext('Deleting this Group Configuration is permanent and cannot be undone.'), - gettext('Delete'), - function() { - return ViewUtils.runOperationShowingMessage( - gettext('Deleting'), - function () { - return self.model.destroy({ wait: true }); - } - ); - } - ); - }, - - render: function() { - // Removes a view from the DOM, and calls stopListening to remove - // any bound events that the view has listened to. - if (this.view) { - this.view.remove(); - } - - if (this.model.get('editing')) { - this.view = new GroupConfigurationEdit({ - model: this.model - }); - } else { - this.view = new GroupConfigurationDetails({ - model: this.model - }); - } - - this.$el.html(this.view.render().el); - - return this; + createDetailsView: function() { + return new GroupConfigurationDetailsView({model: this.model}); } }); - return GroupConfigurationsItem; + return GroupConfigurationItemView; }); diff --git a/cms/static/js/views/group_configurations_list.js b/cms/static/js/views/group_configurations_list.js index 9cce9a1435..33b76a0d94 100644 --- a/cms/static/js/views/group_configurations_list.js +++ b/cms/static/js/views/group_configurations_list.js @@ -1,71 +1,26 @@ +/** + * This class defines a list view for content experiment group configurations. + * It is expected to be backed by a GroupConfiguration collection. + */ define([ - 'js/views/baseview', 'jquery', 'js/views/group_configuration_item' -], function( - BaseView, $, GroupConfigurationItemView -) { + 'js/views/list', 'js/views/group_configuration_item', 'gettext' +], function(ListView, GroupConfigurationItemView, gettext) { 'use strict'; - var GroupConfigurationsList = BaseView.extend({ + + var GroupConfigurationsListView = ListView.extend({ tagName: 'div', + className: 'group-configurations-list', - events: { - 'click .new-button': 'addOne' - }, - initialize: function() { - this.emptyTemplate = this.loadTemplate('no-group-configurations'); - this.listenTo(this.collection, 'add', this.addNewItemView); - this.listenTo(this.collection, 'remove', this.handleDestory); - }, + newModelOptions: {addDefaultGroups: true}, - render: function() { - var configurations = this.collection; + // Translators: this refers to a collection of groups. + itemCategoryDisplayName: gettext('group configuration'), - if(configurations.length === 0) { - this.$el.html(this.emptyTemplate()); - } else { - var frag = document.createDocumentFragment(); - - configurations.each(function(configuration) { - var view = new GroupConfigurationItemView({ - model: configuration - }); - - frag.appendChild(view.render().el); - }); - - this.$el.html([frag]); - } - - return this; - }, - - addNewItemView: function (model) { - var view = new GroupConfigurationItemView({ - model: model - }); - - // If items already exist, just append one new. Otherwise, overwrite - // no-content message. - if (this.collection.length > 1) { - this.$el.append(view.render().el); - } else { - this.$el.html(view.render().el); - } - - view.$el.focus(); - }, - - addOne: function(event) { - if(event && event.preventDefault) { event.preventDefault(); } - this.collection.add([{ editing: true }]); - }, - - handleDestory: function () { - if(this.collection.length === 0) { - this.$el.html(this.emptyTemplate()); - } + createItemView: function(options) { + return new GroupConfigurationItemView(options); } }); - return GroupConfigurationsList; + return GroupConfigurationsListView; }); diff --git a/cms/static/js/views/list.js b/cms/static/js/views/list.js new file mode 100644 index 0000000000..7975586b63 --- /dev/null +++ b/cms/static/js/views/list.js @@ -0,0 +1,95 @@ +/** + * A generic list view class. + * + * Expects the following properties to be overriden: + * render when the collection is empty. + * - createItemView (function): Create and return an item view for a + * model in the collection. + * - newModelOptions (object): Options to pass to models which are + * added to the collection. + * - itemCategoryDisplayName (string): Display name for the category + * of items this list contains. For example, 'Group Configuration'. + * Note that it must be translated. + */ +define([ + 'js/views/baseview' +], function(BaseView) { + 'use strict'; + var ListView = BaseView.extend({ + events: { + 'click .action-add': 'onAddItem', + 'click .new-button': 'onAddItem' + }, + + initialize: function() { + this.listenTo(this.collection, 'add', this.addNewItemView); + this.listenTo(this.collection, 'remove', this.onRemoveItem); + this.template = this.loadTemplate('list'); + + // Don't render the add button when editing a form + this.listenTo(this.collection, 'change:editing', this.toggleAddButton); + this.listenTo(this.collection, 'add', this.toggleAddButton); + this.listenTo(this.collection, 'remove', this.toggleAddButton); + }, + + render: function(model) { + this.$el.html(this.template({ + itemCategoryDisplayName: this.itemCategoryDisplayName, + length: this.collection.length, + isEditing: model && model.get('editing') + })); + + this.collection.each(function(model) { + this.$('.content-groups').append(this.createItemView({model: model}).render().el); + }, this); + + return this; + }, + + hideOrShowAddButton: function(shouldShow) { + var addButtonCss = '.action-add'; + if (this.collection.length) { + if (shouldShow) { + this.$(addButtonCss).removeClass('is-hidden'); + } else { + this.$(addButtonCss).addClass('is-hidden'); + } + } + }, + + toggleAddButton: function(model) { + if (model.get('editing') && this.collection.contains(model)) { + this.hideOrShowAddButton(false); + } else { + this.hideOrShowAddButton(true); + } + }, + + addNewItemView: function (model) { + var view = this.createItemView({model: model}); + + // If items already exist, just append one new. + // Otherwise re-render the empty list HTML. + if (this.collection.length > 1) { + this.$('.content-groups').append(view.render().el); + } else { + this.render(); + } + + view.$el.focus(); + }, + + onAddItem: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + this.collection.add({editing: true}, this.newModelOptions); + }, + + onRemoveItem: function () { + if (this.collection.length === 0) { + this.render(); + } + } + }); + + return ListView; +}); diff --git a/cms/static/js/views/list_item.js b/cms/static/js/views/list_item.js new file mode 100644 index 0000000000..df22a823dd --- /dev/null +++ b/cms/static/js/views/list_item.js @@ -0,0 +1,77 @@ +/** + * A generic view to represent an editable item in a list. The item + * has a edit view and a details view. + * + * Subclasses must implement: + * - itemDisplayName (string): Display name for the list item. + * Must be translated. + * - createEditView (function): Render and append the edit view to the + * DOM. + * - createDetailsView (function): Render and append the details view + * to the DOM. + */ +define([ + 'js/views/baseview', 'jquery', "gettext", "js/views/utils/view_utils" +], function( + BaseView, $, gettext, ViewUtils +) { + 'use strict'; + + var ListItemView = BaseView.extend({ + canDelete: false, + + initialize: function() { + this.listenTo(this.model, 'change:editing', this.render); + this.listenTo(this.model, 'remove', this.remove); + }, + + deleteItem: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + if (!this.canDelete) { return; } + var model = this.model, + itemDisplayName = this.itemDisplayName; + ViewUtils.confirmThenRunOperation( + interpolate( + // Translators: "item_display_name" is the name of the item to be deleted. + gettext('Delete this %(item_display_name)s?'), + {item_display_name: itemDisplayName}, true + ), + interpolate( + // Translators: "item_display_name" is the name of the item to be deleted. + gettext('Deleting this %(item_display_name)s is permanent and cannot be undone.'), + {item_display_name: itemDisplayName}, + true + ), + gettext('Delete'), + function() { + return ViewUtils.runOperationShowingMessage( + gettext('Deleting'), + function () { + return model.destroy({wait: true}); + } + ); + } + ); + }, + + render: function() { + // Removes a view from the DOM, and calls stopListening to remove + // any bound events that the view has listened to. + if (this.view) { + this.view.remove(); + } + + if (this.model.get('editing')) { + this.view = this.createEditView(); + } else { + this.view = this.createDetailsView(); + } + + this.$el.html(this.view.render().el); + + return this; + } + }); + + return ListItemView; +}); diff --git a/cms/static/js/views/list_item_editor.js b/cms/static/js/views/list_item_editor.js new file mode 100644 index 0000000000..29638082bf --- /dev/null +++ b/cms/static/js/views/list_item_editor.js @@ -0,0 +1,76 @@ +/** + * A generic view to represent a list item in its editing state. + * + * Subclasses must implement: + * - getTemplateOptions (function): Return an object to pass to the + * template. + * - setValues (function): Set values on the model according to the + * DOM. + * - getSaveableModel (function): Return the model which should be + * saved by this view. Note this may be a parent model. + */ +define([ + 'js/views/baseview', 'js/views/utils/view_utils', 'underscore', 'gettext' +], function(BaseView, ViewUtils, _, gettext) { + 'use strict'; + + var ListItemEditorView = BaseView.extend({ + initialize: function() { + this.listenTo(this.model, 'invalid', this.render); + }, + + render: function() { + this.$el.html(this.template(_.extend({ + error: this.model.validationError + }, this.getTemplateOptions()))); + }, + + setAndClose: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + + this.setValues(); + if (!this.model.isValid()) { + return false; + } + + ViewUtils.runOperationShowingMessage( + gettext('Saving'), + function () { + var dfd = $.Deferred(); + var actionableModel = this.getSaveableModel(); + + actionableModel.save({}, { + success: function() { + actionableModel.setOriginalAttributes(); + this.close(); + dfd.resolve(); + }.bind(this) + }); + + return dfd; + }.bind(this)); + }, + + cancel: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + + this.getSaveableModel().reset(); + return this.close(); + }, + + close: function() { + this.remove(); + if (this.model.isNew() && !_.isUndefined(this.model.collection)) { + // if the item has never been saved, remove it + this.model.collection.remove(this.model); + } else { + // tell the model that it's no longer being edited + this.model.set('editing', false); + } + + return this; + } + }); + + return ListItemEditorView; +}); diff --git a/cms/static/js/views/pages/group_configurations.js b/cms/static/js/views/pages/group_configurations.js index 2bce4afe6d..c0e0ed6952 100644 --- a/cms/static/js/views/pages/group_configurations.js +++ b/cms/static/js/views/pages/group_configurations.js @@ -1,21 +1,31 @@ define([ 'jquery', 'underscore', 'gettext', 'js/views/pages/base_page', - 'js/views/group_configurations_list' + 'js/views/group_configurations_list', 'js/views/content_group_list' ], -function ($, _, gettext, BasePage, GroupConfigurationsList) { +function ($, _, gettext, BasePage, GroupConfigurationsListView, ContentGroupListView) { 'use strict'; var GroupConfigurationsPage = BasePage.extend({ - initialize: function() { + initialize: function(options) { BasePage.prototype.initialize.call(this); - this.listView = new GroupConfigurationsList({ - collection: this.collection + this.experimentsEnabled = options.experimentsEnabled; + if (this.experimentsEnabled) { + this.experimentGroupConfigurations = options.experimentGroupConfigurations; + this.experimentGroupsListView = new GroupConfigurationsListView({ + collection: this.experimentGroupConfigurations + }); + } + this.contentGroupConfiguration = options.contentGroupConfiguration; + this.cohortGroupsListView = new ContentGroupListView({ + collection: this.contentGroupConfiguration.get('groups') }); }, renderPage: function() { var hash = this.getLocationHash(); - this.$('.content-primary').append(this.listView.render().el); - this.addButtonActions(); + if (this.experimentsEnabled) { + this.$('.experiment-groups').append(this.experimentGroupsListView.render().el); + } + this.$('.cohort-groups').append(this.cohortGroupsListView.render().el); this.addWindowActions(); if (hash) { // Strip leading '#' to get id string to match @@ -24,22 +34,17 @@ function ($, _, gettext, BasePage, GroupConfigurationsList) { return $.Deferred().resolve().promise(); }, - addButtonActions: function () { - this.$('.nav-actions .new-button').click(function (event) { - this.listView.addOne(event); - }.bind(this)); - }, - addWindowActions: function () { $(window).on('beforeunload', this.onBeforeUnload.bind(this)); }, onBeforeUnload: function () { - var dirty = this.collection.find(function(configuration) { - return configuration.isDirty(); - }); + var dirty = this.contentGroupConfiguration.isDirty() || + (this.experimentsEnabled && this.experimentGroupConfigurations.find(function(configuration) { + return configuration.isDirty(); + })); - if(dirty) { + if (dirty) { return gettext('You have unsaved changes. Do you really want to leave this page?'); } }, @@ -57,7 +62,7 @@ function ($, _, gettext, BasePage, GroupConfigurationsList) { * @param {String|Number} Id of the group configuration. */ expandConfiguration: function (id) { - var groupConfig = this.collection.findWhere({ + var groupConfig = this.experimentsEnabled && this.experimentGroupConfigurations.findWhere({ id: parseInt(id) }); diff --git a/cms/static/sass/views/_group-configuration.scss b/cms/static/sass/views/_group-configuration.scss index ca5af460f7..37944360a0 100644 --- a/cms/static/sass/views/_group-configuration.scss +++ b/cms/static/sass/views/_group-configuration.scss @@ -253,7 +253,7 @@ color: $gray-l3; } - .is-focused .tip{ + .is-focused .tip { color: $gray; } @@ -274,12 +274,12 @@ // add a group is below with groups styling .action-primary { @extend %btn-primary-blue; - padding: ($baseline*.3) $baseline; + padding: ($baseline/4) $baseline; } .action-secondary { @extend %btn-secondary-gray; - padding: ($baseline*.3) $baseline; + padding: ($baseline/4) $baseline; } .wrapper-delete-button { @@ -495,6 +495,25 @@ padding: ($baseline/2); } } + + // add/new items + .action-add { + @extend %ui-btn-flat-outline; + display: block; + width: 100%; + margin-top: ($baseline*0.75); + padding: ($baseline/2) $baseline; + + &.is-hidden { + display: none; + } + + .icon { + display: inline-block; + vertical-align: middle; + @include margin-right($baseline/2); + } + } } .content-supplementary { diff --git a/cms/templates/group_configurations.html b/cms/templates/group_configurations.html index fcad0e3508..ec1bd722a4 100644 --- a/cms/templates/group_configurations.html +++ b/cms/templates/group_configurations.html @@ -1,5 +1,6 @@ <%inherit file="base.html" /> -<%def name="online_help_token()"><% return "group_configurations" %> +<%def name="content_groups_help_token()"><% return "content_groups" %> +<%def name="experiment_group_configurations_help_token()"><% return "group_configurations" %> <%namespace name='static' file='static_content.html'/> <%! import json %> <%! @@ -11,7 +12,7 @@ <%block name="bodyclass">is-signedin course view-group-configurations <%block name="header_extras"> -% for template_name in ["group-configuration-details", "group-configuration-edit", "no-group-configurations", "group-edit", "basic-modal", "modal-button"]: +% for template_name in ["group-configuration-details", "group-configuration-editor", "group-edit", "content-group-editor", "content-group-details", "basic-modal", "modal-button", "list"]: @@ -19,11 +20,9 @@ <%block name="requirejs"> -% if configurations is not None: require(["js/factories/group_configurations"], function(GroupConfigurationsFactory) { - GroupConfigurationsFactory(${json.dumps(configurations)}, "${group_configuration_url}", "${course_outline_url}"); + GroupConfigurationsFactory(${json.dumps(should_show_experiment_groups)}, ${json.dumps(experiment_group_configurations)}, ${json.dumps(content_group_configuration)}, "${group_configuration_url}", "${course_outline_url}"); }); -% endif <%block name="content"> @@ -33,32 +32,23 @@ ${_("Settings")} > ${_("Group Configurations")} -
-
-

${_("Cohorted Content")}

-

${_("A cohorted content group configuration allows different groups of students to view separate content. You can then define what units each cohort can see from the {a_start}Course Outline{a_end}. [Copy TBD]").format(a_start='', a_end="")}

-
-

${_("You haven't created any cohort groups yet.")} ${_("Add your first Cohort Group")}

+
+

${_("Content Groups")}

+
+

${_("Loading")}

-
+
+ % if should_show_experiment_groups:
-

${_("Experiment Groups")}

-

${_("An experiment group configuration defines how many groups of students are in an experiment. You can then add a content experiment to any unit in the {a_start}Course Outline{a_end}. [Copy TBD]").format(a_start='', a_end="")}

- % if configurations is None: +

${_("Experiment Group Configurations")}

+ % if experiment_group_configurations is None:

${_("This module is disabled at the moment.")} @@ -70,20 +60,28 @@

% endif
+ % endif