Convert CourseKeyField to ForeignKey(CourseOverview) on enrollments and course modes

common/djangoapps/course_modes/admin.py

@@ -60,6 +60,9 @@ class CourseModeForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super(CourseModeForm, self).__init__(*args, **kwargs)
 
+        if self.data.get('course'):
+            self.data['course'] = CourseKey.from_string(self.data['course'])
+
         default_tz = timezone(settings.TIME_ZONE)
 
         if self.instance._expiration_datetime:  # pylint: disable=protected-access
@@ -82,7 +85,7 @@ class CourseModeForm(forms.ModelForm):
             )
 
     def clean_course_id(self):
-        course_id = self.cleaned_data['course_id']
+        course_id = self.cleaned_data['course']
         try:
             course_key = CourseKey.from_string(course_id)
         except InvalidKeyError:
@@ -154,7 +157,7 @@ class CourseModeForm(forms.ModelForm):
         """
         # Trigger validation so we can access cleaned data
         if self.is_valid():
-            course_key = self.cleaned_data.get("course_id")
+            course = self.cleaned_data.get("course")
             verification_deadline = self.cleaned_data.get("verification_deadline")
             mode_slug = self.cleaned_data.get("mode_slug")
 
@@ -162,8 +165,11 @@ class CourseModeForm(forms.ModelForm):
             # we need to handle saving this ourselves.
             # Note that verification deadline can be `None` here if
             # the deadline is being disabled.
-            if course_key is not None and mode_slug in CourseMode.VERIFIED_MODES:
-                verification_models.VerificationDeadline.set_deadline(course_key, verification_deadline)
+            if course is not None and mode_slug in CourseMode.VERIFIED_MODES:
+                verification_models.VerificationDeadline.set_deadline(
+                    course.id,
+                    verification_deadline
+                )
 
         return super(CourseModeForm, self).save(commit=commit)
 
@@ -173,7 +179,7 @@ class CourseModeAdmin(admin.ModelAdmin):
     form = CourseModeForm
 
     fields = (
-        'course_id',
+        'course',
         'mode_slug',
         'mode_display_name',
         'min_price',
@@ -184,11 +190,11 @@ class CourseModeAdmin(admin.ModelAdmin):
         'bulk_sku'
     )
 
-    search_fields = ('course_id',)
+    search_fields = ('course',)
 
     list_display = (
         'id',
-        'course_id',
+        'course',
         'mode_slug',
         'min_price',
         'expiration_datetime_custom',

common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import openedx.core.djangoapps.xmodule_django.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_overviews', '0013_courseoverview_language'),
+        ('course_modes', '0007_coursemode_bulk_sku'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='coursemode',
+            name='course_id',
+            field=openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True, verbose_name="Course", db_column='course_id'),
+        ),
+        migrations.RenameField(
+            model_name='coursemode',
+            old_name='course_id',
+            new_name='course',
+        ),
+        migrations.AlterField(
+            model_name='coursemode',
+            name='course',
+            field=models.ForeignKey(related_name='modes', db_constraint=False, default=None, to='course_overviews.CourseOverview'),
+            preserve_default=False,
+        ),
+        migrations.AlterUniqueTogether(
+            name='coursemode',
+            unique_together=set([('course', 'mode_slug', 'currency')]),
+        ),
+    ]

common/djangoapps/course_modes/models.py

@@ -13,7 +13,9 @@ from django.db.models import Q
 from django.dispatch import receiver
 from django.utils.translation import ugettext_lazy as _
 from django.utils.encoding import force_text
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 
+from opaque_keys.edx.keys import CourseKey
 from openedx.core.djangoapps.xmodule_django.models import CourseKeyField
 from request_cache.middleware import RequestCache, ns_request_cached
 
@@ -39,8 +41,18 @@ class CourseMode(models.Model):
     class Meta(object):
         app_label = "course_modes"
 
-    # the course that this mode is attached to
-    course_id = CourseKeyField(max_length=255, db_index=True, verbose_name=_("Course"))
+    course = models.ForeignKey(CourseOverview, db_constraint=False, db_index=True, related_name='modes')
+
+    @property
+    def course_id(self):
+        return self._course_id
+
+    @course_id.setter
+    def course_id(self, value):
+        if isinstance(value, basestring):
+            self._course_id = CourseKey.from_string(value)
+        else:
+            self._course_id = value
 
     # the reference to this mode that can be used by Enrollments to generate
     # similar behavior for the same slug across courses
@@ -164,7 +176,7 @@ class CourseMode(models.Model):
     CACHE_NAMESPACE = u"course_modes.CourseMode.cache."
 
     class Meta(object):
-        unique_together = ('course_id', 'mode_slug', 'currency')
+        unique_together = ('course', 'mode_slug', 'currency')
 
     def clean(self):
         """
