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

Merge pull request #19673 from cpennington/fbe-per-course-config

Browse Source 
Allow overriding Content Type Gating and Course Duration Limits on a …
This commit is contained in:
Calen Pennington
2019-03-12 12:05:22 -04:00
committed by GitHub
parent ac38d5b413 fa2ee761d9
commit 27dfbd0240
5 changed files with 195 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

152
openedx/core/djangoapps/config_model_utils/models.py
View File

@@ -29,7 +29,8 @@ class Provenance(Enum):
"""
Provenance enum
"""
course = 'Course'
run = 'Course Run'
org_course = 'Org/Course'
org = 'Org'
site = 'Site'
global_ = 'Global'
@@ -38,7 +39,7 @@ class Provenance(Enum):
class StackedConfigurationModel(ConfigurationModel):
"""
A ConfigurationModel that stacks Global, Site, Org, and Course level
A ConfigurationModel that stacks Global, Site, Org, Course, and Course Run level
configuration values.
"""
class Meta(object):
@@ -46,24 +47,58 @@ class StackedConfigurationModel(ConfigurationModel):
indexes = [
# This index optimizes the .object.current_set() query
# by preventing a filesort
models.Index(fields=['site', 'org', 'course'])
models.Index(fields=['site', 'org', 'course']),
models.Index(fields=['site', 'org', 'org_course', 'course'])
]
KEY_FIELDS = ('site', 'org', 'course')
KEY_FIELDS = ('site', 'org', 'org_course', 'course')
STACKABLE_FIELDS = ('enabled',)
enabled = models.NullBooleanField(default=None, verbose_name=_("Enabled"))
site = models.ForeignKey(Site, on_delete=models.CASCADE, null=True, blank=True)
org = models.CharField(max_length=255, db_index=True, null=True, blank=True)
site = models.ForeignKey(
Site,
on_delete=models.CASCADE,
null=True,
blank=True,
help_text=_(
"Configure values for all course runs associated with this site."
),
)
org = models.CharField(
max_length=255,
db_index=True,
null=True,
blank=True,
help_text=_(
"Configure values for all course runs associated with this "
"Organization. This is the organization string (i.e. edX, MITx)."
)
)
org_course = models.CharField(
max_length=255,
db_index=True,
null=True,
blank=True,
verbose_name=_("Course in Org"),
help_text=_(
"Configure values for all course runs associated with this course. "
"This is should be formatted as 'org/course' (i.e. MITx/6.002x, HarvardX/CS50)."
),
)
course = models.ForeignKey(
CourseOverview,
on_delete=models.DO_NOTHING,
null=True,
blank=True,
verbose_name=_("Course Run"),
help_text=_(
"Configure values for this course run. This should be "
"formatted as the CourseKey (i.e. course-v1://MITx+6.002x+2019_Q1)"
),
)
@classmethod
def current(cls, site=None, org=None, course_key=None): # pylint: disable=arguments-differ
def current(cls, site=None, org=None, org_course=None, course_key=None): # pylint: disable=arguments-differ
"""
Return the current overridden configuration at the specified level.
@@ -74,8 +109,9 @@ class StackedConfigurationModel(ConfigurationModel):
Global: Feature Disabled
edx.org (Site): Feature Enabled
Harvard (org): Feature Disabled
CS50 (course): Feature Enabled
HarvardX (org): Feature Disabled
HarvardX/CS50 (org_course): Feature Enabled
CS50 in 2018_Q1 (course run): Feature Disabled
Assuming these values had been properly configured, these calls would result
@@ -84,10 +120,15 @@ class StackedConfigurationModel(ConfigurationModel):
MyStackedConfig.current(site=Site(domain='whitelabel.edx.org')) # False -- value derived from global setting
MyStackedConfig.current(org='HarvardX') # False
MyStackedConfig.current(org='MITx') # True -- value derived from edx.org site
MyStackedConfig.current(course=CourseKey(org='HarvardX', course='CS50', run='2018_Q1')) # True
MyStackedConfig.current(org_course='HarvardX/CS50') # True
MyStackedConfig.current(org_course='HarvardX/Bio101') # False -- value derived from HarvardX setting
MyStackedConfig.current(course_key=CourseKey(org='HarvardX', course='CS50', run='2018_Q1')) # False
MyStackedConfig.current(
course_key=CourseKey(org='HarvardX', course='CS50', run='2019_Q1')
) # True -- value derived from HarvardX/CS50 setting
cs50 = CourseKey(org='HarvardX', course='Bio101', run='2018_Q1')
MyStackedConfig.current(course=cs50) # False -- value derived from HarvardX org
bio101 = CourseKey(org='HarvardX', course='Bio101', run='2018_Q1')
MyStackedConfig.current(course_key=cs50) # False -- value derived from HarvardX org
The following calls would result in errors due to overspecification:
@@ -98,25 +139,29 @@ class StackedConfigurationModel(ConfigurationModel):
Arguments:
site: The Site to check current values for
org: The org to check current values for
course: The course to check current values for
org_course: The course in a specific org to check current values for
course_key: The course to check current values for
Returns:
An instance of :class:`cls.attribute_tuple()` with the overridden values
specified down to the level of the supplied argument (or global values if
no arguments are supplied).
"""
cache_key_name = cls.cache_key_name(site, org, course_key=course_key)
cache_key_name = cls.cache_key_name(site, org, org_course, course_key)
cached = cache.get(cache_key_name)
if cached is not None:
return cached
# Raise an error if more than one of site/org/course are specified simultaneously.
if len([arg for arg in [site, org, course_key] if arg is not None]) > 1:
raise ValueError("Only one of site, org, and course can be specified")
if len([arg for arg in [site, org, org_course, course_key] if arg is not None]) > 1:
raise ValueError("Only one of site, org, org_course, and course can be specified")
if org is None and course_key is not None:
org = cls._org_from_course_key(course_key)
if org_course is None and course_key is not None:
org_course = cls._org_course_from_course_key(course_key)
if org is None and org_course is not None:
org = cls._org_from_org_course(org_course)
if site is None and org is not None:
site = cls._site_from_org(org)
@@ -129,34 +174,48 @@ class StackedConfigurationModel(ConfigurationModel):
values = field_defaults.copy()
global_override_q = Q(site=None, org=None, course_id=None)
site_override_q = Q(site=site, org=None, course_id=None)
org_override_q = Q(site=None, org=org, course_id=None)
course_override_q = Q(site=None, org=None, course_id=course_key)
global_override_q = Q(site=None, org=None, org_course=None, course_id=None)
site_override_q = Q(site=site, org=None, org_course=None, course_id=None)
org_override_q = Q(site=None, org=org, org_course=None, course_id=None)
org_course_override_q = Q(site=None, org=None, org_course=org_course, course_id=None)
course_override_q = Q(site=None, org=None, org_course=None, course_id=course_key)
overrides = cls.objects.current_set().filter(
global_override_q |
site_override_q |
org_override_q |
org_course_override_q |
course_override_q
)
provenances = defaultdict(lambda: Provenance.default)
# We are sorting in python to avoid doing a filesort in the database for
# what will only be 4 rows at maximum
for override in sorted(
overrides,
# This particular sort order sorts None before not-None (because False < True)
# It sorts global first (because all entries are None), then site entries
# (because course_id and org are None), then org and course (by the same logic)
key=lambda o: (o.course_id is not None, o.org is not None, o.site_id is not None)
):
def sort_key(override):
"""
Sort overrides in increasing specificity.
This particular sort order sorts None before not-None (because False < True)
It sorts global first (because all entries are None), then site entries
(because course_id and org are None), then org, org_course and course (by the same logic)
"""
return (
override.course_id is not None,
override.org_course is not None,
override.org is not None,
override.site_id is not None
)
for override in sorted(overrides, key=sort_key):
for field in stackable_fields:
value = field.value_from_object(override)
if value != field_defaults[field.name]:
values[field.name] = value
if override.course_id is not None:
provenances[field.name] = Provenance.course
provenances[field.name] = Provenance.run
elif override.org_course is not None:
provenances[field.name] = Provenance.org_course
elif override.org is not None:
provenances[field.name] = Provenance.org
elif override.site_id is not None:
@@ -197,7 +256,7 @@ class StackedConfigurationModel(ConfigurationModel):
all_overrides = cls.objects.current_set()
overrides = {
(override.site_id, override.org, override.course_id): override
(override.site_id, override.org, override.org_course, override.course_id): override
for override in all_overrides
}
@@ -211,11 +270,15 @@ class StackedConfigurationModel(ConfigurationModel):
"""
Return provenance for given field
"""
org_course = cls._org_course_from_course_key(course.id)
org = cls._org_from_org_course(org_course)
for (config_key, provenance) in [
((None, None, course.id), Provenance.course),
((None, course.id.org, None), Provenance.org),
((sites_by_org[course.id.org].id, None, None), Provenance.site),
((None, None, None), Provenance.global_),
((None, None, None, course.id), Provenance.run),
((None, None, org_course, None), Provenance.org_course),
((None, org, None, None), Provenance.org),
((sites_by_org[course.id.org].id, None, None, None), Provenance.site),
((None, None, None, None), Provenance.global_),
]:
config = overrides.get(config_key)
if config is None:
@@ -235,22 +298,21 @@ class StackedConfigurationModel(ConfigurationModel):
}
@classmethod
def cache_key_name(cls, site, org, course=None, course_key=None): # pylint: disable=arguments-differ
if course is not None and course_key is not None:
raise ValueError("Only one of course and course_key can be specified at a time")
if course is not None:
course_key = course
def cache_key_name(cls, site, org, org_course, course_key): # pylint: disable=arguments-differ
if site is None:
site_id = None
else:
site_id = site.id
return super(StackedConfigurationModel, cls).cache_key_name(site_id, org, course_key)
return super(StackedConfigurationModel, cls).cache_key_name(site_id, org, org_course, course_key)
@classmethod
def _org_from_course_key(cls, course_key):
return course_key.org
def _org_from_org_course(cls, org_course):
return org_course.partition('+')[0]
@classmethod
def _org_course_from_course_key(cls, course_key):
return u"{}+{}".format(course_key.org, course_key.course)
@classmethod
@request_cached()
@@ -267,7 +329,7 @@ class StackedConfigurationModel(ConfigurationModel):
def clean(self):
# fail validation if more than one of site/org/course are specified simultaneously
if len([arg for arg in [self.site, self.org, self.course] if arg is not None]) > 1:
if len([arg for arg in [self.site, self.org, self.org_course, self.course] if arg is not None]) > 1:
raise ValidationError(
_('Configuration may not be specified at more than one level at once.')
)

