diff --git a/openedx/features/discounts/admin.py b/openedx/features/discounts/admin.py index 4996d83099..7efc67cc84 100644 --- a/openedx/features/discounts/admin.py +++ b/openedx/features/discounts/admin.py @@ -21,7 +21,7 @@ class DiscountRestrictionConfigAdmin(StackedConfigModelAdmin): ('Context', { 'fields': DiscountRestrictionConfig.KEY_FIELDS, 'description': _( - 'These define the context to enable lms-controlled discounts on. ' + 'These define the context to disable 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.
' @@ -31,12 +31,12 @@ class DiscountRestrictionConfigAdmin(StackedConfigModelAdmin): ), }), ('Configuration', { - 'fields': ('enabled',), + 'fields': ('disabled',), '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.' + 'For example, if "Disabled" is left as "Unknown" in the course context, then that ' + 'course will be Disabled only if the org that it is in is Disabled.' ), }) ) diff --git a/openedx/features/discounts/applicability.py b/openedx/features/discounts/applicability.py index b3c7b88bc5..4caa0a8d8b 100644 --- a/openedx/features/discounts/applicability.py +++ b/openedx/features/discounts/applicability.py @@ -52,7 +52,7 @@ def can_receive_discount(user, course): # pylint: disable=unused-argument return False # Site, Partner, Course or Course Run not excluded from lms-controlled discounts - if not DiscountRestrictionConfig.enabled_for_course(course): + if DiscountRestrictionConfig.disabled_for_course_stacked_config(course): return False return True diff --git a/openedx/features/discounts/migrations/0001_initial.py b/openedx/features/discounts/migrations/0001_initial.py index 73eb1482dd..515c4a3a87 100644 --- a/openedx/features/discounts/migrations/0001_initial.py +++ b/openedx/features/discounts/migrations/0001_initial.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.11.20 on 2019-05-22 18:31 +# Generated by Django 1.11.20 on 2019-05-31 20:08 from __future__ import unicode_literals from django.conf import settings @@ -27,6 +27,7 @@ class Migration(migrations.Migration): ('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')), + ('disabled', models.NullBooleanField(default=None, verbose_name='Disabled')), ('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')), diff --git a/openedx/features/discounts/models.py b/openedx/features/discounts/models.py index bf32d6d231..0023320224 100644 --- a/openedx/features/discounts/models.py +++ b/openedx/features/discounts/models.py @@ -5,7 +5,9 @@ DiscountRestrictionConfig Models # -*- coding: utf-8 -*- from __future__ import absolute_import, unicode_literals +from django.db import models from django.utils.encoding import python_2_unicode_compatible +from django.utils.translation import ugettext_lazy as _ from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel @@ -16,20 +18,24 @@ class DiscountRestrictionConfig(StackedConfigurationModel): A ConfigurationModel used to manage restrictons for lms-controlled discounts """ - STACKABLE_FIELDS = ('enabled',) + STACKABLE_FIELDS = ('disabled',) + # Since this config disables a feature, it seemed it would be clearer to use a disabled flag instead of enabled. + # The enabled field still exists but is not used or shown in the admin. + disabled = models.NullBooleanField(default=None, verbose_name=_("Disabled")) @classmethod - def enabled_for_course(cls, course): + def disabled_for_course_stacked_config(cls, course): """ - Return whether lms-controlled discounts can be enabled for this course. + Return whether lms-controlled discounts are disabled for this course. + Checks if discounts are disabled for attributes of this course like Site, Partner, Course or Course Run. Arguments: course: The CourseOverview of the course being queried. """ current_config = cls.current(course_key=course.id) - return current_config.enabled + return current_config.disabled def __str__(self): - return "DiscountRestrictionConfig(enabled={!r})".format( - self.enabled + return "DiscountRestrictionConfig(disabled={!r})".format( + self.disabled ) diff --git a/openedx/features/discounts/tests/test_applicability.py b/openedx/features/discounts/tests/test_applicability.py index 2835fe3405..b0e9a3bd88 100644 --- a/openedx/features/discounts/tests/test_applicability.py +++ b/openedx/features/discounts/tests/test_applicability.py @@ -25,8 +25,6 @@ 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): @@ -43,13 +41,16 @@ 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) + + disabled_course = CourseFactory() + CourseModeFactory.create(course_id=disabled_course.id, mode_slug='verified') + disabled_course_overview = CourseOverview.get_from_id(disabled_course.id) + DiscountRestrictionConfig.objects.create(disabled=True, course=disabled_course_overview) + applicability = can_receive_discount(user=self.user, course=disabled_course) + 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 index f532c92f5a..8a1146d4b2 100644 --- a/openedx/features/discounts/tests/test_discount_restriction_models.py +++ b/openedx/features/discounts/tests/test_discount_restriction_models.py @@ -27,19 +27,19 @@ class TestDiscountRestrictionConfig(CacheIsolationTestCase): super(TestDiscountRestrictionConfig, self).setUp() @ddt.data(True, False) - def test_enabled_for_course( + def test_disabled_for_course_stacked_config( self, - enabled, + disabled, ): DiscountRestrictionConfig.objects.create( - enabled=enabled, + disabled=disabled, course=self.course_overview, ) course_key = self.course_overview.id self.assertEqual( - enabled, - DiscountRestrictionConfig.current(course_key=course_key).enabled + disabled, + DiscountRestrictionConfig.current(course_key=course_key).disabled ) @ddt.data( @@ -56,40 +56,40 @@ class TestDiscountRestrictionConfig(CacheIsolationTestCase): """ # 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_course_enabled = CourseOverviewFactory.create(org='non-test-org-enabled') non_test_site_cfg_disabled = SiteConfigurationFactory.create( values={'course_org_filter': non_test_course_disabled.org} ) + non_test_site_cfg_enabled = SiteConfigurationFactory.create( + values={'course_org_filter': non_test_course_enabled.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) + DiscountRestrictionConfig.objects.create(course=non_test_course_disabled, disabled=True) + DiscountRestrictionConfig.objects.create(course=non_test_course_enabled, disabled=False) + DiscountRestrictionConfig.objects.create(org=non_test_course_disabled.org, disabled=True) + DiscountRestrictionConfig.objects.create(org=non_test_course_enabled.org, disabled=False) + DiscountRestrictionConfig.objects.create(site=non_test_site_cfg_disabled.site, disabled=True) + DiscountRestrictionConfig.objects.create(site=non_test_site_cfg_enabled.site, disabled=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) + DiscountRestrictionConfig.objects.create(disabled=global_setting) + DiscountRestrictionConfig.objects.create(course=test_course, disabled=course_setting) + DiscountRestrictionConfig.objects.create(org=test_course.org, disabled=org_setting) + DiscountRestrictionConfig.objects.create(site=test_site_cfg.site, disabled=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) + self.assertEqual(expected_global_setting, DiscountRestrictionConfig.current().disabled) + self.assertEqual(expected_site_setting, DiscountRestrictionConfig.current(site=test_site_cfg.site).disabled) + self.assertEqual(expected_org_setting, DiscountRestrictionConfig.current(org=test_course.org).disabled) + self.assertEqual(expected_course_setting, DiscountRestrictionConfig.current(course_key=test_course.id).disabled) def _resolve_settings(self, settings): if all(setting is None for setting in settings):