From 2c572e1b59008ec03a1e0ac36d5192ebeffa03f3 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Tue, 27 Aug 2019 15:05:30 -0400 Subject: [PATCH] Add new ProgramCourseEnrollment uniqueness constriant (#21463) ProgramCourseEnrollments were already unique on (program_enrollment, course_enrollment) by nature of the OneToOneField on course_enrollment. However, this only affects realized enrollments. For waiting enrollments, we need to add a uniqueness constraint on (program_enrollment, course_key). --- ...ting_programcourseenrollment_constraint.py | 19 ++++++++ lms/djangoapps/program_enrollments/models.py | 11 +++++ .../program_enrollments/tests/factories.py | 13 +++++- .../program_enrollments/tests/test_models.py | 31 +++++++++++++ .../program_enrollments/tests/test_tasks.py | 44 +++++++++++++++---- 5 files changed, 108 insertions(+), 10 deletions(-) create mode 100644 lms/djangoapps/program_enrollments/migrations/0007_waiting_programcourseenrollment_constraint.py diff --git a/lms/djangoapps/program_enrollments/migrations/0007_waiting_programcourseenrollment_constraint.py b/lms/djangoapps/program_enrollments/migrations/0007_waiting_programcourseenrollment_constraint.py new file mode 100644 index 0000000000..1b8aa75a34 --- /dev/null +++ b/lms/djangoapps/program_enrollments/migrations/0007_waiting_programcourseenrollment_constraint.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.23 on 2019-08-27 13:11 +from __future__ import unicode_literals + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('program_enrollments', '0006_add_the_correct_constraints'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='programcourseenrollment', + unique_together=set([('program_enrollment', 'course_key')]), + ), + ] diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 4307658c06..08943e4917 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -127,6 +127,17 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin class Meta(object): app_label = "program_enrollments" + # For each program enrollment, there may be only one + # waiting program-course enrollment per course key. + # This same constraint is implicitly enforced for + # completed program-course enrollments by the + # OneToOneField on `course_enrollment`, which mandates that + # there may be at most one program-course enrollment per + # (user, course) pair. + unique_together = ( + ('program_enrollment', 'course_key'), + ) + program_enrollment = models.ForeignKey( ProgramEnrollment, on_delete=models.CASCADE, diff --git a/lms/djangoapps/program_enrollments/tests/factories.py b/lms/djangoapps/program_enrollments/tests/factories.py index ac0c75f36d..f21a053c39 100644 --- a/lms/djangoapps/program_enrollments/tests/factories.py +++ b/lms/djangoapps/program_enrollments/tests/factories.py @@ -25,6 +25,11 @@ class ProgramEnrollmentFactory(DjangoModelFactory): status = 'enrolled' +PROGRAM_COURSE_ENROLLMENT_DEFAULT_COURSE_KEY = ( + CourseKey.from_string("course-v1:edX+DemoX+Demo_Course") +) + + class ProgramCourseEnrollmentFactory(DjangoModelFactory): """ A factory for the ProgramCourseEnrollment model. """ class Meta(object): @@ -32,5 +37,11 @@ class ProgramCourseEnrollmentFactory(DjangoModelFactory): program_enrollment = factory.SubFactory(ProgramEnrollmentFactory) course_enrollment = factory.SubFactory(CourseEnrollmentFactory) - course_key = CourseKey.from_string("course-v1:edX+DemoX+Demo_Course") + course_key = factory.LazyAttribute( + lambda pce: ( + pce.course_enrollment.course_id + if pce.course_enrollment + else PROGRAM_COURSE_ENROLLMENT_DEFAULT_COURSE_KEY + ) + ) status = 'active' diff --git a/lms/djangoapps/program_enrollments/tests/test_models.py b/lms/djangoapps/program_enrollments/tests/test_models.py index 1f1181e2ed..01c6738b00 100644 --- a/lms/djangoapps/program_enrollments/tests/test_models.py +++ b/lms/djangoapps/program_enrollments/tests/test_models.py @@ -155,6 +155,37 @@ class ProgramCourseEnrollmentModelTests(TestCase): self.course_key = CourseKey.from_string(generate_course_run_key()) CourseOverviewFactory(id=self.course_key) + def test_unique_completed_enrollment(self): + """ + A record with the same (program_enrollment, course_enrollment) + cannot be created. + """ + pce = self._create_completed_program_course_enrollment() + with self.assertRaises(IntegrityError): + # Purposefully mis-set the course_key in order to test + # that there is a constraint on + # (program_enrollment, course_enrollment) alone. + ProgramCourseEnrollment.objects.create( + program_enrollment=pce.program_enrollment, + course_key="course-v1:dummy+value+101", + course_enrollment=pce.course_enrollment, + status="inactive", + ) + + def test_unique_waiting_enrollment(self): + """ + A record with the same (program_enrollment, course_key) + cannot be created. + """ + pce = self._create_waiting_program_course_enrollment() + with self.assertRaises(IntegrityError): + ProgramCourseEnrollment.objects.create( + program_enrollment=pce.program_enrollment, + course_key=pce.course_key, + course_enrollment=None, + status="inactive", + ) + def _create_completed_program_course_enrollment(self): """ helper function create program course enrollment """ course_enrollment = CourseEnrollmentFactory.create( diff --git a/lms/djangoapps/program_enrollments/tests/test_tasks.py b/lms/djangoapps/program_enrollments/tests/test_tasks.py index 494d8bcd2a..c0c3dda58d 100644 --- a/lms/djangoapps/program_enrollments/tests/test_tasks.py +++ b/lms/djangoapps/program_enrollments/tests/test_tasks.py @@ -9,36 +9,62 @@ from django.db.models.base import ObjectDoesNotExist from django.test import TestCase from django.utils import timezone from freezegun import freeze_time +from opaque_keys.edx.keys import CourseKey from testfixtures import LogCapture +from course_modes.models import CourseMode from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment from lms.djangoapps.program_enrollments.tasks import expire_waiting_enrollments, log from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory -from student.tests.factories import UserFactory +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from student.tests.factories import CourseEnrollmentFactory, UserFactory class ExpireWaitingEnrollmentsTest(TestCase): """ Test expire_waiting_enrollments task """ - def _setup_enrollments(self, external_user_key, user, created_date): + @classmethod + def setUpClass(cls): + super(cls, ExpireWaitingEnrollmentsTest).setUpClass() + cls.timed_course_key = CourseKey.from_string('course-v1:edX+TestExpire+Timed') + cls.fresh_course_key = CourseKey.from_string('course-v1:edX+TestExpire+Fresh') + CourseOverviewFactory(id=cls.timed_course_key) + CourseOverviewFactory(id=cls.fresh_course_key) + + def _set_up_course_enrollment(self, user, program_enrollment, course_key): + """ helper function to set up a program course enrollment """ + if user: + ProgramCourseEnrollmentFactory( + program_enrollment=program_enrollment, + course_enrollment=CourseEnrollmentFactory( + course_id=course_key, user=user, mode=CourseMode.MASTERS + ) + ) + else: + ProgramCourseEnrollmentFactory( + program_enrollment=program_enrollment, + course_key=course_key, + ) + + def _set_up_enrollments(self, external_user_key, user, created_date): """ helper function to setup enrollments """ with freeze_time(created_date): program_enrollment = ProgramEnrollmentFactory( user=user, external_user_key=external_user_key, ) - ProgramCourseEnrollmentFactory( - program_enrollment=program_enrollment + self._set_up_course_enrollment( + user, program_enrollment, self.timed_course_key ) # additional course enrollment that is always fresh - ProgramCourseEnrollmentFactory( - program_enrollment=program_enrollment + self._set_up_course_enrollment( + user, program_enrollment, self.fresh_course_key ) def test_expire(self): - self._setup_enrollments('student_expired_waiting', None, timezone.now() - timedelta(60)) - self._setup_enrollments('student_waiting', None, timezone.now() - timedelta(59)) - self._setup_enrollments('student_actualized', UserFactory(), timezone.now() - timedelta(90)) + self._set_up_enrollments('student_expired_waiting', None, timezone.now() - timedelta(60)) + self._set_up_enrollments('student_waiting', None, timezone.now() - timedelta(59)) + self._set_up_enrollments('student_actualized', UserFactory(), timezone.now() - timedelta(90)) expired_program_enrollment = ProgramEnrollment.objects.get( external_user_key='student_expired_waiting'