40
openedx/features/content_type_gating/migrations/0007_auto_20190311_1919.py Normal file
View File

@@ -0,0 +1,40 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-03-11 19:19
from __future__ import unicode_literals
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('content_type_gating', '0006_auto_20190308_1447'),
]
operations = [
migrations.AddField(
model_name='contenttypegatingconfig',
name='org_course',
field=models.CharField(blank=True, db_index=True, help_text="Configure values for all course runs associated with this course. This is should be formatted as 'org/course' (i.e. MITx/6.002x, HarvardX/CS50).", max_length=255, null=True, verbose_name='Course in Org'),
),
migrations.AlterField(
model_name='contenttypegatingconfig',
name='course',
field=models.ForeignKey(blank=True, help_text='Configure values for this course run. This should be formatted as the CourseKey (i.e. course-v1://MITx+6.002x+2019_Q1)', null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview', verbose_name='Course Run'),
),
migrations.AlterField(
model_name='contenttypegatingconfig',
name='org',
field=models.CharField(blank=True, db_index=True, help_text='Configure values for all course runs associated with this Organization. This is the organization string (i.e. edX, MITx).', max_length=255, null=True),
),
migrations.AlterField(
model_name='contenttypegatingconfig',
name='site',
field=models.ForeignKey(blank=True, help_text='Configure values for all course runs associated with this site.', null=True, on_delete=django.db.models.deletion.CASCADE, to='sites.Site'),
),
migrations.AddIndex(
model_name='contenttypegatingconfig',
index=models.Index(fields=['site', 'org', 'org_course', 'course'], name='content_typ_site_id_650310_idx'),
),
]

8
openedx/features/content_type_gating/tests/test_models.py
View File

@@ -202,15 +202,15 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
all_configs[CourseLocator('7-True', 'test_course', 'run-None')],
{
'enabled': (True, Provenance.org),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
'studio_override_enabled': (None, Provenance.default),
}
)
self.assertEqual(
all_configs[CourseLocator('7-True', 'test_course', 'run-False')],
{
'enabled': (False, Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled': (False, Provenance.run),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
'studio_override_enabled': (None, Provenance.default),
}
)
@@ -218,7 +218,7 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase):
all_configs[CourseLocator('7-None', 'test_course', 'run-None')],
{
'enabled': (True, Provenance.site),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
'studio_override_enabled': (None, Provenance.default),
}
)

