Merge pull request openedx#20653 from edx/zhancock/complete-waiting-e…

…nrollments

complete waiting enrollments. fixed

common/djangoapps/third_party_auth/migrations/0024_fix_edit_disallowed.py

@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.20 on 2019-05-20 20:13
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('third_party_auth', '0023_auto_20190418_2033'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='ltiproviderconfig',
+            name='organization',
+            field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'),
+        ),
+        migrations.AlterField(
+            model_name='oauth2providerconfig',
+            name='organization',
+            field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'),
+        ),
+        migrations.AlterField(
+            model_name='samlproviderconfig',
+            name='organization',
+            field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'),
+        ),
+    ]

common/djangoapps/third_party_auth/models.py

@@ -126,7 +126,7 @@ class ProviderConfig(ConfigurationModel):
             'in a separate list of "Institution" login providers.'
         ),
     )
-    organization = models.OneToOneField(
+    organization = models.ForeignKey(
         Organization,
         blank=True,
         null=True,

lms/djangoapps/program_enrollments/api/v1/tests/test_views.py

@@ -20,6 +20,7 @@
 from six.moves import range, zip
 
 from bulk_email.models import BulkEmailFlag, Optout
+from course_modes.models import CourseMode
 from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory
 from lms.djangoapps.certificates.models import CertificateStatuses
 from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory
@@ -29,6 +30,7 @@
     MAX_ENROLLMENT_RECORDS,
     REQUEST_STUDENT_KEY,
 )
+from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory
 from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment
 from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL
 from openedx.core.djangoapps.catalog.tests.factories import CourseFactory
@@ -40,7 +42,6 @@
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
 from xmodule.modulestore.tests.factories import CourseFactory as ModulestoreCourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
-from .factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory
 
 
 class ListViewTestMixin(object):
@@ -357,6 +358,7 @@ def create_program_course_enrollment(self, program_enrollment, course_status='ac
             course_enrollment = CourseEnrollmentFactory.create(
                 course_id=self.course_key,
                 user=program_enrollment.user,
+                mode=CourseMode.MASTERS
             )
             course_enrollment.is_active = course_status == "active"
             course_enrollment.save()
@@ -371,7 +373,7 @@ def create_program_and_course_enrollments(self, external_user_key, user=False, c
         program_enrollment = self.create_program_enrollment(external_user_key, user)
         return self.create_program_course_enrollment(program_enrollment, course_status=course_status)
 
-    def assert_program_course_enrollment(self, external_user_key, expected_status, has_user):
+    def assert_program_course_enrollment(self, external_user_key, expected_status, has_user, mode=CourseMode.MASTERS):
         """
         Convenience method to assert that a ProgramCourseEnrollment exists,
         and potentially that a CourseEnrollment also exists
@@ -387,6 +389,7 @@ def assert_program_course_enrollment(self, external_user_key, expected_status, h
             self.assertIsNotNone(course_enrollment)
             self.assertEqual(expected_status == "active", course_enrollment.is_active)
             self.assertEqual(self.course_key, course_enrollment.course_id)
+            self.assertEqual(mode, course_enrollment.mode)
         else:
             self.assertIsNone(course_enrollment)
 
@@ -499,13 +502,44 @@ def test_create_enrollments(self):
         self.assert_program_course_enrollment("learner-3", "active", False)
         self.assert_program_course_enrollment("learner-4", "inactive", False)
 
-    def test_user_already_enrolled_in_course(self):
+    def test_program_course_enrollment_exists(self):
+        """
+        The program enrollments application already has a program_course_enrollment
+        record for this user and course
+        """
         self.create_program_and_course_enrollments('learner-1')
         post_data = [self.learner_enrollment("learner-1")]
         response = self.request(self.default_url, post_data)
         self.assertEqual(422, response.status_code)
         self.assertDictEqual({'learner-1': CourseStatuses.CONFLICT}, response.data)
 
+    def test_user_currently_enrolled_in_course(self):
+        """
+        If a user is already enrolled in a course through a different method
+        that enrollment should be linked but not overwritten as masters.
+        """
+        CourseEnrollmentFactory.create(
+            course_id=self.course_key,
+            user=self.student,
+            mode=CourseMode.VERIFIED
+        )
+
+        self.create_program_enrollment('learner-1', user=self.student)
+
+        post_data = [
+            self.learner_enrollment("learner-1", "active")
+        ]
+        response = self.request(self.default_url, post_data)
+
+        self.assertEqual(200, response.status_code)
+        self.assertDictEqual(
+            {
+                "learner-1": "active"
+            },
+            response.data
+        )
+        self.assert_program_course_enrollment("learner-1", "active", True, mode=CourseMode.VERIFIED)
+
     def test_207_multistatus(self):
         self.create_program_enrollment('learner-1')
         post_data = [self.learner_enrollment("learner-1"), self.learner_enrollment("learner-2")]

lms/djangoapps/program_enrollments/api/v1/views.py

@@ -758,7 +758,8 @@ def enroll_learner_in_course(self, enrollment_request, program_enrollment, progr
         """
         if program_course_enrollment:
             return CourseEnrollmentResponseStatuses.CONFLICT
-        return ProgramCourseEnrollment.enroll(
+
+        return ProgramCourseEnrollment.create_program_course_enrollment(
             program_enrollment,
             self.course_key,
             enrollment_request['status']

lms/djangoapps/program_enrollments/models.py

@@ -10,11 +10,13 @@
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 from course_modes.models import CourseMode
-from lms.djangoapps.program_enrollments.api.v1.constants import CourseEnrollmentResponseStatuses
+from lms.djangoapps.program_enrollments.api.v1.constants import (
+    CourseEnrollmentResponseStatuses as ProgramCourseEnrollmentResponseStatuses
+)
 from model_utils.models import TimeStampedModel
 from opaque_keys.edx.django.models import CourseKeyField
 from simple_history.models import HistoricalRecords
-from student.models import CourseEnrollment as StudentCourseEnrollment
+from student.models import AlreadyEnrolledError, CourseEnrollment
 
 logger = logging.getLogger(__name__)  # pylint: disable=invalid-name
 
@@ -131,7 +133,7 @@ class Meta(object):
         related_name="program_course_enrollments"
     )
     course_enrollment = models.OneToOneField(
-        StudentCourseEnrollment,
+        CourseEnrollment,
         null=True,
         blank=True,
     )
@@ -143,27 +145,19 @@ def __str__(self):
         return '[ProgramCourseEnrollment id={}]'.format(self.id)
 
     @classmethod
-    def enroll(cls, program_enrollment, course_key, status):
+    def create_program_course_enrollment(cls, program_enrollment, course_key, status):
         """
         Create ProgramCourseEnrollment for the given course and program enrollment
         """
-        course_enrollment = None
-        if program_enrollment.user:
-            course_enrollment = StudentCourseEnrollment.enroll(
-                program_enrollment.user,
-                course_key,
-                mode=CourseMode.MASTERS,
-                check_access=True,
-            )
-            if status == CourseEnrollmentResponseStatuses.INACTIVE:
-                course_enrollment.deactivate()
-
         program_course_enrollment = ProgramCourseEnrollment.objects.create(
             program_enrollment=program_enrollment,
-            course_enrollment=course_enrollment,
             course_key=course_key,
             status=status,
         )
+
+        if program_enrollment.user:
+            program_course_enrollment.enroll(program_enrollment.user)
+
         return program_course_enrollment.status
 
     def change_status(self, status):
@@ -175,18 +169,18 @@ def change_status(self, status):
 
         self.status = status
         if self.course_enrollment:
-            if status == CourseEnrollmentResponseStatuses.ACTIVE:
+            if status == ProgramCourseEnrollmentResponseStatuses.ACTIVE:
                 self.course_enrollment.activate()
-            elif status == CourseEnrollmentResponseStatuses.INACTIVE:
+            elif status == ProgramCourseEnrollmentResponseStatuses.INACTIVE:
                 self.course_enrollment.deactivate()
             else:
                 message = ("Changed {enrollment} status to {status}, not changing course_enrollment"
                            " status because status is not '{active}' or '{inactive}'")
                 logger.warn(message.format(
                     enrollment=self,
                     status=status,
-                    active=CourseEnrollmentResponseStatuses.ACTIVE,
-                    inactive=CourseEnrollmentResponseStatuses.INACTIVE
+                    active=ProgramCourseEnrollmentResponseStatuses.ACTIVE,
+                    inactive=ProgramCourseEnrollmentResponseStatuses.INACTIVE
                 ))
         elif self.program_enrollment.user:
             logger.warn("User {user} {program_enrollment} {course_key} has no course_enrollment".format(
@@ -196,3 +190,30 @@ def change_status(self, status):
             ))
         self.save()
         return self.status
+
+    def enroll(self, user):
+        """
+        Create a CourseEnrollment to enroll user in course
+        """
+        try:
+            self.course_enrollment = CourseEnrollment.enroll(
+                user,
+                self.course_key,
+                mode=CourseMode.MASTERS,
+                check_access=True,
+            )
+        except AlreadyEnrolledError:
+            course_enrollment = CourseEnrollment.objects.get(
+                user=user,
+                course_id=self.course_key,
+            )
+            if course_enrollment.mode == CourseMode.AUDIT or course_enrollment.mode == CourseMode.HONOR:
+                course_enrollment.mode = CourseMode.MASTERS
+                course_enrollment.save()
+            self.course_enrollment = course_enrollment
+            message = ("Attempted to create course enrollment for user={user} and course={course}"
+                       " but an enrollment already exists. Existing enrollment will be used instead")
+            logger.info(message.format(user=user.id, course=self.course_key))
+        if self.status == ProgramCourseEnrollmentResponseStatuses.INACTIVE:
+            self.course_enrollment.deactivate()
+        self.save()

lms/djangoapps/program_enrollments/signals.py

@@ -3,10 +3,17 @@
 """
 from __future__ import absolute_import
 
-from django.dispatch.dispatcher import receiver
-
-from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC
+import logging
+from django.db.models.signals import post_save
+from django.dispatch import receiver
+from social_django.models import UserSocialAuth
 from lms.djangoapps.program_enrollments.models import ProgramEnrollment
+from openedx.core.djangoapps.catalog.utils import get_programs
+from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC
+from student.models import CourseEnrollmentException
+from third_party_auth.models import SAMLProviderConfig
+
+logger = logging.getLogger(__name__)
 
 
 @receiver(USER_RETIRE_LMS_MISC)
@@ -16,3 +23,69 @@ def _listen_for_lms_retire(sender, **kwargs):  # pylint: disable=unused-argument
     """
     user = kwargs.get('user')
     ProgramEnrollment.retire_user(user.id)
+
+
+@receiver(post_save, sender=UserSocialAuth)
+def matriculate_learner(sender, instance, created, **kwargs):  # pylint: disable=unused-argument
+    """
+    Post-save signal to update any waiting program enrollments with a user,
+    and enroll the user in any waiting course enrollments.
+
+    In most cases this will just short-circuit. Enrollments will only be updated
+    for a SAML provider with a linked organization.
+    """
+    if not created:
+        return
+
+    try:
+        user = instance.user
+        provider_slug, external_user_key = instance.uid.split(':')
+        authorizing_org = SAMLProviderConfig.objects.current_set().get(slug=provider_slug).organization
+
+        if not authorizing_org:
+            return
+    except (AttributeError, ValueError):
+        logger.info(u'Ignoring non-saml social auth entry for user=%s', user.id)
+        return
+    except SAMLProviderConfig.DoesNotExist:
+        logger.warning(u'Got incoming social auth for provider=%s but no such provider exists', provider_slug)
+        return
+    except SAMLProviderConfig.MultipleObjectsReturned:
+        logger.warning(
+            u'Unable to activate waiting enrollments for user=%s.'
+            u'  Multiple active SAML configurations found for slug=%s. Expected one.',
+            user.id,
+            provider_slug)
+        return
+
+    incomplete_enrollments = ProgramEnrollment.objects.filter(
+        external_user_key=external_user_key,
+        user=None,
+    ).prefetch_related('program_course_enrollments')
+
+    for enrollment in incomplete_enrollments:
+        try:
+            enrollment_org = get_programs(uuid=enrollment.program_uuid)['authoring_organizations'][0]
+            if enrollment_org['key'] != authorizing_org.short_name:
+                continue
+        except (KeyError, TypeError):
+            logger.warning(
+                u'Failed to complete waiting enrollments for organization=%s.'
+                u' No catalog programs with matching authoring_organization exist.',
+                authorizing_org.short_name
+            )
+            continue
+
+        enrollment.user = user
+        enrollment.save()
+        for program_course_enrollment in enrollment.program_course_enrollments.all():
+            try:
+                program_course_enrollment.enroll(user)
+            except CourseEnrollmentException as e:
+                logger.warning(
+                    u'Failed to enroll user=%s with waiting program_course_enrollment=%s: %s',
+                    user.id,
+                    program_course_enrollment.id,
+                    e,
+                )
+                raise e