From b32ed9ff2b4f60cc16319e6fb765fa1fafcf54f9 Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Wed, 22 May 2019 15:50:49 -0400 Subject: [PATCH] add discount restriction stacked config models --- openedx/features/discounts/admin.py | 45 ++++++++ openedx/features/discounts/applicability.py | 5 + .../discounts/migrations/0001_initial.py | 46 ++++++++ .../features/discounts/migrations/__init__.py | 0 openedx/features/discounts/models.py | 35 ++++++ .../discounts/tests/test_applicability.py | 8 ++ .../tests/test_discount_restriction_models.py | 103 ++++++++++++++++++ 7 files changed, 242 insertions(+) create mode 100644 openedx/features/discounts/admin.py create mode 100644 openedx/features/discounts/migrations/0001_initial.py create mode 100644 openedx/features/discounts/migrations/__init__.py create mode 100644 openedx/features/discounts/models.py create mode 100644 openedx/features/discounts/tests/test_discount_restriction_models.py diff --git a/openedx/features/discounts/admin.py b/openedx/features/discounts/admin.py new file mode 100644 index 0000000000..4996d83099 --- /dev/null +++ b/openedx/features/discounts/admin.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +""" +Django Admin pages for DiscountRestrictionConfig. +""" + +from __future__ import absolute_import, unicode_literals + +from django.contrib import admin +from django.utils.translation import ugettext_lazy as _ + +from openedx.core.djangoapps.config_model_utils.admin import StackedConfigModelAdmin + +from .models import DiscountRestrictionConfig + + +class DiscountRestrictionConfigAdmin(StackedConfigModelAdmin): + """ + Admin to configure discount restrictions + """ + fieldsets = ( + ('Context', { + 'fields': DiscountRestrictionConfig.KEY_FIELDS, + 'description': _( + 'These define the context to enable lms-controlled discounts on. ' + 'If no values are set, then the configuration applies globally. ' + 'If a single value is set, then the configuration applies to all courses ' + 'within that context. At most one value can be set at a time.
' + 'If multiple contexts apply to a course (for example, if configuration ' + 'is specified for the course specifically, and for the org that the course ' + 'is in, then the more specific context overrides the more general context.' + ), + }), + ('Configuration', { + 'fields': ('enabled',), + 'description': _( + 'If any of these values is left empty or "Unknown", then their value ' + 'at runtime will be retrieved from the next most specific context that applies. ' + 'For example, if "Enabled" is left as "Unknown" in the course context, then that ' + 'course will be Enabled only if the org that it is in is Enabled.' + ), + }) + ) + raw_id_fields = ('course',) + +admin.site.register(DiscountRestrictionConfig, DiscountRestrictionConfigAdmin) diff --git a/openedx/features/discounts/applicability.py b/openedx/features/discounts/applicability.py index 43114a8248..b3c7b88bc5 100644 --- a/openedx/features/discounts/applicability.py +++ b/openedx/features/discounts/applicability.py @@ -10,6 +10,7 @@ not other discounts like coupons or enterprise/program offers configured in ecom """ from course_modes.models import CourseMode from openedx.core.djangoapps.waffle_utils import WaffleFlag, WaffleFlagNamespace +from openedx.features.discounts.models import DiscountRestrictionConfig # .. feature_toggle_name: discounts.enable_discounting # .. feature_toggle_type: flag @@ -50,6 +51,10 @@ def can_receive_discount(user, course): # pylint: disable=unused-argument if not verified_mode: return False + # Site, Partner, Course or Course Run not excluded from lms-controlled discounts + if not DiscountRestrictionConfig.enabled_for_course(course): + return False + return True diff --git a/openedx/features/discounts/migrations/0001_initial.py b/openedx/features/discounts/migrations/0001_initial.py new file mode 100644 index 0000000000..73eb1482dd --- /dev/null +++ b/openedx/features/discounts/migrations/0001_initial.py @@ -0,0 +1,46 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-22 18:31 +from __future__ import unicode_literals + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import openedx.core.djangoapps.config_model_utils.models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('course_overviews', '0014_courseoverview_certificate_available_date'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('sites', '0002_alter_domain_unique'), + ] + + operations = [ + migrations.CreateModel( + name='DiscountRestrictionConfig', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.NullBooleanField(default=None, verbose_name='Enabled')), + ('org', 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)), + ('org_course', 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, validators=[openedx.core.djangoapps.config_model_utils.models.validate_course_in_org], verbose_name='Course in Org')), + ('changed_by', models.ForeignKey(editable=False, null=True, on_delete=django.db.models.deletion.PROTECT, to=settings.AUTH_USER_MODEL, verbose_name='Changed by')), + ('course', 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')), + ('site', 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')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddIndex( + model_name='discountrestrictionconfig', + index=models.Index(fields=['site', 'org', 'course'], name='discounts_d_site_id_d67da3_idx'), + ), + migrations.AddIndex( + model_name='discountrestrictionconfig', + index=models.Index(fields=['site', 'org', 'org_course', 'course'], name='discounts_d_site_id_f83727_idx'), + ), + ] diff --git a/openedx/features/discounts/migrations/__init__.py b/openedx/features/discounts/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/features/discounts/models.py b/openedx/features/discounts/models.py new file mode 100644 index 0000000000..bf32d6d231 --- /dev/null +++ b/openedx/features/discounts/models.py @@ -0,0 +1,35 @@ +""" +DiscountRestrictionConfig Models +""" + +# -*- coding: utf-8 -*- +from __future__ import absolute_import, unicode_literals + +from django.utils.encoding import python_2_unicode_compatible + +from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel + + +@python_2_unicode_compatible +class DiscountRestrictionConfig(StackedConfigurationModel): + """ + A ConfigurationModel used to manage restrictons for lms-controlled discounts + """ + + STACKABLE_FIELDS = ('enabled',) + + @classmethod + def enabled_for_course(cls, course): + """ + Return whether lms-controlled discounts can be enabled for this course. + + Arguments: + course: The CourseOverview of the course being queried. + """ + current_config = cls.current(course_key=course.id) + return current_config.enabled + + def __str__(self): + return "DiscountRestrictionConfig(enabled={!r})".format( + self.enabled + ) diff --git a/openedx/features/discounts/tests/test_applicability.py b/openedx/features/discounts/tests/test_applicability.py index 5ebbe89bf0..2835fe3405 100644 --- a/openedx/features/discounts/tests/test_applicability.py +++ b/openedx/features/discounts/tests/test_applicability.py @@ -5,7 +5,9 @@ from datetime import timedelta from django.utils.timezone import now from course_modes.tests.factories import CourseModeFactory +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.waffle_utils.testutils import override_waffle_flag +from openedx.features.discounts.models import DiscountRestrictionConfig from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -23,6 +25,8 @@ class TestApplicability(ModuleStoreTestCase): super(TestApplicability, self).setUp() self.user = UserFactory.create() self.course = CourseFactory.create(run='test', display_name='test') + course_overview = CourseOverview.get_from_id(self.course.id) + DiscountRestrictionConfig.objects.create(enabled=True, course=course_overview) CourseModeFactory.create(course_id=self.course.id, mode_slug='verified') def test_can_receive_discount(self): @@ -39,9 +43,13 @@ class TestApplicability(ModuleStoreTestCase): self.assertEqual(applicability, True) no_verified_mode_course = CourseFactory(end=now() + timedelta(days=30)) + no_verified_mode_course_overview = CourseOverview.get_from_id(no_verified_mode_course.id) + DiscountRestrictionConfig.objects.create(enabled=True, course=no_verified_mode_course_overview) applicability = can_receive_discount(user=self.user, course=no_verified_mode_course) self.assertEqual(applicability, False) course_that_has_ended = CourseFactory(end=now() - timedelta(days=30)) + course_that_has_ended_overview = CourseOverview.get_from_id(course_that_has_ended.id) + DiscountRestrictionConfig.objects.create(enabled=True, course=course_that_has_ended_overview) applicability = can_receive_discount(user=self.user, course=course_that_has_ended) self.assertEqual(applicability, False) diff --git a/openedx/features/discounts/tests/test_discount_restriction_models.py b/openedx/features/discounts/tests/test_discount_restriction_models.py new file mode 100644 index 0000000000..f532c92f5a --- /dev/null +++ b/openedx/features/discounts/tests/test_discount_restriction_models.py @@ -0,0 +1,103 @@ +""" +Test discount restriction config +""" +from __future__ import absolute_import + +import itertools + +import ddt + +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory +from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from openedx.features.discounts.models import DiscountRestrictionConfig +from student.tests.factories import UserFactory + + +@ddt.ddt +class TestDiscountRestrictionConfig(CacheIsolationTestCase): + """ + Test discount restriction config + """ + ENABLED_CACHES = ['default'] + + def setUp(self): + self.course_overview = CourseOverviewFactory.create() + self.user = UserFactory.create() + super(TestDiscountRestrictionConfig, self).setUp() + + @ddt.data(True, False) + def test_enabled_for_course( + self, + enabled, + ): + DiscountRestrictionConfig.objects.create( + enabled=enabled, + course=self.course_overview, + ) + course_key = self.course_overview.id + + self.assertEqual( + enabled, + DiscountRestrictionConfig.current(course_key=course_key).enabled + ) + + @ddt.data( + # Generate all combinations of setting each configuration level to True/False/None + *itertools.product(*[(True, False, None)] * 4) + ) + @ddt.unpack + def test_config_overrides(self, global_setting, site_setting, org_setting, course_setting): + """ + Test that the stacked configuration overrides happen in the correct order and priority. + + This is tested by exhaustively setting each combination of contexts, and validating that only + the lowest level context that is set to not-None is applied. + """ + # Add a bunch of configuration outside the contexts that are being tested, to make sure + # there are no leaks of configuration across contexts + non_test_course_enabled = CourseOverviewFactory.create(org='non-test-org-enabled') + non_test_course_disabled = CourseOverviewFactory.create(org='non-test-org-disabled') + non_test_site_cfg_enabled = SiteConfigurationFactory.create( + values={'course_org_filter': non_test_course_enabled.org} + ) + non_test_site_cfg_disabled = SiteConfigurationFactory.create( + values={'course_org_filter': non_test_course_disabled.org} + ) + + DiscountRestrictionConfig.objects.create(course=non_test_course_enabled, enabled=True) + DiscountRestrictionConfig.objects.create(course=non_test_course_disabled, enabled=False) + DiscountRestrictionConfig.objects.create(org=non_test_course_enabled.org, enabled=True) + DiscountRestrictionConfig.objects.create(org=non_test_course_disabled.org, enabled=False) + DiscountRestrictionConfig.objects.create(site=non_test_site_cfg_enabled.site, enabled=True) + DiscountRestrictionConfig.objects.create(site=non_test_site_cfg_disabled.site, enabled=False) + + # Set up test objects + test_course = CourseOverviewFactory.create(org='test-org') + test_site_cfg = SiteConfigurationFactory.create(values={'course_org_filter': test_course.org}) + + DiscountRestrictionConfig.objects.create(enabled=global_setting) + DiscountRestrictionConfig.objects.create(course=test_course, enabled=course_setting) + DiscountRestrictionConfig.objects.create(org=test_course.org, enabled=org_setting) + DiscountRestrictionConfig.objects.create(site=test_site_cfg.site, enabled=site_setting) + + expected_global_setting = self._resolve_settings([global_setting]) + expected_site_setting = self._resolve_settings([global_setting, site_setting]) + expected_org_setting = self._resolve_settings([global_setting, site_setting, org_setting]) + expected_course_setting = self._resolve_settings([global_setting, site_setting, org_setting, course_setting]) + + self.assertEqual(expected_global_setting, DiscountRestrictionConfig.current().enabled) + self.assertEqual(expected_site_setting, DiscountRestrictionConfig.current(site=test_site_cfg.site).enabled) + self.assertEqual(expected_org_setting, DiscountRestrictionConfig.current(org=test_course.org).enabled) + self.assertEqual(expected_course_setting, DiscountRestrictionConfig.current(course_key=test_course.id).enabled) + + def _resolve_settings(self, settings): + if all(setting is None for setting in settings): + return None + + return [ + setting + for setting + in settings + if setting is not None + ][-1]