diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index b4bd61f34b..4f76a6b8bd 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -4,6 +4,7 @@ Views related to operations on course objects import json import random import string # pylint: disable=W0402 +import logging from django.utils.translation import ugettext as _ import django.utils @@ -32,7 +33,8 @@ from contentstore.utils import ( get_lms_link_for_item, add_extra_panel_tab, remove_extra_panel_tab, - reverse_course_url + reverse_course_url, + reverse_usage_url, ) from models.settings.course_details import CourseDetails, CourseSettingsEncoder @@ -70,6 +72,8 @@ __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler' 'textbooks_list_handler', 'textbooks_detail_handler', 'group_configurations_list_handler', 'group_configurations_detail_handler'] +log = logging.getLogger(__name__) + class AccessListFallback(Exception): """ @@ -949,6 +953,62 @@ class GroupConfiguration(object): groups ) + @staticmethod + def _get_usage_info(course, modulestore): + """ + Get all units names and their urls that have experiments and associated + with configurations. + + Returns: + {'user_partition_id': + [ + {'label': 'Unit Name / Experiment Name', 'url': 'url_to_unit_1'}, + {'label': 'Another Unit Name / Another Experiment Name', 'url': 'url_to_unit_1'} + ], + } + """ + usage_info = {} + descriptors = modulestore.get_items(course.id, category='split_test') + for split_test in descriptors: + if split_test.user_partition_id not in usage_info: + usage_info[split_test.user_partition_id] = [] + + unit_location = modulestore.get_parent_location(split_test.location) + if not unit_location: + log.warning("Parent location of split_test module not found: %s", split_test.location) + continue + + try: + unit = modulestore.get_item(unit_location) + except ItemNotFoundError: + log.warning("Unit not found: %s", unit_location) + continue + + unit_url = reverse_usage_url( + 'unit_handler', + course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name) + ) + usage_info[split_test.user_partition_id].append({ + 'label': '{} / {}'.format(unit.display_name, split_test.display_name), + 'url': unit_url + }) + return usage_info + + @staticmethod + def add_usage_info(course, modulestore): + """ + Add usage information to group configurations json. + + Returns json of group configurations updated with usage information. + """ + usage_info = GroupConfiguration._get_usage_info(course, modulestore) + configurations = [] + for partition in course.user_partitions: + configuration = partition.to_json() + configuration['usage'] = usage_info.get(partition.id, []) + configurations.append(configuration) + return configurations + @require_http_methods(("GET", "POST")) @login_required @@ -968,12 +1028,16 @@ 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) split_test_enabled = SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules + configurations = GroupConfiguration.add_usage_info(course, store) + 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 split_test_enabled else None, + 'course_outline_url': course_outline_url, + 'configurations': configurations if split_test_enabled else None, }) elif "application/json" in request.META.get('HTTP_ACCEPT'): if request.method == 'POST': diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index 318806dbff..34e9466762 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -6,8 +6,10 @@ from unittest import skipUnless from django.conf import settings from contentstore.utils import reverse_course_url from contentstore.views.component import SPLIT_TEST_COMPONENT_TYPE +from contentstore.views.course import GroupConfiguration from contentstore.tests.utils import CourseTestCase from xmodule.partitions.partitions import Group, UserPartition +from xmodule.modulestore.tests.factories import ItemFactory GROUP_CONFIGURATION_JSON = { @@ -20,6 +22,44 @@ GROUP_CONFIGURATION_JSON = { } +class HelperMethods(object): + """ + Mixin that provides useful methods for Group Configuration tests. + """ + def _create_content_experiment(self, cid=None, name_suffix=''): + """ + Create content experiment. + + Assign Group Configuration to the experiment if cid is provided. + """ + vertical = ItemFactory.create( + category='vertical', + parent_location=self.course.location, + display_name='Test Unit {}'.format(name_suffix) + ) + split_test = ItemFactory.create( + category='split_test', + parent_location=vertical.location, + user_partition_id=cid, + display_name='Test Content Experiment {}'.format(name_suffix) + ) + self.save_course() + return (vertical, split_test) + + def _add_user_partitions(self, count=1): + """ + Create user partitions for the course. + """ + partitions = [ + UserPartition( + i, 'Name ' + str(i), 'Description ' + str(i), [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')] + ) for i in xrange(count) + ] + + self.course.user_partitions = partitions + self.save_course() + + # pylint: disable=no-member class GroupConfigurationsBaseTestCase(object): """ @@ -286,3 +326,118 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio self.assertEqual(len(user_partititons[0].groups), 2) self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name') self.assertEqual(user_partititons[0].groups[1].name, u'Group C') + + +# pylint: disable=no-member +@skipUnless(settings.FEATURES.get('ENABLE_GROUP_CONFIGURATIONS'), 'Tests Group Configurations feature') +class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): + """ + Tests for usage information of configurations. + """ + def setUp(self): + super(GroupConfigurationsUsageInfoTestCase, self).setUp() + + def test_group_configuration_not_used(self): + """ + 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) + expected = [{ + u'id': 0, + u'name': u'Name 0', + u'description': u'Description 0', + u'version': 1, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1}, + {u'id': 1, u'name': u'Group B', u'version': 1}, + {u'id': 2, u'name': u'Group C', u'version': 1}, + ], + u'usage': [], + }] + self.assertEqual(actual, expected) + + def test_can_get_correct_usage_info(self): + """ + Test if group configurations json updated successfully with usage information. + """ + self._add_user_partitions(count=2) + 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) + + expected = [{ + u'id': 0, + u'name': u'Name 0', + u'description': u'Description 0', + u'version': 1, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1}, + {u'id': 1, u'name': u'Group B', u'version': 1}, + {u'id': 2, u'name': u'Group C', u'version': 1}, + ], + u'usage': [{ + 'url': '/unit/i4x://MITx/999/vertical/Test_Unit_0', + 'label': 'Test Unit 0 / Test Content Experiment 0', + }], + }, { + u'id': 1, + u'name': u'Name 1', + u'description': u'Description 1', + u'version': 1, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1}, + {u'id': 1, u'name': u'Group B', u'version': 1}, + {u'id': 2, u'name': u'Group C', u'version': 1}, + ], + u'usage': [], + }] + + self.assertEqual(actual, expected) + + def test_can_use_one_configuration_in_multiple_experiments(self): + """ + Test if multiple experiments are present in usage info when they use same + group configuration. + """ + self._add_user_partitions() + self._create_content_experiment(cid=0, name_suffix='0') + self._create_content_experiment(cid=0, name_suffix='1') + + actual = GroupConfiguration.add_usage_info(self.course, self.store) + + expected = [{ + u'id': 0, + u'name': u'Name 0', + u'description': u'Description 0', + u'version': 1, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1}, + {u'id': 1, u'name': u'Group B', u'version': 1}, + {u'id': 2, u'name': u'Group C', u'version': 1}, + ], + u'usage': [{ + 'url': '/unit/i4x://MITx/999/vertical/Test_Unit_0', + 'label': 'Test Unit 0 / Test Content Experiment 0', + }, { + 'url': '/unit/i4x://MITx/999/vertical/Test_Unit_1', + 'label': 'Test Unit 1 / Test Content Experiment 1', + }], + }] + self.assertEqual(actual, expected) + + def test_can_handle_without_parent(self): + """ + Test if it possible to handle case when split_test has no parent. + """ + self._add_user_partitions() + # Create split test without parent. + ItemFactory.create( + category='split_test', + user_partition_id=0, + display_name='Test Content Experiment' + ) + self.save_course() + actual = GroupConfiguration._get_usage_info(self.course, self.store) + self.assertEqual(actual, {0: []}) diff --git a/cms/static/js/models/group_configuration.js b/cms/static/js/models/group_configuration.js index 54c209dd4f..f2e42a6531 100644 --- a/cms/static/js/models/group_configuration.js +++ b/cms/static/js/models/group_configuration.js @@ -22,7 +22,8 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { } ]), showGroups: false, - editing: false + editing: false, + usage: [] }; }, diff --git a/cms/static/js/spec/models/group_configuration_spec.js b/cms/static/js/spec/models/group_configuration_spec.js index e57a12d363..550d1426bc 100644 --- a/cms/static/js/spec/models/group_configuration_spec.js +++ b/cms/static/js/spec/models/group_configuration_spec.js @@ -9,6 +9,9 @@ define([ this.addMatchers({ toBeInstanceOf: function(expected) { return this.actual instanceof expected; + }, + toBeEmpty: function() { + return this.actual.length === 0; } }); }); @@ -40,6 +43,10 @@ define([ expect(groups.at(1).get('name')).toBe('Group B'); }); + it('should have an empty usage by default', function() { + expect(this.model.get('usage')).toBeEmpty(); + }); + it('should be able to reset itself', function() { this.model.set('name', 'foobar'); this.model.reset(); @@ -120,7 +127,8 @@ define([ 'order': 1, 'name': 'Group 2' } - ] + ], + 'usage': [] }, model = new GroupConfigurationModel( serverModelSpec, { parse: true } diff --git a/cms/static/js/spec/views/group_configuration_spec.js b/cms/static/js/spec/views/group_configuration_spec.js index 4aaa6f335d..100ae9890c 100644 --- a/cms/static/js/spec/views/group_configuration_spec.js +++ b/cms/static/js/spec/views/group_configuration_spec.js @@ -28,6 +28,12 @@ define([ inputGroupName: '.group-name', inputName: '.group-configuration-name-input', inputDescription: '.group-configuration-description-input', + usageCount: '.group-configuration-usage-count', + usage: '.group-configuration-usage', + usageText: '.group-configuration-usage-text', + usageTextAnchor: '.group-configuration-usage-text > a', + usageUnit: '.group-configuration-usage-unit', + usageUnitAnchor: '.group-configuration-usage-unit > a' }; beforeEach(function() { @@ -89,6 +95,7 @@ define([ }); this.collection = new GroupConfigurationCollection([ this.model ]); + this.collection.outlineUrl = '/outline'; this.view = new GroupConfigurationDetails({ model: this.model }); @@ -126,6 +133,70 @@ define([ expect(this.view.$(SELECTORS.description)).not.toExist(); expect(this.view.$(SELECTORS.groupsAllocation)).not.toExist(); }); + + it('should show empty usage appropriately', function() { + this.model.set('showGroups', false); + this.view.$('.show-groups').click(); + + expect(this.view.$(SELECTORS.usageCount)).not.toExist(); + expect(this.view.$(SELECTORS.usageText)) + .toContainText('This Group Configuration is not in use. ' + + 'Start by adding a content experiment to any ' + + 'Unit via the'); + expect(this.view.$(SELECTORS.usageTextAnchor)).toExist(); + expect(this.view.$(SELECTORS.usageUnit)).not.toExist(); + }); + + it('should hide empty usage appropriately', function() { + this.model.set('showGroups', true); + this.view.$('.hide-groups').click(); + + expect(this.view.$(SELECTORS.usageText)).not.toExist(); + expect(this.view.$(SELECTORS.usageUnit)).not.toExist(); + expect(this.view.$(SELECTORS.usageCount)) + .toContainText('Not in Use'); + }); + + it('should show non-empty usage appropriately', function() { + var usageUnitAnchors; + + this.model.set('usage', + [ + {'label': 'label1', 'url': 'url1'}, + {'label': 'label2', 'url': 'url2'} + ] + ); + this.model.set('showGroups', false); + this.view.$('.show-groups').click(); + + usageUnitAnchors = this.view.$(SELECTORS.usageUnitAnchor); + + expect(this.view.$(SELECTORS.usageCount)).not.toExist(); + expect(this.view.$(SELECTORS.usageText)) + .toContainText('This Group Configuration is used in:'); + expect(this.view.$(SELECTORS.usageUnit).length).toBe(2); + expect(usageUnitAnchors.length).toBe(2); + expect(usageUnitAnchors.eq(0)).toContainText('label1'); + expect(usageUnitAnchors.eq(0).attr('href')).toBe('url1'); + expect(usageUnitAnchors.eq(1)).toContainText('label2'); + expect(usageUnitAnchors.eq(1).attr('href')).toBe('url2'); + }); + + it('should hide non-empty usage appropriately', function() { + this.model.set('usage', + [ + {'label': 'label1', 'url': 'url1'}, + {'label': 'label2', 'url': 'url2'} + ] + ); + this.model.set('showGroups', true); + this.view.$('.hide-groups').click(); + + expect(this.view.$(SELECTORS.usageText)).not.toExist(); + expect(this.view.$(SELECTORS.usageUnit)).not.toExist(); + expect(this.view.$(SELECTORS.usageCount)) + .toContainText('Used in 2 units'); + }); }); describe('GroupConfigurationEdit', function() { @@ -418,5 +489,3 @@ define([ }); }); }); - - diff --git a/cms/static/js/views/group_configuration_details.js b/cms/static/js/views/group_configuration_details.js index f00e7d00b6..ac73a52610 100644 --- a/cms/static/js/views/group_configuration_details.js +++ b/cms/static/js/views/group_configuration_details.js @@ -1,7 +1,7 @@ define([ - 'js/views/baseview', 'underscore', 'gettext' + 'js/views/baseview', 'underscore', 'gettext', 'underscore.string' ], -function(BaseView, _, gettext) { +function(BaseView, _, gettext, str) { 'use strict'; var GroupConfigurationDetails = BaseView.extend({ tagName: 'div', @@ -30,6 +30,8 @@ function(BaseView, _, gettext) { render: function() { var attrs = $.extend({}, this.model.attributes, { groupsCountMessage: this.getGroupsCountTitle(), + usageCountMessage: this.getUsageCountTitle(), + outlineAnchorMessage: this.getOutlineAnchorMessage(), index: this.model.collection.indexOf(this.model) }); @@ -64,6 +66,44 @@ function(BaseView, _, gettext) { ); return interpolate(message, { count: count }, true); + }, + + getUsageCountTitle: function () { + var count = this.model.get('usage').length, message; + + if (count === 0) { + message = gettext('Not in Use'); + } else { + message = ngettext( + /* + Translators: 'count' is number of units that the group + configuration is used in. + */ + 'Used in %(count)s unit', 'Used in %(count)s units', + count + ); + } + + return interpolate(message, { count: count }, true); + }, + + getOutlineAnchorMessage: function () { + var message = gettext( + /* + Translators: 'outlineAnchor' is an anchor pointing to + the course outline page. + */ + 'This Group Configuration is not in use. Start by adding a content experiment to any Unit via the %(outlineAnchor)s.' + ), + anchor = str.sprintf( + '%(text)s', + { + url: this.model.collection.outlineUrl, + text: gettext('Course Outline') + } + ); + + return str.sprintf(message, {outlineAnchor: anchor}); } }); diff --git a/cms/static/sass/views/_group-configuration.scss b/cms/static/sass/views/_group-configuration.scss index 9382690ae1..311750c570 100644 --- a/cms/static/sass/views/_group-configuration.scss +++ b/cms/static/sass/views/_group-configuration.scss @@ -42,142 +42,164 @@ outline: none; .group-configuration-details { - padding: $baseline ($baseline*1.5); + .wrapper-group-configuration { + padding: $baseline ($baseline*1.5); - .group-configuration-header { - margin-bottom: 0; - border-bottom: 0; - } - - .group-configuration-title { - @extend %t-title; - @include font-size(22); - @include line-height(22); - overflow: hidden; - text-overflow: ellipsis; - margin-right: ($baseline*14); - font-weight: bold; - - .group-toggle { - display: inline-block; - padding-left: $baseline; - color: $black; - - &:hover, &:focus { - color: $blue; - } - } - } - - .group-configuration-info { - @extend %t-copy-sub1; - color: $gray-l1; - margin-left: $baseline; - - &.group-configuration-info-inline { - display: table; - width: 70%; - margin: ($baseline/4) 0 ($baseline/2) $baseline; - - li { - @include box-sizing(border-box); - display: table-cell; - margin-right: 1%; - } + .group-configuration-header { + margin-bottom: 0; + border-bottom: 0; } - &.group-configuration-info-block { - li { - padding: ($baseline/4) 0; - } - } - - .group-configuration-label { - text-transform: uppercase; - } - - .group-configuration-description { + .group-configuration-title { + @extend %t-title; + @include font-size(22); + @include line-height(22); overflow: hidden; text-overflow: ellipsis; - } - } + margin-right: ($baseline*14); + font-weight: bold; - .ui-toggle-expansion { - @include transition(rotate .15s ease-in-out .25s); - @include font-size(21); - display: inline-block; - width: ($baseline*0.75); - vertical-align: baseline; - margin-left: -$baseline; - } + .group-toggle { + display: inline-block; + padding-left: $baseline; + color: $black; - &.is-selectable { - cursor: pointer; - - &:hover { - color: $blue; - - .ui-toggle-expansion { - color: $blue; + &:hover, &:focus { + color: $blue; + } } } - } - .groups { - margin-left: $baseline; - margin-bottom: ($baseline*0.75); + .group-configuration-info { + @extend %t-copy-sub1; + color: $gray-l1; + margin-left: $baseline; - .group { - @extend %t-copy-sub2; - @include font-size(18); - @include line-height(16); - padding: ($baseline/7) 0 ($baseline/4); - border-top: 1px solid $gray-l4; - white-space: nowrap; + &.group-configuration-info-inline { + display: table; + width: 70%; + margin: ($baseline/4) 0 ($baseline/2) $baseline; - &:first-child { - border-top: none; + li { + @include box-sizing(border-box); + display: table-cell; + margin-right: 1%; + + &.group-configuration-usage-count { + font-style: italic; + } + } } - .group-name { + &.group-configuration-info-block { + li { + padding: ($baseline/4) 0; + } + } + + .group-configuration-label { + text-transform: uppercase; + } + + .group-configuration-description { overflow: hidden; text-overflow: ellipsis; - display: inline-block; - vertical-align: middle; - width: 75%; - margin-right: 5%; } + } - .group-allocation { + .ui-toggle-expansion { + @include transition(rotate .15s ease-in-out .25s); + @include font-size(21); + display: inline-block; + width: ($baseline*0.75); + vertical-align: baseline; + margin-left: -$baseline; + } + + &.is-selectable { + cursor: pointer; + + &:hover { + color: $blue; + + .ui-toggle-expansion { + color: $blue; + } + } + } + + .groups { + margin-left: $baseline; + margin-bottom: ($baseline*0.75); + + .group { + @extend %t-copy-sub2; + @include font-size(18); + @include line-height(16); + padding: ($baseline/7) 0 ($baseline/4); + border-top: 1px solid $gray-l4; + white-space: nowrap; + + &:first-child { + border-top: none; + } + + .group-name { + overflow: hidden; + text-overflow: ellipsis; + display: inline-block; + vertical-align: middle; + width: 75%; + margin-right: 5%; + } + + .group-allocation { + display: inline-block; + vertical-align: middle; + width: 20%; + color: $gray-l1; + text-align: right; + } + } + } + + .actions { + @include transition(opacity .15s .25s ease-in-out); + opacity: 0.0; + position: absolute; + top: $baseline; + right: $baseline; + + .action { display: inline-block; - vertical-align: middle; - width: 20%; - color: $gray-l1; - text-align: right; + margin-right: ($baseline/4); + + .edit { + @include blue-button; + @extend %t-action4; + } } } } - .actions { - @include transition(opacity .15s .25s ease-in-out); - opacity: 0.0; - position: absolute; - top: $baseline; - right: $baseline; + .wrapper-group-configuration-usages { + @include font-size(14); + background-color: #f8f8f8; + box-shadow: 0 2px 2px 0 $shadow inset; + padding: $baseline ($baseline*1.5) $baseline ($baseline*2.5); - .action { - display: inline-block; - margin-right: ($baseline/4); + .group-configuration-usage { + color: $gray-l1; + margin-left: $baseline; - .edit { - @include blue-button; - @extend %t-action4; + .group-configuration-usage-unit { + padding: ($baseline/4) 0; } } } } - &:hover .actions { + &:hover .wrapper-group-configuration .actions { opacity: 1.0; } } diff --git a/cms/templates/group_configurations.html b/cms/templates/group_configurations.html index 80aef06cd1..537688573e 100644 --- a/cms/templates/group_configurations.html +++ b/cms/templates/group_configurations.html @@ -26,6 +26,7 @@ function(doc, GroupConfigurationCollection, GroupConfigurationsPage) { var collection = new GroupConfigurationCollection(${json.dumps(configurations)}, { parse: true }); collection.url = "${group_configuration_url}"; + collection.outlineUrl = "${course_outline_url}"; new GroupConfigurationsPage({ el: $('#content'), collection: collection diff --git a/cms/templates/js/group-configuration-details.underscore b/cms/templates/js/group-configuration-details.underscore index ebaa20a1ad..e96f2a24bf 100644 --- a/cms/templates/js/group-configuration-details.underscore +++ b/cms/templates/js/group-configuration-details.underscore @@ -23,19 +23,22 @@
  • <%= groupsCountMessage %>
    • +
  • + <%= usageCountMessage %> +
    • <% } %> <% if(showGroups) { %> <% allocation = Math.floor(100 / groups.length) %> -
      - <% groups.each(function(group, groupIndex) { %> -
    1. <%= group.get('name') %><%= allocation %>%
      2. - <% }) %> -
    +
      + <% groups.each(function(group, groupIndex) { %> +
    1. + <%= group.get('name') %> + <%= allocation %>% +
      2. + <% }) %> +
    <% } %> +<% if(showGroups) { %> +
    + <% if (!_.isEmpty(usage)) { %> +

    <%= gettext('This Group Configuration is used in:') %>

    +
      + <% _.each(usage, function(unit) { %> +
    1. + ><%= unit.label %> +
      2. + <% }) %> +
    + <% } else { %> +

    + <%= outlineAnchorMessage %> +

    + <% } %> +
    +<% } %> diff --git a/common/test/acceptance/fixtures/course.py b/common/test/acceptance/fixtures/course.py index 8c6bb98f65..2c7b5d5c54 100644 --- a/common/test/acceptance/fixtures/course.py +++ b/common/test/acceptance/fixtures/course.py @@ -99,6 +99,7 @@ class XBlockFixtureDesc(object): self.grader_type = grader_type self.publish = publish self.children = [] + self.locator = None def add_children(self, *args): """ @@ -137,11 +138,12 @@ class XBlockFixtureDesc(object): metadata={2}, grader_type={3}, publish={4}, - children={5} + children={5}, + locator={6}, > """).strip().format( self.category, self.data, self.metadata, - self.grader_type, self.publish, self.children + self.grader_type, self.publish, self.children, self.locator ) @@ -199,7 +201,7 @@ class CourseFixture(StudioApiFixture): self._updates = [] self._handouts = [] - self._children = [] + self.children = [] self._assets = [] self._advanced_settings = {} @@ -216,7 +218,7 @@ class CourseFixture(StudioApiFixture): Returns the course fixture to allow chaining. """ - self._children.extend(args) + self.children.extend(args) return self def add_update(self, update): @@ -257,7 +259,7 @@ class CourseFixture(StudioApiFixture): self._configure_course() self._upload_assets() self._add_advanced_settings() - self._create_xblock_children(self._course_location, self._children) + self._create_xblock_children(self._course_location, self.children) return self @@ -362,7 +364,7 @@ class CourseFixture(StudioApiFixture): # Construct HTML with each of the handout links handouts_li = [ '
  • Example Handout
    • '.format(handout=handout) - for handout in self._handouts + for handout in self._handouts ] handouts_html = '
      {}
    '.format("".join(handouts_li)) @@ -446,12 +448,31 @@ class CourseFixture(StudioApiFixture): Recursively create XBlock children. """ for desc in xblock_descriptions: - loc = self._create_xblock(parent_loc, desc) + loc = self.create_xblock(parent_loc, desc) self._create_xblock_children(loc, desc.children) self._publish_xblock(parent_loc) - def _create_xblock(self, parent_loc, xblock_desc): + def get_nested_xblocks(self, category=None): + """ + Return a list of nested XBlocks for the course that can be filtered by + category. + """ + xblocks = self._get_nested_xblocks(self) + if category: + xblocks = filter(lambda x: x.category == category, xblocks) + return xblocks + + def _get_nested_xblocks(self, xblock_descriptor): + """ + Return a list of nested XBlocks for the course. + """ + xblocks = list(xblock_descriptor.children) + for child in xblock_descriptor.children: + xblocks.extend(self._get_nested_xblocks(child)) + return xblocks + + def create_xblock(self, parent_loc, xblock_desc): """ Create an XBlock with `parent_loc` (the location of the parent block) and `xblock_desc` (an `XBlockFixtureDesc` instance). @@ -477,6 +498,7 @@ class CourseFixture(StudioApiFixture): try: loc = response.json().get('locator') + xblock_desc.locator = loc except ValueError: raise CourseFixtureError("Could not decode JSON from '{0}'".format(response.content)) diff --git a/common/test/acceptance/pages/studio/overview.py b/common/test/acceptance/pages/studio/overview.py index 67a1eefeb4..00e3ebeb0e 100644 --- a/common/test/acceptance/pages/studio/overview.py +++ b/common/test/acceptance/pages/studio/overview.py @@ -89,6 +89,9 @@ class CourseOutlineUnit(CourseOutlineChild): """ return UnitPage(self.browser, self.locator).visit() + def is_browser_on_page(self): + return self.q(css=self.BODY_SELECTOR).present + class CourseOutlineSubsection(CourseOutlineChild, CourseOutlineContainer): """ @@ -197,4 +200,3 @@ class CourseOutlinePage(CoursePage, CourseOutlineContainer): Open release date edit modal of first section in course outline """ self.q(css='div.section-published-date a.edit-release-date').first.click() - diff --git a/common/test/acceptance/pages/studio/settings_group_configurations.py b/common/test/acceptance/pages/studio/settings_group_configurations.py index 478805a423..2968f32c84 100644 --- a/common/test/acceptance/pages/studio/settings_group_configurations.py +++ b/common/test/acceptance/pages/studio/settings_group_configurations.py @@ -15,6 +15,7 @@ class GroupConfigurationsPage(CoursePage): def is_browser_on_page(self): return self.q(css='body.view-group-configurations').present + @property def group_configurations(self): """ Return list of the group configurations for the course. @@ -68,6 +69,20 @@ class GroupConfiguration(object): """ return self.find_css(css).first.text[0] + def click_outline_anchor(self): + """ + Click on the `Course Outline` link. + """ + css = 'p.group-configuration-usage-text a' + self.find_css(css).first.click() + + def click_unit_anchor(self, index=0): + """ + Click on the link to the unit. + """ + css = 'li.group-configuration-usage-unit a' + self.find_css(css).nth(index).click() + def edit(self): """ Open editing view for the group configuration. @@ -114,6 +129,14 @@ class GroupConfiguration(object): """ return self.get_text('.message-status.error') + @property + def usages(self): + """ + Return list of usages. + """ + css = '.group-configuration-usage-unit' + return self.find_css(css).text + @property def name(self): """ diff --git a/common/test/acceptance/pages/studio/unit.py b/common/test/acceptance/pages/studio/unit.py index 1639f8ca54..e30ec1f037 100644 --- a/common/test/acceptance/pages/studio/unit.py +++ b/common/test/acceptance/pages/studio/unit.py @@ -14,6 +14,8 @@ class UnitPage(PageObject): Unit page in Studio """ + NAME_SELECTOR = '#unit-display-name-input' + def __init__(self, browser, unit_locator): super(UnitPage, self).__init__(browser) self.unit_locator = unit_locator @@ -38,6 +40,10 @@ class UnitPage(PageObject): Promise(_is_finished_loading, 'Finished rendering the xblocks in the unit.').fulfill() ) + @property + def name(self): + return self.q(css=self.NAME_SELECTOR).attrs('value')[0] + @property def components(self): """ @@ -87,6 +93,7 @@ COMPONENT_BUTTONS = { 'save_settings': '.action-save', } + class Component(PageObject): """ A PageObject representing an XBlock child on the Studio UnitPage (including diff --git a/common/test/acceptance/tests/test_studio_split_test.py b/common/test/acceptance/tests/test_studio_split_test.py index b3ae6446b5..99eec41a47 100644 --- a/common/test/acceptance/tests/test_studio_split_test.py +++ b/common/test/acceptance/tests/test_studio_split_test.py @@ -8,13 +8,15 @@ import math from unittest import skip, skipUnless from xmodule.partitions.partitions import Group, UserPartition -from bok_choy.promise import Promise +from bok_choy.promise import Promise, EmptyPromise from ..fixtures.course import XBlockFixtureDesc from ..pages.studio.component_editor import ComponentEditorView +from ..pages.studio.overview import CourseOutlinePage from ..pages.studio.settings_advanced import AdvancedSettingsPage from ..pages.studio.settings_group_configurations import GroupConfigurationsPage from ..pages.studio.utils import add_advanced_component +from ..pages.studio.unit import UnitPage from ..pages.xblock.utils import wait_for_xblock_initialization from acceptance.tests.base_studio_test import StudioCourseTest @@ -238,6 +240,13 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.course_info['run'] ) + self.outline_page = CourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + def _assert_fields(self, config, cid=None, name='', description='', groups=None): self.assertEqual(config.mode, 'details') @@ -317,7 +326,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): }) self.page.visit() - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] # no groups when the the configuration is collapsed self.assertEqual(len(config.groups), 0) self._assert_fields( @@ -327,7 +336,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): groups=["Group 0", "Group 1"] ) - config = self.page.group_configurations()[1] + config = self.page.group_configurations[1] self._assert_fields( config, @@ -350,10 +359,10 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): Then I see the group configuration is saved successfully and has the new data """ self.page.visit() - self.assertEqual(len(self.page.group_configurations()), 0) + self.assertEqual(len(self.page.group_configurations), 0) # Create new group configuration self.page.create() - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.name = "New Group Configuration Name" config.description = "New Description of the group configuration." config.groups[1].name = "New Group Name" @@ -418,7 +427,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.page.visit() # Create new group configuration self.page.create() - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.name = "New Group Configuration Name" # Add new group config.add_group() @@ -435,7 +444,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): self.verify_groups(container, ['Group A', 'Group B', 'New group'], []) self.page.visit() - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.edit() config.name = "Second Group Configuration Name" # Add new group @@ -476,11 +485,11 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): """ self.page.visit() - self.assertEqual(len(self.page.group_configurations()), 0) + self.assertEqual(len(self.page.group_configurations), 0) # Create new group configuration self.page.create() - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.name = "Name of the Group Configuration" config.description = "Description of the group configuration." # Add new group @@ -488,7 +497,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): # Cancel the configuration config.cancel() - self.assertEqual(len(self.page.group_configurations()), 0) + self.assertEqual(len(self.page.group_configurations), 0) def test_can_cancel_editing_of_group_configuration(self): """ @@ -508,8 +517,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): }, }) self.page.visit() - - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.name = "New Group Configuration Name" config.description = "New Description of the group configuration." # Add 2 new groups @@ -552,7 +560,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): # Create new group configuration self.page.create() # Leave empty required field - config = self.page.group_configurations()[0] + config = self.page.group_configurations[0] config.description = "Description of the group configuration." try_to_save_and_verify_error_message("Group Configuration name is required") @@ -574,3 +582,76 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): description="Description of the group configuration.", groups=["Group A", "Group B"] ) + + def test_group_configuration_empty_usage(self): + """ + Scenario: When group configuration is not used, ensure that the link to outline page works correctly. + Given I have a course without group configurations + And I create new group configuration with 2 default groups + Then I see a link to the outline page + When I click on the outline link + Then I see the outline page + """ + # Create a new group configurations + self.course_fixture._update_xblock(self.course_fixture._course_location, { + "metadata": { + u"user_partitions": [ + UserPartition(0, "Name", "Description.", [Group("0", "Group A"), Group("1", "Group B")]).to_json(), + ], + }, + }) + + # Go to the Group Configuration Page and click on outline anchor + self.page.visit() + config = self.page.group_configurations[0] + config.toggle() + config.click_outline_anchor() + + # Waiting for the page load and verify that we've landed on course outline page + EmptyPromise( + lambda: self.outline_page.is_browser_on_page(), "loaded page {!r}".format(self.outline_page), + timeout=30 + ).fulfill() + + def test_group_configuration_non_empty_usage(self): + """ + Scenario: When group configuration is used, ensure that the links to units using a group configuration work correctly. + Given I have a course without group configurations + And I create new group configuration with 2 default groups + And I create a unit and assign the newly created group configuration + And open the Group Configuration page + Then I see a link to the newly created unit + When I click on the unit link + Then I see correct unit page + """ + # Create a new group configurations + self.course_fixture._update_xblock(self.course_fixture._course_location, { + "metadata": { + u"user_partitions": [ + UserPartition(0, "Name", "Description.", [Group("0", "Group A"), Group("1", "Group B")]).to_json(), + ], + }, + }) + + # Assign newly created group configuration to unit + vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0] + self.course_fixture.create_xblock( + vertical.locator, + XBlockFixtureDesc('split_test', 'Test Content Experiment', metadata={'user_partition_id': 0}) + ) + unit = UnitPage(self.browser, vertical.locator) + + # Go to the Group Configuration Page and click unit anchor + self.page.visit() + config = self.page.group_configurations[0] + config.toggle() + usage = config.usages[0] + config.click_unit_anchor() + + # Waiting for the page load and verify that we've landed on the unit page + EmptyPromise( + lambda: unit.is_browser_on_page(), "loaded page {!r}".format(unit), + timeout=30 + ).fulfill() + + self.assertIn(unit.name, usage)