From d38b51cb4aed0649c2ae2c509ac62febb73975c9 Mon Sep 17 00:00:00 2001 From: polesye Date: Fri, 20 Jun 2014 12:46:24 +0300 Subject: [PATCH] BLD-1117: Add read-only list of Group Configurations. --- .../contentstore/views/component.py | 5 +- cms/djangoapps/contentstore/views/course.py | 31 ++- .../contentstore/views/tests/test_item.py | 54 ++++- cms/static/coffee/spec/main.coffee | 4 + cms/static/js/collections/group.js | 20 ++ .../js/collections/group_configuration.js | 11 + cms/static/js/models/group.js | 29 +++ cms/static/js/models/group_configuration.js | 87 +++++++ .../spec/models/group_configuration_spec.js | 228 ++++++++++++++++++ .../js/spec/views/group_configuration_spec.js | 145 +++++++++++ .../views/pages/group_configurations_spec.js | 71 ++++++ .../js/views/group_configuration_details.js | 54 +++++ .../js/views/group_configurations_list.js | 36 +++ .../js/views/pages/group_configurations.js | 40 +++ cms/static/sass/elements/_navigation.scss | 6 + cms/static/sass/style-app-extend1.scss | 1 + .../sass/views/_group-configuration.scss | 160 ++++++++++++ cms/templates/group_configurations.html | 84 +++++++ .../js/group-configuration-details.underscore | 42 ++++ .../mock-group-configuration-page.underscore | 22 ++ .../js/no-group-configurations.underscore | 3 + cms/templates/settings.html | 4 + cms/templates/settings_advanced.html | 4 + cms/templates/settings_graders.html | 4 + cms/templates/widgets/header.html | 6 + cms/urls.py | 1 + .../pages/studio/settings_advanced.py | 28 +++ .../studio/settings_group_configurations.py | 123 ++++++++++ common/test/acceptance/pages/studio/utils.py | 29 +++ .../tests/test_studio_split_test.py | 150 ++++++++++++ 30 files changed, 1465 insertions(+), 17 deletions(-) create mode 100644 cms/static/js/collections/group.js create mode 100644 cms/static/js/collections/group_configuration.js create mode 100644 cms/static/js/models/group.js create mode 100644 cms/static/js/models/group_configuration.js create mode 100644 cms/static/js/spec/models/group_configuration_spec.js create mode 100644 cms/static/js/spec/views/group_configuration_spec.js create mode 100644 cms/static/js/spec/views/pages/group_configurations_spec.js create mode 100644 cms/static/js/views/group_configuration_details.js create mode 100644 cms/static/js/views/group_configurations_list.js create mode 100644 cms/static/js/views/pages/group_configurations.js create mode 100644 cms/static/sass/views/_group-configuration.scss create mode 100644 cms/templates/group_configurations.html create mode 100644 cms/templates/js/group-configuration-details.underscore create mode 100644 cms/templates/js/mock/mock-group-configuration-page.underscore create mode 100644 cms/templates/js/no-group-configurations.underscore create mode 100644 common/test/acceptance/pages/studio/settings_group_configurations.py diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 2499ff9123..cd1405bed2 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -43,6 +43,7 @@ log = logging.getLogger(__name__) # NOTE: unit_handler assumes this list is disjoint from ADVANCED_COMPONENT_TYPES COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video'] +SPLIT_TEST_COMPONENT_TYPE = 'split_test' OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"] NOTE_COMPONENT_TYPES = ['notes'] @@ -61,11 +62,11 @@ else: # XBlocks from pmitros repos are prototypes. They should not be used # except for edX Learning Sciences experiments on edge.edx.org without # further work to make them robust, maintainable, finalize data formats, - # etc. + # etc. 'concept', # Concept mapper. See https://github.com/pmitros/ConceptXBlock 'done', # Lets students mark things as done. See https://github.com/pmitros/DoneXBlock 'audio', # Embed an audio file. See https://github.com/pmitros/AudioXBlock - 'split_test' + SPLIT_TEST_COMPONENT_TYPE, # Adds A/B test support ] + OPEN_ENDED_COMPONENT_TYPES + NOTE_COMPONENT_TYPES ADVANCED_COMPONENT_CATEGORY = 'advanced' diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 8935013e97..4ff410fecb 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -44,7 +44,8 @@ from .access import has_course_access from .component import ( OPEN_ENDED_COMPONENT_TYPES, NOTE_COMPONENT_TYPES, - ADVANCED_COMPONENT_POLICY_KEY + ADVANCED_COMPONENT_POLICY_KEY, + SPLIT_TEST_COMPONENT_TYPE, ) from django_comment_common.models import assign_default_role @@ -65,7 +66,8 @@ __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler' 'settings_handler', 'grading_handler', 'advanced_settings_handler', - 'textbooks_list_handler', 'textbooks_detail_handler'] + 'textbooks_list_handler', 'textbooks_detail_handler', + 'group_configurations_list_handler'] class AccessListFallback(Exception): @@ -267,7 +269,6 @@ def course_index(request, course_key): lms_link = get_lms_link_for_item(course_module.location) sections = course_module.get_children() - return render_to_response('overview.html', { 'context_course': course_module, 'lms_link': lms_link, @@ -604,7 +605,7 @@ def _config_course_advanced_components(request, course_module): # Indicate that tabs should not be filtered out of # the metadata filter_tabs = False # Set this flag to avoid the tab removal code below. - found_ac_type = True #break + found_ac_type = True # break # If we did not find a module type in the advanced settings, # we may need to remove the tab from the course. @@ -854,6 +855,28 @@ def textbooks_detail_handler(request, course_key_string, textbook_id): return JsonResponse() +@require_http_methods(("GET")) +@login_required +@ensure_csrf_cookie +def group_configurations_list_handler(request, course_key_string): + """ + A RESTful handler for Group Configurations + + GET + html: return Group Configurations list page (Backbone application) + """ + course_key = CourseKey.from_string(course_key_string) + course = _get_course_module(course_key, request.user) + group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) + splite_test_enabled = SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules + + return render_to_response('group_configurations.html', { + 'context_course': course, + 'group_configuration_url': group_configuration_url, + 'configurations': [u.to_json() for u in course.user_partitions] if splite_test_enabled else None, + }) + + 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_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 7bb31fcd10..4c4e2134ab 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -12,9 +12,12 @@ 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 +from contentstore.utils import reverse_usage_url, reverse_course_url -from contentstore.views.component import component_handler, get_component_templates +from contentstore.views.component import ( + component_handler, get_component_templates, + SPLIT_TEST_COMPONENT_TYPE +) from contentstore.tests.utils import CourseTestCase from student.tests.factories import UserFactory @@ -824,7 +827,7 @@ class TestEditSplitModule(ItemTest): def test_create_groups(self): """ - Test that verticals are created for the experiment groups when + Test that verticals are created for the configuration groups when a spit test module is edited. """ split_test = self.get_item_from_modulestore(self.split_test_usage_key, verify_is_draft=True) @@ -851,15 +854,16 @@ class TestEditSplitModule(ItemTest): def test_change_user_partition_id(self): """ - Test what happens when the user_partition_id is changed to a different experiment. + Test what happens when the user_partition_id is changed to a different groups + group configuration. """ - # Set to first experiment. + # Set to first group configuration. split_test = self._update_partition_id(0) self.assertEqual(2, len(split_test.children)) initial_vertical_0_location = split_test.children[0] initial_vertical_1_location = split_test.children[1] - # Set to second experiment + # Set to second group configuration split_test = self._update_partition_id(1) # We don't remove existing children. self.assertEqual(5, len(split_test.children)) @@ -881,12 +885,12 @@ class TestEditSplitModule(ItemTest): """ Test that nothing happens when the user_partition_id is set to the same value twice. """ - # Set to first experiment. + # Set to first group configuration. split_test = self._update_partition_id(0) self.assertEqual(2, len(split_test.children)) initial_group_id_to_child = split_test.group_id_to_child - # Set again to first experiment. + # Set again to first group configuration. split_test = self._update_partition_id(0) self.assertEqual(2, len(split_test.children)) self.assertEqual(initial_group_id_to_child, split_test.group_id_to_child) @@ -897,12 +901,12 @@ class TestEditSplitModule(ItemTest): The user_partition_id will be updated, but children and group_id_to_child map will not change. """ - # Set to first experiment. + # Set to first group configuration. split_test = self._update_partition_id(0) self.assertEqual(2, len(split_test.children)) initial_group_id_to_child = split_test.group_id_to_child - # Set to an experiment that doesn't exist. + # Set to an group configuration that doesn't exist. split_test = self._update_partition_id(-50) self.assertEqual(2, len(split_test.children)) self.assertEqual(initial_group_id_to_child, split_test.group_id_to_child) @@ -913,7 +917,7 @@ class TestEditSplitModule(ItemTest): Also test that deleting a child not in the group_id_to_child_map behaves properly. """ - # Set to first experiment. + # Set to first group configuration. self._update_partition_id(0) split_test = self._assert_children(2) vertical_1_usage_key = split_test.children[1] @@ -981,6 +985,34 @@ 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 e55a28dda3..4ed431b066 100644 --- a/cms/static/coffee/spec/main.coffee +++ b/cms/static/coffee/spec/main.coffee @@ -214,6 +214,7 @@ 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", @@ -229,10 +230,13 @@ define([ "js/spec/views/xblock_editor_spec", "js/spec/views/pages/container_spec", + "js/spec/views/pages/group_configurations_spec", "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/js/collections/group.js b/cms/static/js/collections/group.js new file mode 100644 index 0000000000..5ebcdc6edb --- /dev/null +++ b/cms/static/js/collections/group.js @@ -0,0 +1,20 @@ +define([ + 'backbone', 'js/models/group' +], +function (Backbone, GroupModel) { + 'use strict'; + var GroupCollection = Backbone.Collection.extend({ + model: GroupModel, + /** + * Indicates if the collection is empty when all the models are empty + * or the collection does not include any models. + **/ + isEmpty: function() { + return this.length === 0 || this.every(function(m) { + return m.isEmpty(); + }); + } + }); + + return GroupCollection; +}); diff --git a/cms/static/js/collections/group_configuration.js b/cms/static/js/collections/group_configuration.js new file mode 100644 index 0000000000..e322995d76 --- /dev/null +++ b/cms/static/js/collections/group_configuration.js @@ -0,0 +1,11 @@ +define([ + 'backbone', 'js/models/group_configuration' +], +function(Backbone, GroupConfigurationModel) { + 'use strict'; + var GroupConfigurationCollection = Backbone.Collection.extend({ + model: GroupConfigurationModel + }); + + return GroupConfigurationCollection; +}); diff --git a/cms/static/js/models/group.js b/cms/static/js/models/group.js new file mode 100644 index 0000000000..d87c414873 --- /dev/null +++ b/cms/static/js/models/group.js @@ -0,0 +1,29 @@ +define([ + 'backbone', 'gettext', 'backbone.associations' +], function(Backbone, gettext) { + 'use strict'; + var Group = Backbone.AssociatedModel.extend({ + defaults: function() { + return { name: '' }; + }, + + isEmpty: function() { + return !this.get('name'); + }, + + toJSON: function() { + return { name: this.get('name') }; + }, + + validate: function(attrs) { + if (!attrs.name) { + return { + message: gettext('Group name is required'), + attributes: { name: true } + }; + } + } + }); + + return Group; +}); diff --git a/cms/static/js/models/group_configuration.js b/cms/static/js/models/group_configuration.js new file mode 100644 index 0000000000..178c0a432f --- /dev/null +++ b/cms/static/js/models/group_configuration.js @@ -0,0 +1,87 @@ +define([ + 'backbone', 'underscore', 'gettext', 'js/models/group', + 'js/collections/group', 'backbone.associations', 'coffee/src/main' +], +function(Backbone, _, gettext, GroupModel, GroupCollection) { + 'use strict'; + var GroupConfiguration = Backbone.AssociatedModel.extend({ + defaults: function() { + return { + id: null, + name: '', + description: '', + groups: new GroupCollection([{}, {}]), + showGroups: false + }; + }, + + relations: [{ + type: Backbone.Many, + key: 'groups', + relatedModel: GroupModel, + collectionType: GroupCollection + }], + + initialize: function() { + this.setOriginalAttributes(); + return this; + }, + + setOriginalAttributes: function() { + this._originalAttributes = this.toJSON(); + }, + + reset: function() { + this.set(this._originalAttributes); + }, + + isDirty: function() { + return !_.isEqual( + this._originalAttributes, this.toJSON() + ); + }, + + isEmpty: function() { + return !this.get('name') && this.get('groups').isEmpty(); + }, + + toJSON: function() { + return { + id: this.get('id'), + name: this.get('name'), + description: this.get('description'), + groups: this.get('groups').toJSON() + }; + }, + + validate: function(attrs) { + if (!attrs.name) { + return { + message: gettext('Group Configuration name is required'), + attributes: {name: true} + }; + } + if (attrs.groups.length === 0) { + return { + message: gettext('Please add at least one group'), + 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 new file mode 100644 index 0000000000..6ca7e8f6d1 --- /dev/null +++ b/cms/static/js/spec/models/group_configuration_spec.js @@ -0,0 +1,228 @@ +define([ + 'backbone', 'js/models/group_configuration', + 'js/collections/group_configuration', 'js/models/group', + 'js/collections/group', 'coffee/src/main' +], function( + Backbone, GroupConfiguration, GroupConfigurationSet, Group, GroupSet, main +) { + 'use strict'; + beforeEach(function() { + this.addMatchers({ + toBeInstanceOf: function(expected) { + return this.actual instanceof expected; + } + }); + }); + + describe('GroupConfiguration model', function() { + beforeEach(function() { + main(); + this.model = new GroupConfiguration(); + }); + + describe('Basic', function() { + it('should have an empty name by default', function() { + expect(this.model.get('name')).toEqual(''); + }); + + it('should have an empty description by default', function() { + expect(this.model.get('description')).toEqual(''); + }); + + it('should not show groups by default', function() { + expect(this.model.get('showGroups')).toBeFalsy(); + }); + + it('should have a GroupSet with two groups by default', function() { + var groups = this.model.get('groups'); + + expect(groups).toBeInstanceOf(GroupSet); + expect(groups.length).toEqual(2); + expect(groups.at(0).isEmpty()).toBeTruthy(); + expect(groups.at(1).isEmpty()).toBeTruthy(); + }); + + it('should be empty by default', function() { + expect(this.model.isEmpty()).toBeTruthy(); + }); + + it('should be able to reset itself', function() { + this.model.set('name', 'foobar'); + this.model.reset(); + + expect(this.model.get('name')).toEqual(''); + }); + + it('should not be dirty by default', function() { + expect(this.model.isDirty()).toBeFalsy(); + }); + + it('should be dirty after it\'s been changed', function() { + this.model.set('name', 'foobar'); + + expect(this.model.isDirty()).toBeTruthy(); + }); + + it('should not be dirty after calling setOriginalAttributes', function() { + this.model.set('name', 'foobar'); + this.model.setOriginalAttributes(); + + expect(this.model.isDirty()).toBeFalsy(); + }); + }); + + describe('Input/Output', function() { + var deepAttributes = function(obj) { + if (obj instanceof Backbone.Model) { + 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 = {}; + + for (var prop in obj) { + if (obj.hasOwnProperty(prop)) { + attributes[prop] = deepAttributes(obj[prop]); + } + } + return attributes; + } else { + return obj; + } + }; + + it('should match server model to client model', function() { + var serverModelSpec = { + 'id': 10, + 'name': 'My GroupConfiguration', + 'description': 'Some description', + 'groups': [ + { + 'name': 'Group 1' + }, { + 'name': 'Group 2' + } + ] + }, + clientModelSpec = { + 'id': 10, + 'name': 'My GroupConfiguration', + 'description': 'Some description', + 'showGroups': false, + 'groups': [ + { + 'name': 'Group 1' + }, { + 'name': 'Group 2' + } + ] + }, + model = new GroupConfiguration(serverModelSpec); + + expect(deepAttributes(model)).toEqual(clientModelSpec); + expect(model.toJSON()).toEqual(serverModelSpec); + }); + }); + + describe('Validation', function() { + it('requires a name', function() { + var model = new GroupConfiguration({ name: '' }); + + expect(model.isValid()).toBeFalsy(); + }); + + it('requires at least one group', function() { + var model = new GroupConfiguration({ name: 'foo' }); + model.get('groups').reset(); + + expect(model.isValid()).toBeFalsy(); + }); + + it('requires a valid group', function() { + var group = new Group(), + model = new GroupConfiguration({ name: 'foo' }); + + group.isValid = function() { return false; }; + model.get('groups').reset([group]); + + expect(model.isValid()).toBeFalsy(); + }); + + it('requires all groups to be valid', function() { + var group1 = new Group(), + group2 = new Group(), + model = new GroupConfiguration({ name: 'foo' }); + + group1.isValid = function() { return true; }; + group2.isValid = function() { return false; }; + model.get('groups').reset([group1, group2]); + + expect(model.isValid()).toBeFalsy(); + }); + + it('can pass validation', function() { + var group = new Group(), + model = new GroupConfiguration({ name: 'foo' }); + + group.isValid = function() { return true; }; + model.get('groups').reset([group]); + + expect(model.isValid()).toBeTruthy(); + }); + }); + }); + + describe('Group model', function() { + beforeEach(function() { + this.model = new Group(); + }); + + describe('Basic', function() { + it('should have a name by default', function() { + expect(this.model.get('name')).toEqual(''); + }); + + it('should be empty by default', function() { + expect(this.model.isEmpty()).toBeTruthy(); + }); + }); + + describe('Validation', function() { + it('requires a name', function() { + var model = new Group({ name: '' }); + + expect(model.isValid()).toBeFalsy(); + }); + + it('can pass validation', function() { + var model = new Group({ name: 'a' }); + + expect(model.isValid()).toBeTruthy(); + }); + }); + }); + + describe('Group collection', function() { + beforeEach(function() { + this.collection = new GroupSet(); + }); + + it('is empty by default', function() { + expect(this.collection.isEmpty()).toBeTruthy(); + }); + + it('is empty if all groups are empty', function() { + this.collection.add([{}, {}, {}]); + + expect(this.collection.isEmpty()).toBeTruthy(); + }); + + it('is not empty if a group is not empty', function() { + this.collection.add([{}, { name: 'full' }, {} ]); + + expect(this.collection.isEmpty()).toBeFalsy(); + }); + }); +}); diff --git a/cms/static/js/spec/views/group_configuration_spec.js b/cms/static/js/spec/views/group_configuration_spec.js new file mode 100644 index 0000000000..ade63adaf6 --- /dev/null +++ b/cms/static/js/spec/views/group_configuration_spec.js @@ -0,0 +1,145 @@ +define([ + 'js/models/group_configuration', 'js/models/course', + 'js/collections/group_configuration', 'js/views/group_configuration_details', + 'js/views/group_configurations_list', 'jasmine-stealth' +], function( + GroupConfigurationModel, Course, GroupConfigurationSet, + GroupConfigurationDetails, GroupConfigurationsList +) { + 'use strict'; + beforeEach(function() { + window.course = new Course({ + id: '5', + name: 'Course Name', + url_name: 'course_name', + org: 'course_org', + num: 'course_num', + revision: 'course_rev' + }); + + this.addMatchers({ + toContainText: function(text) { + var trimmedText = $.trim(this.actual.text()); + + if (text && $.isFunction(text.test)) { + return text.test(trimmedText); + } else { + return trimmedText.indexOf(text) !== -1; + } + } + }); + }); + + afterEach(function() { + delete window.course; + }); + + describe('GroupConfigurationDetails', function() { + var tpl = readFixtures('group-configuration-details.underscore'); + + beforeEach(function() { + setFixtures($(' +% endfor + + +<%block name="jsextra"> + + + +<%block name="content"> +
+
+

