Merge pull request #10490 from edx/release

Release 4 November 2015 to master

common/djangoapps/student/models.py

@@ -1377,7 +1377,7 @@ class CourseEnrollment(models.Model):
 
         # If it is after the refundable cutoff date they should not be refunded.
         refund_cutoff_date = self.refund_cutoff_date()
-        if refund_cutoff_date and datetime.now() > refund_cutoff_date:
+        if refund_cutoff_date and datetime.now(UTC) > refund_cutoff_date:
             return False
 
         course_mode = CourseMode.mode_for_course(self.course_id, 'verified')
@@ -1400,7 +1400,7 @@ class CourseEnrollment(models.Model):
             self.course_overview.start.replace(tzinfo=None)
         )
 
-        return refund_window_start_date + EnrollmentRefundConfiguration.current().refund_window
+        return refund_window_start_date.replace(tzinfo=UTC) + EnrollmentRefundConfiguration.current().refund_window
 
     @property
     def username(self):

common/djangoapps/student/tests/test_refunds.py

@@ -113,10 +113,10 @@ class RefundableTest(SharedModuleStoreTestCase):
         self.assertTrue(self.enrollment.refundable())
 
         with patch('student.models.CourseEnrollment.refund_cutoff_date') as cutoff_date:
-            cutoff_date.return_value = datetime.now() - timedelta(days=1)
+            cutoff_date.return_value = datetime.now(pytz.UTC) - timedelta(minutes=5)
             self.assertFalse(self.enrollment.refundable())
 
-            cutoff_date.return_value = datetime.now() + timedelta(days=1)
+            cutoff_date.return_value = datetime.now(pytz.UTC) + timedelta(minutes=5)
             self.assertTrue(self.enrollment.refundable())
 
     @ddt.data(
@@ -132,7 +132,7 @@ class RefundableTest(SharedModuleStoreTestCase):
         """
         Assert that the later date is used with the configurable refund period in calculating the returned cutoff date.
         """
-        now = datetime.now().replace(microsecond=0)
+        now = datetime.now(pytz.UTC).replace(microsecond=0)
         order_date = now + order_date_delta
         course_start = now + course_start_delta
         expected_date = now + expected_date_delta

common/djangoapps/student/tests/test_views.py

@@ -11,6 +11,7 @@ from django.conf import settings
 
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 from student.models import CourseEnrollment
+from student.helpers import DISABLE_UNENROLL_CERT_STATES
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -38,7 +39,10 @@ class TestStudentDashboardUnenrollments(ModuleStoreTestCase):
     def mock_cert(self, _user, _course_overview, _course_mode):  # pylint: disable=unused-argument
         """ Return a preset certificate status. """
         if self.cert_status is not None:
-            return {'status': self.cert_status}
+            return {
+                'status': self.cert_status,
+                'can_unenroll': self.cert_status not in DISABLE_UNENROLL_CERT_STATES
+            }
         else:
             return {}
 
@@ -85,3 +89,17 @@ class TestStudentDashboardUnenrollments(ModuleStoreTestCase):
                 course_enrollment.assert_called_with(self.user, self.course.id)
             else:
                 course_enrollment.assert_not_called()
+
+    def test_no_cert_status(self):
+        """ Assert that the dashboard loads when cert_status is None."""
+        with patch('student.views.cert_info', return_value=None):
+            response = self.client.get(reverse('dashboard'))
+
+            self.assertEqual(response.status_code, 200)
+
+    def test_cant_unenroll_status(self):
+        """ Assert that the dashboard loads when cert_status does not allow for unenrollment"""
+        with patch('certificates.models.certificate_status_for_student', return_value={'status': 'ready'}):
+            response = self.client.get(reverse('dashboard'))
+
+            self.assertEqual(response.status_code, 200)

common/djangoapps/student/tests/tests.py

@@ -79,6 +79,7 @@ class CourseEndingTest(TestCase):
                 'show_disabled_download_button': False,
                 'show_download_url': False,
                 'show_survey_button': False,
+                'can_unenroll': True,
             }
         )
 
@@ -91,7 +92,8 @@ class CourseEndingTest(TestCase):
                 'show_download_url': False,
                 'show_survey_button': False,
                 'mode': None,
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': True,
             }
         )
 
