Merge pull request #23583 from edx/feanil/test_propert_removal

WIP Remove the course_id property on student.CourseEnrollment.

common/djangoapps/student/models.py

@@ -13,6 +13,7 @@ file and check it in at the same time as your model changes. To do that,
 
 
 import hashlib
+import inspect
 import json
 import logging
 import uuid
@@ -1094,17 +1095,6 @@ class CourseEnrollment(models.Model):
     def course_price(self):
         return get_cosmetic_verified_display_price(self.course)
 
-    @property
-    def course_id(self):
-        return self._course_id
-
-    @course_id.setter
-    def course_id(self, value):
-        if isinstance(value, six.string_types):
-            self._course_id = CourseKey.from_string(value)
-        else:
-            self._course_id = value
-
     created = models.DateTimeField(auto_now_add=True, null=True, db_index=True)
 
     # If is_active is False, then the student is not considered to be enrolled
@@ -1136,6 +1126,13 @@ class CourseEnrollment(models.Model):
         ordering = ('user', 'course')
 
     def __init__(self, *args, **kwargs):
+        if 'course_id' in kwargs:
+            course_id = kwargs['course_id']
+            if isinstance(course_id, str):
+                kwargs['course_id'] = CourseKey.from_string(course_id)
+                call_location = "\n".join("%30s : %s:%d" % (t[3], t[1], t[2]) for t in inspect.stack()[::-1])
+                log.warning("Forced to coerce course_id in CourseEnrollment instantiation: %s", call_location)
+
         super(CourseEnrollment, self).__init__(*args, **kwargs)
 
         # Private variable for storing course_overview to minimize calls to the database.

common/djangoapps/student/tests/factories.py

@@ -142,6 +142,11 @@ class CourseEnrollmentFactory(DjangoModelFactory):
             course_id = kwargs.get('course_id')
             course_overview = None
             if course_id is not None:
+                # 'course_id' is not needed by the model when course is passed.
+                # This arg used to be called course_id before we added the CourseOverview
+                # foreign key constraint to CourseEnrollment.
+                del kwargs['course_id']
+
                 if isinstance(course_id, six.string_types):
                     course_id = CourseKey.from_string(course_id)
                     course_kwargs.setdefault('id', course_id)
@@ -152,6 +157,9 @@ class CourseEnrollmentFactory(DjangoModelFactory):
                     pass
 
             if course_overview is None:
+                if 'id' not in course_kwargs and course_id:
+                    course_kwargs['id'] = course_id
+
                 course_overview = CourseOverviewFactory(**course_kwargs)
             kwargs['course'] = course_overview
 

common/djangoapps/student/tests/test_views.py

