Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source
This commit is contained in:
Calen Pennington
2017-08-04 11:41:44 -04:00
parent 599de36066
commit 84075efc81
7 changed files with 179 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
common/djangoapps/course_modes/admin.py
View File

@@ -59,6 +59,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
@@ -81,7 +84,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:
@@ -150,7 +153,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")
@@ -158,8 +161,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)
@@ -169,7 +175,7 @@ class CourseModeAdmin(admin.ModelAdmin):
form = CourseModeForm
fields = (
'course_id',
'course',
'mode_slug',
'mode_display_name',
'min_price',
@@ -180,11 +186,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',

47
common/djangoapps/course_modes/migrations/0008_course_key_field_to_foreign_key.py Normal file
View File

@@ -0,0 +1,47 @@
# -*- 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 = [
# Pin the name of the column in the database so that we can rename the field
# in Django without generating any sql changes
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'),
),
# Change the field name in Django to match our target field name
migrations.RenameField(
model_name='coursemode',
old_name='course_id',
new_name='course',
),
# Change the type of the field in Django to be a foreign key
# N.B. we don't need the db_column declaration because the default
# for Django is to use ${field_name}_id (which is what we pinned the column
# name to above).
# We deliberately leave db_constraint set to False because the column
# isn't currently constrained
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,
),
# Change the Django unique-together constraint (this is Django-level only
# since the database column constraint already exists).
migrations.AlterUniqueTogether(
name='coursemode',
unique_together=set([('course', 'mode_slug', 'currency')]),
),
]

33
common/djangoapps/course_modes/models.py
View File

@@ -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,20 @@ 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')
# Django sets the `course_id` property in __init__ with the value from the database
# This pair of properties converts that into a proper CourseKey
@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 +178,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):
"""
@@ -738,8 +752,6 @@ def get_course_prices(course, verified_only=False):
settings.PAID_COURSE_REGISTRATION_CURRENCY[0]
)
currency_symbol = settings.PAID_COURSE_REGISTRATION_CURRENCY[1]
if registration_price > 0:
price = registration_price
# Handle course overview objects which have no cosmetic_display_price
@@ -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):

8
common/djangoapps/course_modes/tests/test_admin.py
View File

@@ -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': unicode(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": unicode(self.course.id),
"mode_slug": mode,
"mode_display_name": mode,
"_expiration_datetime": upgrade_deadline,

66
common/djangoapps/student/migrations/0011_course_key_field_to_foreign_key.py Normal file
View File

@@ -0,0 +1,66 @@
# -*- 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 = [
# Pin the db_columns to the names already in the database
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'),
),
# Rename the fields in Django to the new names that we want them to have
migrations.RenameField(
model_name='courseenrollment',
old_name='course_id',
new_name='course',
),
migrations.RenameField(
model_name='historicalcourseenrollment',
old_name='course_id',
new_name='course',
),
# Alter the fields to make them ForeignKeys (leaving off the db_constraint so
# that we don't create it at migration time). The db_column is left off because
# it defaults to ${field_name}_id, which we pinned it to up above.
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,
),
# Set the Django-side unique-together and ordering configuration (no SQL required)
migrations.AlterModelOptions(
name='courseenrollment',
options={'ordering': ('user', 'course')},
),
migrations.AlterUniqueTogether(
name='courseenrollment',
unique_together=set([('user', 'course')]),
),
]

31
common/djangoapps/student/models.py
View File

@@ -992,10 +992,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
@@ -1017,8 +1031,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)
@@ -1181,7 +1195,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
@@ -1433,7 +1447,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:
@@ -1654,11 +1668,6 @@ class CourseEnrollment(models.Model):
def username(self):
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):
"""

2
lms/djangoapps/experiments/utils.py
View File

@@ -38,7 +38,7 @@ def get_experiment_user_metadata_context(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',