From eb3e36b82206cc0a408c6b259d97bbe46c831193 Mon Sep 17 00:00:00 2001 From: Simon Chen Date: Wed, 26 Feb 2020 12:18:28 -0500 Subject: [PATCH] MST-121 We should allow the business case where a learner can be enrolled into a program, then unenrolled, then enroll into another program with the same course --- common/djangoapps/student/models.py | 20 -- .../grades/rest_api/v1/gradebook_views.py | 8 +- .../program_enrollments/api/__init__.py | 3 +- .../program_enrollments/api/reading.py | 52 ++++- .../api/tests/test_reading.py | 57 +++++- .../api/tests/test_writing.py | 6 +- .../program_enrollments/api/writing.py | 100 ++++++++-- .../commands/tests/test_migrate_saml_uids.py | 6 +- ..._course_enrollment_field_to_foreign_key.py | 21 +++ lms/djangoapps/program_enrollments/models.py | 9 +- .../rest_api/v1/tests/test_views.py | 177 +++++++++++++++++- .../program_enrollments/tests/test_models.py | 21 +-- 12 files changed, 396 insertions(+), 84 deletions(-) create mode 100644 lms/djangoapps/program_enrollments/migrations/0009_update_course_enrollment_field_to_foreign_key.py diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 42d7a9fbea..69f59f480a 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1240,26 +1240,6 @@ class CourseEnrollment(models.Model): except cls.DoesNotExist: return None - @classmethod - def get_program_enrollment(cls, user, course_id): - """ - Return the ProgramEnrollment associated with the CourseEnrollment specified - by the user and course_id. - Return None if there is no ProgramEnrollment. - - Arguments: - user (User): the user for whom we want the program enrollment - course_id (CourseKey): the id of the course the user has a course enrollment in - - Returns: - ProgramEnrollment object or None - """ - try: - course_enrollment = cls.objects.get(user=user, course_id=course_id) - return course_enrollment.programcourseenrollment.program_enrollment - except (ObjectDoesNotExist): - return None - @classmethod def is_enrollment_closed(cls, user, course): """ diff --git a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py index 7800a913a8..5ea63c1779 100644 --- a/lms/djangoapps/grades/rest_api/v1/gradebook_views.py +++ b/lms/djangoapps/grades/rest_api/v1/gradebook_views.py @@ -46,6 +46,7 @@ from lms.djangoapps.grades.subsection_grade import CreateSubsectionGrade from lms.djangoapps.grades.subsection_grade_factory import SubsectionGradeFactory from lms.djangoapps.grades.tasks import recalculate_subsection_grade_v3 from lms.djangoapps.course_blocks.api import get_course_blocks +from lms.djangoapps.program_enrollments.api import get_external_key_by_user_and_course from openedx.core.djangoapps.course_groups import cohorts from openedx.core.djangoapps.util.forms import to_bool from openedx.core.lib.api.view_utils import ( @@ -498,17 +499,12 @@ class GradebookView(GradeViewMixin, PaginatedAPIView): user_entry['user_id'] = user.id user_entry['full_name'] = user.profile.name - external_user_key = self._get_external_user_key(user, course.id) + external_user_key = get_external_key_by_user_and_course(user, course.id) if external_user_key: user_entry['external_user_key'] = external_user_key return user_entry - @staticmethod - def _get_external_user_key(user, course_id): - program_enrollment = CourseEnrollment.get_program_enrollment(user, course_id) - return getattr(program_enrollment, 'external_user_key', None) - @verify_course_exists @verify_writable_gradebook_enabled @course_author_access_required diff --git a/lms/djangoapps/program_enrollments/api/__init__.py b/lms/djangoapps/program_enrollments/api/__init__.py index 49b292c440..3f76108502 100644 --- a/lms/djangoapps/program_enrollments/api/__init__.py +++ b/lms/djangoapps/program_enrollments/api/__init__.py @@ -18,9 +18,10 @@ from .grades import iter_program_course_grades from .linking import link_program_enrollment_to_lms_user, link_program_enrollments from .reading import ( fetch_program_course_enrollments, - fetch_program_course_enrollments_by_student, + fetch_program_course_enrollments_by_students, fetch_program_enrollments, fetch_program_enrollments_by_student, + get_external_key_by_user_and_course, get_program_course_enrollment, get_program_enrollment, get_provider_slug, diff --git a/lms/djangoapps/program_enrollments/api/reading.py b/lms/djangoapps/program_enrollments/api/reading.py index 4a95bf7415..67b646de82 100644 --- a/lms/djangoapps/program_enrollments/api/reading.py +++ b/lms/djangoapps/program_enrollments/api/reading.py @@ -27,6 +27,10 @@ _STUDENT_ARG_ERROR_MESSAGE = ( _REALIZED_FILTER_ERROR_TEMPLATE = ( "{} and {} are mutually exclusive; at most one of them may be passed in as True." ) +_STUDENT_LIST_ARG_ERROR_MESSAGE = ( + 'user list and external_user_key_list are both empty or None;' + ' At least one of the lists must be provided.' +) def get_program_enrollment( @@ -265,9 +269,9 @@ def fetch_program_enrollments_by_student( return ProgramEnrollment.objects.filter(**_remove_none_values(filters)) -def fetch_program_course_enrollments_by_student( - user=None, - external_user_key=None, +def fetch_program_course_enrollments_by_students( + users=None, + external_user_keys=None, program_uuids=None, curriculum_uuids=None, course_keys=None, @@ -278,11 +282,11 @@ def fetch_program_course_enrollments_by_student( waiting_only=False, ): """ - Fetch program-course enrollments for a specific student. + Fetch program-course enrollments for a specific list of students. Required arguments (at least one must be provided): - * user (User) - * external_user_key (str) + * users (iterable[User]) + * external_user_keys (iterable[str]) Optional arguments: * provided_uuids (iterable[UUID|str]) @@ -298,8 +302,9 @@ def fetch_program_course_enrollments_by_student( Returns: queryset[ProgramCourseEnrollment] """ - if not (user or external_user_key): - raise ValueError(_STUDENT_ARG_ERROR_MESSAGE) + if not (users or external_user_keys): + raise ValueError(_STUDENT_LIST_ARG_ERROR_MESSAGE) + if active_only and inactive_only: raise ValueError( _REALIZED_FILTER_ERROR_TEMPLATE.format("active_only", "inactive_only") @@ -309,8 +314,8 @@ def fetch_program_course_enrollments_by_student( _REALIZED_FILTER_ERROR_TEMPLATE.format("realized_only", "waiting_only") ) filters = { - "program_enrollment__user": user, - "program_enrollment__external_user_key": external_user_key, + "program_enrollment__user__in": users, + "program_enrollment__external_user_key__in": external_user_keys, "program_enrollment__program_uuid__in": program_uuids, "program_enrollment__curriculum_uuid__in": curriculum_uuids, "course_key__in": course_keys, @@ -377,6 +382,33 @@ def get_users_by_external_keys(program_uuid, external_user_keys): return users_by_external_keys +def get_external_key_by_user_and_course(user, course_key): + """ + Returns the external_user_key of the edX account/user + enrolled into the course + + Arguments: + user (User): + The edX account representing the user in auth_user table + course_key (CourseKey|str): + The course key of the course user is enrolled in + + Returns: external_user_key (str|None) + The external user key provided by Masters degree provider + Or None if cannot find edX user to Masters learner mapping + """ + program_course_enrollments = ProgramCourseEnrollment.objects.filter( + course_enrollment__user=user, + course_key=course_key + ).order_by('status', '-modified') + + if not program_course_enrollments: + return None + + relevant_pce = program_course_enrollments.first() + return relevant_pce.program_enrollment.external_user_key + + def get_saml_provider_for_program(program_uuid): """ Return currently configured SAML provider for the Organization diff --git a/lms/djangoapps/program_enrollments/api/tests/test_reading.py b/lms/djangoapps/program_enrollments/api/tests/test_reading.py index 1ad65ed612..ba5bd55093 100644 --- a/lms/djangoapps/program_enrollments/api/tests/test_reading.py +++ b/lms/djangoapps/program_enrollments/api/tests/test_reading.py @@ -34,9 +34,10 @@ from third_party_auth.tests.factories import SAMLProviderConfigFactory from ..reading import ( fetch_program_course_enrollments, - fetch_program_course_enrollments_by_student, + fetch_program_course_enrollments_by_students, fetch_program_enrollments, fetch_program_enrollments_by_student, + get_external_key_by_user_and_course, get_program_course_enrollment, get_program_enrollment, get_users_by_external_keys @@ -368,14 +369,14 @@ class ProgramEnrollmentReadingTests(TestCase): # User with no program enrollments ( - {'username': username_0}, + {'usernames': [username_0]}, set(), ), # Course keys and active-only filters ( { - 'external_user_key': ext_4, + 'external_user_keys': [ext_4], 'course_keys': {course_key_p, course_key_q}, 'active_only': True, }, @@ -384,26 +385,26 @@ class ProgramEnrollmentReadingTests(TestCase): # Curriculum filter ( - {'username': username_3, 'curriculum_uuids': {curriculum_uuid_b}}, + {'usernames': [username_3], 'curriculum_uuids': {curriculum_uuid_b}}, {5}, ), # Program filter ( - {'username': username_3, 'program_uuids': {program_uuid_y}}, + {'usernames': [username_3], 'program_uuids': {program_uuid_y}}, {12}, ), # Realized-only filter ( - {'external_user_key': ext_4, 'realized_only': True}, + {'external_user_keys': [ext_4], 'realized_only': True}, set(), ), # Waiting-only and inactive-only filter ( { - 'external_user_key': ext_4, + 'external_user_keys': [ext_4], 'waiting_only': True, 'inactive_only': True, }, @@ -411,9 +412,9 @@ class ProgramEnrollmentReadingTests(TestCase): ), ) @ddt.unpack - def test_fetch_program_course_enrollments_by_student(self, kwargs, expected_enrollment_ids): - kwargs = self._username_to_user(kwargs) - actual_enrollments = fetch_program_course_enrollments_by_student(**kwargs) + def test_fetch_program_course_enrollments_by_students(self, kwargs, expected_enrollment_ids): + kwargs = self._usernames_to_users(kwargs) + actual_enrollments = fetch_program_course_enrollments_by_students(**kwargs) actual_enrollment_ids = {enrollment.id for enrollment in actual_enrollments} assert actual_enrollment_ids == expected_enrollment_ids @@ -443,6 +444,42 @@ class ProgramEnrollmentReadingTests(TestCase): del result['usernames'] return result + @ddt.data( + ( + {'username': username_0, 'course_key': course_key_p}, + None + ), + ( + {'username': username_1, 'course_key': course_key_p}, + None + ), + ( + {'username': username_1, 'course_key': course_key_r}, + None + ), + ( + {'username': username_2, 'course_key': course_key_p}, + None + ), + ( + {'username': username_3, 'course_key': course_key_p}, + ext_3 + ), + ( + {'username': username_3, 'course_key': course_key_r}, + None + ), + ( + {'username': username_4, 'course_key': course_key_p}, + None + ) + ) + @ddt.unpack + def test_get_external_key_by_user_and_course(self, kwargs, expected_external_user_key): + kwarg = self._username_to_user(kwargs) + external_user_key = get_external_key_by_user_and_course(**kwarg) + assert expected_external_user_key == external_user_key + class GetUsersByExternalKeysTests(CacheIsolationTestCase): """ diff --git a/lms/djangoapps/program_enrollments/api/tests/test_writing.py b/lms/djangoapps/program_enrollments/api/tests/test_writing.py index d075e0f910..83883016ee 100644 --- a/lms/djangoapps/program_enrollments/api/tests/test_writing.py +++ b/lms/djangoapps/program_enrollments/api/tests/test_writing.py @@ -11,8 +11,8 @@ mocks in the view tests. from uuid import UUID -from organizations.tests.factories import OrganizationFactory from django.core.cache import cache +from organizations.tests.factories import OrganizationFactory from lms.djangoapps.program_enrollments.constants import ProgramEnrollmentStatuses as PEStatuses from lms.djangoapps.program_enrollments.models import ProgramEnrollment @@ -22,9 +22,7 @@ from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangolib.testing.utils import CacheIsolationTestCase from third_party_auth.tests.factories import SAMLProviderConfigFactory -from ..writing import ( - write_program_enrollments -) +from ..writing import write_program_enrollments class WritingProgramEnrollmentTest(CacheIsolationTestCase): diff --git a/lms/djangoapps/program_enrollments/api/writing.py b/lms/djangoapps/program_enrollments/api/writing.py index 259c43e6d4..323bad5ba8 100644 --- a/lms/djangoapps/program_enrollments/api/writing.py +++ b/lms/djangoapps/program_enrollments/api/writing.py @@ -18,7 +18,7 @@ from ..constants import ProgramEnrollmentStatuses from ..constants import ProgramOperationStatuses as ProgramOpStatuses from ..exceptions import ProviderDoesNotExistException from ..models import ProgramCourseEnrollment, ProgramEnrollment -from .reading import fetch_program_course_enrollments, fetch_program_enrollments, get_users_by_external_keys +from .reading import fetch_program_course_enrollments_by_students, fetch_program_enrollments, get_users_by_external_keys logger = logging.getLogger(__name__) @@ -193,30 +193,59 @@ def write_program_course_enrollments( if not (create or update): raise ValueError("At least one of (create, update) must be True") requests_by_key, duplicated_keys = _organize_requests_by_external_key(enrollment_requests) - external_keys = set(requests_by_key) + processable_external_keys = set(requests_by_key) + + results = {} + results.update({ + key: ProgramCourseOpStatuses.DUPLICATED for key in duplicated_keys + }) + + if not processable_external_keys: + return results + program_enrollments = fetch_program_enrollments( program_uuid=program_uuid, - external_user_keys=external_keys, + external_user_keys=processable_external_keys, ).prefetch_related('program_course_enrollments') program_enrollments_by_key = { enrollment.external_user_key: enrollment for enrollment in program_enrollments } - # Fetch existing program-course enrollments. - existing_course_enrollments = fetch_program_course_enrollments( - program_uuid, course_key, program_enrollments=program_enrollments, + # Fetch enrollments regardless of anchored Program Enrollments + existing_course_enrollments = fetch_program_course_enrollments_by_students( + external_user_keys=processable_external_keys, + course_keys=[course_key], + ).select_related('program_enrollment') + + conflicting_user_key_and_status = _get_conflicting_active_course_enrollments( + requests_by_key, + existing_course_enrollments, + program_uuid, + course_key ) - existing_course_enrollments_by_key = {key: None for key in external_keys} + + # Remove the conflicted items from the requests dictionary, + # so we can continue our processing below + for conflicting_user_key in conflicting_user_key_and_status: + del requests_by_key[conflicting_user_key] + + results.update(conflicting_user_key_and_status) + + # Now, limit the course enrollments to the same program uuid + existing_course_enrollments_of_program_enrollment = existing_course_enrollments.filter( + program_enrollment__program_uuid=program_uuid + ) + + existing_course_enrollments_by_key = {key: None for key in processable_external_keys} existing_course_enrollments_by_key.update({ enrollment.program_enrollment.external_user_key: enrollment - for enrollment in existing_course_enrollments + for enrollment in existing_course_enrollments_of_program_enrollment }) # 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 (Django 2.2 will add bulk-update support). # For each operation, update `results` with the new status or an error status. - results = {} to_save = [] for external_key, request in requests_by_key.items(): status = request['status'] @@ -251,9 +280,6 @@ def write_program_course_enrollments( if to_save: ProgramCourseEnrollment.objects.bulk_create(to_save) - results.update({ - key: ProgramCourseOpStatuses.DUPLICATED for key in duplicated_keys - }) return results @@ -424,3 +450,53 @@ def _organize_requests_by_external_key(enrollment_requests): continue requests_by_key[key] = request return requests_by_key, duplicated_keys + + +def _get_conflicting_active_course_enrollments( + requests_by_key, + existing_course_enrollments, + program_uuid, + course_key +): + """ + Process the list of existing course enrollments together with + the enrollment request list stored in 'requests_by_key'. Detect + whether we have conflicting ACTIVE ProgramCourseEnrollment entries. + When detected, log about it and return the conflicting entry with + duplicated status. + + Arguments: + requests_by_key (dict) + existing_course_enrollments (queryset[ProgramCourseEnrollment]), + program_uuid (UUID|str), + course_key (str) + + Returns: + results (dict) with detected conflict entry, or empty dict. + """ + conflicted_by_user_key = {} + + requested_statuses_by_user_key = { + key: request.get('status') for key, request in requests_by_key.items() + } + + for existing_enrollment in existing_course_enrollments: + external_user_key = existing_enrollment.program_enrollment.external_user_key + requested_status = requested_statuses_by_user_key.get( + existing_enrollment.program_enrollment.external_user_key + ) + if ( + requested_status + and requested_status == ProgramCourseEnrollmentStatuses.ACTIVE + and existing_enrollment.is_active + and str(existing_enrollment.program_enrollment.program_uuid) != str(program_uuid) + ): + logger.error( + u'Detected conflicting active ProgramCourseEnrollment. This is happening on' + u' The program_uuid [{}] with course_key [{}] for external_user_key [{}]'.format( + program_uuid, + course_key, + external_user_key + )) + conflicted_by_user_key[external_user_key] = ProgramCourseOpStatuses.CONFLICT + return conflicted_by_user_key diff --git a/lms/djangoapps/program_enrollments/management/commands/tests/test_migrate_saml_uids.py b/lms/djangoapps/program_enrollments/management/commands/tests/test_migrate_saml_uids.py index 00e6137aec..6a97c091b2 100644 --- a/lms/djangoapps/program_enrollments/management/commands/tests/test_migrate_saml_uids.py +++ b/lms/djangoapps/program_enrollments/management/commands/tests/test_migrate_saml_uids.py @@ -3,15 +3,15 @@ Tests for the migrate_saml_uids management command. """ +import six from django.core.management import call_command from django.test import TestCase - -import six from factory import LazyAttributeSequence, SubFactory from factory.django import DjangoModelFactory -from lms.djangoapps.program_enrollments.management.commands import migrate_saml_uids from mock import mock_open, patch from social_django.models import UserSocialAuth + +from lms.djangoapps.program_enrollments.management.commands import migrate_saml_uids from student.tests.factories import UserFactory _COMMAND_PATH = 'lms.djangoapps.program_enrollments.management.commands.migrate_saml_uids' diff --git a/lms/djangoapps/program_enrollments/migrations/0009_update_course_enrollment_field_to_foreign_key.py b/lms/djangoapps/program_enrollments/migrations/0009_update_course_enrollment_field_to_foreign_key.py new file mode 100644 index 0000000000..2e2111c8bd --- /dev/null +++ b/lms/djangoapps/program_enrollments/migrations/0009_update_course_enrollment_field_to_foreign_key.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.28 on 2020-02-26 21:20 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('program_enrollments', '0008_add_ended_programenrollment_status'), + ] + + operations = [ + migrations.AlterField( + model_name='programcourseenrollment', + name='course_enrollment', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='student.CourseEnrollment'), + ), + ] diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 0ee6989111..8ff6abed43 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -107,7 +107,10 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin on_delete=models.CASCADE, related_name="program_course_enrollments" ) - course_enrollment = models.OneToOneField( + # In Django 2.x, we should add a conditional unique constraint to this field so + # no duplicated tuple of (course_enrollment_id, status=active) exists + # MST-168 is the Jira ticket to accomplish this once Django is upgraded + course_enrollment = models.ForeignKey( CourseEnrollment, null=True, blank=True, @@ -116,5 +119,9 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin status = models.CharField(max_length=9, choices=STATUS_CHOICES) historical_records = HistoricalRecords() + @property + def is_active(self): + return self.status == ProgramCourseEnrollmentStatuses.ACTIVE + def __str__(self): return '[ProgramCourseEnrollment id={}]'.format(self.id) diff --git a/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py b/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py index 969ac87eac..9a864a2a69 100644 --- a/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py +++ b/lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py @@ -23,6 +23,7 @@ from rest_framework import status from rest_framework.test import APITestCase from six import text_type from six.moves import range, zip +from social_django.models import UserSocialAuth from bulk_email.models import BulkEmailFlag, Optout from course_modes.models import CourseMode @@ -98,8 +99,8 @@ class EnrollmentsDataMixin(ProgramCacheMixin): super(EnrollmentsDataMixin, cls).setUpClass() cls.start_cache_isolation() cls.organization_key = "testorg" - catalog_org = OrganizationFactory(key=cls.organization_key) - LMSOrganizationFactory(short_name=cls.organization_key) + cls.catalog_org = OrganizationFactory(key=cls.organization_key) + cls.lms_org = LMSOrganizationFactory(short_name=cls.organization_key) cls.program_uuid = UUID('00000000-1111-2222-3333-444444444444') cls.program_uuid_tmpl = '00000000-1111-2222-3333-4444444444{0:02d}' cls.curriculum_uuid = UUID('aaaaaaaa-1111-2222-3333-444444444444') @@ -111,12 +112,12 @@ class EnrollmentsDataMixin(ProgramCacheMixin): cls.course_id = CourseKey.from_string(course_run_id_str) CourseOverviewFactory(id=cls.course_id) course_run = CourseRunFactory(key=course_run_id_str) - course = CourseFactory(key=catalog_course_id_str, course_runs=[course_run]) + cls.course = CourseFactory(key=catalog_course_id_str, course_runs=[course_run]) inactive_curriculum = CurriculumFactory(uuid=inactive_curriculum_uuid, is_active=False) - cls.curriculum = CurriculumFactory(uuid=cls.curriculum_uuid, courses=[course]) + cls.curriculum = CurriculumFactory(uuid=cls.curriculum_uuid, courses=[cls.course]) cls.program = ProgramFactory( uuid=cls.program_uuid, - authoring_organizations=[catalog_org], + authoring_organizations=[cls.catalog_org], curricula=[inactive_curriculum, cls.curriculum], ) @@ -1202,6 +1203,172 @@ class ProgramCourseEnrollmentsPatchTests(ProgramCourseEnrollmentsModifyMixin, AP self.create_program_and_course_enrollments('learner-4', course_status=initial_statuses[3], user=None) +@ddt.ddt +class MultiprogramEnrollmentsTest(EnrollmentsDataMixin, APITestCase): + """ Tests for the Multiple Program with same course scenario """ + @classmethod + def setUpClass(cls): + super(MultiprogramEnrollmentsTest, cls).setUpClass() + cls.another_curriculum_uuid = UUID('bbbbbbbb-8888-9999-7777-666666666666') + cls.another_curriculum = CurriculumFactory( + uuid=cls.another_curriculum_uuid, + courses=[cls.course] + ) + cls.another_program_uuid = UUID(cls.program_uuid_tmpl.format(99)) + cls.another_program = ProgramFactory( + uuid=cls.another_program_uuid, + authoring_organizations=[cls.catalog_org], + curricula=[cls.another_curriculum] + ) + cls.external_user_key = 'aabbcc' + cls.user = UserFactory.create(username='multiprogram_user') + + def setUp(self): + super(MultiprogramEnrollmentsTest, self).setUp() + self.set_program_in_catalog_cache(self.another_program_uuid, self.another_program) + self.client.login(username=self.global_staff.username, password=self.password) + + def get_program_url(self, program_uuid): + return reverse('programs_api:v1:program_enrollments', kwargs={ + 'program_uuid': program_uuid + }) + + def get_program_course_url(self, program_uuid, course_id): + return reverse('programs_api:v1:program_course_enrollments', kwargs={ + 'program_uuid': program_uuid, + 'course_id': course_id + }) + + def write_program_enrollment( + self, + method, + program_uuid, + curriculum_uuid, + enrollment_status, + existing_user + ): + """ Create or update the program enrollment through API """ + write_data = [{ + 'status': enrollment_status, + REQUEST_STUDENT_KEY: self.external_user_key, + 'curriculum_uuid': str(curriculum_uuid) + }] + url = self.get_program_url(program_uuid=program_uuid) + mock_user = defaultdict(lambda: None) + if existing_user: + mock_user = {self.external_user_key: self.user} + with mock.patch( + _get_users_patch_path, + autospec=True, + return_value=mock_user, + ): + response = getattr(self.client, method)( + url, + json.dumps(write_data), + content_type='application/json' + ) + return response + + def write_program_course_enrollment( + self, + method, + program_uuid, + course_id, + enrollment_status + ): + """ Create or update the program course enrollment through API """ + course_post_data = [{ + 'student_key': self.external_user_key, + 'status': enrollment_status + }] + course_url = self.get_program_course_url(program_uuid, course_id) + response = getattr(self.client, method)( + course_url, + json.dumps(course_post_data), + content_type='application/json' + ) + return response + + def link_user_social_auth(self): + """ Create the UserSocialAuth record to trigger the linkage django signal """ + SAMLProviderConfigFactory( + organization=self.lms_org, + slug=self.organization_key + ) + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.organization_key, self.external_user_key), + provider=self.organization_key + ) + + @ddt.data(True, False) + def test_enrollment_in_same_course_multi_program(self, existing_user): + response = self.write_program_enrollment( + 'post', self.program_uuid, self.curriculum_uuid, 'enrolled', existing_user + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.write_program_course_enrollment( + 'post', self.program_uuid, self.course_id, 'active' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + response = self.write_program_enrollment( + 'put', self.program_uuid, self.curriculum_uuid, 'canceled', existing_user + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.write_program_course_enrollment( + 'put', self.program_uuid, self.course_id, 'inactive' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + response = self.write_program_enrollment( + 'post', self.another_program_uuid, self.another_curriculum_uuid, 'enrolled', existing_user + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.write_program_course_enrollment( + 'post', self.another_program_uuid, self.course_id, 'active') + self.assertEqual(response.status_code, status.HTTP_200_OK) + + if not existing_user: + self.link_user_social_auth() + program_course_enrollment = ProgramCourseEnrollment.objects.get( + program_enrollment__external_user_key=self.external_user_key, + program_enrollment__program_uuid=self.another_program_uuid + ) + self.assertIsNotNone(program_course_enrollment.program_enrollment.user) + + @ddt.data(True, False) + @mock.patch('lms.djangoapps.program_enrollments.api.writing.logger') + def test_enrollment_in_same_course_both_program_enrollments_active(self, existing_user, mock_log): + response = self.write_program_enrollment( + 'post', self.program_uuid, self.curriculum_uuid, 'enrolled', existing_user + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.write_program_course_enrollment( + 'post', self.program_uuid, self.course_id, 'active' + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + response = self.write_program_enrollment( + 'post', self.another_program_uuid, self.another_curriculum_uuid, 'enrolled', existing_user + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.write_program_course_enrollment( + 'post', self.another_program_uuid, self.course_id, 'active' + ) + self.assertEqual(response.status_code, 422) + mock_log.error.assert_called_with( + u'Detected conflicting active ProgramCourseEnrollment. This is happening on' + u' The program_uuid [{}] with course_key [{}] for external_user_key [{}]'.format( + self.another_program_uuid, + self.course_id, + self.external_user_key + ) + ) + expected_results = {self.external_user_key: CourseStatuses.CONFLICT} + self.assertDictEqual(expected_results, response.data) + + class ProgramCourseEnrollmentsPutTests(ProgramCourseEnrollmentsModifyMixin, APITestCase): """ Tests for course enrollment PUT """ diff --git a/lms/djangoapps/program_enrollments/tests/test_models.py b/lms/djangoapps/program_enrollments/tests/test_models.py index 7089d07432..5a742112c3 100644 --- a/lms/djangoapps/program_enrollments/tests/test_models.py +++ b/lms/djangoapps/program_enrollments/tests/test_models.py @@ -116,22 +116,19 @@ class ProgramCourseEnrollmentModelTests(TestCase): self.course_key = CourseKey.from_string(generate_course_run_key()) CourseOverviewFactory(id=self.course_key) - def test_unique_completed_enrollment(self): + def test_duplicate_enrollments_allowed(self): """ A record with the same (program_enrollment, course_enrollment) - cannot be created. + can be created as long as only one record is active for the + same course_enrollment """ 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", - ) + 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): """