@@ -106,7 +108,8 @@ class CourseEndingTest(TestCase):
                 'survey_url': survey_url,
                 'grade': '67',
                 'mode': 'honor',
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': False,
             }
         )
 
@@ -121,7 +124,8 @@ class CourseEndingTest(TestCase):
                 'survey_url': survey_url,
                 'grade': '67',
                 'mode': 'verified',
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': False,
             }
         )
 
@@ -143,7 +147,8 @@ class CourseEndingTest(TestCase):
                 'survey_url': survey_url,
                 'grade': '67',
                 'mode': 'honor',
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': False,
             }
         )
 
@@ -162,7 +167,8 @@ class CourseEndingTest(TestCase):
                 'survey_url': survey_url,
                 'grade': '67',
                 'mode': 'honor',
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': True,
             }
         )
 
@@ -181,21 +187,22 @@ class CourseEndingTest(TestCase):
                 'show_survey_button': False,
                 'grade': '67',
                 'mode': 'honor',
-                'linked_in_url': None
+                'linked_in_url': None,
+                'can_unenroll': True,
             }
         )
 
         # test when the display is unavailable or notpassing, we get the correct results out
         course2.certificates_display_behavior = 'early_no_info'
         cert_status = {'status': 'unavailable'}
-        self.assertIsNone(_cert_info(user, course2, cert_status, course_mode))
+        self.assertEqual(_cert_info(user, course2, cert_status, course_mode), {})
 
         cert_status = {
             'status': 'notpassing', 'grade': '67',
             'download_url': download_url,
             'mode': 'honor'
         }
-        self.assertIsNone(_cert_info(user, course2, cert_status, course_mode))
+        self.assertEqual(_cert_info(user, course2, cert_status, course_mode), {})
 
 
 @ddt.ddt
@@ -1033,6 +1040,32 @@ class DashboardTestXSeriesPrograms(ModuleStoreTestCase, ProgramsApiConfigMixin):
             else:
                 self.assertIn('xseries-border-btn', response.content)
 