+ ${_("Settings")} + > ${_("Group Configurations")} +

+
+
+ +
+
+
+ % if configurations is None: +
+

+ ${_("This module is disabled at the moment.")} +

+
+ % else: +
+

${_("Loading...")}

+
+ % endif +
+ +
+
+ diff --git a/cms/templates/js/group-configuration-details.underscore b/cms/templates/js/group-configuration-details.underscore new file mode 100644 index 0000000000..79a746c5b0 --- /dev/null +++ b/cms/templates/js/group-configuration-details.underscore @@ -0,0 +1,42 @@ +
+
+
+

+ + + <%= name %> + +

+
+ +
    + <% if (_.isNumber(id)) { %> +
  1. <%= gettext('ID') %>: <%= id %>
    2. + <% } %> + <% if (showGroups) { %> +
  2. + <%= description %> +
    3. + <% } else { %> +
  3. + <%= groupsCountMessage %> +
    4. + <% } %> +
+ + <% if(showGroups) { %> + <% allocation = Math.floor(100 / groups.length) %> +
    + <% groups.each(function(group, groupIndex) { %> +
  1. <%= group.get('name') %><%= allocation %>%
    2. + <% }) %> +
+ <% } %> +
+
diff --git a/cms/templates/js/mock/mock-group-configuration-page.underscore b/cms/templates/js/mock/mock-group-configuration-page.underscore new file mode 100644 index 0000000000..8cab146d9c --- /dev/null +++ b/cms/templates/js/mock/mock-group-configuration-page.underscore @@ -0,0 +1,22 @@ +
+ +
+
+