40
openedx/features/course_duration_limits/migrations/0007_auto_20190311_1919.py Normal file
View File

@@ -0,0 +1,40 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-03-11 19:19
from __future__ import unicode_literals
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('course_duration_limits', '0006_auto_20190308_1447'),
]
operations = [
migrations.AddField(
model_name='coursedurationlimitconfig',
name='org_course',
field=models.CharField(blank=True, db_index=True, help_text="Configure values for all course runs associated with this course. This is should be formatted as 'org/course' (i.e. MITx/6.002x, HarvardX/CS50).", max_length=255, null=True, verbose_name='Course in Org'),
),
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='course',
field=models.ForeignKey(blank=True, help_text='Configure values for this course run. This should be formatted as the CourseKey (i.e. course-v1://MITx+6.002x+2019_Q1)', null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='course_overviews.CourseOverview', verbose_name='Course Run'),
),
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='org',
field=models.CharField(blank=True, db_index=True, help_text='Configure values for all course runs associated with this Organization. This is the organization string (i.e. edX, MITx).', max_length=255, null=True),
),
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='site',
field=models.ForeignKey(blank=True, help_text='Configure values for all course runs associated with this site.', null=True, on_delete=django.db.models.deletion.CASCADE, to='sites.Site'),
),
migrations.AddIndex(
model_name='coursedurationlimitconfig',
index=models.Index(fields=['site', 'org', 'org_course', 'course'], name='course_dura_site_id_b5bbcd_idx'),
),
]

8
openedx/features/course_duration_limits/tests/test_models.py
View File

@@ -232,21 +232,21 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase):
all_configs[CourseLocator('7-True', 'test_course', 'run-None')],
{
'enabled': (True, Provenance.org),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
}
)
self.assertEqual(
all_configs[CourseLocator('7-True', 'test_course', 'run-False')],
{
'enabled': (False, Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled': (False, Provenance.run),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
}
)
self.assertEqual(
all_configs[CourseLocator('7-None', 'test_course', 'run-None')],
{
'enabled': (True, Provenance.site),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.course),
'enabled_as_of': (datetime(2018, 1, 1, 5, tzinfo=pytz.UTC), Provenance.run),
}
)