Merge pull request #11336 from edx/mrehan/wrong-units-in-group-tnl-3727

Fix - skip orphans from content group usage information for split

cms/djangoapps/contentstore/course_group_config.py

@@ -190,7 +190,7 @@ class GroupConfiguration(object):
         """
         Get usage information for content groups.
         """
-        items = store.get_items(course.id, settings={'group_access': {'$exists': True}})
+        items = store.get_items(course.id, settings={'group_access': {'$exists': True}}, include_orphans=False)
 
         return GroupConfiguration._get_content_groups_usage_info(course, items)
 

cms/djangoapps/contentstore/views/tests/test_group_configurations.py

@@ -4,12 +4,14 @@
 Group Configuration Tests.
 """
 import json
+import ddt
 from mock import patch
+
 from contentstore.utils import reverse_course_url, reverse_usage_url
 from contentstore.course_group_config import GroupConfiguration
 from contentstore.tests.utils import CourseTestCase
 from xmodule.partitions.partitions import Group, UserPartition
-from xmodule.modulestore.tests.factories import ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.validation import StudioValidation, StudioValidationMessage
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
@@ -110,6 +112,7 @@ class HelperMethods(object):
             data={'metadata': group_access_content}
         )
 
+        self.course.children.append(vertical.location)
         self.save_course()
 
         return vertical, problem
@@ -600,6 +603,7 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
         self.assertEqual(user_partititons[0].name, 'Name 0')
 
 
+@ddt.ddt
 class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
     """
     Tests for usage information of configurations and content groups.
@@ -675,6 +679,45 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
 
         self.assertEqual(actual, expected)
 
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_can_get_correct_usage_info_with_orphan(self, module_store_type):
+        """
+        Test if content group json updated successfully with usage information even if there is
+        an orphan in content group.
+        """
+        self.course = CourseFactory.create(default_store=module_store_type)
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        vertical, problem = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0')
+
+        # Assert that there is no orphan in the course yet.
+        self.assertEqual(len(self.store.get_orphans(self.course.id)), 0)
+
+        # Update problem(created earlier) to an orphan.
+        with self.store.branch_setting(ModuleStoreEnum.Branch.published_only):
+            vertical = self.store.get_item(vertical.location)
+            vertical.children.remove(problem.location)
+            self.store.update_item(vertical, self.user.id)
+
+        # Assert that the problem is orphan now.
+        self.assertIn(problem.location, self.store.get_orphans(self.course.id))
+
+        # Get the expected content group information based on module store.
+        if module_store_type == ModuleStoreEnum.Type.mongo:
+            expected = self._get_expected_content_group(usage_for_group=[
+                {
+                    'url': '/container/{}'.format(vertical.location),
+                    'label': 'Test Unit 0 / Test Problem 0'
+                }
+            ])
+        else:
+            expected = self._get_expected_content_group(usage_for_group=[])
+
+        # Get the actual content group information
+        actual = GroupConfiguration.get_or_create_content_group(self.store, self.course)
+
+        # Assert that actual content group information is same as expected one.
+        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
@@ -687,13 +730,13 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         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'
+            },
+            {
+                'url': '/container/{}'.format(vertical.location),
+                'label': 'Test Unit 0 / Test Problem 0'
             }
         ])