+ ${_("Settings")} + > ${_("Group Configurations")} +

+
+
+ +
+
+
+
+

${_("Loading...")}

+
+
+ +
+
+
diff --git a/cms/templates/js/no-group-configurations.underscore b/cms/templates/js/no-group-configurations.underscore new file mode 100644 index 0000000000..de103d9a2c --- /dev/null +++ b/cms/templates/js/no-group-configurations.underscore @@ -0,0 +1,3 @@ +
+

<%= gettext("You haven't created any group configurations yet.") %>

+
diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 990b73678e..631e20e86c 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -309,6 +309,7 @@ require(["domReady!", "jquery", "js/models/settings/course_details", "js/views/s course_team_url = utils.reverse_course_url('course_team_handler', context_course.id) grading_config_url = utils.reverse_course_url('grading_handler', context_course.id) advanced_config_url = utils.reverse_course_url('advanced_settings_handler', context_course.id) + group_configurations_config_url = utils.reverse_course_url('group_configurations_list_handler', context_course.id) %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/settings_advanced.html b/cms/templates/settings_advanced.html index 3ed9146680..9bfb60d97f 100644 --- a/cms/templates/settings_advanced.html +++ b/cms/templates/settings_advanced.html @@ -118,6 +118,7 @@ require(["domReady!", "jquery", "gettext", "js/models/settings/advanced", "js/vi details_url = utils.reverse_course_url('settings_handler', context_course.id) grading_url = utils.reverse_course_url('grading_handler', context_course.id) course_team_url = utils.reverse_course_url('course_team_handler', context_course.id) + group_configurations_config_url = utils.reverse_course_url('group_configurations_list_handler', context_course.id) %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index 6e67ed3c5e..1a82d88e67 100644 --- a/cms/templates/settings_graders.html +++ b/cms/templates/settings_graders.html @@ -141,6 +141,7 @@ require(["domReady!", "jquery", "js/views/settings/grading", "js/models/settings detailed_settings_url = utils.reverse_course_url('settings_handler', context_course.id) course_team_url = utils.reverse_course_url('course_team_handler', context_course.id) advanced_settings_url = utils.reverse_course_url('advanced_settings_handler', context_course.id) + group_configurations_config_url = utils.reverse_course_url('group_configurations_list_handler', context_course.id) %>

${_("Other Course Settings")}

% endif diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index 32cf79b96b..d66087e794 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -27,6 +27,7 @@ settings_url = reverse('contentstore.views.settings_handler', kwargs={'course_key_string': unicode(course_key)}) grading_url = reverse('contentstore.views.grading_handler', kwargs={'course_key_string': unicode(course_key)}) advanced_settings_url = reverse('contentstore.views.advanced_settings_handler', kwargs={'course_key_string': unicode(course_key)}) + group_configurations_config_url = reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)}) tabs_url = reverse('contentstore.views.tabs_handler', kwargs={'course_key_string': unicode(course_key)}) %>

@@ -84,6 +85,11 @@
  • ${_("Advanced Settings")}
    • + % if "split_test" in context_course.advanced_modules: +
  • + ${_("Group Configurations")} +
    • + % endif diff --git a/cms/urls.py b/cms/urls.py index 8dba754cc8..f1bc4828fc 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -90,6 +90,7 @@ urlpatterns += patterns( url(r'^settings/advanced/(?P[^/]+)$', 'advanced_settings_handler'), url(r'^textbooks/(?P[^/]+)$', 'textbooks_list_handler'), url(r'^textbooks/(?P[^/]+)/(?P\d[^/]*)$', 'textbooks_detail_handler'), + url(r'^group_configurations/(?P[^/]+)$', 'group_configurations_list_handler'), ) js_info_dict = { diff --git a/common/test/acceptance/pages/studio/settings_advanced.py b/common/test/acceptance/pages/studio/settings_advanced.py index 0e98078308..2c514ed23e 100644 --- a/common/test/acceptance/pages/studio/settings_advanced.py +++ b/common/test/acceptance/pages/studio/settings_advanced.py @@ -3,6 +3,10 @@ Course Advanced Settings page """ from .course_page import CoursePage +from .utils import press_the_notification_button, type_in_codemirror, get_codemirror_value + + +KEY_CSS = '.key h3.title' class AdvancedSettingsPage(CoursePage): @@ -14,3 +18,27 @@ class AdvancedSettingsPage(CoursePage): def is_browser_on_page(self): return self.q(css='body.advanced').present + + def _get_index_of(self, expected_key): + for i, element in enumerate(self.q(css=KEY_CSS)): + # Sometimes get stale reference if I hold on to the array of elements + key = self.q(css=KEY_CSS).nth(i).text[0] + if key == expected_key: + return i + + return -1 + + def save(self): + press_the_notification_button(self, "Save") + + def cancel(self): + press_the_notification_button(self, "Cancel") + + def set(self, key, new_value): + index = self._get_index_of(key) + type_in_codemirror(self, index, new_value) + self.save() + + def get(self, key): + index = self._get_index_of(key) + return get_codemirror_value(self, index) diff --git a/common/test/acceptance/pages/studio/settings_group_configurations.py b/common/test/acceptance/pages/studio/settings_group_configurations.py new file mode 100644 index 0000000000..20cb6fa1d3 --- /dev/null +++ b/common/test/acceptance/pages/studio/settings_group_configurations.py @@ -0,0 +1,123 @@ +""" +Course Group Configurations page. +""" + +from .course_page import CoursePage + + +class GroupConfigurationsPage(CoursePage): + """ + Course Group Configurations page. + """ + + url_path = "group_configurations" + + def is_browser_on_page(self): + return self.q(css='body.view-group-configurations').present + + def group_configurations(self): + """ + Returns list of the group configurations for the course. + """ + css = '.wrapper-group-configuration' + return [GroupConfiguration(self, index) for index in xrange(len(self.q(css=css)))] + + +class GroupConfiguration(object): + """ + Group Configuration wrapper. + """ + + def __init__(self, page, index): + self.page = page + self.SELECTOR = '.view-group-configuration-{}'.format(index) + self.index = index + + def get_selector(self, css=''): + return ' '.join([self.SELECTOR, css]) + + def find_css(self, selector): + """ + Find elements as defined by css locator. + """ + return self.page.q(css=self.get_selector(css=selector)) + + def toggle(self): + """ + Expand/collapse group configuration. + """ + css = 'a.group-toggle' + self.find_css(css).first.click() + + @property + def id(self): + """ + Returns group configuration id. + """ + css = '.group-configuration-id .group-configuration-value' + return self.find_css(css).first.text[0] + + @property + def name(self): + """ + Returns group configuration name. + """ + css = '.group-configuration-title' + return self.find_css(css).first.text[0] + + @property + def description(self): + """ + Returns group configuration description. + """ + css = '.group-configuration-description' + return self.find_css(css).first.text[0] + + @property + def groups(self): + """ + Returns list of groups. + """ + css = '.group' + + def group_selector(config_index, group_index): + return self.get_selector('.groups-{} .group-{} '.format(config_index, group_index)) + + return [Group(self.page, group_selector(self.index, index)) for index, element in enumerate(self.find_css(css))] + + def __repr__(self): + return "<{}:{}>".format(self.__class__.__name__, self.name) + + +class Group(object): + """ + Group wrapper. + """ + def __init__(self, page, prefix_selector): + self.page = page + self.prefix = prefix_selector + + def find_css(self, selector): + """ + Find elements as defined by css locator. + """ + return self.page.q(css=self.prefix + selector) + + @property + def name(self): + """ + Returns group name. + """ + css = '.group-name' + return self.find_css(css).first.text[0] + + @property + def allocation(self): + """ + Returns allocation for the group. + """ + css = '.group-allocation' + return self.find_css(css).first.text[0] + + def __repr__(self): + return "<{}:{}>".format(self.__class__.__name__, self.name) diff --git a/common/test/acceptance/pages/studio/utils.py b/common/test/acceptance/pages/studio/utils.py index 731ded5b68..bb675deb44 100644 --- a/common/test/acceptance/pages/studio/utils.py +++ b/common/test/acceptance/pages/studio/utils.py @@ -37,6 +37,18 @@ def wait_for_notification(page): Promise(_is_saving_done, 'Notification should have been hidden.', timeout=60).fulfill() +def press_the_notification_button(page, name): + # Because the notification uses a CSS transition, + # Selenium will always report it as being visible. + # This makes it very difficult to successfully click + # the "Save" button at the UI level. + # Instead, we use JavaScript to reliably click + # the button. + btn_css = 'div#page-notification a.action-%s' % name.lower() + page.browser.execute_script("$('{}').focus().click()".format(btn_css)) + page.wait_for_ajax() + + def add_discussion(page, menu_index): """ Add a new instance of the discussion category. @@ -66,3 +78,20 @@ def add_advanced_component(page, menu_index, name): Promise(is_advanced_components_showing, "Advanced component menu not showing").fulfill() click_css(page, 'a[data-category={}]'.format(name)) + + +def type_in_codemirror(page, index, text, find_prefix="$"): + script = """ + var cm = {find_prefix}('div.CodeMirror:eq({index})').get(0).CodeMirror; + CodeMirror.signal(cm, "focus", cm); + cm.setValue(arguments[0]); + CodeMirror.signal(cm, "blur", cm);""".format(index=index, find_prefix=find_prefix) + page.browser.execute_script(script, str(text)) + + +def get_codemirror_value(page, index=0, find_prefix="$"): + return page.browser.execute_script( + """ + return {find_prefix}('div.CodeMirror:eq({index})').get(0).CodeMirror.getValue(); + """.format(index=index, find_prefix=find_prefix) + ) diff --git a/common/test/acceptance/tests/test_studio_split_test.py b/common/test/acceptance/tests/test_studio_split_test.py index 7f705bcdb3..0ed671b2e6 100644 --- a/common/test/acceptance/tests/test_studio_split_test.py +++ b/common/test/acceptance/tests/test_studio_split_test.py @@ -2,16 +2,22 @@ Acceptance tests for Studio related to the split_test module. """ +import json from unittest import skip from ..fixtures.course import CourseFixture, XBlockFixtureDesc from ..pages.studio.component_editor import ComponentEditorView +from ..pages.studio.settings_advanced import AdvancedSettingsPage +from ..pages.studio.settings_group_configurations import GroupConfigurationsPage +from ..pages.studio.auto_auth import AutoAuthPage from test_studio_container import ContainerBase from ..pages.studio.utils import add_advanced_component from xmodule.partitions.partitions import Group, UserPartition from bok_choy.promise import Promise +from .helpers import UniqueCourseTest + class SplitTest(ContainerBase): """ @@ -154,3 +160,147 @@ class SplitTest(ContainerBase): container = self.create_poorly_configured_split_instance() container.delete(0) self.verify_groups(container, ['alpha'], [], verify_missing_groups_not_present=False) + + +class SettingsMenuTest(UniqueCourseTest): + """ + Tests that Setting menu is rendered correctly in Studio + """ + + def setUp(self): + super(SettingsMenuTest, self).setUp() + + course_fix = CourseFixture(**self.course_info) + course_fix.install() + + self.auth_page = AutoAuthPage( + self.browser, + staff=False, + username=course_fix.user.get('username'), + email=course_fix.user.get('email'), + password=course_fix.user.get('password') + ) + self.auth_page.visit() + + self.advanced_settings = AdvancedSettingsPage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + self.advanced_settings.visit() + + def test_link_exist_if_split_test_enabled(self): + """ + Ensure that the link to the "Group Configurations" page is shown in the + Settings menu. + """ + link_css = 'li.nav-course-settings-group-configurations a' + self.assertFalse(self.advanced_settings.q(css=link_css).present) + + self.advanced_settings.set('Advanced Module List', '["split_test"]') + + self.browser.refresh() + self.advanced_settings.wait_for_page() + + self.assertIn( + "split_test", + json.loads(self.advanced_settings.get('Advanced Module List')), + ) + + self.assertTrue(self.advanced_settings.q(css=link_css).present) + + def test_link_does_not_exist_if_split_test_disabled(self): + """ + Ensure that the link to the "Group Configurations" page does not exist + in the Settings menu. + """ + link_css = 'li.nav-course-settings-group-configurations a' + self.advanced_settings.set('Advanced Module List', '[]') + self.browser.refresh() + self.advanced_settings.wait_for_page() + self.assertFalse(self.advanced_settings.q(css=link_css).present) + + +class GroupConfigurationsTest(UniqueCourseTest): + """ + Tests that Group Configurations page works correctly with previously + added configurations in Studio + """ + + def setUp(self): + super(GroupConfigurationsTest, self).setUp() + + course_fix = CourseFixture(**self.course_info) + course_fix.add_advanced_settings({ + u"advanced_modules": {"value": ["split_test"]}, + }) + + course_fix.install() + self.course_fix = course_fix + self.user = course_fix.user + + self.auth_page = AutoAuthPage( + self.browser, + staff=False, + username=course_fix.user.get('username'), + email=course_fix.user.get('email'), + password=course_fix.user.get('password') + ) + self.auth_page.visit() + + self.page = GroupConfigurationsPage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + def test_no_group_configurations_added(self): + """ + Ensure that message telling me to create a new group configuration is + shown when group configurations were not added. + """ + self.page.visit() + css = ".wrapper-content .no-group-configurations-content" + self.assertTrue(self.page.q(css=css).present) + self.assertIn( + "You haven't created any group configurations yet.", + self.page.q(css=css).text[0] + ) + + def test_group_configurations_have_correct_data(self): + """ + Ensure that the group configuration is rendered correctly in + expanded/collapsed mode. + """ + self.course_fix.add_advanced_settings({ + u"user_partitions": { + "value": [ + UserPartition(0, 'Name of the Group Configuration', 'Description of the group configuration.', [Group("0", 'Group 0'), Group("1", 'Group 1')]).to_json(), + UserPartition(1, 'Name of second Group Configuration', 'Second group configuration.', [Group("0", 'Alpha'), Group("1", 'Beta'), Group("2", 'Gamma')]).to_json() + ], + }, + }) + self.course_fix._add_advanced_settings() + + self.page.visit() + + config = self.page.group_configurations()[0] + self.assertIn("Name of the Group Configuration", config.name) + self.assertEqual(config.id, '0') + config.toggle() + self.assertIn("Description of the group configuration.", config.description) + self.assertEqual(len(config.groups), 2) + + self.assertEqual("Group 0", config.groups[0].name) + self.assertEqual("50%", config.groups[0].allocation) + + config = self.page.group_configurations()[1] + self.assertIn("Name of second Group Configuration", config.name) + self.assertEqual(len(config.groups), 0) # no groups when the partition is collapsed + config.toggle() + self.assertEqual(len(config.groups), 3) + + self.assertEqual("Beta", config.groups[1].name) + self.assertEqual("33%", config.groups[1].allocation)