Add the correct UNIQUE constraints to the ProgramEnrollment model.

lms/djangoapps/program_enrollments/docs/decisions/0001-program-enrollment-data-integrity.rst

@@ -0,0 +1,47 @@
+ProgramEnrollment Model Data Integrity
+--------------------------------------
+
+Status
+======
+
+Accepted (circa August 2019)
+
+
+Context
+=======
+
+For the sake of fundamental data integrity, we are introducing 2 unique
+constraints on the ``program_enrollments.ProgramEnrollment`` model.
+
+Decisions
+=========
+
+The unique constraints are on the following column sets:
+
+* ``('user', 'program_uuid', 'curriculum_uuid')``
+* ``('external_user_key', 'program_uuid', 'curriculum_uuid')``
+
+Note that either the ``user`` column or the ``external_user_key`` column may be null.
+In the future, it would be nice to add a validation step at the Django model layer
+that restricts a model instance from having null values for both of these fields.
+
+Consequences
+============
+
+The first constraint supports the cases in which we save program enrollment records
+that don't have any association with an external organization, e.g. our MicroMasters programs.
+Non-realized enrollments, where the ``user`` value is null, are not affected by this constraint.
+
+As for the second constraint , we want to disallow the ability of anyone to register a learner,
+as identified by ``external_user_key``, into the same program and curriculum more than once.
+No enrollment record with a null ``external_user_key`` is affected by this constraint.
+
+Together, these constraints restrict the duplication of learner records in a specific
+program/curriculum, where the learner is identified either by their ``auth.User.id`` or
+some ``external_user_key``.
+
+This constraint set does NOT support the use case of a single ``auth.User`` being enrolled
+in the same program/curriculum with two or more different ``external_user_keys``.  Supporting
+this case leads to problematic situations, e.g. how to decide which of these program enrollment
+records to link to a program-course enrollment?  If needed, we could introduce an additional
+set of models to support this situation.

lms/djangoapps/program_enrollments/migrations/0006_add_the_correct_constraints.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.23 on 2019-08-23 15:37
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('program_enrollments', '0005_canceled_not_withdrawn'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='programenrollment',
+            unique_together=set([('user', 'program_uuid', 'curriculum_uuid'), ('external_user_key', 'program_uuid', 'curriculum_uuid')]),
+        ),
+    ]

lms/djangoapps/program_enrollments/models.py

@@ -39,12 +39,12 @@ class ProgramEnrollment(TimeStampedModel):  # pylint: disable=model-missing-unic
 
     class Meta(object):
         app_label = "program_enrollments"
-        unique_together = ('external_user_key', 'program_uuid', 'curriculum_uuid')
 
         # A student enrolled in a given (program, curriculum) should always
         # have a non-null ``user`` or ``external_user_key`` field (or both).
         unique_together = (
-            ('user', 'external_user_key', 'program_uuid', 'curriculum_uuid'),
+            ('user', 'program_uuid', 'curriculum_uuid'),
+            ('external_user_key', 'program_uuid', 'curriculum_uuid'),
         )
 
     user = models.ForeignKey(

lms/djangoapps/program_enrollments/tests/factories.py

@@ -20,8 +20,8 @@ class ProgramEnrollmentFactory(DjangoModelFactory):
 
     user = factory.SubFactory(UserFactory)
     external_user_key = None
-    program_uuid = uuid4()
-    curriculum_uuid = uuid4()
+    program_uuid = factory.LazyFunction(uuid4)
+    curriculum_uuid = factory.LazyFunction(uuid4)
     status = 'enrolled'
 
 

lms/djangoapps/program_enrollments/tests/test_models.py

@@ -6,6 +6,7 @@ from __future__ import absolute_import, unicode_literals
 from uuid import uuid4
 
 import ddt
+from django.db.utils import IntegrityError
 from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey
 from six.moves import range
@@ -32,14 +33,37 @@ class ProgramEnrollmentModelTests(TestCase):
         self.user = UserFactory.create()
         self.program_uuid = uuid4()
         self.other_program_uuid = uuid4()
+        self.curriculum_uuid = uuid4()
         self.enrollment = ProgramEnrollment.objects.create(
             user=self.user,
             external_user_key='abc',
             program_uuid=self.program_uuid,
-            curriculum_uuid=uuid4(),
+            curriculum_uuid=self.curriculum_uuid,
             status='enrolled'
         )
 
+    def test_unique_external_key_program_curriculum(self):
+        """ A record with the same (external_user_key, program_uuid, curriculum_uuid) cannot be duplicated. """
+        with self.assertRaises(IntegrityError):
+            _ = ProgramEnrollment.objects.create(
+                user=None,
+                external_user_key='abc',
+                program_uuid=self.program_uuid,
+                curriculum_uuid=self.curriculum_uuid,
+                status='pending',
+            )
+
+    def test_unique_user_program_curriculum(self):
+        """ A record with the same (user, program_uuid, curriculum_uuid) cannot be duplicated. """
+        with self.assertRaises(IntegrityError):
+            _ = ProgramEnrollment.objects.create(
+                user=self.user,
+                external_user_key=None,
+                program_uuid=self.program_uuid,
+                curriculum_uuid=self.curriculum_uuid,
+                status='suspended',
+            )
+
     def test_bulk_read_by_student_key(self):
         curriculum_a = uuid4()
         curriculum_b = uuid4()