From 3541643dd55f82cecd398481482bfd5058559d63 Mon Sep 17 00:00:00 2001 From: Julia Eskew Date: Fri, 3 Jan 2020 16:16:16 -0500 Subject: [PATCH] Rename values in SiteConfiguration (2/3) This stage does the following: - Includes a data migration to copy the values from old to new field. - Changes business logic to switch to using new field. - Deletes all code references of the old field. --- common/djangoapps/student/admin.py | 2 +- .../student/tests/test_admin_views.py | 4 +- common/djangoapps/util/tests/test_db.py | 8 ++- lms/djangoapps/branding/tests/test_views.py | 4 +- .../discussion/tests/test_signals.py | 2 - lms/djangoapps/discussion/tests/test_tasks.py | 2 +- lms/djangoapps/instructor/tests/test_api.py | 6 +- .../tests/views/test_instructor_dashboard.py | 2 - .../ace_common/tests/test_tracking.py | 3 - openedx/core/djangoapps/catalog/views.py | 3 +- .../djangoapps/config_model_utils/models.py | 4 +- .../commands/tests/test_notify_credentials.py | 3 +- .../credentials/tests/test_signals.py | 5 +- .../programs/tasks/v1/tests/test_tasks.py | 2 +- .../djangoapps/programs/tests/test_signals.py | 5 +- .../commands/tests/send_email_base.py | 6 +- .../schedules/tests/test_resolvers.py | 7 +-- .../djangoapps/site_configuration/admin.py | 6 +- .../djangoapps/site_configuration/helpers.py | 2 +- .../create_or_update_site_configuration.py | 4 -- ...est_create_or_update_site_configuration.py | 14 ++--- .../0005_copy_values_to_site_values.py | 40 ++++++++++++ .../djangoapps/site_configuration/models.py | 58 ++++++++++------- .../site_configuration/tests/factories.py | 2 +- .../site_configuration/tests/mixins.py | 12 +--- .../tests/test_middleware.py | 6 -- .../site_configuration/tests/test_models.py | 63 +++++++++---------- .../site_configuration/tests/test_util.py | 6 +- .../create_sites_and_configurations.py | 1 - .../tests/test_sync_hubspot_contacts.py | 9 ++- .../content_type_gating/tests/test_models.py | 20 +++--- .../tests/test_models.py | 20 +++--- .../tests/test_discount_restriction_models.py | 9 +-- 33 files changed, 171 insertions(+), 169 deletions(-) create mode 100644 openedx/core/djangoapps/site_configuration/migrations/0005_copy_values_to_site_values.py diff --git a/common/djangoapps/student/admin.py b/common/djangoapps/student/admin.py index 55e3407eba..cb740c894e 100644 --- a/common/djangoapps/student/admin.py +++ b/common/djangoapps/student/admin.py @@ -465,7 +465,7 @@ class AllowedAuthUserForm(forms.ModelForm): if not allowed_site_email_domain: raise forms.ValidationError( _("Please add a key/value 'THIRD_PARTY_AUTH_ONLY_DOMAIN/{site_email_domain}' in SiteConfiguration " - "model's values field.") + "model's site_values field.") ) elif email_domain != allowed_site_email_domain: raise forms.ValidationError( diff --git a/common/djangoapps/student/tests/test_admin_views.py b/common/djangoapps/student/tests/test_admin_views.py index 7a5f363b3e..21840fe245 100644 --- a/common/djangoapps/student/tests/test_admin_views.py +++ b/common/djangoapps/student/tests/test_admin_views.py @@ -454,7 +454,7 @@ class AllowedAuthUserFormTest(SiteMixin, TestCase): def _update_site_configuration(self): """ Updates the site's configuration """ - self.site.configuration.values = {'THIRD_PARTY_AUTH_ONLY_DOMAIN': self.email_domain_name} + self.site.configuration.site_values = {'THIRD_PARTY_AUTH_ONLY_DOMAIN': self.email_domain_name} self.site.configuration.save() def _assert_form(self, site, email, is_valid_form=False): @@ -480,7 +480,7 @@ class AllowedAuthUserFormTest(SiteMixin, TestCase): self.assertEqual( error, "Please add a key/value 'THIRD_PARTY_AUTH_ONLY_DOMAIN/{site_email_domain}' in SiteConfiguration " - "model's values field." + "model's site_values field." ) def test_form_with_invalid_domain_name(self): diff --git a/common/djangoapps/util/tests/test_db.py b/common/djangoapps/util/tests/test_db.py index 8fdcf40b9f..565ad10d60 100644 --- a/common/djangoapps/util/tests/test_db.py +++ b/common/djangoapps/util/tests/test_db.py @@ -222,7 +222,9 @@ class MigrationTests(TestCase): Tests for migrations. """ - @unittest.skip("Need to skip as part of renaming a field in schedules app. This will be unskipped in DE-1825") + @unittest.skip( + "Need to skip as part of renaming a field in schedules app. This will be unskipped in DE-1825. ALSO need to skip as part of renaming a field in the site_configuration app. This will be unskipped in DENG-18." + ) @override_settings(MIGRATION_MODULES={}) def test_migrations_are_in_sync(self): """ @@ -237,6 +239,6 @@ class MigrationTests(TestCase): release afterwards, this test doesn't fail. """ out = StringIO() - call_command('makemigrations', dry_run=True, verbosity=3, stdout=out) + call_command("makemigrations", dry_run=True, verbosity=3, stdout=out) output = out.getvalue() - self.assertIn('No changes detected', output) + self.assertIn("No changes detected", output) diff --git a/lms/djangoapps/branding/tests/test_views.py b/lms/djangoapps/branding/tests/test_views.py index 8b2ffc95d2..1f11b2da81 100644 --- a/lms/djangoapps/branding/tests/test_views.py +++ b/lms/djangoapps/branding/tests/test_views.py @@ -297,7 +297,7 @@ class TestIndex(SiteMixin, TestCase): response = self.client.get(reverse("root")) self.assertRedirects( response, - self.site_configuration_other.values["MKTG_URLS"]["ROOT"], + self.site_configuration_other.site_values["MKTG_URLS"]["ROOT"], fetch_redirect_response=False ) @@ -309,4 +309,4 @@ class TestIndex(SiteMixin, TestCase): self.use_site(self.site_other) self.client.login(username=self.user.username, password="password") response = self.client.get(reverse("dashboard")) - self.assertIn(self.site_configuration_other.values["MKTG_URLS"]["ROOT"], response.content.decode('utf-8')) + self.assertIn(self.site_configuration_other.site_values["MKTG_URLS"]["ROOT"], response.content.decode('utf-8')) diff --git a/lms/djangoapps/discussion/tests/test_signals.py b/lms/djangoapps/discussion/tests/test_signals.py index e303bc11c9..14d5887144 100644 --- a/lms/djangoapps/discussion/tests/test_signals.py +++ b/lms/djangoapps/discussion/tests/test_signals.py @@ -30,7 +30,6 @@ class SendMessageHandlerTestCase(TestCase): site_config = SiteConfigurationFactory.create(site=self.site) enable_notifications_cfg = {ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY: True} site_config.site_values = enable_notifications_cfg - site_config.values = enable_notifications_cfg site_config.save() mock_get_current_site.return_value = self.site signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) @@ -60,7 +59,6 @@ class SendMessageHandlerTestCase(TestCase): site_config = SiteConfigurationFactory.create(site=self.site) enable_notifications_cfg = {ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY: False} site_config.site_values = enable_notifications_cfg - site_config.values = enable_notifications_cfg site_config.save() mock_get_current_site.return_value = self.site signals.comment_created.send(sender=self.sender, user=self.user, post=self.post) diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index c6a8b64ebb..4161792a98 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -193,7 +193,7 @@ class TaskTestCase(ModuleStoreTestCase): comment = cc.Comment.find(id=self.comment['id']).retrieve() site = Site.objects.get_current() site_config = SiteConfigurationFactory.create(site=site) - site_config.values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True + site_config.site_values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True site_config.save() with mock.patch('lms.djangoapps.discussion.signals.handlers.get_current_site', return_value=site): comment_created.send(sender=None, user=user, post=comment) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 5fa668f87f..4424cd6338 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -4000,8 +4000,8 @@ class TestInstructorSendEmail(SiteMixin, SharedModuleStoreTestCase, LoginEnrollm self.assertEqual(response.status_code, 400) def test_send_email_with_site_template_and_from_addr(self): - site_email = self.site_configuration.values.get('course_email_from_addr') - site_template = self.site_configuration.values.get('course_email_template_name') + site_email = self.site_configuration.site_values.get('course_email_from_addr') + site_template = self.site_configuration.site_values.get('course_email_template_name') CourseEmailTemplate.objects.create(name=site_template) url = reverse('send_email', kwargs={'course_id': text_type(self.course.id)}) response = self.client.post(url, self.full_test_message) @@ -4019,7 +4019,7 @@ class TestInstructorSendEmail(SiteMixin, SharedModuleStoreTestCase, LoginEnrollm org_email = 'fake_org@example.com' org_template = 'fake_org_email_template' CourseEmailTemplate.objects.create(name=org_template) - self.site_configuration.values.update({ + self.site_configuration.site_values.update({ 'course_email_from_addr': {self.course.id.org: org_email}, 'course_email_template_name': {self.course.id.org: org_template} }) diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index 58d29ae5c1..3bbfdbf54f 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -169,7 +169,6 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT SiteConfiguration.objects.create( site=site, site_values=configuration_values, - values=configuration_values, enabled=True ) @@ -202,7 +201,6 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase, XssT SiteConfiguration.objects.create( site=site, site_values=configuration_values, - values=configuration_values, enabled=True ) url = reverse( diff --git a/openedx/core/djangoapps/ace_common/tests/test_tracking.py b/openedx/core/djangoapps/ace_common/tests/test_tracking.py index 1412acf252..6201362055 100644 --- a/openedx/core/djangoapps/ace_common/tests/test_tracking.py +++ b/openedx/core/djangoapps/ace_common/tests/test_tracking.py @@ -124,9 +124,6 @@ class TestGoogleAnalyticsTrackingPixel(QueryStringAssertionMixin, CacheIsolation site_config = SiteConfigurationFactory.create( site_values=dict( GOOGLE_ANALYTICS_ACCOUNT='UA-654321-1' - ), - values=dict( - GOOGLE_ANALYTICS_ACCOUNT='UA-654321-1' ) ) pixel = GoogleAnalyticsTrackingPixel(site=site_config.site) diff --git a/openedx/core/djangoapps/catalog/views.py b/openedx/core/djangoapps/catalog/views.py index 41874667c1..a6c1ba3704 100644 --- a/openedx/core/djangoapps/catalog/views.py +++ b/openedx/core/djangoapps/catalog/views.py @@ -27,8 +27,7 @@ def cache_programs(request): SiteConfiguration.objects.create( site=request.site, enabled=True, - site_values={"COURSE_CATALOG_API_URL": "{catalog_url}/api/v1/".format(catalog_url=CATALOG_STUB_URL)}, - values={"COURSE_CATALOG_API_URL": "{catalog_url}/api/v1/".format(catalog_url=CATALOG_STUB_URL)} + site_values={"COURSE_CATALOG_API_URL": "{catalog_url}/api/v1/".format(catalog_url=CATALOG_STUB_URL)} ) if settings.FEATURES.get('EXPOSE_CACHE_PROGRAMS_ENDPOINT'): diff --git a/openedx/core/djangoapps/config_model_utils/models.py b/openedx/core/djangoapps/config_model_utils/models.py index abf6b5a038..5a46541a93 100644 --- a/openedx/core/djangoapps/config_model_utils/models.py +++ b/openedx/core/djangoapps/config_model_utils/models.py @@ -244,7 +244,7 @@ class StackedConfigurationModel(ConfigurationModel): """ all_courses = CourseOverview.objects.all() all_site_configs = SiteConfiguration.objects.filter( - values__contains='course_org_filter', enabled=True + site_values__contains='course_org_filter', enabled=True ).select_related('site') try: @@ -254,7 +254,7 @@ class StackedConfigurationModel(ConfigurationModel): sites_by_org = defaultdict(lambda: default_site) site_cfg_org_filters = ( - (site_cfg.site, site_cfg.values['course_org_filter']) + (site_cfg.site, site_cfg.site_values['course_org_filter']) for site_cfg in all_site_configs ) sites_by_org.update({ diff --git a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py index d84ad0669b..63ed6e7101 100644 --- a/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py +++ b/openedx/core/djangoapps/credentials/management/commands/tests/test_notify_credentials.py @@ -157,8 +157,7 @@ class TestNotifyCredentials(TestCase): @mock.patch(COMMAND_MODULE + '.handle_cert_change') def test_site(self, mock_grade_interesting, mock_cert_change): site_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': ['testX']}, - values={'course_org_filter': ['testX']}, + site_values={'course_org_filter': ['testX']} ) call_command(Command(), '--site', site_config.site.domain, '--start-date', '2017-01-01') diff --git a/openedx/core/djangoapps/credentials/tests/test_signals.py b/openedx/core/djangoapps/credentials/tests/test_signals.py index ef2a8196d5..311be58cc0 100644 --- a/openedx/core/djangoapps/credentials/tests/test_signals.py +++ b/openedx/core/djangoapps/credentials/tests/test_signals.py @@ -110,8 +110,7 @@ class TestCredentialsSignalsSendGrade(TestCase): def test_send_grade_records_enabled(self, _mock_is_course_run_in_a_program, mock_send_grade_to_credentials, _mock_is_learner_issuance_enabled): site_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': [self.key.org]}, - values={'course_org_filter': [self.key.org]}, + site_values={'course_org_filter': [self.key.org]} ) # Correctly sent @@ -120,7 +119,7 @@ class TestCredentialsSignalsSendGrade(TestCase): mock_send_grade_to_credentials.delay.reset_mock() # Correctly not sent - site_config.values['ENABLE_LEARNER_RECORDS'] = False + site_config.site_values['ENABLE_LEARNER_RECORDS'] = False site_config.save() send_grade_if_interesting(self.user, self.key, 'verified', 'downloadable', None, None) self.assertFalse(mock_send_grade_to_credentials.delay.called) diff --git a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py index 96d6a3ecb9..290ae6abba 100644 --- a/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py +++ b/openedx/core/djangoapps/programs/tasks/v1/tests/test_tasks.py @@ -196,7 +196,7 @@ class AwardProgramCertificatesTestCase(CatalogIntegrationMixin, CredentialsApiCo mock_get_certified_programs.return_value = [1] # programs to be skipped - self.site_configuration.values = { + self.site_configuration.site_values = { "programs_without_certificates": [2] } self.site_configuration.save() diff --git a/openedx/core/djangoapps/programs/tests/test_signals.py b/openedx/core/djangoapps/programs/tests/test_signals.py index 82c5299cb7..364dc089f6 100644 --- a/openedx/core/djangoapps/programs/tests/test_signals.py +++ b/openedx/core/djangoapps/programs/tests/test_signals.py @@ -146,8 +146,7 @@ class CertChangedReceiverTest(TestCase): mock_is_learner_issuance_enabled.return_value = True site_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': ['edX']}, - values={'course_org_filter': ['edX']}, + site_values={'course_org_filter': ['edX']} ) # Correctly sent @@ -156,7 +155,7 @@ class CertChangedReceiverTest(TestCase): mock_task.reset_mock() # Correctly not sent - site_config.values['ENABLE_LEARNER_RECORDS'] = False + site_config.site_values['ENABLE_LEARNER_RECORDS'] = False site_config.save() handle_course_cert_changed(**self.signal_kwargs) self.assertFalse(mock_task.called) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index 9da5d0e811..b76054d8fb 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -308,12 +308,10 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): filtered_org = 'filtered_org' unfiltered_org = 'unfiltered_org' this_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': this_org_list}, - values={'course_org_filter': this_org_list} + site_values={'course_org_filter': this_org_list} ) other_config = SiteConfigurationFactory.create( - site_values={'course_org_filter': other_org_list}, - values={'course_org_filter': other_org_list} + site_values={'course_org_filter': other_org_list} ) for config in (this_config, other_config): diff --git a/openedx/core/djangoapps/schedules/tests/test_resolvers.py b/openedx/core/djangoapps/schedules/tests/test_resolvers.py index 4c792b860f..aa60ec7cab 100644 --- a/openedx/core/djangoapps/schedules/tests/test_resolvers.py +++ b/openedx/core/djangoapps/schedules/tests/test_resolvers.py @@ -61,7 +61,7 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): 'course1' ) def test_get_course_org_filter_equal(self, course_org_filter): - self.site_config.values['course_org_filter'] = course_org_filter + self.site_config.site_values['course_org_filter'] = course_org_filter self.site_config.save() mock_query = Mock() result = self.resolver.filter_by_org(mock_query) @@ -73,7 +73,7 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): (['course1', 'course2'], ['course1', 'course2']) ) def test_get_course_org_filter_include__in(self, course_org_filter, expected_org_list): - self.site_config.values['course_org_filter'] = course_org_filter + self.site_config.site_values['course_org_filter'] = course_org_filter self.site_config.save() mock_query = Mock() result = self.resolver.filter_by_org(mock_query) @@ -88,8 +88,7 @@ class TestBinnedSchedulesBaseResolver(SchedulesResolverTestMixin, TestCase): ) def test_get_course_org_filter_exclude__in(self, course_org_filter, expected_org_list): SiteConfigurationFactory.create( - site_values={'course_org_filter': course_org_filter}, - values={'course_org_filter': course_org_filter}, + site_values={'course_org_filter': course_org_filter} ) mock_query = Mock() result = self.resolver.filter_by_org(mock_query) diff --git a/openedx/core/djangoapps/site_configuration/admin.py b/openedx/core/djangoapps/site_configuration/admin.py index 6fda05f472..d8b52647b6 100644 --- a/openedx/core/djangoapps/site_configuration/admin.py +++ b/openedx/core/djangoapps/site_configuration/admin.py @@ -12,8 +12,8 @@ class SiteConfigurationAdmin(admin.ModelAdmin): """ Admin interface for the SiteConfiguration object. """ - list_display = ('site', 'enabled', 'values') - search_fields = ('site__domain', 'values') + list_display = ('site', 'enabled', 'site_values') + search_fields = ('site__domain', 'site_values') class Meta(object): """ @@ -29,7 +29,7 @@ class SiteConfigurationHistoryAdmin(admin.ModelAdmin): Admin interface for the SiteConfigurationHistory object. """ list_display = ('site', 'enabled', 'created', 'modified') - search_fields = ('site__domain', 'values', 'created', 'modified') + search_fields = ('site__domain', 'site_values', 'created', 'modified') ordering = ['-created'] diff --git a/openedx/core/djangoapps/site_configuration/helpers.py b/openedx/core/djangoapps/site_configuration/helpers.py index 07cb3ae9a9..45663497f3 100644 --- a/openedx/core/djangoapps/site_configuration/helpers.py +++ b/openedx/core/djangoapps/site_configuration/helpers.py @@ -53,7 +53,7 @@ def has_configuration_override(name): (bool): True if given key is present in the configuration. """ configuration = get_current_site_configuration() - if configuration and name in configuration.values: + if configuration and name in configuration.site_values: return True return False diff --git a/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py index 2c670b513f..edcb0f19a4 100644 --- a/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py +++ b/openedx/core/djangoapps/site_configuration/management/commands/create_or_update_site_configuration.py @@ -103,10 +103,6 @@ class Command(BaseCommand): site_configuration_values = configuration or config_file_data if site_configuration_values: - if site_configuration.values: - site_configuration.values.update(site_configuration_values) - else: - site_configuration.values = site_configuration_values if site_configuration.site_values: site_configuration.site_values.update(site_configuration_values) else: diff --git a/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py b/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py index 0a0d4974ad..9a686ae0cd 100644 --- a/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py +++ b/openedx/core/djangoapps/site_configuration/management/commands/tests/test_create_or_update_site_configuration.py @@ -56,7 +56,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): def create_fixture_site_configuration(self, enabled): SiteConfiguration.objects.update_or_create( site=self.site, - defaults={'enabled': enabled, 'values': {'ABC': 'abc', 'B': 'b'}} + defaults={'enabled': enabled, 'site_values': {'ABC': 'abc', 'B': 'b'}} ) def test_command_no_args(self): @@ -105,7 +105,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): call_command(self.command, *self.site_id_arg) site_configuration = SiteConfiguration.objects.get(site=self.site) - self.assertFalse(site_configuration.values) + self.assertFalse(site_configuration.site_values) self.assertFalse(site_configuration.enabled) def test_both_enabled_disabled_flags(self): @@ -126,7 +126,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): self.assert_site_configuration_does_not_exist() call_command(self.command, '--{}'.format(flag), *self.site_id_arg) site_configuration = SiteConfiguration.objects.get(site=self.site) - self.assertFalse(site_configuration.values) + self.assertFalse(site_configuration.site_values) self.assertEqual(enabled, site_configuration.enabled) def test_site_configuration_created_with_parameters(self): @@ -136,7 +136,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): self.assert_site_configuration_does_not_exist() call_command(self.command, '--configuration', json.dumps(self.input_configuration), *self.site_id_arg) site_configuration = self.get_site_configuration() - self.assertDictEqual(site_configuration.values, self.input_configuration) + self.assertDictEqual(site_configuration.site_values, self.input_configuration) def test_site_configuration_created_with_json_file_parameters(self): """ @@ -145,7 +145,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): self.assert_site_configuration_does_not_exist() call_command(self.command, '-f', str(self.json_file_path.abspath()), *self.site_id_arg) site_configuration = self.get_site_configuration() - self.assertEqual(site_configuration.values, {'ABC': 123, 'XYZ': '789'}) + self.assertEqual(site_configuration.site_values, {'ABC': 123, 'XYZ': '789'}) @ddt.data(True, False) def test_site_configuration_updated_with_parameters(self, enabled): @@ -156,7 +156,7 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): call_command(self.command, '--configuration', json.dumps(self.input_configuration), *self.site_id_arg) site_configuration = self.get_site_configuration() self.assertEqual( - site_configuration.values, + site_configuration.site_values, {'ABC': 123, 'B': 'b', 'FEATURE_FLAG': True, 'SERVICE_URL': 'https://foo.bar'} ) self.assertEqual(site_configuration.enabled, enabled) @@ -172,5 +172,5 @@ class CreateOrUpdateSiteConfigurationTest(TestCase): expected_site_configuration = {'ABC': 'abc', 'B': 'b'} with codecs.open(self.json_file_path, encoding='utf-8') as f: expected_site_configuration.update(json.load(f)) - self.assertEqual(site_configuration.values, expected_site_configuration) + self.assertEqual(site_configuration.site_values, expected_site_configuration) self.assertEqual(site_configuration.enabled, enabled) diff --git a/openedx/core/djangoapps/site_configuration/migrations/0005_copy_values_to_site_values.py b/openedx/core/djangoapps/site_configuration/migrations/0005_copy_values_to_site_values.py new file mode 100644 index 0000000000..8993f724fd --- /dev/null +++ b/openedx/core/djangoapps/site_configuration/migrations/0005_copy_values_to_site_values.py @@ -0,0 +1,40 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.27 on 2020-01-03 20:57 +from __future__ import unicode_literals + +from django.db import migrations + + +def copy_column_values(apps, schema_editor): + """ + Copy the contents of the values field into the site_values field in both + SiteConfiguration and SiteConfigurationHistory. + """ + # Update all values in the model. + SiteConfiguration = apps.get_model('site_configuration', 'SiteConfiguration') + for site_configuration in SiteConfiguration.objects.all(): + site_configuration.site_values = site_configuration.values + # It would be incorrect to record these saves in the history table since it is + # just backfilling data. Use save_without_historical_record() instead of + # save(). + site_configuration.save_without_historical_record() + + # Update all values in the history model. + SiteConfigurationHistory = apps.get_model('site_configuration', 'SiteConfigurationHistory') + for historical_site_configuration in SiteConfigurationHistory.objects.all(): + historical_site_configuration.site_values = historical_site_configuration.values + historical_site_configuration.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ('site_configuration', '0004_add_site_values_field'), + ] + + operations = [ + migrations.RunPython( + copy_column_values, + reverse_code=migrations.RunPython.noop, # Allow reverse migrations, but make it a no-op. + ), + ] diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index 683fae6af5..e7c64fbd69 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -40,13 +40,6 @@ class SiteConfiguration(models.Model): default=dict, load_kwargs={'object_pairs_hook': collections.OrderedDict} ) - # TODO: Delete this deprecated field during the later stages of the rollout - # which renames 'values' to 'site_values'. - values = JSONField( - null=False, - blank=True, - load_kwargs={'object_pairs_hook': collections.OrderedDict} - ) def __str__(self): return u"".format(site=self.site) # xss-lint: disable=python-wrap-html @@ -69,7 +62,7 @@ class SiteConfiguration(models.Model): """ if self.enabled: try: - return self.values.get(name, default) + return self.site_values.get(name, default) except AttributeError as error: logger.exception(u'Invalid JSON data. \n [%s]', error) else: @@ -87,7 +80,7 @@ class SiteConfiguration(models.Model): org (str): Org to use to filter SiteConfigurations select_related (list or None): A list of values to pass as arguments to select_related """ - query = cls.objects.filter(values__contains=org, enabled=True).all() + query = cls.objects.filter(site_values__contains=org, enabled=True).all() if select_related is not None: query = query.select_related(*select_related) for configuration in query: @@ -131,7 +124,7 @@ class SiteConfiguration(models.Model): """ org_filter_set = set() - for configuration in cls.objects.filter(values__contains='course_org_filter', enabled=True).all(): + for configuration in cls.objects.filter(site_values__contains='course_org_filter', enabled=True).all(): course_org_filter = configuration.get_value('course_org_filter', []) if not isinstance(course_org_filter, list): course_org_filter = [course_org_filter] @@ -148,6 +141,21 @@ class SiteConfiguration(models.Model): """ return org in cls.get_all_orgs() + def save_without_historical_record(self, *args, **kwargs): + """ + Save model without saving a historical record + + Make sure you know what you're doing before you use this method. + + Note: this method is copied verbatim from django-simple-history. + """ + self.skip_history_when_saving = True # pylint: disable=attribute-defined-outside-init + try: + ret = self.save(*args, **kwargs) + finally: + del self.skip_history_when_saving + return ret + @python_2_unicode_compatible class SiteConfigurationHistory(TimeStampedModel): @@ -168,13 +176,6 @@ class SiteConfigurationHistory(TimeStampedModel): blank=True, load_kwargs={'object_pairs_hook': collections.OrderedDict} ) - # TODO: Delete this deprecated field during the later stages of the rollout - # which renames 'values' to 'site_values'. - values = JSONField( - null=False, - blank=True, - load_kwargs={'object_pairs_hook': collections.OrderedDict} - ) class Meta: get_latest_by = 'modified' @@ -192,18 +193,27 @@ class SiteConfigurationHistory(TimeStampedModel): @receiver(post_save, sender=SiteConfiguration) -def update_site_configuration_history(sender, instance, **kwargs): # pylint: disable=unused-argument +def update_site_configuration_history(sender, instance, created, **kwargs): # pylint: disable=unused-argument """ Add site configuration changes to site configuration history. + Recording history on updates and deletes can be skipped by first setting + the `skip_history_when_saving` attribute on the instace, e.g.: + + site_config.skip_history_when_saving = True + site_config.save() + Args: sender: sender of the signal i.e. SiteConfiguration model instance: SiteConfiguration instance associated with the current signal + created (bool): True if a new record was created. **kwargs: extra key word arguments """ - SiteConfigurationHistory.objects.create( - site=instance.site, - site_values=instance.values, - values=instance.values, - enabled=instance.enabled, - ) + # Skip writing history when asked by the caller. This skip feature only + # works for non-creates. + if created or not hasattr(instance, "skip_history_when_saving"): + SiteConfigurationHistory.objects.create( + site=instance.site, + site_values=instance.site_values, + enabled=instance.enabled, + ) diff --git a/openedx/core/djangoapps/site_configuration/tests/factories.py b/openedx/core/djangoapps/site_configuration/tests/factories.py index 4943ca3670..db1bb7377f 100644 --- a/openedx/core/djangoapps/site_configuration/tests/factories.py +++ b/openedx/core/djangoapps/site_configuration/tests/factories.py @@ -33,5 +33,5 @@ class SiteConfigurationFactory(DjangoModelFactory): site = SubFactory(SiteFactory) @lazy_attribute - def values(self): + def site_values(self): return {} diff --git a/openedx/core/djangoapps/site_configuration/tests/mixins.py b/openedx/core/djangoapps/site_configuration/tests/mixins.py index 95294d2891..f08aca3541 100644 --- a/openedx/core/djangoapps/site_configuration/tests/mixins.py +++ b/openedx/core/djangoapps/site_configuration/tests/mixins.py @@ -22,9 +22,7 @@ class SiteMixin(object): } self.site_configuration = SiteConfigurationFactory.create( site=self.site, - site_values=site_config, - # TODO: Remove this deprecated value eventually. - values=site_config + site_values=site_config ) self.site_other = SiteFactory.create( @@ -44,9 +42,7 @@ class SiteMixin(object): } self.site_configuration_other = SiteConfigurationFactory.create( site=self.site_other, - site_values=site_config_other, - # TODO: Remove this deprecated value eventually. - values=site_config_other + site_values=site_config_other ) # Initialize client with default site domain @@ -62,9 +58,7 @@ class SiteMixin(object): ) __ = SiteConfigurationFactory.create( site=site, - site_values=site_configuration_values, - # TODO: Remove this deprecated value eventually. - values=site_configuration_values + site_values=site_configuration_values ) self.use_site(site) return site diff --git a/openedx/core/djangoapps/site_configuration/tests/test_middleware.py b/openedx/core/djangoapps/site_configuration/tests/test_middleware.py index c1dcc8dd37..124ba1f4a0 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_middleware.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_middleware.py @@ -40,9 +40,6 @@ class SessionCookieDomainTests(TestCase): site=self.site, site_values={ "SESSION_COOKIE_DOMAIN": self.site.domain, - }, - values={ - "SESSION_COOKIE_DOMAIN": self.site.domain, } ) @@ -78,9 +75,6 @@ class SessionCookieDomainSiteConfigurationOverrideTests(TestCase): site=self.site, site_values={ "SESSION_COOKIE_DOMAIN": self.site.domain, - }, - values={ - "SESSION_COOKIE_DOMAIN": self.site.domain, } ) self.client = Client() diff --git a/openedx/core/djangoapps/site_configuration/tests/test_models.py b/openedx/core/djangoapps/site_configuration/tests/test_models.py index 4b69d57f58..ea83f8c271 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_models.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_models.py @@ -83,8 +83,6 @@ class SiteConfigurationTests(TestCase): ) site_configuration.site_values = {'test': 'test'} - # TODO: Remove this deprecated value eventually. - site_configuration.values = {'test': 'test'} site_configuration.save() # Verify an entry to SiteConfigurationHistory was added. @@ -92,9 +90,30 @@ class SiteConfigurationTests(TestCase): site=site_configuration.site, ).all() - # Make sure two entries (one for save and one for update) are saved for SiteConfiguration + # Make sure two entries (one for create and one for update) are saved for SiteConfiguration self.assertEqual(len(site_configuration_history), 2) + def test_site_configuration_post_update_receiver_with_skip(self): + """ + Test that and entry is NOT added to SiteConfigurationHistory each time a + SiteConfiguration is updated with save_without_historical_record(). + """ + # add SiteConfiguration to database + site_configuration = SiteConfigurationFactory.create( + site=self.site, + ) + + site_configuration.site_values = {'test': 'test'} + site_configuration.save_without_historical_record() # Instead of save(). + + # Verify an entry to SiteConfigurationHistory was NOT added. + site_configuration_history = SiteConfigurationHistory.objects.filter( + site=site_configuration.site, + ).all() + + # Make sure one entry (one for create and NONE for update) is saved for SiteConfiguration + self.assertEqual(len(site_configuration_history), 1) + def test_no_entry_is_saved_for_errors(self): """ Test that and entry is not added to SiteConfigurationHistory if there is an error while @@ -133,9 +152,7 @@ class SiteConfigurationTests(TestCase): # add SiteConfiguration to database site_configuration = SiteConfigurationFactory.create( site=self.site, - site_values=self.test_config1, - # TODO: Remove this deprecated value eventually. - values=self.test_config1, + site_values=self.test_config1 ) # Make sure entry is saved and retrieved correctly @@ -183,9 +200,7 @@ class SiteConfigurationTests(TestCase): # add SiteConfiguration to database site_configuration = SiteConfigurationFactory.create( site=self.site, - site_values=invalid_data, - # TODO: Remove this deprecated value eventually. - values=invalid_data, + site_values=invalid_data ) # make sure get_value logs an error for invalid json data @@ -206,15 +221,11 @@ class SiteConfigurationTests(TestCase): # add SiteConfiguration to database SiteConfigurationFactory.create( site=self.site, - site_values=self.test_config1, - # TODO: Remove this deprecated value eventually. - values=self.test_config1, + site_values=self.test_config1 ) SiteConfigurationFactory.create( site=self.site2, - site_values=self.test_config2, - # TODO: Remove this deprecated value eventually. - values=self.test_config2, + site_values=self.test_config2 ) # Make sure entry is saved and retrieved correctly @@ -295,15 +306,11 @@ class SiteConfigurationTests(TestCase): # add SiteConfiguration to database config1 = SiteConfigurationFactory.create( site=self.site, - site_values=self.test_config1, - # TODO: Remove this deprecated value eventually. - values=self.test_config1, + site_values=self.test_config1 ) config2 = SiteConfigurationFactory.create( site=self.site2, - site_values=self.test_config2, - # TODO: Remove this deprecated value eventually. - values=self.test_config2, + site_values=self.test_config2 ) # Make sure entry is saved and retrieved correctly @@ -328,15 +335,11 @@ class SiteConfigurationTests(TestCase): # add SiteConfiguration to database SiteConfigurationFactory.create( site=self.site, - site_values=self.test_config1, - # TODO: Remove this deprecated value eventually. - values=self.test_config1, + site_values=self.test_config1 ) SiteConfigurationFactory.create( site=self.site2, - site_values=self.test_config2, - # TODO: Remove this deprecated value eventually. - values=self.test_config2, + site_values=self.test_config2 ) # Test that the default value is returned if the value for the given key is not found in the configuration @@ -351,15 +354,11 @@ class SiteConfigurationTests(TestCase): SiteConfigurationFactory.create( site=self.site, site_values=self.test_config1, - # TODO: Remove this deprecated value eventually. - values=self.test_config1, enabled=False, ) SiteConfigurationFactory.create( site=self.site2, - site_values=self.test_config2, - # TODO: Remove this deprecated value eventually. - values=self.test_config2, + site_values=self.test_config2 ) # Test that the default value is returned if the value for the given key is not found in the configuration diff --git a/openedx/core/djangoapps/site_configuration/tests/test_util.py b/openedx/core/djangoapps/site_configuration/tests/test_util.py index 6c13a5fadc..6bfb5b760e 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_util.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_util.py @@ -29,11 +29,10 @@ def with_site_configuration(domain="test.localhost", configuration=None): site, __ = Site.objects.get_or_create(domain=domain, name=domain) site_configuration, created = SiteConfiguration.objects.get_or_create( site=site, - defaults={"enabled": True, "site_values": configuration, "values": configuration}, + defaults={"enabled": True, "site_values": configuration}, ) if not created: site_configuration.site_values = configuration - site_configuration.values = configuration site_configuration.save() with patch('openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration', @@ -57,11 +56,10 @@ def with_site_configuration_context(domain="test.localhost", configuration=None) site, __ = Site.objects.get_or_create(domain=domain, name=domain) site_configuration, created = SiteConfiguration.objects.get_or_create( site=site, - defaults={"enabled": True, "site_values": configuration, "values": configuration}, + defaults={"enabled": True, "site_values": configuration}, ) if not created: site_configuration.site_values = configuration - site_configuration.values = configuration site_configuration.save() with patch('openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration', diff --git a/openedx/core/djangoapps/theming/management/commands/create_sites_and_configurations.py b/openedx/core/djangoapps/theming/management/commands/create_sites_and_configurations.py index 41e3ad59de..e83a2b38b8 100644 --- a/openedx/core/djangoapps/theming/management/commands/create_sites_and_configurations.py +++ b/openedx/core/djangoapps/theming/management/commands/create_sites_and_configurations.py @@ -116,7 +116,6 @@ class Command(BaseCommand): SiteConfiguration.objects.create( site=site, site_values=site_configuration, - values=site_configuration, enabled=True ) else: diff --git a/openedx/core/djangoapps/user_api/management/tests/test_sync_hubspot_contacts.py b/openedx/core/djangoapps/user_api/management/tests/test_sync_hubspot_contacts.py index d61bd05f48..2a966f48c9 100644 --- a/openedx/core/djangoapps/user_api/management/tests/test_sync_hubspot_contacts.py +++ b/openedx/core/djangoapps/user_api/management/tests/test_sync_hubspot_contacts.py @@ -31,8 +31,7 @@ class TestHubspotSyncCommand(TestCase): super(TestHubspotSyncCommand, cls).setUpClass() cls.site_config = SiteConfigurationFactory() cls.hubspot_site_config = SiteConfigurationFactory.create( - site_values={'HUBSPOT_API_KEY': 'test_key'}, - values={'HUBSPOT_API_KEY': 'test_key'}, + site_values={'HUBSPOT_API_KEY': 'test_key'} ) cls.users = [] cls._create_users(cls.hubspot_site_config) # users for a site with hubspot integration enabled @@ -64,8 +63,8 @@ class TestHubspotSyncCommand(TestCase): """ Test no _sync_site call is made if hubspot integration is not enabled for any site """ - orig_values = self.hubspot_site_config.values - self.hubspot_site_config.values = {} + orig_values = self.hubspot_site_config.site_values + self.hubspot_site_config.site_values = {} self.hubspot_site_config.save() sync_site = patch.object(sync_command, '_sync_site') mock_sync_site = sync_site.start() @@ -73,7 +72,7 @@ class TestHubspotSyncCommand(TestCase): self.assertFalse(mock_sync_site.called, "_sync_site should not be called") sync_site.stop() # put values back - self.hubspot_site_config.values = orig_values + self.hubspot_site_config.site_values = orig_values self.hubspot_site_config.save() def test_with_initial_sync_days(self): diff --git a/openedx/features/content_type_gating/tests/test_models.py b/openedx/features/content_type_gating/tests/test_models.py index c6cf276bdc..89c66668d0 100644 --- a/openedx/features/content_type_gating/tests/test_models.py +++ b/openedx/features/content_type_gating/tests/test_models.py @@ -132,12 +132,10 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): 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( - site_values={'course_org_filter': non_test_course_enabled.org}, - values={'course_org_filter': non_test_course_enabled.org} + site_values={'course_org_filter': non_test_course_enabled.org} ) non_test_site_cfg_disabled = SiteConfigurationFactory.create( - site_values={'course_org_filter': non_test_course_disabled.org}, - values={'course_org_filter': non_test_course_disabled.org} + site_values={'course_org_filter': non_test_course_disabled.org} ) ContentTypeGatingConfig.objects.create(course=non_test_course_enabled, enabled=True, enabled_as_of=datetime(2018, 1, 1)) @@ -150,8 +148,7 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): # Set up test objects test_course = CourseOverviewFactory.create(org='test-org') test_site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': test_course.org}, - values={'course_org_filter': test_course.org} + site_values={'course_org_filter': test_course.org} ) ContentTypeGatingConfig.objects.create(enabled=global_setting, enabled_as_of=datetime(2018, 1, 1)) @@ -176,14 +173,13 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): ContentTypeGatingConfig.objects.create(enabled=global_setting, enabled_as_of=datetime(2018, 1, 1)) for site_setting in (True, False, None): test_site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': []}, - values={'course_org_filter': []} + site_values={'course_org_filter': []} ) ContentTypeGatingConfig.objects.create(site=test_site_cfg.site, enabled=site_setting, enabled_as_of=datetime(2018, 1, 1)) for org_setting in (True, False, None): test_org = "{}-{}".format(test_site_cfg.id, org_setting) - test_site_cfg.values['course_org_filter'].append(test_org) + test_site_cfg.site_values['course_org_filter'].append(test_org) test_site_cfg.save() ContentTypeGatingConfig.objects.create(org=test_org, enabled=org_setting, enabled_as_of=datetime(2018, 1, 1)) @@ -292,8 +288,7 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): def test_caching_org(self): course = CourseOverviewFactory.create(org='test-org') site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': course.org}, - values={'course_org_filter': course.org} + site_values={'course_org_filter': course.org} ) org_config = ContentTypeGatingConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() @@ -340,8 +335,7 @@ class TestContentTypeGatingConfig(CacheIsolationTestCase): def test_caching_course(self): course = CourseOverviewFactory.create(org='test-org') site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': course.org}, - values={'course_org_filter': course.org} + site_values={'course_org_filter': course.org} ) course_config = ContentTypeGatingConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1)) course_config.save() diff --git a/openedx/features/course_duration_limits/tests/test_models.py b/openedx/features/course_duration_limits/tests/test_models.py index fe4865fd77..16414054a3 100644 --- a/openedx/features/course_duration_limits/tests/test_models.py +++ b/openedx/features/course_duration_limits/tests/test_models.py @@ -142,12 +142,10 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): 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( - site_values={'course_org_filter': non_test_course_enabled.org}, - values={'course_org_filter': non_test_course_enabled.org} + site_values={'course_org_filter': non_test_course_enabled.org} ) non_test_site_cfg_disabled = SiteConfigurationFactory.create( - site_values={'course_org_filter': non_test_course_disabled.org}, - values={'course_org_filter': non_test_course_disabled.org} + site_values={'course_org_filter': non_test_course_disabled.org} ) CourseDurationLimitConfig.objects.create(course=non_test_course_enabled, enabled=True) @@ -160,8 +158,7 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): # Set up test objects test_course = CourseOverviewFactory.create(org='test-org') test_site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': test_course.org}, - values={'course_org_filter': test_course.org} + site_values={'course_org_filter': test_course.org} ) if reverse_order: @@ -191,8 +188,7 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): CourseDurationLimitConfig.objects.create(enabled=global_setting, enabled_as_of=datetime(2018, 1, 1)) for site_setting in (True, False, None): test_site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': []}, - values={'course_org_filter': []} + site_values={'course_org_filter': []} ) CourseDurationLimitConfig.objects.create( site=test_site_cfg.site, enabled=site_setting, enabled_as_of=datetime(2018, 1, 1) @@ -200,7 +196,7 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): for org_setting in (True, False, None): test_org = "{}-{}".format(test_site_cfg.id, org_setting) - test_site_cfg.values['course_org_filter'].append(test_org) + test_site_cfg.site_values['course_org_filter'].append(test_org) test_site_cfg.save() CourseDurationLimitConfig.objects.create( @@ -310,8 +306,7 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): def test_caching_org(self): course = CourseOverviewFactory.create(org='test-org') site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': course.org}, - values={'course_org_filter': course.org} + site_values={'course_org_filter': course.org} ) org_config = CourseDurationLimitConfig(org=course.org, enabled=True, enabled_as_of=datetime(2018, 1, 1)) org_config.save() @@ -358,8 +353,7 @@ class TestCourseDurationLimitConfig(CacheIsolationTestCase): def test_caching_course(self): course = CourseOverviewFactory.create(org='test-org') site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': course.org}, - values={'course_org_filter': course.org} + site_values={'course_org_filter': course.org} ) course_config = CourseDurationLimitConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1)) course_config.save() diff --git a/openedx/features/discounts/tests/test_discount_restriction_models.py b/openedx/features/discounts/tests/test_discount_restriction_models.py index 1de2ca7c72..1288d202e3 100644 --- a/openedx/features/discounts/tests/test_discount_restriction_models.py +++ b/openedx/features/discounts/tests/test_discount_restriction_models.py @@ -59,12 +59,10 @@ class TestDiscountRestrictionConfig(CacheIsolationTestCase): non_test_course_disabled = CourseOverviewFactory.create(org='non-test-org-disabled') non_test_course_enabled = CourseOverviewFactory.create(org='non-test-org-enabled') non_test_site_cfg_disabled = SiteConfigurationFactory.create( - site_values={'course_org_filter': non_test_course_disabled.org}, - values={'course_org_filter': non_test_course_disabled.org} + site_values={'course_org_filter': non_test_course_disabled.org} ) non_test_site_cfg_enabled = SiteConfigurationFactory.create( - site_values={'course_org_filter': non_test_course_enabled.org}, - values={'course_org_filter': non_test_course_enabled.org} + site_values={'course_org_filter': non_test_course_enabled.org} ) DiscountRestrictionConfig.objects.create(course=non_test_course_disabled, disabled=True) @@ -77,8 +75,7 @@ class TestDiscountRestrictionConfig(CacheIsolationTestCase): # Set up test objects test_course = CourseOverviewFactory.create(org='test-org') test_site_cfg = SiteConfigurationFactory.create( - site_values={'course_org_filter': test_course.org}, - values={'course_org_filter': test_course.org} + site_values={'course_org_filter': test_course.org} ) DiscountRestrictionConfig.objects.create(disabled=global_setting)