From 83ae677b83812e92edc305dfb1091638371e3bb7 Mon Sep 17 00:00:00 2001 From: cahrens Date: Mon, 24 Jul 2017 10:50:47 -0400 Subject: [PATCH] Don't add groups in experiment configurations twice. EDUCATOR-947 --- cms/djangoapps/contentstore/views/course.py | 27 +++++++++++-------- .../views/tests/test_group_configurations.py | 3 ++- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f306c6f9ea..87904bc8fc 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1547,22 +1547,27 @@ def group_configurations_list_handler(request, course_key_string): all_partitions = GroupConfiguration.get_all_user_partition_details(store, course) should_show_enrollment_track = False - group_schemes = [] + has_content_groups = False + displayable_partitions = [] for partition in all_partitions: - group_schemes.append(partition['scheme']) - if partition['scheme'] == ENROLLMENT_SCHEME: - enrollment_track_configuration = partition - should_show_enrollment_track = len(enrollment_track_configuration['groups']) > 1 + if partition['scheme'] == COHORT_SCHEME: + has_content_groups = True + displayable_partitions.append(partition) + elif partition['scheme'] == ENROLLMENT_SCHEME: + should_show_enrollment_track = len(partition['groups']) > 1 - # Remove the enrollment track partition and add it to the front of the list if it should be shown. - all_partitions.remove(partition) + # Add it to the front of the list if it should be shown. if should_show_enrollment_track: - all_partitions.insert(0, partition) + displayable_partitions.insert(0, partition) + elif partition['scheme'] != RANDOM_SCHEME: + # Experiment group configurations are handled explicitly above. We don't + # want to display their groups twice. + displayable_partitions.append(partition) # Add empty content group if there is no COHORT User Partition in the list. # This will add ability to add new groups in the view. - if COHORT_SCHEME not in group_schemes: - all_partitions.append(GroupConfiguration.get_or_create_content_group(store, course)) + if not has_content_groups: + displayable_partitions.append(GroupConfiguration.get_or_create_content_group(store, course)) return render_to_response('group_configurations.html', { 'context_course': course, @@ -1570,7 +1575,7 @@ def group_configurations_list_handler(request, course_key_string): 'course_outline_url': course_outline_url, 'experiment_group_configurations': experiment_group_configurations, 'should_show_experiment_groups': should_show_experiment_groups, - 'all_group_configurations': all_partitions, + 'all_group_configurations': displayable_partitions, 'should_show_enrollment_track': should_show_enrollment_track }) elif "application/json" in request.META.get('HTTP_ACCEPT'): diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index ac0b234c68..1ce1980465 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -250,6 +250,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations Basic check that the groups configuration page responds correctly. """ + # This creates a random UserPartition. self.course.user_partitions = [ UserPartition(0, 'First name', 'First description', [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')]), ] @@ -261,7 +262,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations response = self.client.get(self._url()) self.assertEqual(response.status_code, 200) - self.assertContains(response, 'First name') + self.assertContains(response, 'First name', count=1) self.assertContains(response, 'Group C') self.assertContains(response, CONTENT_GROUP_CONFIGURATION_NAME)