diff --git a/lms/djangoapps/program_enrollments/tests/factories.py b/lms/djangoapps/program_enrollments/api/v1/tests/factories.py similarity index 100% rename from lms/djangoapps/program_enrollments/tests/factories.py rename to lms/djangoapps/program_enrollments/api/v1/tests/factories.py diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py b/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py index d954d6ae19..c9cc3c28f6 100644 --- a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py +++ b/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py @@ -16,19 +16,18 @@ from rest_framework import status from rest_framework.test import APITestCase from six import text_type -from course_modes.models import CourseMode from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory from lms.djangoapps.program_enrollments.api.v1.constants import MAX_ENROLLMENT_RECORDS, REQUEST_STUDENT_KEY from lms.djangoapps.program_enrollments.api.v1.constants import CourseEnrollmentResponseStatuses as CourseStatuses -from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment -from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory +from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment +from student.tests.factories import UserFactory, CourseEnrollmentFactory from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL from openedx.core.djangoapps.catalog.tests.factories import CourseFactory from openedx.core.djangoapps.catalog.tests.factories import OrganizationFactory as CatalogOrganizationFactory from openedx.core.djangoapps.catalog.tests.factories import ProgramFactory from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangolib.testing.utils import CacheIsolationMixin -from student.tests.factories import CourseEnrollmentFactory, UserFactory +from .factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory class ListViewTestMixin(object): @@ -310,7 +309,6 @@ class BaseCourseEnrollmentTestsMixin(ProgramCacheTestCaseMixin): course_enrollment = CourseEnrollmentFactory.create( course_id=self.course_key, user=program_enrollment.user, - mode=CourseMode.MASTERS ) course_enrollment.is_active = course_status == "active" course_enrollment.save() @@ -325,7 +323,7 @@ class BaseCourseEnrollmentTestsMixin(ProgramCacheTestCaseMixin): program_enrollment = self.create_program_enrollment(external_user_key, user) return self.create_program_course_enrollment(program_enrollment, course_status=course_status) - def assert_program_course_enrollment(self, external_user_key, expected_status, has_user, mode=CourseMode.MASTERS): + def assert_program_course_enrollment(self, external_user_key, expected_status, has_user): """ Convenience method to assert that a ProgramCourseEnrollment exists, and potentially that a CourseEnrollment also exists @@ -341,7 +339,6 @@ class BaseCourseEnrollmentTestsMixin(ProgramCacheTestCaseMixin): self.assertIsNotNone(course_enrollment) self.assertEqual(expected_status == "active", course_enrollment.is_active) self.assertEqual(self.course_key, course_enrollment.course_id) - self.assertEqual(mode, course_enrollment.mode) else: self.assertIsNone(course_enrollment) @@ -454,44 +451,13 @@ class CourseEnrollmentPostTests(BaseCourseEnrollmentTestsMixin, APITestCase): self.assert_program_course_enrollment("learner-3", "active", False) self.assert_program_course_enrollment("learner-4", "inactive", False) - def test_program_course_enrollment_exists(self): - """ - The program enrollments application already has a program_course_enrollment - record for this user and course - """ + def test_user_already_enrolled_in_course(self): self.create_program_and_course_enrollments('learner-1') post_data = [self.learner_enrollment("learner-1")] response = self.request(self.default_url, post_data) self.assertEqual(422, response.status_code) self.assertDictEqual({'learner-1': CourseStatuses.CONFLICT}, response.data) - def test_user_currently_enrolled_in_course(self): - """ - If a user is already enrolled in a course through a different method - that enrollment should be linked but not overwritten as masters. - """ - CourseEnrollmentFactory.create( - course_id=self.course_key, - user=self.student, - mode=CourseMode.VERIFIED - ) - - self.create_program_enrollment('learner-1', user=self.student) - - post_data = [ - self.learner_enrollment("learner-1", "active") - ] - response = self.request(self.default_url, post_data) - - self.assertEqual(200, response.status_code) - self.assertDictEqual( - { - "learner-1": "active" - }, - response.data - ) - self.assert_program_course_enrollment("learner-1", "active", True, mode=CourseMode.VERIFIED) - def test_207_multistatus(self): self.create_program_enrollment('learner-1') post_data = [self.learner_enrollment("learner-1"), self.learner_enrollment("learner-2")] diff --git a/lms/djangoapps/program_enrollments/api/v1/views.py b/lms/djangoapps/program_enrollments/api/v1/views.py index ceadcc8d8d..4d7dd3c576 100644 --- a/lms/djangoapps/program_enrollments/api/v1/views.py +++ b/lms/djangoapps/program_enrollments/api/v1/views.py @@ -689,8 +689,7 @@ class ProgramCourseEnrollmentsView(DeveloperErrorViewMixin, ProgramCourseRunSpec """ if program_course_enrollment: return CourseEnrollmentResponseStatuses.CONFLICT - - return ProgramCourseEnrollment.create_program_course_enrollment( + return ProgramCourseEnrollment.enroll( program_enrollment, self.course_key, enrollment_request['status'] diff --git a/lms/djangoapps/program_enrollments/apps.py b/lms/djangoapps/program_enrollments/apps.py index 2ec8923c19..7cdabf684a 100644 --- a/lms/djangoapps/program_enrollments/apps.py +++ b/lms/djangoapps/program_enrollments/apps.py @@ -24,9 +24,3 @@ class ProgramEnrollmentsConfig(AppConfig): } }, } - - def ready(self): - """ - Connect handlers to signals. - """ - from . import signals # pylint: disable=unused-variable diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 05698f915c..896a13f5bc 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -14,7 +14,7 @@ from lms.djangoapps.program_enrollments.api.v1.constants import CourseEnrollment from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField from simple_history.models import HistoricalRecords -from student.models import AlreadyEnrolledError, CourseEnrollment as StudentCourseEnrollment +from student.models import CourseEnrollment as StudentCourseEnrollment logger = logging.getLogger(__name__) # pylint: disable=invalid-name @@ -143,19 +143,27 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin return '[ProgramCourseEnrollment id={}]'.format(self.id) @classmethod - def create_program_course_enrollment(cls, program_enrollment, course_key, status): + def enroll(cls, program_enrollment, course_key, status): """ Create ProgramCourseEnrollment for the given course and program enrollment """ + course_enrollment = None + if program_enrollment.user: + course_enrollment = StudentCourseEnrollment.enroll( + program_enrollment.user, + course_key, + mode=CourseMode.MASTERS, + check_access=True, + ) + if status == CourseEnrollmentResponseStatuses.INACTIVE: + course_enrollment.deactivate() + program_course_enrollment = ProgramCourseEnrollment.objects.create( program_enrollment=program_enrollment, + course_enrollment=course_enrollment, course_key=course_key, status=status, ) - - if program_enrollment.user: - program_course_enrollment.enroll(program_enrollment.user) - return program_course_enrollment.status def change_status(self, status): @@ -188,30 +196,3 @@ class ProgramCourseEnrollment(TimeStampedModel): # pylint: disable=model-missin )) self.save() return self.status - - def enroll(self, user): - """ - Create a StudentCourseEnrollment to enroll user in course - """ - try: - self.course_enrollment = StudentCourseEnrollment.enroll( - user, - self.course_key, - mode=CourseMode.MASTERS, - check_access=True, - ) - except AlreadyEnrolledError: - course_enrollment = StudentCourseEnrollment.objects.get( - user=user, - course_id=self.course_key, - ) - if course_enrollment.mode == CourseMode.AUDIT or course_enrollment.mode == CourseMode.HONOR: - course_enrollment.mode = CourseMode.MASTERS - course_enrollment.save() - self.course_enrollment = course_enrollment - message = ("Attempted to create course enrollment for user={user} and course={course}" - " but an enrollment already exists. Existing enrollment will be used instead") - logger.info(message.format(user=user.id, course=self.course_key)) - if self.status == CourseEnrollmentResponseStatuses.INACTIVE: - self.course_enrollment.deactivate() - self.save() diff --git a/lms/djangoapps/program_enrollments/signals.py b/lms/djangoapps/program_enrollments/signals.py deleted file mode 100644 index f24bf84269..0000000000 --- a/lms/djangoapps/program_enrollments/signals.py +++ /dev/null @@ -1,50 +0,0 @@ -""" -Signal handlers for program enrollments -""" -import logging -from django.db.models.signals import post_save -from django.dispatch import receiver -from social_django.models import UserSocialAuth -from student.models import CourseEnrollmentException -from third_party_auth.models import SAMLProviderConfig - -from lms.djangoapps.program_enrollments.models import ProgramEnrollment - -logger = logging.getLogger(__name__) - - -@receiver(post_save, sender=UserSocialAuth) -def martriculate_learner(sender, instance, **kwargs): # pylint: disable=unused-argument - """ - Post-save signal to update any waiting program enrollments with a user, - and enroll the user in any waiting course enrollments. - - In most cases this will just short-circuit. Enrollments will only be updated - for a SAML provider with a linked organization. - """ - try: - user = instance.user - provider_slug, external_user_key = instance.uid.split(':') - if not SAMLProviderConfig.objects.get(slug=provider_slug).organization: - return - except (AttributeError, ValueError, SAMLProviderConfig.DoesNotExist): - return - - incomplete_enrollments = ProgramEnrollment.objects.filter( - external_user_key=external_user_key - ).prefetch_related('program_course_enrollments') - - incomplete_enrollments.update(user=user) - - for enrollment in incomplete_enrollments: - for program_course_enrollment in enrollment.program_course_enrollments.all(): - try: - program_course_enrollment.enroll(user) - except CourseEnrollmentException as e: - logger.warning( - u'Failed to enroll user=%s with waiting program_course_enrollment=%s: %s', - user.id, - program_course_enrollment.id, - e, - ) - raise e diff --git a/lms/djangoapps/program_enrollments/tests/test_models.py b/lms/djangoapps/program_enrollments/tests/test_models.py index 8a51dd41ae..19572d6c46 100644 --- a/lms/djangoapps/program_enrollments/tests/test_models.py +++ b/lms/djangoapps/program_enrollments/tests/test_models.py @@ -6,17 +6,12 @@ from __future__ import unicode_literals from uuid import uuid4 from testfixtures import LogCapture -import ddt from django.test import TestCase from opaque_keys.edx.keys import CourseKey -from course_modes.models import CourseMode -from edx_django_utils.cache import RequestCache from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment -from student.models import CourseEnrollment from student.tests.factories import UserFactory, CourseEnrollmentFactory from openedx.core.djangoapps.catalog.tests.factories import generate_course_run_key -from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory class ProgramEnrollmentModelTests(TestCase): @@ -107,7 +102,6 @@ class ProgramEnrollmentModelTests(TestCase): self.assertEquals(record.external_user_key, None) -@ddt.ddt class ProgramCourseEnrollmentModelTests(TestCase): """ Tests for the ProgramCourseEnrollment model. @@ -117,7 +111,6 @@ class ProgramCourseEnrollmentModelTests(TestCase): Set up test data """ super(ProgramCourseEnrollmentModelTests, self).setUp() - RequestCache.clear_all_namespaces() self.user = UserFactory.create() self.program_uuid = uuid4() self.program_enrollment = ProgramEnrollment.objects.create( @@ -128,37 +121,21 @@ class ProgramCourseEnrollmentModelTests(TestCase): status='enrolled' ) self.course_key = CourseKey.from_string(generate_course_run_key()) - CourseOverviewFactory(id=self.course_key) - - def _create_completed_program_course_enrollment(self): - """ helper function create program course enrollment """ - course_enrollment = CourseEnrollmentFactory.create( + self.course_enrollment = CourseEnrollmentFactory.create( course_id=self.course_key, user=self.user, - mode=CourseMode.MASTERS ) - program_course_enrollment = ProgramCourseEnrollment.objects.create( + self.program_course_enrollment = ProgramCourseEnrollment.objects.create( program_enrollment=self.program_enrollment, course_key=self.course_key, - course_enrollment=course_enrollment, - status="active" - ) - return program_course_enrollment - - def _create_waiting_program_course_enrollment(self): - """ helper function create program course enrollment with no lms user """ - return ProgramCourseEnrollment.objects.create( - program_enrollment=self.program_enrollment, - course_key=self.course_key, - course_enrollment=None, + course_enrollment=self.course_enrollment, status="active" ) def test_change_status_no_enrollment(self): - program_course_enrollment = self._create_completed_program_course_enrollment() with LogCapture() as capture: - program_course_enrollment.course_enrollment = None - program_course_enrollment.change_status("inactive") + self.program_course_enrollment.course_enrollment = None + self.program_course_enrollment.change_status("inactive") expected_message = "User {} {} {} has no course_enrollment".format( self.user, self.program_enrollment, @@ -169,43 +146,12 @@ class ProgramCourseEnrollmentModelTests(TestCase): ) def test_change_status_not_active_or_inactive(self): - program_course_enrollment = self._create_completed_program_course_enrollment() with LogCapture() as capture: status = "potential-future-status-0123" - program_course_enrollment.change_status(status) + self.program_course_enrollment.change_status(status) message = ("Changed {} status to {}, not changing course_enrollment" " status because status is not 'active' or 'inactive'") - expected_message = message.format(program_course_enrollment, status) + expected_message = message.format(self.program_course_enrollment, status) capture.check( ('lms.djangoapps.program_enrollments.models', 'WARNING', expected_message) ) - - def test_enroll_new_course_enrollment(self): - program_course_enrollment = self._create_waiting_program_course_enrollment() - program_course_enrollment.enroll(self.user) - - course_enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_key) - self.assertEqual(course_enrollment.user, self.user) - self.assertEqual(course_enrollment.course.id, self.course_key) - self.assertEqual(course_enrollment.mode, CourseMode.MASTERS) - - @ddt.data( - (CourseMode.VERIFIED, CourseMode.VERIFIED), - (CourseMode.AUDIT, CourseMode.MASTERS), - (CourseMode.HONOR, CourseMode.MASTERS) - ) - @ddt.unpack - def test_enroll_existing_course_enrollment(self, original_mode, result_mode): - course_enrollment = CourseEnrollmentFactory.create( - course_id=self.course_key, - user=self.user, - mode=original_mode - ) - program_course_enrollment = self._create_waiting_program_course_enrollment() - - program_course_enrollment.enroll(self.user) - - course_enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_key) - self.assertEqual(course_enrollment.user, self.user) - self.assertEqual(course_enrollment.course.id, self.course_key) - self.assertEqual(course_enrollment.mode, result_mode) diff --git a/lms/djangoapps/program_enrollments/tests/test_signals.py b/lms/djangoapps/program_enrollments/tests/test_signals.py deleted file mode 100644 index 63b05efdb5..0000000000 --- a/lms/djangoapps/program_enrollments/tests/test_signals.py +++ /dev/null @@ -1,176 +0,0 @@ -""" -Unit tests for completing program course enrollments -once a social auth entry for the user is created. -""" -from django.test import TestCase -import mock -from opaque_keys.edx.keys import CourseKey -import pytest -from social_django.models import UserSocialAuth -from testfixtures import LogCapture - -from course_modes.models import CourseMode -from edx_django_utils.cache import RequestCache -from lms.djangoapps.program_enrollments.signals import logger -from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory -from organizations.tests.factories import OrganizationFactory -from openedx.core.djangoapps.content.course_overviews.models import CourseOverview -from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory -from student.models import CourseEnrollmentException -from student.tests.factories import CourseEnrollmentFactory, UserFactory -from third_party_auth.tests.factories import SAMLProviderConfigFactory - - -class SocialAuthEnrollmentCompletionSignalTest(TestCase): - """ - Test post-save handler on UserSocialAuth - """ - - def setUp(self): - super(SocialAuthEnrollmentCompletionSignalTest, self).setUp() - RequestCache.clear_all_namespaces() - - @classmethod - def setUpClass(cls): - super(SocialAuthEnrollmentCompletionSignalTest, cls).setUpClass() - - cls.external_id = '0000' - cls.provider_slug = 'uox' - cls.course_keys = [ - CourseKey.from_string('course-v1:edX+DemoX+Test_Course'), - CourseKey.from_string('course-v1:edX+DemoX+Another_Test_Course'), - ] - cls.organization = OrganizationFactory.create() - cls.user = UserFactory.create() - - for course_key in cls.course_keys: - CourseOverviewFactory(id=course_key) - SAMLProviderConfigFactory.create(organization=cls.organization, slug=cls.provider_slug) - - def _create_waiting_program_enrollment(self): - """ helper method to create a waiting program enrollment """ - return ProgramEnrollmentFactory.create( - user=None, - external_user_key=self.external_id - ) - - def _create_waiting_course_enrollments(self, program_enrollment): - """ helper method to create waiting course enrollments """ - return [ - ProgramCourseEnrollmentFactory( - program_enrollment=program_enrollment, - course_enrollment=None, - course_key=course_key - ) - for course_key in self.course_keys - ] - - def _assert_program_enrollment_user(self, program_enrollment): - """ validate program enrollment has a user """ - program_enrollment.refresh_from_db() - self.assertEqual(program_enrollment.user, self.user) - - def _assert_program_course_enrollment(self, program_course_enrollment, mode=CourseMode.MASTERS): - """ validate program course enrollment has a valid course enrollment """ - program_course_enrollment.refresh_from_db() - student_course_enrollment = program_course_enrollment.course_enrollment - self.assertEqual(student_course_enrollment.user, self.user) - self.assertEqual(student_course_enrollment.course.id, program_course_enrollment.course_key) - self.assertEqual(student_course_enrollment.mode, mode) - - def test_waiting_course_enrollments_completed(self): - program_enrollment = self._create_waiting_program_enrollment() - program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) - - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(self.provider_slug, self.external_id) - ) - - self._assert_program_enrollment_user(program_enrollment) - for program_course_enrollment in program_course_enrollments: - self._assert_program_course_enrollment(program_course_enrollment) - - def test_learner_already_enrolled_in_course(self): - course_key = self.course_keys[0] - course = CourseOverview.objects.get(id=course_key) - CourseEnrollmentFactory(user=self.user, course=course, mode=CourseMode.VERIFIED) - - program_enrollment = self._create_waiting_program_enrollment() - program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) - - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(self.provider_slug, self.external_id) - ) - - self._assert_program_enrollment_user(program_enrollment) - - duplicate_program_course_enrollment = program_course_enrollments[0] - self._assert_program_course_enrollment(duplicate_program_course_enrollment, CourseMode.VERIFIED) - - program_course_enrollment = program_course_enrollments[1] - self._assert_program_course_enrollment(program_course_enrollment) - - def test_enrolled_with_no_course_enrollments(self): - program_enrollment = self._create_waiting_program_enrollment() - - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(self.provider_slug, self.external_id) - ) - - self._assert_program_enrollment_user(program_enrollment) - - def test_create_social_auth_with_no_waiting_enrollments(self): - """ - No exceptions should be raised if there are no enrollments to update - """ - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(self.provider_slug, self.external_id) - ) - - def test_create_social_auth_provider_has_no_organization(self): - """ - No exceptions should be raised if provider is not linked to an organization - """ - provider = SAMLProviderConfigFactory.create() - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(provider.slug, self.external_id) - ) - - def test_create_social_auth_non_saml_provider(self): - """ - No exceptions should be raised for a non-SAML uid or if a SAML provider cannot be found - """ - UserSocialAuth.objects.create( - user=self.user, - uid='google-oauth-user@gmail.com' - ) - UserSocialAuth.objects.create( - user=self.user, - uid='123:123:123' - ) - - def test_log_on_enrollment_failure(self): - program_enrollment = self._create_waiting_program_enrollment() - program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) - - with mock.patch('student.models.CourseEnrollment.enroll') as enrollMock: - enrollMock.side_effect = CourseEnrollmentException('something has gone wrong') - with LogCapture(logger.name) as log: - with pytest.raises(CourseEnrollmentException): - UserSocialAuth.objects.create( - user=self.user, - uid='{0}:{1}'.format(self.provider_slug, self.external_id) - ) - error_tmpl = u'Failed to enroll user={} with waiting program_course_enrollment={}: {}' - log.check_present( - ( - logger.name, - 'WARNING', - error_tmpl.format(self.user.id, program_course_enrollments[0].id, 'something has gone wrong') - ) - )