From 2fd3592586ff2f54ca17c31c4b25b0b5be79c2fe Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 27 Jul 2018 09:26:20 +0930 Subject: [PATCH 1/4] Do not delete course enrollments when the associated CourseOverview gets recreated. --- common/djangoapps/student/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 8a689bb41f..5b325b7514 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1162,7 +1162,7 @@ class CourseEnrollment(models.Model): course = models.ForeignKey( CourseOverview, db_constraint=False, - on_delete=models.CASCADE, + on_delete=models.DO_NOTHING, ) @property From 84eefa0e87dfcbdc495db076dbb35da239b308b8 Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 27 Jul 2018 11:18:32 +0930 Subject: [PATCH 2/4] Adds migration --- ...senrollment_course_on_delete_do_nothing.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 common/djangoapps/student/migrations/0016_coursenrollment_course_on_delete_do_nothing.py diff --git a/common/djangoapps/student/migrations/0016_coursenrollment_course_on_delete_do_nothing.py b/common/djangoapps/student/migrations/0016_coursenrollment_course_on_delete_do_nothing.py new file mode 100644 index 0000000000..cb1cef83c5 --- /dev/null +++ b/common/djangoapps/student/migrations/0016_coursenrollment_course_on_delete_do_nothing.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.14 on 2018-07-27 01:44 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0015_manualenrollmentaudit_add_role'), + ] + + operations = [ + migrations.AlterField( + model_name='courseenrollment', + name='course', + field=models.ForeignKey(db_constraint=False, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview'), + ), + ] From 7a63af9216f1dc37d56b0e7027980ad4bdccb36f Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 27 Jul 2018 09:56:14 +0930 Subject: [PATCH 3/4] Add a test --- .../djangoapps/student/tests/test_models.py | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 4f7134b81c..1ea2ab3df0 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -207,6 +207,43 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): self.assertIsNotNone(enrollment.schedule) self.assertIsNone(enrollment.upgrade_deadline) + @skip_unless_lms + # NOTE: We mute the post_save signal to prevent Schedules from being created for new enrollments + @factory.django.mute_signals(signals.post_save) + def test_enrollments_not_deleted(self): + """ Recreating a CourseOverview with an outdated version should not delete the associated enrollment. """ + course = CourseFactory(self_paced=True) + CourseModeFactory( + course_id=course.id, + mode_slug=CourseMode.VERIFIED, + # This must be in the future to ensure it is returned by downstream code. + expiration_datetime=datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30), + ) + + # Create a CourseOverview with an outdated version + course_overview = CourseOverview.load_from_module_store(course.id) + course_overview.version = CourseOverview.VERSION - 1 + course_overview.save() + + # Create an inactive enrollment with this course overview + enrollment = CourseEnrollmentFactory( + user=self.user, + course_id=course.id, + mode=CourseMode.AUDIT, + course=course_overview, + ) + + # Re-fetch the CourseOverview record, which as a side effect, + # will recreate the record to update the version. + course_overview_new = CourseOverview.get_from_id(course.id) + assert(course_overview_new.version == CourseOverview.VERSION) + assert(course_overview_new.id != course_overview.id) + + # Ensure that the enrollment record was not deleted during this re-creation + enrollment_refetched = CourseEnrollment.objects.filter(id=enrollment.id) + assert(enrollment_refetched.exists()) + assert(enrollment_refetched.all()[0] == enrollment) + class PendingNameChangeTests(SharedModuleStoreTestCase): """ From 2667c14e1907c5c9619121183ed262287d20e06a Mon Sep 17 00:00:00 2001 From: Jillian Vogel Date: Fri, 27 Jul 2018 13:50:06 +0930 Subject: [PATCH 4/4] Fix failing test and use the self.assert* convention used by the other tests --- common/djangoapps/student/tests/test_models.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/student/tests/test_models.py b/common/djangoapps/student/tests/test_models.py index 1ea2ab3df0..6b34bb7e5e 100644 --- a/common/djangoapps/student/tests/test_models.py +++ b/common/djangoapps/student/tests/test_models.py @@ -233,16 +233,15 @@ class CourseEnrollmentTests(SharedModuleStoreTestCase): course=course_overview, ) - # Re-fetch the CourseOverview record, which as a side effect, - # will recreate the record to update the version. + # Re-fetch the CourseOverview record. + # As a side effect, this will recreate the record, and update the version. course_overview_new = CourseOverview.get_from_id(course.id) - assert(course_overview_new.version == CourseOverview.VERSION) - assert(course_overview_new.id != course_overview.id) + self.assertEqual(course_overview_new.version, CourseOverview.VERSION) - # Ensure that the enrollment record was not deleted during this re-creation + # Ensure that the enrollment record was unchanged during this re-creation enrollment_refetched = CourseEnrollment.objects.filter(id=enrollment.id) - assert(enrollment_refetched.exists()) - assert(enrollment_refetched.all()[0] == enrollment) + self.assertTrue(enrollment_refetched.exists()) + self.assertEqual(enrollment_refetched.all()[0], enrollment) class PendingNameChangeTests(SharedModuleStoreTestCase):