+    @patch.dict('django.conf.settings.FEATURES', {'DISABLE_START_DATES': False})
+    @ddt.data((-2, -1), (-1, 1), (1, 2))
+    @ddt.unpack
+    def test_start_end_offsets(self, start_days_offset, end_days_offset):
+        """Test that the xseries upsell messaging displays whether the course
+        has not yet started, is in session, or has already ended.
+        """
+        self.course_1.start = datetime.now(pytz.UTC) + timedelta(days=start_days_offset)
+        self.course_1.end = datetime.now(pytz.UTC) + timedelta(days=end_days_offset)
+        self.update_course(self.course_1, self.user.id)
+        CourseEnrollment.enroll(self.user, self.course_1.id, mode='verified')
+
+        self.client.login(username="jack", password="test")
+        self.create_config(enabled=True, enable_student_dashboard=True)
+
+        with patch(
+            'student.views.get_course_programs_for_dashboard',
+            return_value=self._create_program_data([(self.course_1.id, 'active')])
+        ) as mock_get_programs:
+            response = self.client.get(reverse('dashboard'))
+            # ensure that our course id was included in the API call regardless of start/end dates
+            __, course_ids = mock_get_programs.call_args[0]
+            self.assertEqual(list(course_ids), [self.course_1.id])
+            # count total courses appearing on student dashboard
+            self._assert_responses(response, 1)
+
     @ddt.data(
         ('unpublished', 'unpublished', 'unpublished', 0),
         ('active', 'unpublished', 'unpublished', 1),

common/djangoapps/student/views.py

@@ -202,6 +202,7 @@ def cert_info(user, course_overview, course_mode):
             'show_survey_button': bool
             'survey_url': url, only if show_survey_button is True
             'grade': if status is not 'processing'
+            'can_unenroll': if status allows for unenrollment
     """
     if not course_overview.may_certify():
         return {}
@@ -302,6 +303,7 @@ def _cert_info(user, course_overview, cert_status, course_mode):  # pylint: disa
                     'show_disabled_download_button': False,
                     'show_download_url': False,
                     'show_survey_button': False,
+                    'can_unenroll': True
                     }
 
     if cert_status is None:
@@ -310,7 +312,7 @@ def _cert_info(user, course_overview, cert_status, course_mode):  # pylint: disa
     is_hidden_status = cert_status['status'] in ('unavailable', 'processing', 'generating', 'notpassing')
 
     if course_overview.certificates_display_behavior == 'early_no_info' and is_hidden_status:
-        return None
+        return {}
 
     status = template_state.get(cert_status['status'], default_status)
 
@@ -319,7 +321,8 @@ def _cert_info(user, course_overview, cert_status, course_mode):  # pylint: disa
         'show_download_url': status == 'ready',
         'show_disabled_download_button': status == 'generating',
         'mode': cert_status.get('mode', None),
-        'linked_in_url': None
+        'linked_in_url': None,
+        'can_unenroll': status not in DISABLE_UNENROLL_CERT_STATES,
     }
 
     if (status in ('generating', 'ready', 'notpassing', 'restricted') and
@@ -581,7 +584,7 @@ def dashboard(request):
     # program-related information on the dashboard view.
     course_programs = {}
     if is_student_dashboard_programs_enabled():
-        course_programs = _get_course_programs(user, show_courseware_links_for)
+        course_programs = _get_course_programs(user, [enrollment.course_id for enrollment in course_enrollments])
 
     # Construct a dictionary of course mode information
     # used to render the course list.  We re-use the course modes dict
@@ -1030,8 +1033,8 @@ def change_enrollment(request, check_access=True):
         if not enrollment:
             return HttpResponseBadRequest(_("You are not enrolled in this course"))
 
-        certicifate_info = cert_info(user, enrollment.course_overview, enrollment.mode)
-        if certicifate_info.get('status') in DISABLE_UNENROLL_CERT_STATES:
+        certificate_info = cert_info(user, enrollment.course_overview, enrollment.mode)
+        if certificate_info.get('status') in DISABLE_UNENROLL_CERT_STATES:
             return HttpResponseBadRequest(_("Your certificate prevents you from unenrolling from this course"))
 
         CourseEnrollment.unenroll(user, course_id)

common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py

@@ -245,7 +245,7 @@ class ProctoredExamsTest(BaseInstructorDashboardTest):
         # Stop the timed exam.
         self.courseware_page.stop_timed_exam()
 
-    @flaky  # TODO fix this SOL-1183
+    @flaky  # TODO fix this SOL-1182
     def test_can_add_remove_allowance(self):
         """
         Make sure that allowances can be added and removed.
@@ -263,6 +263,7 @@ class ProctoredExamsTest(BaseInstructorDashboardTest):
         # Then I can add Allowance to that exam for a student
         self.assertTrue(allowance_section.is_add_allowance_button_visible)
 
+    @flaky  # TODO fix this SOL-1182
     def test_can_reset_attempts(self):
         """
         Make sure that Exam attempts are visible and can be reset.

lms/djangoapps/certificates/admin.py

@@ -11,6 +11,7 @@ from certificates.models import (
     BadgeImageConfiguration,
     CertificateTemplate,
     CertificateTemplateAsset,
+    GeneratedCertificate,
 )
 
 
@@ -46,8 +47,17 @@ class CertificateTemplateAssetAdmin(admin.ModelAdmin):
     list_display = ('description', '__unicode__')
 
 
+class GeneratedCertificateAdmin(admin.ModelAdmin):
+    """
+    Django admin customizations for GeneratedCertificate model
+    """
+    search_fields = ('course_id', 'user__username')
+    list_display = ('id', 'course_id', 'mode', 'user')
+
+
 admin.site.register(CertificateGenerationConfiguration)
 admin.site.register(CertificateHtmlViewConfiguration, ConfigurationModelAdmin)
 admin.site.register(BadgeImageConfiguration)
 admin.site.register(CertificateTemplate, CertificateTemplateAdmin)
 admin.site.register(CertificateTemplateAsset, CertificateTemplateAssetAdmin)
+admin.site.register(GeneratedCertificate, GeneratedCertificateAdmin)

lms/djangoapps/courseware/module_render.py

@@ -1037,7 +1037,7 @@ def _invoke_xblock_handler(request, course_id, usage_id, handler, suffix, course
         # New Relic. The suffix is necessary for XModule handlers because the
         # "handler" in those cases is always just "xmodule_handler".
         nr_tx_name = "{}.{}".format(instance.__class__.__name__, handler)
-        nr_tx_name += "/{}".format(suffix) if suffix else ""
+        nr_tx_name += "/{}".format(suffix) if (suffix and handler == "xmodule_handler") else ""
         newrelic.agent.set_transaction_name(nr_tx_name, group="Python/XBlock/Handler")
 
         tracking_context_name = 'module_callback_handler'

lms/templates/dashboard.html

@@ -87,6 +87,7 @@ import json
       % for dashboard_index, enrollment in enumerate(course_enrollments):
         <% show_courseware_link = (enrollment.course_id in show_courseware_links_for) %>
         <% cert_status = cert_statuses.get(enrollment.course_id) %>
+        <% can_unenroll = (not cert_status) or cert_status.get('can_unenroll') %>
         <% credit_status = credit_statuses.get(enrollment.course_id) %>
         <% show_email_settings = (enrollment.course_id in show_email_settings_for) %>
         <% course_mode_info = all_course_modes.get(enrollment.course_id) %>
@@ -96,7 +97,7 @@ import json
         <% course_verification_status = verification_status_by_course.get(enrollment.course_id, {}) %>
         <% course_requirements = courses_requirements_not_met.get(enrollment.course_id) %>
         <% course_program_info = course_programs.get(unicode(enrollment.course_id)) %>
-        <%include file = 'dashboard/_dashboard_course_listing.html' args="course_overview=enrollment.course_overview, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option=show_refund_option, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, course_program_info=course_program_info" />
+        <%include file = 'dashboard/_dashboard_course_listing.html' args="course_overview=enrollment.course_overview, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option=show_refund_option, is_paid_course=is_paid_course, is_course_blocked=is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, course_program_info=course_program_info" />
       % endfor
 
       </ul>

lms/templates/dashboard/_dashboard_course_listing.html

-<%page args="course_overview, enrollment, show_courseware_link, cert_status, credit_status, show_email_settings, course_mode_info, show_refund_option, is_paid_course, is_course_blocked, verification_status, course_requirements, dashboard_index, share_settings, course_program_info" />
+<%page args="course_overview, enrollment, show_courseware_link, cert_status, can_unenroll, credit_status, show_email_settings, course_mode_info, show_refund_option, is_paid_course, is_course_blocked, verification_status, course_requirements, dashboard_index, share_settings, course_program_info" />
 
 <%!
 import urllib
@@ -178,7 +178,7 @@ from student.helpers import (
               </a>
               <div class="actions-dropdown" id="actions-dropdown-${dashboard_index}" aria-label="${_('Additional Actions Menu')}">
                 <ul class="actions-dropdown-list" id="actions-dropdown-list-${dashboard_index}" aria-label="${_('Available Actions')}" role="menu">
-                  % if cert_status.get('status') not in DISABLE_UNENROLL_CERT_STATES:
+                  % if can_unenroll:
                     <li class="actions-item" id="actions-item-unenroll-${dashboard_index}">
                     % if is_paid_course and show_refund_option:
                       % if not is_course_blocked:

openedx/core/djangoapps/course_groups/management/commands/post_cohort_membership_fix.py

+"""
+Intended to fix any inconsistencies that may arise during the rollout of the CohortMembership model.
+Illustration: https://gist.github.com/efischer19/d62f8ee42b7fbfbc6c9a
+"""
+from django.core.management.base import BaseCommand
+from django.db import IntegrityError
+
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup, CohortMembership
+
+
+class Command(BaseCommand):
+    """
+    Repair any inconsistencies between CourseUserGroup and CohortMembership. To be run after migration 0006.
+    """
+    help = '''
+    Repairs any potential inconsistencies made in the window between running migrations 0005 and 0006, and deploying
+    the code changes to enforce use of CohortMembership that go with said migrations.
+    |commit|: optional argument. If not provided, will dry-run and list number of operations that would be made.
+    '''
+
+    def handle(self, *args, **options):
+        """
+        Execute the command. Since this is designed to fix any issues cause by running pre-CohortMembership code
+        with the database already migrated to post-CohortMembership state, we will use the pre-CohortMembership
+        table CourseUserGroup as the canonical source of truth. This way, changes made in the window are persisted.
+        """
+        commit = False
+        if len(args) == 1:
+            commit = args[0] == 'commit'
+
+        memberships_to_delete = 0
+        memberships_to_add = 0
+
+        # Begin by removing any data in CohortMemberships that does not match CourseUserGroups data
+        for membership in CohortMembership.objects.all():
+            try:
+                CourseUserGroup.objects.get(
+                    group_type=CourseUserGroup.COHORT,
+                    users__id=membership.user.id,
+                    course_id=membership.course_id,
+                    id=membership.course_user_group.id
+                )
+            except CourseUserGroup.DoesNotExist:
+                memberships_to_delete += 1
+                if commit:
+                    membership.delete()
+
+        # Now we can add any CourseUserGroup data that is missing a backing CohortMembership
+        for course_group in CourseUserGroup.objects.filter(group_type=CourseUserGroup.COHORT):
+            for user in course_group.users.all():
+                try:
+                    CohortMembership.objects.get(
+                        user=user,
+                        course_id=course_group.course_id,
+                        course_user_group_id=course_group.id
+                    )
+                except CohortMembership.DoesNotExist:
+                    memberships_to_add += 1
+                    if commit:
+                        membership = CohortMembership(
+                            course_user_group=course_group,
+                            user=user,
+                            course_id=course_group.course_id
+                        )
+                        try:
+                            membership.save()
+                        except IntegrityError:  # If the user is in multiple cohorts, we arbitrarily choose between them
+                            # In this case, allow the pre-existing entry to be "correct"
+                            course_group.users.remove(user)
+                            user.course_groups.remove(course_group)
+
+        print '{} CohortMemberships did not match the CourseUserGroup table and will be deleted'.format(
+            memberships_to_delete
+        )
+        print '{} CourseUserGroup users do not have a CohortMembership; one will be added if it is valid'.format(
+            memberships_to_add
+        )
+        if commit:
+            print 'Changes have been made and saved.'
+        else:
+            print 'Dry run, changes have not been saved. Run again with "commit" argument to save changes'

openedx/core/djangoapps/course_groups/management/commands/remove_users_from_multiple_cohorts.py

-"""
-Script for removing users with multiple cohorts of a course from cohorts
-to ensure user's uniqueness for a course cohorts
-"""
-from django.contrib.auth.models import User
-from django.core.management.base import BaseCommand
-from django.db.models import Count
-
-from openedx.core.djangoapps.course_groups.models import CourseUserGroup
-
-
-class Command(BaseCommand):
-    """
-    Remove users with multiple cohorts of a course from all cohorts
-    """
-    help = 'Remove all users from multiple cohorts (except one) of each course'
-
-    def handle(self, *args, **options):
-        """
-        Execute the command
-        """
-        # Get entries of cohorts which have same user added multiple times for a single course
-        multiple_objects_cohorts = CourseUserGroup.objects.filter(group_type=CourseUserGroup.COHORT).\
-            values_list('users', 'course_id').annotate(user_count=Count('users')).filter(user_count__gt=1).\
-            order_by('users')
-        multiple_objects_cohorts_count = multiple_objects_cohorts.count()
-        multiple_course_cohorts_users = set(multiple_objects_cohorts.values_list('users', flat=True))
-        users_failed_to_cleanup = []
-
-        for user in User.objects.filter(id__in=multiple_course_cohorts_users):
-            print u"Removing user with id '{0}' from cohort groups".format(user.id)
-            try:
-                # remove user from only cohorts
-                user.course_groups.remove(*user.course_groups.filter(group_type=CourseUserGroup.COHORT))
-            except AttributeError as err:
-                users_failed_to_cleanup.append(user.email)
-                print u"Failed to remove user with id {0} from cohort groups, error: {1}".format(user.id, err)
-
-        print "=" * 80
-        print u"=" * 30 + u"> Cohorts summary"
-        print(
-            u"Total number of CourseUserGroup of type '{0}' with multiple users: {1}".format(
-                CourseUserGroup.COHORT, multiple_objects_cohorts_count
-            )
-        )
-        print(
-            u"Total number of unique users with multiple course cohorts: {0}".format(
-                len(multiple_course_cohorts_users)
-            )
-        )
-        print(
-            u"Users which failed on cohorts cleanup [{0}]: [{1}]".format(
-                len(users_failed_to_cleanup), (', '.join(users_failed_to_cleanup))
-            )
-        )
-        print "=" * 80

openedx/core/djangoapps/course_groups/management/commands/tests/test_post_cohort_membership_fix.py

+"""
+Test for the post-migration fix commands that are included with this djangoapp
+"""
+from django.core.management import call_command
+from django.test.client import RequestFactory
+
+from openedx.core.djangoapps.course_groups.views import cohort_handler
+from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_name
+from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts
+from openedx.core.djangoapps.course_groups.models import CohortMembership
+from student.tests.factories import UserFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+
+
+class TestPostMigrationFix(ModuleStoreTestCase):
+    """
+    Base class for testing post-migration fix commands
+    """
+    def setUp(self):
+        """
+        setup course, user and request for tests
+        """
+        super(TestPostMigrationFix, self).setUp()
+        self.course1 = CourseFactory.create()
+        self.course2 = CourseFactory.create()
+        self.user1 = UserFactory(is_staff=True)
+        self.user2 = UserFactory(is_staff=True)
+        self.request = RequestFactory().get("dummy_url")
+        self.request.user = self.user1
+
+    def test_post_cohortmembership_fix(self):
+        """
+        Test that changes made *after* migration, but *before* turning on new code are handled properly
+        """
+        # First, we're going to simulate some problem states that can arise during this window
+        config_course_cohorts(self.course1, is_cohorted=True, auto_cohorts=["Course1AutoGroup1", "Course1AutoGroup2"])
+
+        # Get the cohorts from the courses, which will cause auto cohorts to be created
+        cohort_handler(self.request, unicode(self.course1.id))
+        course_1_auto_cohort_1 = get_cohort_by_name(self.course1.id, "Course1AutoGroup1")
+        course_1_auto_cohort_2 = get_cohort_by_name(self.course1.id, "Course1AutoGroup2")
+
+        # When migrations were first run, the users were assigned to CohortMemberships correctly
+        membership1 = CohortMembership(
+            course_id=course_1_auto_cohort_1.course_id,
+            user=self.user1,
+            course_user_group=course_1_auto_cohort_1
+        )
+        membership1.save()
+        membership2 = CohortMembership(
+            course_id=course_1_auto_cohort_1.course_id,
+            user=self.user2,
+            course_user_group=course_1_auto_cohort_1
+        )
+        membership2.save()
+
+        # But before CohortMembership code was turned on, some changes were made:
+        course_1_auto_cohort_2.users.add(self.user1)  # user1 is now in 2 cohorts in the same course!
+        course_1_auto_cohort_2.users.add(self.user2)
+        course_1_auto_cohort_1.users.remove(self.user2)  # and user2 was moved, but no one told CohortMembership!
+
+        # run the post-CohortMembership command, dry-run
+        call_command('post_cohort_membership_fix')
+
+        # Verify nothing was changed in dry-run mode.
+        self.assertEqual(self.user1.course_groups.count(), 2)  # CourseUserGroup has 2 entries for user1
+
+        self.assertEqual(CohortMembership.objects.get(user=self.user2).course_user_group.name, 'Course1AutoGroup1')
+        user2_cohorts = list(self.user2.course_groups.values_list('name', flat=True))
+        self.assertEqual(user2_cohorts, ['Course1AutoGroup2'])  # CourseUserGroup and CohortMembership disagree
+
+        # run the post-CohortMembership command, and commit it
+        call_command('post_cohort_membership_fix', 'commit')
+
+        # verify that both databases agree about the (corrected) state of the memberships
+        self.assertEqual(self.user1.course_groups.count(), 1)
+        self.assertEqual(CohortMembership.objects.filter(user=self.user1).count(), 1)
+
+        self.assertEqual(self.user2.course_groups.count(), 1)
+        self.assertEqual(CohortMembership.objects.filter(user=self.user2).count(), 1)
+        self.assertEqual(CohortMembership.objects.get(user=self.user2).course_user_group.name, 'Course1AutoGroup2')
+        user2_cohorts = list(self.user2.course_groups.values_list('name', flat=True))
+        self.assertEqual(user2_cohorts, ['Course1AutoGroup2'])

openedx/core/djangoapps/course_groups/management/commands/tests/test_remove_users_from_multiple_cohorts.py

-"""
-Tests for cleanup of users which are added in multiple cohorts of a course
-"""
-from django.core.exceptions import MultipleObjectsReturned
-from django.core.management import call_command
-from django.test.client import RequestFactory
-
-from openedx.core.djangoapps.course_groups.views import cohort_handler
-from openedx.core.djangoapps.course_groups.cohorts import get_cohort, get_cohort_by_name
-from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts
-from student.tests.factories import UserFactory
-from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.modulestore.tests.factories import CourseFactory
-
-
-class TestMultipleCohortUsers(ModuleStoreTestCase):
-    """
-    Base class for testing users with multiple cohorts
-    """
-    def setUp(self):
-        """
-        setup course, user and request for tests
-        """
-        super(TestMultipleCohortUsers, self).setUp()
-        self.course1 = CourseFactory.create()
-        self.course2 = CourseFactory.create()
-        self.user1 = UserFactory(is_staff=True)
-        self.user2 = UserFactory(is_staff=True)
-        self.request = RequestFactory().get("dummy_url")
-        self.request.user = self.user1
-
-    def test_users_with_multiple_cohorts_cleanup(self):
-        """
-        Test that user which have been added in multiple cohorts of a course,
-        can get cohorts without error after running cohorts cleanup command
-        """
-        # set two auto_cohort_groups for both courses
-        config_course_cohorts(
-            self.course1, is_cohorted=True, auto_cohorts=["Course1AutoGroup1", "Course1AutoGroup2"]
-        )
-        config_course_cohorts(
-            self.course2, is_cohorted=True, auto_cohorts=["Course2AutoGroup1", "Course2AutoGroup2"]
-        )
-
-        # get the cohorts from the courses, which will cause auto cohorts to be created
-        cohort_handler(self.request, unicode(self.course1.id))
-        cohort_handler(self.request, unicode(self.course2.id))
-        course_1_auto_cohort_1 = get_cohort_by_name(self.course1.id, "Course1AutoGroup1")
-        course_1_auto_cohort_2 = get_cohort_by_name(self.course1.id, "Course1AutoGroup2")
-        course_2_auto_cohort_1 = get_cohort_by_name(self.course2.id, "Course2AutoGroup1")
-
-        # forcefully add user1 in two auto cohorts
-        course_1_auto_cohort_1.users.add(self.user1)
-        course_1_auto_cohort_2.users.add(self.user1)
-        # forcefully add user2 in auto cohorts of both courses
-        course_1_auto_cohort_1.users.add(self.user2)
-        course_2_auto_cohort_1.users.add(self.user2)
-
-        # now check that when user1 goes on discussion page and tries to get
-        # cohorts 'MultipleObjectsReturned' exception is returned
-        with self.assertRaises(MultipleObjectsReturned):
-            get_cohort(self.user1, self.course1.id)
-        # also check that user 2 can go on discussion page of both courses
-        # without any exception
-        get_cohort(self.user2, self.course1.id)
-        get_cohort(self.user2, self.course2.id)
-
-        # call command to remove users added in multiple cohorts of a course
-        # are removed from all cohort groups
-        call_command('remove_users_from_multiple_cohorts')
-
-        # check that only user1 (with multiple cohorts) is removed from cohorts
-        # and user2 is still in auto cohorts of both course after running
-        # 'remove_users_from_multiple_cohorts' management command
-        self.assertEqual(self.user1.course_groups.count(), 0)
-        self.assertEqual(self.user2.course_groups.count(), 2)
-        user2_cohorts = list(self.user2.course_groups.values_list('name', flat=True))
-        self.assertEqual(user2_cohorts, ['Course1AutoGroup1', 'Course2AutoGroup1'])
-
-        # now check that user1 can get cohorts in which he is added
-        response = cohort_handler(self.request, unicode(self.course1.id))
-        self.assertEqual(response.status_code, 200)

openedx/core/djangoapps/course_groups/migrations/rerun_0006.sh

-#!/bin/bash
-if [ $# -eq 0 ]; then
-    echo "$0: usage: rerun_0006.sh <arguments>. At minimum, '--settings=<environment>' is expected."
-    exit 1
-fi
-
-./manage.py lms migrate course_groups 0005 --fake "$@"
-./manage.py lms migrate course_groups 0006 "$@"