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

@@ -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(

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):

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)

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'))

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)

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)

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_email = self.site_configuration.site_values.get('course_email_from_addr')
-        site_template = self.site_configuration.values.get('course_email_template_name')
+        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}
         })

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(

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)

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)},
+            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)}
         )
 
     if settings.FEATURES.get('EXPOSE_CACHE_PROGRAMS_ENDPOINT'):

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({

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']},
+            site_values={'course_org_filter': ['testX']}
-            values={'course_org_filter': ['testX']},
         )
 
         call_command(Command(), '--site', site_config.site.domain, '--start-date', '2017-01-01')

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]},
+            site_values={'course_org_filter': [self.key.org]}
-            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)

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()

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']},
+            site_values={'course_org_filter': ['edX']}
-            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)

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},
+            site_values={'course_org_filter': this_org_list}
-            values={'course_org_filter': this_org_list}
         )
         other_config = SiteConfigurationFactory.create(
-            site_values={'course_org_filter': other_org_list},
+            site_values={'course_org_filter': other_org_list}
-            values={'course_org_filter': other_org_list}
         )
 
         for config in (this_config, other_config):

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},
+            site_values={'course_org_filter': course_org_filter}
-            values={'course_org_filter': course_org_filter},
         )
         mock_query = Mock()
         result = self.resolver.filter_by_org(mock_query)

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')
+    list_display = ('site', 'enabled', 'site_values')
-    search_fields = ('site__domain', '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']
 

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
 

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:

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)

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.
+        ),
+    ]

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"<SiteConfiguration: {site} >".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(
+    # Skip writing history when asked by the caller.  This skip feature only
-        site=instance.site,
+    # works for non-creates.
-        site_values=instance.values,
+    if created or not hasattr(instance, "skip_history_when_saving"):
-        values=instance.values,
+        SiteConfigurationHistory.objects.create(
-        enabled=instance.enabled,
+            site=instance.site,
-    )
+            site_values=instance.site_values,
+            enabled=instance.enabled,
+        )

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 {}

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,
+            site_values=site_config
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=site_config_other
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=site_configuration_values
-            # TODO: Remove this deprecated value eventually.
-            values=site_configuration_values
         )
         self.use_site(site)
         return site

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()

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,
+            site_values=self.test_config1
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=invalid_data
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=self.test_config1
-            # TODO: Remove this deprecated value eventually.
-            values=self.test_config1,
         )
         SiteConfigurationFactory.create(
             site=self.site2,
-            site_values=self.test_config2,
+            site_values=self.test_config2
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=self.test_config1
-            # TODO: Remove this deprecated value eventually.
-            values=self.test_config1,
         )
         config2 = SiteConfigurationFactory.create(
             site=self.site2,
-            site_values=self.test_config2,
+            site_values=self.test_config2
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=self.test_config1
-            # TODO: Remove this deprecated value eventually.
-            values=self.test_config1,
         )
         SiteConfigurationFactory.create(
             site=self.site2,
-            site_values=self.test_config2,
+            site_values=self.test_config2
-            # TODO: Remove this deprecated value eventually.
-            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,
+            site_values=self.test_config2
-            # TODO: Remove this deprecated value eventually.
-            values=self.test_config2,
         )
 
         # Test that the default value is returned if the value for the given key is not found in the configuration

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',

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:

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'},
+            site_values={'HUBSPOT_API_KEY': 'test_key'}
-            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
+        orig_values = self.hubspot_site_config.site_values
-        self.hubspot_site_config.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):

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},
+            site_values={'course_org_filter': non_test_course_enabled.org}
-            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},
+            site_values={'course_org_filter': non_test_course_disabled.org}
-            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},
+            site_values={'course_org_filter': test_course.org}
-            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': []},
+                    site_values={'course_org_filter': []}
-                    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},
+            site_values={'course_org_filter': course.org}
-            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},
+            site_values={'course_org_filter': course.org}
-            values={'course_org_filter': course.org}
         )
         course_config = ContentTypeGatingConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1))
         course_config.save()

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},
+            site_values={'course_org_filter': non_test_course_enabled.org}
-            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},
+            site_values={'course_org_filter': non_test_course_disabled.org}
-            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},
+            site_values={'course_org_filter': test_course.org}
-            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': []},
+                    site_values={'course_org_filter': []}
-                    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},
+            site_values={'course_org_filter': course.org}
-            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},
+            site_values={'course_org_filter': course.org}
-            values={'course_org_filter': course.org}
         )
         course_config = CourseDurationLimitConfig(course=course, enabled=True, enabled_as_of=datetime(2018, 1, 1))
         course_config.save()

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},
+            site_values={'course_org_filter': non_test_course_disabled.org}
-            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},
+            site_values={'course_org_filter': non_test_course_enabled.org}
-            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},
+            site_values={'course_org_filter': test_course.org}
-            values={'course_org_filter': test_course.org}
         )
 
         DiscountRestrictionConfig.objects.create(disabled=global_setting)