From 2325b3af7ab4792cb703e9e1998178ae18a0f937 Mon Sep 17 00:00:00 2001 From: Zachary Hancock Date: Fri, 13 Mar 2020 12:55:58 -0400 Subject: [PATCH] history on bulk create of enrollments (#23389) fixes bug where bulk creation of enrollments caused no new history records to be created --- .../program_enrollments/api/tests/test_writing.py | 6 +++++- lms/djangoapps/program_enrollments/api/writing.py | 11 ++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/program_enrollments/api/tests/test_writing.py b/lms/djangoapps/program_enrollments/api/tests/test_writing.py index 83883016ee..1e779e7c3b 100644 --- a/lms/djangoapps/program_enrollments/api/tests/test_writing.py +++ b/lms/djangoapps/program_enrollments/api/tests/test_writing.py @@ -55,19 +55,23 @@ class WritingProgramEnrollmentTest(CacheIsolationTestCase): def test_write_program_enrollments_status_ended(self): """ - Successfully updates program enrollment to status ended if requested + Successfully updates program enrollment to status ended if requested. + This also validates history records are created on both create and update. """ assert ProgramEnrollment.objects.count() == 0 + assert ProgramEnrollment.historical_records.count() == 0 # pylint: disable=no-member write_program_enrollments(self.program_uuid_x, [{ 'external_user_key': self.user_0, 'status': PEStatuses.PENDING, 'curriculum_uuid': self.curriculum_uuid_a, }], True, False) assert ProgramEnrollment.objects.count() == 1 + assert ProgramEnrollment.historical_records.count() == 1 # pylint: disable=no-member write_program_enrollments(self.program_uuid_x, [{ 'external_user_key': self.user_0, 'status': PEStatuses.ENDED, 'curriculum_uuid': self.curriculum_uuid_a, }], False, True) assert ProgramEnrollment.objects.count() == 1 + assert ProgramEnrollment.historical_records.count() == 2 # pylint: disable=no-member assert ProgramEnrollment.objects.filter(status=PEStatuses.ENDED).exists() diff --git a/lms/djangoapps/program_enrollments/api/writing.py b/lms/djangoapps/program_enrollments/api/writing.py index 323bad5ba8..fb8ec2d04c 100644 --- a/lms/djangoapps/program_enrollments/api/writing.py +++ b/lms/djangoapps/program_enrollments/api/writing.py @@ -8,6 +8,8 @@ from `lms.djangoapps.program_enrollments.api`. import logging +from simple_history.utils import bulk_create_with_history + from course_modes.models import CourseMode from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment, NonExistentCourseError @@ -67,7 +69,6 @@ def write_program_enrollments(program_uuid, enrollment_requests, create, update) # For each enrollment request, try to create/update: # * For creates, build up list `to_save`, which we will bulk-create afterwards. # * For updates, do them in place. - # (TODO: Django 2.2 will add bulk-update support, which we could use here) # Update `results` with the new status or an error status for each operation. results = {} to_save = [] @@ -100,11 +101,11 @@ def write_program_enrollments(program_uuid, enrollment_requests, create, update) to_save.append(new_enrollment) results[external_key] = new_enrollment.status - # Bulk-create all new program enrollments. + # Bulk-create all new program enrollments and corresponding history records # Note: this will NOT invoke `save()` or `pre_save`/`post_save` signals! # See https://docs.djangoproject.com/en/1.11/ref/models/querysets/#bulk-create. if to_save: - ProgramEnrollment.objects.bulk_create(to_save) + bulk_create_with_history(to_save, ProgramEnrollment) results.update({key: ProgramOpStatuses.DUPLICATED for key in duplicated_keys}) return results @@ -274,11 +275,11 @@ def write_program_course_enrollments( to_save.append(new_course_enrollment) results[external_key] = new_course_enrollment.status - # Bulk-create all new program-course enrollments. + # Bulk-create all new program-course enrollments and corresponding history records. # Note: this will NOT invoke `save()` or `pre_save`/`post_save` signals! # See https://docs.djangoproject.com/en/1.11/ref/models/querysets/#bulk-create. if to_save: - ProgramCourseEnrollment.objects.bulk_create(to_save) + bulk_create_with_history(to_save, ProgramCourseEnrollment) return results