diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index ff6bd819d8..1fe7df91c8 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -85,6 +85,10 @@ from util.milestones_helpers import ( MINIMUM_GROUP_ID = 100 +RANDOM_SCHEME = "random" +COHORT_SCHEME = "cohort" + + # 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.' @@ -1396,19 +1400,38 @@ class GroupConfiguration(object): return UserPartition.from_json(self.configuration) @staticmethod - def get_usage_info(course, store): + def _get_usage_info(course, unit, item, usage_info, group_id, scheme_name=None): + """ + Get usage info for unit/module. + """ + unit_url = reverse_usage_url( + 'container_handler', + course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name) + ) + + usage_dict = {'label': u"{} / {}".format(unit.display_name, item.display_name), 'url': unit_url} + if scheme_name == RANDOM_SCHEME: + validation_summary = item.general_validation_message() + usage_dict.update({'validation': validation_summary.to_json() if validation_summary else None}) + + usage_info[group_id].append(usage_dict) + + return usage_info + + @staticmethod + def get_content_experiment_usage_info(store, course): """ 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) + return GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) @staticmethod - def get_split_test_partitions_with_usage(course, store): + def get_split_test_partitions_with_usage(store, course): """ Returns json split_test group configurations updated with usage information. """ - usage_info = GroupConfiguration.get_usage_info(course, store) + usage_info = GroupConfiguration.get_content_experiment_usage_info(store, course) configurations = [] for partition in get_split_user_partitions(course.user_partitions): configuration = partition.to_json() @@ -1417,7 +1440,7 @@ class GroupConfiguration(object): return configurations @staticmethod - def _get_usage_info(store, course, split_tests): + def _get_content_experiment_usage_info(store, course, split_tests): """ Returns all units names, their urls and validation messages. @@ -1442,28 +1465,70 @@ class GroupConfiguration(object): if split_test.user_partition_id not in usage_info: usage_info[split_test.user_partition_id] = [] - unit_location = store.get_parent_location(split_test.location) - if not unit_location: - log.warning("Parent location of split_test module not found: %s", split_test.location) + unit = split_test.get_parent() + if not unit: + log.warning("Unable to find parent for split_test %s", split_test.location) continue - try: - unit = store.get_item(unit_location) - except ItemNotFoundError: - log.warning("Unit not found: %s", unit_location) - continue - - unit_url = reverse_usage_url( - 'container_handler', - course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name) + usage_info = GroupConfiguration._get_usage_info( + course=course, + unit=unit, + item=split_test, + usage_info=usage_info, + group_id=split_test.user_partition_id, + scheme_name=RANDOM_SCHEME ) + return usage_info + + @staticmethod + def get_content_groups_usage_info(store, course): + """ + Get usage information for content groups. + """ + items = store.get_items(course.id, settings={'group_access': {'$exists': True}}) + + return GroupConfiguration._get_content_groups_usage_info(course, items) + + @staticmethod + def _get_content_groups_usage_info(course, items): + """ + Returns all units names and their urls. + + Returns: + {'group_id': + [ + { + 'label': 'Unit 1 / Problem 1', + 'url': 'url_to_unit_1' + }, + { + 'label': 'Unit 2 / Problem 2', + 'url': 'url_to_unit_2' + } + ], + } + """ + usage_info = {} + for item in items: + if hasattr(item, 'group_access') and item.group_access: + (__, group_ids), = item.group_access.items() + for group_id in group_ids: + if group_id not in usage_info: + usage_info[group_id] = [] + + unit = item.get_parent() + if not unit: + log.warning("Unable to find parent for component %s", item.location) + continue + + usage_info = GroupConfiguration._get_usage_info( + course, + unit=unit, + item=item, + usage_info=usage_info, + group_id=group_id + ) - validation_summary = split_test.general_validation_message() - usage_info[split_test.user_partition_id].append({ - 'label': u"{} / {}".format(unit.display_name, split_test.display_name), - 'url': unit_url, - 'validation': validation_summary.to_json() if validation_summary else None, - }) return usage_info @staticmethod @@ -1473,19 +1538,39 @@ class GroupConfiguration(object): Returns json of particular group configuration updated with usage information. """ - # Get all Experiments that use particular Group Configuration in course. - split_tests = store.get_items( - course.id, - category='split_test', - content={'user_partition_id': configuration.id} - ) - configuration_json = configuration.to_json() - usage_information = GroupConfiguration._get_usage_info(store, course, split_tests) - configuration_json['usage'] = usage_information.get(configuration.id, []) + configuration_json = None + # Get all Experiments that use particular Group Configuration in course. + if configuration.scheme.name == RANDOM_SCHEME: + split_tests = store.get_items( + course.id, + category='split_test', + content={'user_partition_id': configuration.id} + ) + configuration_json = configuration.to_json() + usage_information = GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests) + configuration_json['usage'] = usage_information.get(configuration.id, []) + elif configuration.scheme.name == COHORT_SCHEME: + # In case if scheme is "cohort" + configuration_json = GroupConfiguration.update_content_group_usage_info(store, course, configuration) return configuration_json @staticmethod - def get_or_create_content_group_configuration(course): + def update_content_group_usage_info(store, course, configuration): + """ + Update usage information for particular Content Group Configuration. + + Returns json of particular content group configuration updated with usage information. + """ + usage_info = GroupConfiguration.get_content_groups_usage_info(store, course) + content_group_configuration = configuration.to_json() + + for group in content_group_configuration['groups']: + group['usage'] = usage_info.get(group['id'], []) + + return content_group_configuration + + @staticmethod + def get_or_create_content_group(store, course): """ Returns the first user partition from the course which uses the CohortPartitionScheme, or generates one if no such partition is @@ -1500,11 +1585,60 @@ class GroupConfiguration(object): name=CONTENT_GROUP_CONFIGURATION_NAME, description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION, groups=[], - scheme_id='cohort' + scheme_id=COHORT_SCHEME ) + return content_group_configuration.to_json() + + content_group_configuration = GroupConfiguration.update_content_group_usage_info( + store, + course, + content_group_configuration + ) return content_group_configuration +def remove_content_or_experiment_group(request, store, course, configuration, group_configuration_id, group_id=None): + """ + Remove content group or experiment group configuration only if it's not in use. + """ + configuration_index = course.user_partitions.index(configuration) + if configuration.scheme.name == RANDOM_SCHEME: + usages = GroupConfiguration.get_content_experiment_usage_info(store, course) + used = int(group_configuration_id) in usages + + if used: + return JsonResponse( + {"error": _("This group configuration is in use and cannot be deleted.")}, + status=400 + ) + course.user_partitions.pop(configuration_index) + elif configuration.scheme.name == COHORT_SCHEME: + if not group_id: + return JsonResponse(status=404) + + group_id = int(group_id) + usages = GroupConfiguration.get_content_groups_usage_info(store, course) + used = group_id in usages + + if used: + return JsonResponse( + {"error": _("This content group is in use and cannot be deleted.")}, + status=400 + ) + + matching_groups = [group for group in configuration.groups if group.id == group_id] + if matching_groups: + group_index = configuration.groups.index(matching_groups[0]) + configuration.groups.pop(group_index) + else: + return JsonResponse(status=404) + + course.user_partitions[configuration_index] = configuration + + store.update_item(course, request.user.id) + return JsonResponse(status=204) + + @require_http_methods(("GET", "POST")) @login_required @ensure_csrf_cookie @@ -1527,12 +1661,12 @@ def group_configurations_list_handler(request, course_key_string): course_outline_url = reverse_course_url('course_handler', course_key) 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) + experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(store, course) else: experiment_group_configurations = None - content_group_configuration = GroupConfiguration.get_or_create_content_group_configuration( - course - ).to_json() + + content_group_configuration = GroupConfiguration.get_or_create_content_group(store, course) + return render_to_response('group_configurations.html', { 'context_course': course, 'group_configuration_url': group_configuration_url, @@ -1566,7 +1700,7 @@ def group_configurations_list_handler(request, course_key_string): @login_required @ensure_csrf_cookie @require_http_methods(("POST", "PUT", "DELETE")) -def group_configurations_detail_handler(request, course_key_string, group_configuration_id): +def group_configurations_detail_handler(request, course_key_string, group_configuration_id, group_id=None): """ JSON API endpoint for manipulating a group configuration via its internal ID. Used by the Backbone application. @@ -1600,22 +1734,19 @@ def group_configurations_detail_handler(request, course_key_string, group_config store.update_item(course, request.user.id) configuration = GroupConfiguration.update_usage_info(store, course, new_configuration) return JsonResponse(configuration, status=201) + elif request.method == "DELETE": if not configuration: return JsonResponse(status=404) - # Verify that group configuration is not already in use. - usages = GroupConfiguration.get_usage_info(course, store) - if usages.get(int(group_configuration_id)): - return JsonResponse( - {"error": _("This Group Configuration is already in use and cannot be removed.")}, - status=400 - ) - - index = course.user_partitions.index(configuration) - course.user_partitions.pop(index) - store.update_item(course, request.user.id) - return JsonResponse(status=204) + return remove_content_or_experiment_group( + request=request, + store=store, + course=course, + configuration=configuration, + group_configuration_id=group_configuration_id, + group_id=group_id + ) def are_content_experiments_enabled(course): diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index b8f1ca4af4..e3898f34bc 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -1,4 +1,5 @@ #-*- coding: utf-8 -*- + """ Group Configuration Tests. """ @@ -86,16 +87,45 @@ class HelperMethods(object): self.save_course() return (vertical, split_test) - def _add_user_partitions(self, count=1): + def _create_problem_with_content_group(self, cid, group_id, name_suffix='', special_characters=''): + """ + Create a problem + Assign content group to the problem. + """ + vertical = ItemFactory.create( + category='vertical', + parent_location=self.course.location, + display_name="Test Unit {}".format(name_suffix) + ) + + problem = ItemFactory.create( + category='problem', + parent_location=vertical.location, + display_name=u"Test Problem {}{}".format(name_suffix, special_characters) + ) + + group_access_content = {'group_access': {cid: [group_id]}} + + self.client.ajax_post( + reverse_usage_url("xblock_handler", problem.location), + data={'metadata': group_access_content} + ) + + self.save_course() + + return vertical, problem + + def _add_user_partitions(self, count=1, scheme_id="random"): """ 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')] + i, 'Name ' + str(i), 'Description ' + str(i), + [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')], + scheme=None, scheme_id=scheme_id ) for i in xrange(0, count) ] - self.course.user_partitions = partitions self.save_course() @@ -285,6 +315,144 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio kwargs={'group_configuration_id': cid}, ) + def test_can_create_new_content_group_if_it_does_not_exist(self): + """ + PUT new content group. + """ + expected = { + u'id': 666, + u'name': u'Test name', + u'scheme': u'cohort', + u'description': u'Test description', + u'version': UserPartition.VERSION, + u'groups': [ + {u'id': 0, u'name': u'Group A', u'version': 1, u'usage': []}, + {u'id': 1, u'name': u'Group B', u'version': 1, u'usage': []}, + ], + } + response = self.client.put( + self._url(cid=666), + data=json.dumps(expected), + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + content = json.loads(response.content) + + self.assertEqual(content, expected) + self.reload_course() + # Verify that user_partitions in the course contains the new group configuration. + user_partitions = self.course.user_partitions + self.assertEqual(len(user_partitions), 1) + self.assertEqual(user_partitions[0].name, u'Test name') + self.assertEqual(len(user_partitions[0].groups), 2) + self.assertEqual(user_partitions[0].groups[0].name, u'Group A') + self.assertEqual(user_partitions[0].groups[1].name, u'Group B') + + def test_can_edit_content_group(self): + """ + Edit content group and check its id and modified fields. + """ + self._add_user_partitions(scheme_id='cohort') + self.save_course() + + expected = { + u'id': self.ID, + u'name': u'New Test name', + u'scheme': u'cohort', + u'description': u'New Test description', + u'version': UserPartition.VERSION, + u'groups': [ + {u'id': 0, u'name': u'New Group Name', u'version': 1, u'usage': []}, + {u'id': 2, u'name': u'Group C', u'version': 1, u'usage': []}, + ], + } + + response = self.client.put( + self._url(), + data=json.dumps(expected), + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + content = json.loads(response.content) + self.assertEqual(content, expected) + self.reload_course() + + # Verify that user_partitions is properly updated in the course. + user_partititons = self.course.user_partitions + + self.assertEqual(len(user_partititons), 1) + self.assertEqual(user_partititons[0].name, u'New Test name') + self.assertEqual(len(user_partititons[0].groups), 2) + self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name') + self.assertEqual(user_partititons[0].groups[1].name, u'Group C') + + def test_can_delete_content_group(self): + """ + Delete content group and check user partitions. + """ + self._add_user_partitions(count=1, scheme_id='cohort') + self.save_course() + + details_url_with_group_id = self._url(cid=0) + '/1' + response = self.client.delete( + details_url_with_group_id, + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + self.assertEqual(response.status_code, 204) + self.reload_course() + # Verify that group and partition is properly updated in the course. + user_partititons = self.course.user_partitions + self.assertEqual(len(user_partititons), 1) + self.assertEqual(user_partititons[0].name, 'Name 0') + self.assertEqual(len(user_partititons[0].groups), 2) + self.assertEqual(user_partititons[0].groups[1].name, 'Group C') + + def test_cannot_delete_used_content_group(self): + """ + Cannot delete content group if it is in use. + """ + self._add_user_partitions(count=1, scheme_id='cohort') + self._create_problem_with_content_group(cid=0, group_id=1) + + details_url_with_group_id = self._url(cid=0) + '/1' + response = self.client.delete( + details_url_with_group_id, + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + self.assertEqual(response.status_code, 400) + content = json.loads(response.content) + self.assertTrue(content['error']) + self.reload_course() + # Verify that user_partitions and groups are still the same. + user_partititons = self.course.user_partitions + self.assertEqual(len(user_partititons), 1) + self.assertEqual(len(user_partititons[0].groups), 3) + self.assertEqual(user_partititons[0].groups[1].name, 'Group B') + + def test_cannot_delete_non_existent_content_group(self): + """ + Cannot delete content group if it is doesn't exist. + """ + self._add_user_partitions(count=1, scheme_id='cohort') + details_url_with_group_id = self._url(cid=0) + '/90' + response = self.client.delete( + details_url_with_group_id, + content_type="application/json", + HTTP_ACCEPT="application/json", + HTTP_X_REQUESTED_WITH="XMLHttpRequest", + ) + self.assertEqual(response.status_code, 404) + # Verify that user_partitions is still the same. + user_partititons = self.course.user_partitions + self.assertEqual(len(user_partititons), 1) + self.assertEqual(len(user_partititons[0].groups), 3) + def test_can_create_new_group_configuration_if_it_does_not_exist(self): """ PUT new group configuration when no configurations exist in the course. @@ -423,18 +591,107 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio # pylint: disable=no-member class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): """ - Tests for usage information of configurations. + Tests for usage information of configurations and content groups. """ def setUp(self): super(GroupConfigurationsUsageInfoTestCase, self).setUp() + def _get_expected_content_group(self, usage_for_group): + """ + Returns the expected configuration with particular usage. + """ + return { + 'id': 0, + 'name': 'Name 0', + 'scheme': 'cohort', + 'description': 'Description 0', + 'version': UserPartition.VERSION, + 'groups': [ + {'id': 0, 'name': 'Group A', 'version': 1, 'usage': []}, + {'id': 1, 'name': 'Group B', 'version': 1, 'usage': usage_for_group}, + {'id': 2, 'name': 'Group C', 'version': 1, 'usage': []}, + ], + } + + def test_content_group_not_used(self): + """ + Test that right data structure will be created if content group is not used. + """ + self._add_user_partitions(scheme_id='cohort') + actual = GroupConfiguration.get_or_create_content_group(self.store, self.course) + expected = self._get_expected_content_group(usage_for_group=[]) + self.assertEqual(actual, expected) + + def test_can_get_correct_usage_info_when_special_characters_are_in_content(self): + """ + Test if content group json updated successfully with usage information. + """ + self._add_user_partitions(count=1, scheme_id='cohort') + vertical, __ = self._create_problem_with_content_group( + cid=0, group_id=1, name_suffix='0', special_characters=u"JOSÉ ANDRÉS" + ) + + actual = GroupConfiguration.get_or_create_content_group(self.store, self.course) + expected = self._get_expected_content_group( + usage_for_group=[ + { + 'url': u"/container/{}".format(vertical.location), + 'label': u"Test Unit 0 / Test Problem 0JOSÉ ANDRÉS" + } + ] + ) + + self.assertEqual(actual, expected) + + def test_can_get_correct_usage_info_for_content_groups(self): + """ + Test if content group json updated successfully with usage information. + """ + self._add_user_partitions(count=1, scheme_id='cohort') + vertical, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0') + + actual = GroupConfiguration.get_or_create_content_group(self.store, self.course) + + expected = self._get_expected_content_group(usage_for_group=[ + { + 'url': '/container/{}'.format(vertical.location), + 'label': 'Test Unit 0 / Test Problem 0' + } + ]) + + self.assertEqual(actual, expected) + + def test_can_use_one_content_group_in_multiple_problems(self): + """ + Test if multiple problems are present in usage info when they use same + content group. + """ + self._add_user_partitions(scheme_id='cohort') + vertical, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0') + vertical1, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='1') + + actual = GroupConfiguration.get_or_create_content_group(self.store, self.course) + + expected = self._get_expected_content_group(usage_for_group=[ + { + 'url': '/container/{}'.format(vertical.location), + 'label': 'Test Unit 0 / Test Problem 0' + }, + { + 'url': '/container/{}'.format(vertical1.location), + 'label': 'Test Unit 1 / Test Problem 1' + } + ]) + + self.assertEqual(actual, expected) + 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.get_split_test_partitions_with_usage(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course) expected = [{ 'id': 0, 'name': 'Name 0', @@ -458,7 +715,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): vertical, __ = self._create_content_experiment(cid=0, name_suffix='0') self._create_content_experiment(name_suffix='1') - actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course) expected = [{ 'id': 0, @@ -500,7 +757,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): self._add_user_partitions(count=1) vertical, __ = self._create_content_experiment(cid=0, name_suffix='0', special_characters=u"JOSÉ ANDRÉS") - actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course, ) expected = [{ 'id': 0, @@ -531,7 +788,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.get_split_test_partitions_with_usage(self.course, self.store) + actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course) expected = [{ 'id': 0, @@ -572,7 +829,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): modulestore().update_item(orphan, ModuleStoreEnum.UserID.test) self.save_course() - actual = GroupConfiguration.get_usage_info(self.course, self.store) + actual = GroupConfiguration.get_content_experiment_usage_info(self.store, self.course) self.assertEqual(actual, {0: []}) @@ -595,7 +852,7 @@ class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods): validation.add(mocked_message) mocked_validation_messages.return_value = validation - group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)[0] + group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course)[0] self.assertEqual(expected_result.to_json(), group_configuration['usage'][0]['validation']) def test_error_message_present(self): diff --git a/cms/static/js/factories/group_configurations.js b/cms/static/js/factories/group_configurations.js index d013c4c16d..920a5137ec 100644 --- a/cms/static/js/factories/group_configurations.js +++ b/cms/static/js/factories/group_configurations.js @@ -14,6 +14,7 @@ define([ experimentGroupConfigurations.url = groupConfigurationUrl; experimentGroupConfigurations.outlineUrl = courseOutlineUrl; contentGroupConfiguration.urlRoot = groupConfigurationUrl; + contentGroupConfiguration.outlineUrl = courseOutlineUrl; new GroupConfigurationsPage({ el: $('#content'), experimentsEnabled: experimentsEnabled, diff --git a/cms/static/js/models/group.js b/cms/static/js/models/group.js index 9456c2c56a..ba0629f99c 100644 --- a/cms/static/js/models/group.js +++ b/cms/static/js/models/group.js @@ -8,8 +8,17 @@ define([ return { name: '', version: 1, - order: null - }; + order: null, + usage: [] + }; + }, + url : function() { + var parentModel = this.collection.parents[0]; + return parentModel.urlRoot + '/' + encodeURIComponent(parentModel.id) + '/' + encodeURIComponent(this.id); + }, + + reset: function() { + this.set(this._originalAttributes, { parse: true }); }, isEmpty: function() { @@ -20,7 +29,8 @@ define([ return { id: this.get('id'), name: this.get('name'), - version: this.get('version') + version: this.get('version'), + usage: this.get('usage') }; }, diff --git a/cms/static/js/spec/models/group_configuration_spec.js b/cms/static/js/spec/models/group_configuration_spec.js index 1378694b15..f047fbbc82 100644 --- a/cms/static/js/spec/models/group_configuration_spec.js +++ b/cms/static/js/spec/models/group_configuration_spec.js @@ -106,10 +106,12 @@ define([ 'groups': [ { 'version': 1, - 'name': 'Group 1' + 'name': 'Group 1', + 'usage': [] }, { 'version': 1, - 'name': 'Group 2' + 'name': 'Group 2', + 'usage': [] } ] }, @@ -125,11 +127,13 @@ define([ { 'version': 1, 'order': 0, - 'name': 'Group 1' + 'name': 'Group 1', + 'usage': [] }, { 'version': 1, 'order': 1, - 'name': 'Group 2' + 'name': 'Group 2', + 'usage': [] } ], 'usage': [] diff --git a/cms/static/js/spec/views/group_configuration_spec.js b/cms/static/js/spec/views/group_configuration_spec.js index 7ee53ecc9b..400715a858 100644 --- a/cms/static/js/spec/views/group_configuration_spec.js +++ b/cms/static/js/spec/views/group_configuration_spec.js @@ -3,13 +3,14 @@ define([ '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/content_group_details', 'js/views/content_group_editor', 'js/views/content_group_item', 'js/views/feedback_notification', 'js/common_helpers/ajax_helpers', 'js/common_helpers/template_helpers', 'js/spec_helpers/view_helpers', 'jasmine-stealth' ], function( _, Course, GroupConfigurationModel, GroupModel, GroupConfigurationCollection, GroupCollection, GroupConfigurationDetailsView, GroupConfigurationsListView, GroupConfigurationEditorView, - GroupConfigurationItemView, ExperimentGroupEditView, GroupList, Notification, AjaxHelpers, TemplateHelpers, - ViewHelpers + GroupConfigurationItemView, ExperimentGroupEditView, GroupList, ContentGroupDetailsView, + ContentGroupEditorView, ContentGroupItemView, Notification, AjaxHelpers, TemplateHelpers, ViewHelpers ) { 'use strict'; var SELECTORS = { @@ -40,6 +41,134 @@ define([ note: '.wrapper-delete-button' }; + var assertTheDetailsView = function (view, text) { + expect(view.$el).toContainText(text); + expect(view.$el).toContainText('ID: 0'); + expect(view.$('.delete')).toExist(); + }; + var assertShowEmptyUsages = function (view, usageText) { + expect(view.$(SELECTORS.usageCount)).not.toExist(); + expect(view.$(SELECTORS.usageText)).toContainText(usageText); + expect(view.$(SELECTORS.usageTextAnchor)).toExist(); + expect(view.$(SELECTORS.usageUnit)).not.toExist(); + }; + var assertHideEmptyUsages = function (view) { + expect(view.$(SELECTORS.usageText)).not.toExist(); + expect(view.$(SELECTORS.usageUnit)).not.toExist(); + expect(view.$(SELECTORS.usageCount)).toContainText('Not in Use'); + }; + var assertShowNonEmptyUsages = function (view, usageText, toolTipText) { + var usageUnitAnchors = view.$(SELECTORS.usageUnitAnchor); + + expect(view.$(SELECTORS.note)).toHaveAttr( + 'data-tooltip', toolTipText + ); + expect(view.$('.delete')).toHaveClass('is-disabled'); + expect(view.$(SELECTORS.usageCount)).not.toExist(); + expect(view.$(SELECTORS.usageText)).toContainText(usageText); + expect(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'); + }; + var assertHideNonEmptyUsages = function (view) { + expect(view.$('.delete')).toHaveClass('is-disabled'); + expect(view.$(SELECTORS.usageText)).not.toExist(); + expect(view.$(SELECTORS.usageUnit)).not.toExist(); + expect(view.$(SELECTORS.usageCount)).toContainText('Used in 2 units'); + }; + var setUsageInfo = function (model) { + model.set('usage', [ + {'label': 'label1', 'url': 'url1'}, + {'label': 'label2', 'url': 'url2'} + ]); + }; + var assertHideValidationContent = function (view) { + expect(view.$(SELECTORS.usageUnitMessage)).not.toExist(); + expect(view.$(SELECTORS.usageUnitWarningIcon)).not.toExist(); + expect(view.$(SELECTORS.usageUnitErrorIcon)).not.toExist(); + }; + var assertControllerView = function (view, detailsView, editView) { + // Details view by default + expect(view.$(detailsView)).toExist(); + view.$('.action-edit .edit').click(); + expect(view.$(editView)).toExist(); + expect(view.$(detailsView)).not.toExist(); + view.$('.action-cancel').click(); + expect(view.$(detailsView)).toExist(); + expect(view.$(editView)).not.toExist(); + }; + var clickDeleteItem = function (that, promptSpy, promptText) { + that.view.$('.delete').click(); + ViewHelpers.verifyPromptShowing(promptSpy, promptText); + ViewHelpers.confirmPrompt(promptSpy); + ViewHelpers.verifyPromptHidden(promptSpy); + }; + var patchAndVerifyRequest = function (requests, url, notificationSpy) { + // Backbone.emulateHTTP is enabled in our system, so setting this + // option will fake PUT, PATCH and DELETE requests with a HTTP POST, + // setting the X-HTTP-Method-Override header with the true method. + AjaxHelpers.expectJsonRequest(requests, 'POST', url); + expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE'); + ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/); + }; + var assertAndDeleteItemError = function (that, url, promptText) { + var requests = AjaxHelpers.requests(that), + promptSpy = ViewHelpers.createPromptSpy(), + notificationSpy = ViewHelpers.createNotificationSpy(); + + clickDeleteItem(that, promptSpy, promptText); + + patchAndVerifyRequest(requests, url, notificationSpy); + + AjaxHelpers.respondToDelete(requests); + ViewHelpers.verifyNotificationHidden(notificationSpy); + expect($(SELECTORS.itemView)).not.toExist(); + }; + var assertAndDeleteItemWithError = function (that, url, listItemView, promptText) { + var requests = AjaxHelpers.requests(that), + promptSpy = ViewHelpers.createPromptSpy(), + notificationSpy = ViewHelpers.createNotificationSpy(); + + clickDeleteItem(that, promptSpy, promptText); + patchAndVerifyRequest(requests, url, notificationSpy); + + AjaxHelpers.respondWithError(requests); + ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/); + expect($(listItemView)).toExist(); + }; + var submitAndVerifyFormSuccess = function (view, requests, notificationSpy) { + view.$('form').submit(); + ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); + requests[0].respond(200); + ViewHelpers.verifyNotificationHidden(notificationSpy); + }; + var submitAndVerifyFormError = function (view, requests, notificationSpy) { + view.$('form').submit(); + ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); + AjaxHelpers.respondWithError(requests); + ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); + }; + var assertCannotDeleteUsed = function (that, toolTipText, warningText){ + setUsageInfo(that.model); + that.view.render(); + expect(that.view.$(SELECTORS.note)).toHaveAttr( + 'data-tooltip', toolTipText + ); + expect(that.view.$(SELECTORS.warningMessage)).toContainText(warningText); + expect(that.view.$(SELECTORS.warningIcon)).toExist(); + expect(that.view.$('.delete')).toHaveClass('is-disabled'); + }; + var assertUnusedOptions = function (that) { + that.model.set('usage', []); + that.view.render(); + expect(that.view.$(SELECTORS.warningMessage)).not.toExist(); + expect(that.view.$(SELECTORS.warningIcon)).not.toExist(); + }; + + beforeEach(function() { window.course = new Course({ id: '5', @@ -107,9 +236,7 @@ define([ }); it('should render properly', function() { - expect(this.view.$el).toContainText('Configuration'); - expect(this.view.$el).toContainText('ID: 0'); - expect(this.view.$('.delete')).toExist(); + assertTheDetailsView(this.view, 'Configuration'); }); it('should show groups appropriately', function() { @@ -142,69 +269,40 @@ define([ 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(); + assertShowEmptyUsages( + this.view, + 'This Group Configuration is not in use. ' + + 'Start by adding a content experiment to any Unit via the' + ); }); 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'); + assertHideEmptyUsages(this.view) }); it('should show non-empty usage appropriately', function() { - var usageUnitAnchors; - - this.model.set('usage', [ - {'label': 'label1', 'url': 'url1'}, - {'label': 'label2', 'url': 'url2'} - ]); + setUsageInfo(this.model); this.model.set('showGroups', false); this.view.$('.show-groups').click(); - usageUnitAnchors = this.view.$(SELECTORS.usageUnitAnchor); - - expect(this.view.$(SELECTORS.note)).toHaveAttr( - 'data-tooltip', 'Cannot delete when in use by an experiment' - ); - expect(this.view.$('.delete')).toHaveClass('is-disabled'); - 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'); + assertShowNonEmptyUsages( + this.view, + 'This Group Configuration is used in:', + 'Cannot delete when in use by an experiment' + ) }); it('should hide non-empty usage appropriately', function() { - this.model.set('usage', [ - {'label': 'label1', 'url': 'url1'}, - {'label': 'label2', 'url': 'url2'} - ]); + setUsageInfo(this.model); this.model.set('showGroups', true); this.view.$('.hide-groups').click(); expect(this.view.$(SELECTORS.note)).toHaveAttr( 'data-tooltip', 'Cannot delete when in use by an experiment' ); - expect(this.view.$('.delete')).toHaveClass('is-disabled'); - expect(this.view.$(SELECTORS.usageText)).not.toExist(); - expect(this.view.$(SELECTORS.usageUnit)).not.toExist(); - expect(this.view.$(SELECTORS.usageCount)) - .toContainText('Used in 2 units'); + assertHideNonEmptyUsages(this.view); }); it('should show validation warning icon and message appropriately', function() { @@ -244,16 +342,11 @@ define([ }); it('should hide validation icons and messages appropriately', function() { - this.model.set('usage', [ - {'label': 'label1', 'url': 'url1'}, - {'label': 'label2', 'url': 'url2'} - ]); + setUsageInfo(this.model); this.model.set('showGroups', true); this.view.$('.hide-groups').click(); - expect(this.view.$(SELECTORS.usageUnitMessage)).not.toExist(); - expect(this.view.$(SELECTORS.usageUnitWarningIcon)).not.toExist(); - expect(this.view.$(SELECTORS.usageUnitErrorIcon)).not.toExist(); + assertHideValidationContent(this.view); }); }); @@ -312,10 +405,7 @@ define([ inputDescription: 'New Description' }); - this.view.$('form').submit(); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); - requests[0].respond(200); - ViewHelpers.verifyNotificationHidden(notificationSpy); + submitAndVerifyFormSuccess(this.view, requests, notificationSpy); expect(this.model).toBeCorrectValuesInModel({ name: 'New Configuration', @@ -333,10 +423,7 @@ define([ notificationSpy = ViewHelpers.createNotificationSpy(); setValuesToInputs(this.view, { inputName: 'New Configuration' }); - this.view.$('form').submit(); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); - AjaxHelpers.respondWithError(requests); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/); + submitAndVerifyFormError(this.view, requests, notificationSpy); }); it('does not save on cancel', function() { @@ -379,7 +466,7 @@ define([ this.view.$('form').submit(); // See error message expect(this.view.$(SELECTORS.errorMessage)).toContainText( - 'Group Configuration name is required.' + 'Group Configuration name is required' ); // No request expect(requests.length).toBe(0); @@ -461,30 +548,17 @@ define([ }); it('cannot be deleted if it is in use', function () { - this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]); - this.view.render(); - expect(this.view.$(SELECTORS.note)).toHaveAttr( - 'data-tooltip', 'Cannot delete when in use by an experiment' - ); - expect(this.view.$('.delete')).toHaveClass('is-disabled'); - }); - - it('contains warning message if it is in use', function () { - this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]); - this.view.render(); - expect(this.view.$(SELECTORS.warningMessage)).toContainText( + assertCannotDeleteUsed( + this, + 'Cannot delete when in use by an experiment', 'This configuration is currently used in content ' + 'experiments. If you make changes to the groups, you may ' + 'need to edit those experiments.' ); - expect(this.view.$(SELECTORS.warningIcon)).toExist(); }); it('does not contain warning message if it is not in use', function () { - this.model.set('usage', []); - this.view.render(); - expect(this.view.$(SELECTORS.warningMessage)).not.toExist(); - expect(this.view.$(SELECTORS.warningIcon)).not.toExist(); + assertUnusedOptions(this); }); }); @@ -535,7 +609,6 @@ define([ }); describe('Experiment group configurations controller view', function() { - var clickDeleteItem; beforeEach(function() { TemplateHelpers.installTemplates([ @@ -550,56 +623,21 @@ define([ appendSetFixtures(this.view.render().el); }); - clickDeleteItem = function (view, promptSpy) { - view.$('.delete').click(); - ViewHelpers.verifyPromptShowing(promptSpy, /Delete this group configuration/); - ViewHelpers.confirmPrompt(promptSpy); - ViewHelpers.verifyPromptHidden(promptSpy); - }; - it('should render properly', function() { - // Details view by default - expect(this.view.$(SELECTORS.detailsView)).toExist(); - this.view.$('.action-edit .edit').click(); - expect(this.view.$(SELECTORS.editView)).toExist(); - expect(this.view.$(SELECTORS.detailsView)).not.toExist(); - this.view.$('.action-cancel').click(); - expect(this.view.$(SELECTORS.detailsView)).toExist(); - expect(this.view.$(SELECTORS.editView)).not.toExist(); + assertControllerView(this.view, SELECTORS.detailsView, SELECTORS.editView); }); it('should destroy itself on confirmation of deleting', function () { - var requests = AjaxHelpers.requests(this), - promptSpy = ViewHelpers.createPromptSpy(), - notificationSpy = ViewHelpers.createNotificationSpy(); - - clickDeleteItem(this.view, promptSpy); - // Backbone.emulateHTTP is enabled in our system, so setting this - // option will fake PUT, PATCH and DELETE requests with a HTTP POST, - // setting the X-HTTP-Method-Override header with the true method. - AjaxHelpers.expectJsonRequest(requests, 'POST', '/group_configurations/0'); - expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE'); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/); - AjaxHelpers.respondToDelete(requests); - ViewHelpers.verifyNotificationHidden(notificationSpy); - expect($(SELECTORS.itemView)).not.toExist(); + assertAndDeleteItemError(this, '/group_configurations/0', 'Delete this group configuration?'); }); it('does not hide deleting message if failure', function() { - var requests = AjaxHelpers.requests(this), - promptSpy = ViewHelpers.createPromptSpy(), - notificationSpy = ViewHelpers.createNotificationSpy(); - - clickDeleteItem(this.view, promptSpy); - // Backbone.emulateHTTP is enabled in our system, so setting this - // option will fake PUT, PATCH and DELETE requests with a HTTP POST, - // setting the X-HTTP-Method-Override header with the true method. - AjaxHelpers.expectJsonRequest(requests, 'POST', '/group_configurations/0'); - expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE'); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/); - AjaxHelpers.respondWithError(requests); - ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/); - expect($(SELECTORS.itemView)).toExist(); + assertAndDeleteItemWithError( + this, + '/group_configurations/0', + SELECTORS.itemView, + 'Delete this group configuration?' + ); }); }); @@ -651,9 +689,9 @@ define([ } }; - createGroups = function (groupNames) { - var groups = new GroupCollection(_.map(groupNames, function (groupName) { - return {name: groupName}; + createGroups = function (groupNamesWithId) { + var groups = new GroupCollection(_.map(groupNamesWithId, function (groupName, id) { + return {id: id, name: groupName}; })), groupConfiguration = new GroupConfigurationModel({ id: 0, @@ -661,11 +699,12 @@ define([ groups: groups }, {canBeEmpty: true}); groupConfiguration.urlRoot = '/mock_url'; + groupConfiguration.outlineUrl = '/mock_url'; return groups; }; - renderView = function(groupNames) { - var view = new GroupList({collection: createGroups(groupNames || [])}).render(); + renderView = function(groupNamesWithId) { + var view = new GroupList({collection: createGroups(groupNamesWithId || {})}).render(); appendSetFixtures(view.el); return view; }; @@ -778,7 +817,7 @@ define([ var requests = AjaxHelpers.requests(this), oldGroupName = 'Old Group Name', newGroupName = 'New Group Name', - view = renderView([oldGroupName]); + view = renderView({1: oldGroupName}); editNewGroup(view, {newName: newGroupName, save: true}); respondToSave(requests, view); verifyEditingGroup(view, false, 1); @@ -837,5 +876,189 @@ define([ view.collection.add({name: 'Editing Group', editing: true}); verifyEditingGroup(view, true); }); + + }); + + describe('Content groups details view', function() { + + beforeEach(function() { + TemplateHelpers.installTemplate('content-group-details', true); + this.model = new GroupModel({name: 'Content Group', id: 0}); + + var saveableModel = new GroupConfigurationModel({ + name: 'Content Group Configuration', + id: 0, + scheme:'cohort', + groups: new GroupCollection([this.model]), + }, {canBeEmpty: true}); + + saveableModel.urlRoot = '/mock_url'; + + this.collection = new GroupConfigurationCollection([ saveableModel ]); + this.collection.outlineUrl = '/outline'; + + this.view = new ContentGroupDetailsView({ + model: this.model + }); + appendSetFixtures(this.view.render().el); + }); + + it('should render properly', function() { + assertTheDetailsView(this.view, 'Content Group'); + }); + + it('should show empty usage appropriately', function() { + this.view.$('.show-groups').click(); + assertShowEmptyUsages(this.view, 'This content group is not in use. '); + }); + + it('should hide empty usage appropriately', function() { + this.view.$('.hide-groups').click(); + assertHideEmptyUsages(this.view) + }); + + it('should show non-empty usage appropriately', function() { + setUsageInfo(this.model); + this.view.$('.show-groups').click(); + + assertShowNonEmptyUsages( + this.view, + 'This content group is used in:', + 'Cannot delete when in use by a unit' + ) + }); + + it('should hide non-empty usage appropriately', function() { + setUsageInfo(this.model); + this.view.$('.hide-groups').click(); + + expect(this.view.$('li.action-delete')).toHaveAttr( + 'data-tooltip', 'Cannot delete when in use by a unit' + ); + assertHideNonEmptyUsages(this.view); + }); + + it('should hide validation icons and messages appropriately', function() { + setUsageInfo(this.model); + this.view.$('.hide-groups').click(); + assertHideValidationContent(this.view); + }); + }); + + describe('Content groups editor view', function() { + + beforeEach(function() { + ViewHelpers.installViewTemplates(); + TemplateHelpers.installTemplates(['content-group-editor']); + + this.model = new GroupModel({name: 'Content Group', id: 0}); + + this.saveableModel = new GroupConfigurationModel({ + name: 'Content Group Configuration', + id: 0, + scheme:'cohort', + groups: new GroupCollection([this.model]), + editing:true + }); + + this.collection = new GroupConfigurationCollection([ this.saveableModel ]); + this.collection.outlineUrl = '/outline'; + this.collection.url = '/group_configurations'; + + this.view = new ContentGroupEditorView({ + model: this.model + }); + appendSetFixtures(this.view.render().el); + }); + + it('should save properly', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy(); + + this.view.$('.action-add').click(); + this.view.$(SELECTORS.inputName).val('New Content Group'); + + submitAndVerifyFormSuccess(this.view, requests, notificationSpy); + + expect(this.model).toBeCorrectValuesInModel({ + name: 'New Content Group' + }); + expect(this.view.$el).not.toExist(); + }); + + it('does not hide saving message if failure', function() { + var requests = AjaxHelpers.requests(this), + notificationSpy = ViewHelpers.createNotificationSpy(); + this.view.$(SELECTORS.inputName).val('New Content Group') + + submitAndVerifyFormError(this.view, requests, notificationSpy) + }); + + it('does not save on cancel', function() { + expect(this.view.$('.action-add')); + this.view.$('.action-add').click(); + this.view.$(SELECTORS.inputName).val('New Content Group'); + + this.view.$('.action-cancel').click(); + expect(this.model).toBeCorrectValuesInModel({ + name: 'Content Group', + }); + // Model is still exist in the collection + expect(this.collection.indexOf(this.saveableModel)).toBeGreaterThan(-1); + expect(this.collection.length).toBe(1); + }); + + it('cannot be deleted if it is in use', function () { + assertCannotDeleteUsed( + this, + 'Cannot delete when in use by a unit', + 'This content group is used in one or more units.' + ); + }); + + it('does not contain warning message if it is not in use', function () { + assertUnusedOptions(this); + }); + }); + + describe('Content group controller view', function() { + beforeEach(function() { + TemplateHelpers.installTemplates([ + 'content-group-editor', 'content-group-details' + ], true); + + this.model = new GroupModel({name: 'Content Group', id: 0}); + + this.saveableModel = new GroupConfigurationModel({ + name: 'Content Group Configuration', + id: 0, + scheme:'cohort', + groups: new GroupCollection([this.model]) + }); + this.saveableModel.urlRoot = '/group_configurations'; + this.collection = new GroupConfigurationCollection([ this.saveableModel ]); + this.collection.url = '/group_configurations'; + this.view = new ContentGroupItemView({ + model: this.model + }); + appendSetFixtures(this.view.render().el); + }); + + it('should render properly', function() { + assertControllerView(this.view, '.content-group-details', '.content-group-edit'); + }); + + it('should destroy itself on confirmation of deleting', function () { + assertAndDeleteItemError(this, '/group_configurations/0/0', 'Delete this content group'); + }); + + it('does not hide deleting message if failure', function() { + assertAndDeleteItemWithError( + this, + '/group_configurations/0/0', + '.content-groups-list-item', + 'Delete this content group' + ); + }); }); }); 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 3ebd6f576f..dbf0a6a76c 100644 --- a/cms/static/js/spec/views/pages/group_configurations_spec.js +++ b/cms/static/js/spec/views/pages/group_configurations_spec.js @@ -108,7 +108,7 @@ define([ }); it('should show a notification message if a content group is changed', function () { - this.view.contentGroupConfiguration.get('groups').add({name: 'Content Group'}); + this.view.contentGroupConfiguration.get('groups').add({id: 0, 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 index 9388db60ed..4357b04aaa 100644 --- a/cms/static/js/views/content_group_details.js +++ b/cms/static/js/views/content_group_details.js @@ -3,16 +3,26 @@ * It is expected to be backed by a Group model. */ define([ - 'js/views/baseview' -], function(BaseView) { + 'js/views/baseview', 'underscore', 'gettext', 'underscore.string' +], function(BaseView, _, gettext, str) { 'use strict'; var ContentGroupDetailsView = BaseView.extend({ tagName: 'div', - className: 'content-group-details collection', - events: { - 'click .edit': 'editGroup' + 'click .edit': 'editGroup', + 'click .show-groups': 'showContentGroupUsages', + 'click .hide-groups': 'hideContentGroupUsages' + }, + + className: function () { + var index = this.model.collection.indexOf(this.model); + + return [ + 'collection', + 'content-group-details', + 'content-group-details-' + index + ].join(' '); }, editGroup: function() { @@ -21,10 +31,66 @@ define([ initialize: function() { this.template = this.loadTemplate('content-group-details'); + this.listenTo(this.model, 'change', this.render); }, - render: function() { - this.$el.html(this.template(this.model.toJSON())); + render: function(showContentGroupUsages) { + var attrs = $.extend({}, this.model.attributes, { + usageCountMessage: this.getUsageCountTitle(), + outlineAnchorMessage: this.getOutlineAnchorMessage(), + index: this.model.collection.indexOf(this.model), + showContentGroupUsages: showContentGroupUsages || false + }); + this.$el.html(this.template(attrs)); + return this; + }, + + showContentGroupUsages: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + this.render(true); + }, + + hideContentGroupUsages: function(event) { + if (event && event.preventDefault) { event.preventDefault(); } + this.render(false); + }, + + 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 content group is not in use. Add a content group to any unit from the %(outlineAnchor)s.' + ), + anchor = str.sprintf( + '%(text)s', + { + url: this.model.collection.parents[0].outlineUrl, + text: gettext('Course Outline') + } + ); + + return str.sprintf(message, {outlineAnchor: anchor}); } }); diff --git a/cms/static/js/views/content_group_editor.js b/cms/static/js/views/content_group_editor.js index 7d87048f02..f249287cbe 100644 --- a/cms/static/js/views/content_group_editor.js +++ b/cms/static/js/views/content_group_editor.js @@ -23,9 +23,11 @@ function(ListItemEditorView, _) { getTemplateOptions: function() { return { + id: this.model.escape('id'), name: this.model.escape('name'), index: this.model.collection.indexOf(this.model), isNew: this.model.isNew(), + usage: this.model.get('usage'), uniqueId: _.uniqueId() }; }, diff --git a/cms/static/js/views/content_group_item.js b/cms/static/js/views/content_group_item.js index 199fe31a0f..06d3b22c87 100644 --- a/cms/static/js/views/content_group_item.js +++ b/cms/static/js/views/content_group_item.js @@ -5,15 +5,30 @@ * 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) { + 'js/views/list_item', 'js/views/content_group_editor', 'js/views/content_group_details', 'gettext', "js/views/utils/view_utils" +], function(ListItemView, ContentGroupEditorView, ContentGroupDetailsView, gettext) { 'use strict'; var ContentGroupItemView = ListItemView.extend({ + events: { + 'click .delete': 'deleteItem' + }, + tagName: 'section', baseClassName: 'content-group', + canDelete: true, + + itemDisplayName: gettext('content group'), + + attributes: function () { + return { + 'id': this.model.get('id'), + 'tabindex': -1 + }; + }, + createEditView: function() { return new ContentGroupEditorView({model: this.model}); }, diff --git a/cms/static/sass/views/_group-configuration.scss b/cms/static/sass/views/_group-configuration.scss index ac6168423f..897860a7c6 100644 --- a/cms/static/sass/views/_group-configuration.scss +++ b/cms/static/sass/views/_group-configuration.scss @@ -100,6 +100,28 @@ 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-usage-count { + font-style: italic; + } + } + } + + &.group-configuration-info-block { + li { + padding: ($baseline/4) 0; + } + } + &.collection-info-inline { display: table; width: 70%; @@ -355,12 +377,31 @@ } } - .field.add-collection-name label { - @extend %t-title5; - display: inline-block; - vertical-align: bottom; + .field.add-collection-name { + + label { + width: 50%; + @extend %t-title5; + display: inline-block; + vertical-align: bottom; + } + + .group-configuration-id { + display: inline-block; + width: 45%; + text-align: right; + vertical-align: top; + color: $gray-l1; + + .group-configuration-value { + @extend %t-strong; + white-space: nowrap; + margin-left: ($baseline*0.5); + } + } } + .actions { box-shadow: inset 0 1px 2px $shadow; border-top: 1px solid $gray-l1; @@ -443,7 +484,7 @@ .collection-header{ .title { - margin-bottom: 0; + margin-bottom: 0; } } } @@ -457,28 +498,6 @@ 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-usage-count { - font-style: italic; - } - } - } - - &.group-configuration-info-block { - li { - padding: ($baseline/4) 0; - } - } - .group-configuration-label { text-transform: uppercase; } @@ -526,27 +545,12 @@ .group-configuration-edit { .add-collection-name label { - width: 50%; padding-right: 5%; overflow: hidden; text-overflow: ellipsis; vertical-align: bottom; } - .group-configuration-id { - display: inline-block; - width: 45%; - text-align: right; - vertical-align: top; - color: $gray-l1; - - .group-configuration-value { - @extend %t-strong; - white-space: nowrap; - margin-left: ($baseline*0.5); - } - } - .field-group { @include clearfix(); margin: 0 0 ($baseline/2) 0; diff --git a/cms/templates/group_configurations.html b/cms/templates/group_configurations.html index f9a6d0ce08..2a9064010f 100644 --- a/cms/templates/group_configurations.html +++ b/cms/templates/group_configurations.html @@ -67,7 +67,8 @@

${_("Content Groups")}

${_("Use content groups to give groups of students access to a specific set of course content. In addition to course content that is intended for all students, each content group sees content that you specifically designate as visible to it. By associating a content group with one or more cohorts, you can customize the content that a particular cohort or cohorts sees in your course.")}

-

${_("Click {em_start}New content group{em_end} to add a new content group. To edit the name of a content group, hover over its box and click {em_start}Edit{em_end}. Content groups cannot be deleted.").format(em_start="", em_end="")}

+

${_("Click {em_start}New content group{em_end} to add a new content group. To edit the name of a content group, hover over its box and click {em_start}Edit{em_end}.").format(em_start="", em_end="")}

+

${_("You can delete a content group only if it is not in use by a unit. To delete a content group, hover over its box and click the delete icon.")}

${_("Learn More")}

diff --git a/cms/templates/js/content-group-details.underscore b/cms/templates/js/content-group-details.underscore index 9c1999fb25..b300f4b862 100644 --- a/cms/templates/js/content-group-details.underscore +++ b/cms/templates/js/content-group-details.underscore @@ -1,13 +1,58 @@ -
+

- <%- name %> + + + <%= name %> +

-
+ +<% if (showContentGroupUsages) { %> +
+ <% if (!_.isEmpty(usage)) { %> +

<%= gettext('This content group is used in:') %>

+
    + <% _.each(usage, function(unit) { %> +
  1. +

    ><%= unit.label %>

    +
    2. + <% }) %> +
+ <% } else { %> +

+ <%= outlineAnchorMessage %> +

+ <% } %> +
+<% } %> diff --git a/cms/templates/js/content-group-editor.underscore b/cms/templates/js/content-group-editor.underscore index b4d3489a5d..0861e7b6ee 100644 --- a/cms/templates/js/content-group-editor.underscore +++ b/cms/templates/js/content-group-editor.underscore @@ -7,13 +7,40 @@
- + <% + if (!_.isUndefined(id) && !_.isEmpty(id)) { + %> + <%= gettext('Content Group ID') %> + <%= id %> + <% + } + %> ">
+ <% if (!_.isEmpty(usage)) { %> +
+ +

+ <%= gettext('This content group is used in one or more units.') %> +

+
+ <% } %>
+ + <% if (!isNew) { %> + <% if (_.isEmpty(usage)) { %> + "> + <%= gettext("Delete") %> + + <% } else { %> + + <%= gettext("Delete") %> + + <% } %> + <% } %>
diff --git a/cms/urls.py b/cms/urls.py index 4ca692f109..89ef86040c 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -108,8 +108,8 @@ urlpatterns += patterns( url(r'^videos/{}$'.format(settings.COURSE_KEY_PATTERN), 'videos_handler'), url(r'^video_encodings_download/{}$'.format(settings.COURSE_KEY_PATTERN), 'video_encodings_download'), url(r'^group_configurations/{}$'.format(settings.COURSE_KEY_PATTERN), 'group_configurations_list_handler'), - url(r'^group_configurations/{}/(?P\d+)/?$'.format(settings.COURSE_KEY_PATTERN), - 'group_configurations_detail_handler'), + url(r'^group_configurations/{}/(?P\d+)(/)?(?P\d+)?$'.format( + settings.COURSE_KEY_PATTERN), 'group_configurations_detail_handler'), url(r'^api/val/v0/', include('edxval.urls')), ) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 132d5c4c6b..b558cd37ce 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -777,10 +777,8 @@ class ModuleStoreRead(ModuleStoreAssetBase): for key, criteria in qualifiers.iteritems(): is_set, value = _is_set_on(key) - if isinstance(criteria, dict) and '$exists' in criteria and criteria['$exists'] == is_set: continue - if not is_set: return False if not self._value_matches(value, criteria): diff --git a/common/test/acceptance/pages/studio/settings_group_configurations.py b/common/test/acceptance/pages/studio/settings_group_configurations.py index 8d1f1c40d7..95159c369a 100644 --- a/common/test/acceptance/pages/studio/settings_group_configurations.py +++ b/common/test/acceptance/pages/studio/settings_group_configurations.py @@ -167,8 +167,11 @@ class GroupConfiguration(object): return self.find_css('.actions .delete.is-disabled').present @property - def delete_button_is_absent(self): - return not self.find_css('.actions .delete').present + def delete_button_is_present(self): + """ + Returns whether or not the delete icon is present. + """ + return self.find_css('.actions .delete').present def delete(self): """ diff --git a/common/test/acceptance/tests/studio/test_studio_settings.py b/common/test/acceptance/tests/studio/test_studio_settings.py index b7238cc4c4..aefd5ba1a6 100644 --- a/common/test/acceptance/tests/studio/test_studio_settings.py +++ b/common/test/acceptance/tests/studio/test_studio_settings.py @@ -5,9 +5,15 @@ Acceptance tests for Studio's Setting pages from nose.plugins.attrib import attr from base_studio_test import StudioCourseTest - +from bok_choy.promise import EmptyPromise +from ...fixtures.course import XBlockFixtureDesc +from ..helpers import create_user_partition_json +from ...pages.studio.overview import CourseOutlinePage from ...pages.studio.settings_advanced import AdvancedSettingsPage from ...pages.studio.settings_group_configurations import GroupConfigurationsPage +from unittest import skip +from textwrap import dedent +from xmodule.partitions.partitions import Group @attr('shard_1') @@ -25,6 +31,26 @@ class ContentGroupConfigurationTest(StudioCourseTest): self.course_info['run'] ) + self.outline_page = CourseOutlinePage( + self.browser, + self.course_info['org'], + self.course_info['number'], + self.course_info['run'] + ) + + def populate_course_fixture(self, course_fixture): + """ + Populates test course with chapter, sequential, and 1 problems. + The problem is visible only to Group "alpha". + """ + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children( + XBlockFixtureDesc('vertical', 'Test Unit') + ) + ) + ) + def create_and_verify_content_group(self, name, existing_groups): """ Creates a new content group and verifies that it was properly created. @@ -38,7 +64,7 @@ class ContentGroupConfigurationTest(StudioCourseTest): config.name = name # Save the content group self.assertEqual(config.get_text('.action-primary'), "Create") - self.assertTrue(config.delete_button_is_absent) + self.assertFalse(config.delete_button_is_present) config.save() self.assertIn(name, config.name) return config @@ -84,16 +110,68 @@ class ContentGroupConfigurationTest(StudioCourseTest): self.assertIn("Updated Second Content Group", second_config.name) - def test_cannot_delete_content_group(self): + def test_cannot_delete_used_content_group(self): """ - Scenario: Delete is not currently supported for content groups. - Given I have a course without content groups - When I create a content group - Then there is no delete button + Scenario: Ensure that the user cannot delete used content group. + Given I have a course with 1 Content Group + And I go to the Group Configuration page + When I try to delete the Content Group with name "New Content Group" + Then I see the delete button is disabled. + """ + self.course_fixture._update_xblock(self.course_fixture._course_location, { + "metadata": { + u"user_partitions": [ + create_user_partition_json( + 0, + 'Configuration alpha,', + 'Content Group Partition', + [Group("0", 'alpha')], + scheme="cohort" + ) + ], + }, + }) + problem_data = dedent(""" + +

Choose Yes.

+ + + Yes + + + + """) + vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0] + self.course_fixture.create_xblock( + vertical.locator, + XBlockFixtureDesc('problem', "VISIBLE TO ALPHA", data=problem_data, metadata={"group_access": {0: [0]}}), + ) + self.group_configurations_page.visit() + config = self.group_configurations_page.content_groups[0] + self.assertTrue(config.delete_button_is_disabled) + + def test_can_delete_unused_content_group(self): + """ + Scenario: Ensure that the user can delete unused content group. + Given I have a course with 1 Content Group + And I go to the Group Configuration page + When I delete the Content Group with name "New Content Group" + Then I see that there is no Content Group + When I refresh the page + Then I see that the content group has been deleted """ self.group_configurations_page.visit() config = self.create_and_verify_content_group("New Content Group", 0) - self.assertTrue(config.delete_button_is_absent) + self.assertTrue(config.delete_button_is_present) + + self.assertEqual(len(self.group_configurations_page.content_groups), 1) + + # Delete content group + config.delete() + self.assertEqual(len(self.group_configurations_page.content_groups), 0) + + self.group_configurations_page.visit() + self.assertEqual(len(self.group_configurations_page.content_groups), 0) def test_must_supply_name(self): """ @@ -129,6 +207,26 @@ class ContentGroupConfigurationTest(StudioCourseTest): config.cancel() self.assertEqual(0, len(self.group_configurations_page.content_groups)) + def test_content_group_empty_usage(self): + """ + Scenario: When content group is not used, ensure that the link to outline page works correctly. + Given I have a course without content group + And I create new content group + Then I see a link to the outline page + When I click on the outline link + Then I see the outline page + """ + self.group_configurations_page.visit() + config = self.create_and_verify_content_group("New Content Group", 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() + @attr('shard_1') class AdvancedSettingsValidationTest(StudioCourseTest): diff --git a/common/test/acceptance/tests/studio/test_studio_split_test.py b/common/test/acceptance/tests/studio/test_studio_split_test.py index 3d8ef20bae..18d10c3390 100644 --- a/common/test/acceptance/tests/studio/test_studio_split_test.py +++ b/common/test/acceptance/tests/studio/test_studio_split_test.py @@ -449,7 +449,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin): # Save the configuration self.assertEqual(config.get_text('.action-primary'), "Create") - self.assertTrue(config.delete_button_is_absent) + self.assertFalse(config.delete_button_is_present) config.save() self._assert_fields(