@@ -341,7 +341,7 @@ class StudentDashboardTests(SharedModuleStoreTestCase, MilestonesTestCaseMixin,
 
         # If an entitlement has already been redeemed by the user for a course run, do not let the run be selectable
         enrollment = CourseEnrollmentFactory(
-            user=self.user, course_id=six.text_type(mock_course_overview.return_value.id), mode=CourseMode.VERIFIED
+            user=self.user, course=mock_course_overview.return_value, mode=CourseMode.VERIFIED
         )
         CourseEntitlementFactory.create(
             user=self.user, course_uuid=program['courses'][0]['uuid'], enrollment_course_run=enrollment

lms/djangoapps/program_enrollments/rest_api/v1/tests/test_views.py

@@ -1728,13 +1728,6 @@ class ProgramCourseEnrollmentOverviewGetTests(
             user=self.student,
         )
 
-        # create course enrollment
-        self.course_enrollment = CourseEnrollmentFactory.create(
-            course_id=self.course_id,
-            user=self.student,
-            mode=CourseMode.MASTERS,
-        )
-
         # create course overview
         self.course_overview = CourseOverviewFactory.create(
             id=self.course_id,
@@ -1742,6 +1735,13 @@ class ProgramCourseEnrollmentOverviewGetTests(
             end=self.tomorrow,
         )
 
+        # create course enrollment
+        self.course_enrollment = CourseEnrollmentFactory.create(
+            course=self.course_overview,
+            user=self.student,
+            mode=CourseMode.MASTERS,
+        )
+
         # create program course enrollment
         self.program_course_enrollment = ProgramCourseEnrollmentFactory.create(
             program_enrollment=self.program_enrollment,

lms/templates/dashboard.html

@@ -183,7 +183,7 @@ from student.models import CourseEnrollment
                     pseudo_session = unfulfilled_entitlement_pseudo_sessions[str(entitlement.uuid)]
                     if not pseudo_session:
                         continue
-                    enrollment = CourseEnrollment(user=user, course_id=pseudo_session['key'], mode=pseudo_session['type'])
+                    enrollment = CourseEnrollment(user=user, course=CourseOverview.get_from_id(pseudo_session['key']), mode=pseudo_session['type'])
                   # We only show email settings for entitlement cards if the entitlement has an associated enrollment
                   show_email_settings = is_fulfilled_entitlement and (entitlement_session.course_id in show_email_settings_for)
                 else:

openedx/core/djangoapps/catalog/tests/test_utils.py

@@ -592,7 +592,7 @@ class TestSessionEntitlement(CatalogIntegrationMixin, TestCase):
         course_overview = CourseOverviewFactory.create(id=course_key, start=self.tomorrow)
         CourseModeFactory.create(mode_slug=CourseMode.VERIFIED, min_price=100, course_id=course_overview.id)
         course_enrollment = CourseEnrollmentFactory(
-            user=self.user, course_id=six.text_type(course_overview.id), mode=CourseMode.VERIFIED
+            user=self.user, course=course_overview, mode=CourseMode.VERIFIED
         )
         entitlement = CourseEntitlementFactory(
             user=self.user, enrollment_course_run=course_enrollment, mode=CourseMode.VERIFIED
@@ -617,7 +617,7 @@ class TestSessionEntitlement(CatalogIntegrationMixin, TestCase):
             expiration_datetime=now() - timedelta(days=1)
         )
         course_enrollment = CourseEnrollmentFactory(
-            user=self.user, course_id=six.text_type(course_overview.id), mode=CourseMode.VERIFIED
+            user=self.user, course=course_overview, mode=CourseMode.VERIFIED
         )
         entitlement = CourseEntitlementFactory(
             user=self.user, enrollment_course_run=course_enrollment, mode=CourseMode.VERIFIED
@@ -643,7 +643,7 @@ class TestSessionEntitlement(CatalogIntegrationMixin, TestCase):
             expiration_datetime=now() - timedelta(days=1)
         )
         course_enrollment = CourseEnrollmentFactory(
-            user=self.user, course_id=six.text_type(course_overview.id), mode=CourseMode.VERIFIED
+            user=self.user, course=course_overview, mode=CourseMode.VERIFIED
         )
         entitlement = CourseEntitlementFactory(
             user=self.user, enrollment_course_run=course_enrollment, mode=CourseMode.VERIFIED

openedx/core/djangoapps/enrollments/tests/test_data.py

@@ -27,7 +27,7 @@ from openedx.core.djangoapps.enrollments.errors import (
 from openedx.core.djangoapps.enrollments.serializers import CourseEnrollmentSerializer
 from openedx.core.lib.exceptions import CourseNotFoundError
 from student.models import AlreadyEnrolledError, CourseEnrollment, CourseFullError, EnrollmentClosedError
-from student.tests.factories import CourseAccessRoleFactory, UserFactory
+from student.tests.factories import CourseAccessRoleFactory, UserFactory, CourseEnrollmentFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
@@ -145,12 +145,15 @@ class EnrollmentDataTest(ModuleStoreTestCase):
         # not exist in database) for the user and check that the method
         # 'get_course_enrollments' ignores course enrollments for invalid
         # or deleted courses
-        CourseEnrollment.objects.create(
+        non_existent_course_id = 'InvalidOrg/InvalidCourse/InvalidRun'
+        enrollement = CourseEnrollmentFactory.create(
             user=self.user,
-            course_id='InvalidOrg/InvalidCourse/InvalidRun',
+            course_id=non_existent_course_id,
             mode='honor',
             is_active=True
         )
+        enrollement.course.delete()
+
         updated_results = data.get_course_enrollments(self.user.username)
         self.assertEqual(results, updated_results)
 

openedx/core/djangoapps/external_user_ids/tests/test_signals.py

@@ -12,7 +12,7 @@ from openedx.core.djangoapps.catalog.tests.factories import (
     CourseFactory,
     ProgramFactory,
 )
-from student.tests.factories import TEST_PASSWORD, UserFactory
+from student.tests.factories import TEST_PASSWORD, UserFactory, CourseEnrollmentFactory
 from openedx.core.djangoapps.catalog.cache import (
     CATALOG_COURSE_PROGRAMS_CACHE_KEY_TPL,
     COURSE_PROGRAMS_CACHE_KEY_TPL,
@@ -93,7 +93,7 @@ class MicrobachelorsExternalIDTest(ModuleStoreTestCase, CacheIsolationTestCase):
         course_run_key = self.program['courses'][0]['course_runs'][0]['key']
 
         # Enroll user
-        enrollment = CourseEnrollment.objects.create(
+        enrollment = CourseEnrollmentFactory.create(
             course_id=course_run_key,
             user=self.user,
             mode=CourseMode.VERIFIED,
@@ -110,7 +110,7 @@ class MicrobachelorsExternalIDTest(ModuleStoreTestCase, CacheIsolationTestCase):
         course_run_key2 = self.program['courses'][1]['course_runs'][0]['key']
 
         # Enroll user
-        CourseEnrollment.objects.create(
+        CourseEnrollmentFactory.create(
             course_id=course_run_key1,
             user=self.user,
             mode=CourseMode.VERIFIED,
@@ -122,7 +122,7 @@ class MicrobachelorsExternalIDTest(ModuleStoreTestCase, CacheIsolationTestCase):
         assert external_id.external_id_type.name == ExternalIdType.MICROBACHELORS_COACHING
         original_external_user_uuid = external_id.external_user_id
 
-        CourseEnrollment.objects.create(
+        CourseEnrollmentFactory.create(
             course_id=course_run_key2,
             user=self.user,
             mode=CourseMode.VERIFIED,