@@ -748,6 +760,15 @@ def get_course_prices(course, verified_only=False):
     else:
         price = None
 
+    return registration_price, format_course_price(price)
+
+
+def format_course_price(price):
+    """
+    Return a formatted price for a course (a string preceded by correct currency, or 'Free').
+    """
+    currency_symbol = settings.PAID_COURSE_REGISTRATION_CURRENCY[1]
+
     if price:
         # Translators: This will look like '$50', where {currency_symbol} is a symbol such as '$' and {price} is a
         # numerical amount in that currency. Adjust this display as needed for your language.
@@ -756,7 +777,7 @@ def get_course_prices(course, verified_only=False):
         # Translators: This refers to the cost of the course. In this case, the course costs nothing so it is free.
         cosmetic_display_price = _('Free')
 
-    return registration_price, force_text(cosmetic_display_price)
+    return cosmetic_display_price
 
 
 class CourseModesArchive(models.Model):

common/djangoapps/course_modes/tests/test_admin.py

@@ -12,6 +12,7 @@ from pytz import UTC, timezone
 from course_modes.admin import CourseModeForm
 from course_modes.models import CourseMode
 from course_modes.tests.factories import CourseModeFactory
+from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 # Technically, we shouldn't be importing verify_student, since it's
 # defined in the LMS and course_modes is in common.  However, the benefits
 # of putting all this configuration in one place outweigh the downsides.
@@ -40,16 +41,16 @@ class AdminCourseModePageTest(ModuleStoreTestCase):
         user.save()
         course = CourseFactory.create()
         expiration = datetime(2015, 10, 20, 1, 10, 23, tzinfo=timezone(settings.TIME_ZONE))
+        CourseOverview.load_from_module_store(course.id)
 
         data = {
-            'course_id': unicode(course.id),
+            'course': course.id,
             'mode_slug': 'verified',
             'mode_display_name': 'verified',
             'min_price': 10,
             'currency': 'usd',
             '_expiration_datetime_0': expiration.date(),  # due to django admin datetime widget passing as separate vals
             '_expiration_datetime_1': expiration.time(),
-
         }
 
         self.client.login(username=user.username, password='test')
@@ -89,6 +90,7 @@ class AdminCourseModeFormTest(ModuleStoreTestCase):
         """
         super(AdminCourseModeFormTest, self).setUp()
         self.course = CourseFactory.create()
+        CourseOverview.load_from_module_store(self.course.id)
 
     @ddt.data(
         ("honor", False),
@@ -197,7 +199,7 @@ class AdminCourseModeFormTest(ModuleStoreTestCase):
             mode_slug=mode,
         )
         return CourseModeForm({
-            "course_id": unicode(self.course.id),
+            "course": self.course.id,
             "mode_slug": mode,
             "mode_display_name": mode,
             "_expiration_datetime": upgrade_deadline,

common/djangoapps/student/migrations/0011_course_key_field_to_foreign_key.py

+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+import openedx.core.djangoapps.xmodule_django.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('course_overviews', '0013_courseoverview_language'),
+        ('student', '0010_auto_20170207_0458'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='courseenrollment',
+            name='course_id',
+            field=openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True, db_column='course_id'),
+        ),
+        migrations.AlterField(
+            model_name='historicalcourseenrollment',
+            name='course_id',
+            field=openedx.core.djangoapps.xmodule_django.models.CourseKeyField(max_length=255, db_index=True, db_column='course_id'),
+        ),
+        migrations.RenameField(
+            model_name='courseenrollment',
+            old_name='course_id',
+            new_name='course',
+        ),
+        migrations.RenameField(
+            model_name='historicalcourseenrollment',
+            old_name='course_id',
+            new_name='course',
+        ),
+        migrations.AlterField(
+            model_name='courseenrollment',
+            name='course',
+            field=models.ForeignKey(db_constraint=False, to='course_overviews.CourseOverview'),
+            preserve_default=True,
+        ),
+        migrations.AlterField(
+            model_name='historicalcourseenrollment',
+            name='course',
+            field=models.ForeignKey(related_name='+', on_delete=django.db.models.deletion.DO_NOTHING, db_constraint=False, blank=True, to='course_overviews.CourseOverview', null=True),
+            preserve_default=True,
+        ),
+
+        migrations.AlterModelOptions(
+            name='courseenrollment',
+            options={'ordering': ('user', 'course')},
+        ),
+        migrations.AlterUniqueTogether(
+            name='courseenrollment',
+            unique_together=set([('user', 'course')]),
+        ),
+    ]

common/djangoapps/student/models.py

@@ -993,10 +993,24 @@ class CourseEnrollment(models.Model):
     checking course dates, user permissions, etc.) This logic is currently
     scattered across our views.
     """
-    MODEL_TAGS = ['course_id', 'is_active', 'mode']
+    MODEL_TAGS = ['course', 'is_active', 'mode']
 
     user = models.ForeignKey(User)
-    course_id = CourseKeyField(max_length=255, db_index=True)
+
+    course = models.ForeignKey(CourseOverview, db_constraint=False)
+
+    @property
+    def course_id(self):
+        return self._course_id
+
+    @course_id.setter
+    def course_id(self, value):
+        if isinstance(value, basestring):
+            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
@@ -1018,8 +1032,8 @@ class CourseEnrollment(models.Model):
     MODE_CACHE_NAMESPACE = u'CourseEnrollment.mode_and_active'
 
     class Meta(object):
-        unique_together = (('user', 'course_id'),)
-        ordering = ('user', 'course_id')
+        unique_together = (('user', 'course'),)
+        ordering = ('user', 'course')
 
     def __init__(self, *args, **kwargs):
         super(CourseEnrollment, self).__init__(*args, **kwargs)
@@ -1182,7 +1196,7 @@ class CourseEnrollment(models.Model):
                                   cost=cost, currency=currency)
 
     @classmethod
-    def send_signal_full(cls, event, user=user, mode=mode, course_id=course_id, cost=None, currency=None):
+    def send_signal_full(cls, event, user=user, mode=mode, course_id=None, cost=None, currency=None):
         """
         Sends a signal announcing changes in course enrollment status.
         This version should be used if you don't already have a CourseEnrollment object
@@ -1434,7 +1448,7 @@ class CourseEnrollment(models.Model):
         try:
             return cls.objects.filter(
                 user=user,
-                course_id__startswith=querystring,
+                course__id__startswith=querystring,
                 is_active=1
             ).exists()
         except cls.DoesNotExist:
@@ -1656,11 +1670,6 @@ class CourseEnrollment(models.Model):
         return self.user.username
 
     @property
-    def course(self):
-        # Deprecated. Please use the `course_overview` property instead.
-        return self.course_overview
-
-    @property
     def course_overview(self):
         """
         Returns a CourseOverview of the course to which this enrollment refers.

lms/djangoapps/experiments/utils.py

@@ -39,7 +39,7 @@ def get_experiment_user_metadata_context(request, course, user):
 
     return {
         'upgrade_link': upgrade_data and upgrade_data.link,
-        'upgrade_price': get_cosmetic_verified_display_price(course),
+        'upgrade_price': unicode(get_cosmetic_verified_display_price(course)),
         'enrollment_mode': enrollment_mode,
         'enrollment_time': enrollment_time,
         'pacing_type': 'self_paced' if course.self_paced else 'instructor_paced',