Skip to content

Commit

Permalink
Revert "Rename values in SiteConfiguration (2/3) attempt #2"
Browse files Browse the repository at this point in the history
  • Loading branch information
pwnage101 authored Feb 27, 2020
1 parent f945687 commit e6f58b6
Show file tree
Hide file tree
Showing 33 changed files with 176 additions and 184 deletions.
2 changes: 1 addition & 1 deletion common/djangoapps/student/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ def clean_email(self):
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 site_values field.")
"model's values field.")
)
elif email_domain != allowed_site_email_domain:
raise forms.ValidationError(
Expand Down
4 changes: 2 additions & 2 deletions common/djangoapps/student/tests/test_admin_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ def setUpClass(cls):

def _update_site_configuration(self):
""" Updates the site's configuration """
self.site.configuration.site_values = {'THIRD_PARTY_AUTH_ONLY_DOMAIN': self.email_domain_name}
self.site.configuration.values = {'THIRD_PARTY_AUTH_ONLY_DOMAIN': self.email_domain_name}
self.site.configuration.save()

def _assert_form(self, site, email, is_valid_form=False):
Expand All @@ -480,7 +480,7 @@ def test_form_with_invalid_site_configuration(self):
self.assertEqual(
error,
"Please add a key/value 'THIRD_PARTY_AUTH_ONLY_DOMAIN/{site_email_domain}' in SiteConfiguration "
"model's site_values field."
"model's values field."
)

def test_form_with_invalid_domain_name(self):
Expand Down
8 changes: 3 additions & 5 deletions common/djangoapps/util/tests/test_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,7 @@ 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. ALSO need to skip as part of renaming a field in the site_configuration app. This will be unskipped in DENG-18."
)
@unittest.skip("Need to skip as part of renaming a field in schedules app. This will be unskipped in DE-1825")
@override_settings(MIGRATION_MODULES={})
def test_migrations_are_in_sync(self):
"""
Expand All @@ -239,6 +237,6 @@ def test_migrations_are_in_sync(self):
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)
4 changes: 2 additions & 2 deletions lms/djangoapps/branding/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ def test_index_redirects_to_marketing_site_with_site_override(self):
response = self.client.get(reverse("root"))
self.assertRedirects(
response,
self.site_configuration_other.site_values["MKTG_URLS"]["ROOT"],
self.site_configuration_other.values["MKTG_URLS"]["ROOT"],
fetch_redirect_response=False
)

Expand All @@ -309,4 +309,4 @@ def test_header_logo_links_to_marketing_site_with_site_override(self):
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.site_values["MKTG_URLS"]["ROOT"], response.content.decode('utf-8'))
self.assertIn(self.site_configuration_other.values["MKTG_URLS"]["ROOT"], response.content.decode('utf-8'))
2 changes: 2 additions & 0 deletions lms/djangoapps/discussion/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def test_comment_created_signal_sends_message(self, mock_send_message, mock_get_
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)
Expand Down Expand Up @@ -59,6 +60,7 @@ def test_comment_created_signal_msg_not_sent_with_site_config_disabled(
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)
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/discussion/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def test_send_discussion_email_notification(self, user_subscribed):
comment = cc.Comment.find(id=self.comment['id']).retrieve()
site = Site.objects.get_current()
site_config = SiteConfigurationFactory.create(site=site)
site_config.site_values[ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY] = True
site_config.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)
Expand Down
6 changes: 3 additions & 3 deletions lms/djangoapps/instructor/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4000,8 +4000,8 @@ def test_send_email_no_message(self):
self.assertEqual(response.status_code, 400)

def test_send_email_with_site_template_and_from_addr(self):
site_email = self.site_configuration.site_values.get('course_email_from_addr')
site_template = self.site_configuration.site_values.get('course_email_template_name')
site_email = self.site_configuration.values.get('course_email_from_addr')
site_template = self.site_configuration.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)
Expand All @@ -4019,7 +4019,7 @@ def test_send_email_with_org_template_and_from_addr(self):
org_email = 'fake_org@example.com'
org_template = 'fake_org_email_template'
CourseEmailTemplate.objects.create(name=org_template)
self.site_configuration.site_values.update({
self.site_configuration.values.update({
'course_email_from_addr': {self.course.id.org: org_email},
'course_email_template_name': {self.course.id.org: org_template}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ def test_membership_reason_field_visibility(self, enbale_reason_field):
SiteConfiguration.objects.create(
site=site,
site_values=configuration_values,
values=configuration_values,
enabled=True
)

Expand Down Expand Up @@ -201,6 +202,7 @@ def test_membership_site_configuration_role(self):
SiteConfiguration.objects.create(
site=site,
site_values=configuration_values,
values=configuration_values,
enabled=True
)
url = reverse(
Expand Down
3 changes: 3 additions & 0 deletions openedx/core/djangoapps/ace_common/tests/test_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ def test_site_config_override(self):
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)
Expand Down
3 changes: 2 additions & 1 deletion openedx/core/djangoapps/catalog/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ 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'):
Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/config_model_utils/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def all_current_course_configs(cls):
"""
all_courses = CourseOverview.objects.all()
all_site_configs = SiteConfiguration.objects.filter(
site_values__contains='course_org_filter', enabled=True
values__contains='course_org_filter', enabled=True
).select_related('site')

try:
Expand All @@ -254,7 +254,7 @@ def all_current_course_configs(cls):

sites_by_org = defaultdict(lambda: default_site)
site_cfg_org_filters = (
(site_cfg.site, site_cfg.site_values['course_org_filter'])
(site_cfg.site, site_cfg.values['course_org_filter'])
for site_cfg in all_site_configs
)
sites_by_org.update({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ def test_page_size(self):
@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')
Expand Down
5 changes: 3 additions & 2 deletions openedx/core/djangoapps/credentials/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ def test_send_grade_without_issuance_enabled(self, _mock_is_course_run_in_a_prog
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
Expand All @@ -119,7 +120,7 @@ def test_send_grade_records_enabled(self, _mock_is_course_run_in_a_program, mock
mock_send_grade_to_credentials.delay.reset_mock()

# Correctly not sent
site_config.site_values['ENABLE_LEARNER_RECORDS'] = False
site_config.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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def test_awarding_certs_with_skip_program_certificate(
mock_get_certified_programs.return_value = [1]

# programs to be skipped
self.site_configuration.site_values = {
self.site_configuration.values = {
"programs_without_certificates": [2]
}
self.site_configuration.save()
Expand Down
5 changes: 3 additions & 2 deletions openedx/core/djangoapps/programs/tests/test_signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ def test_records_enabled(self, mock_is_learner_issuance_enabled, mock_task):
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
Expand All @@ -155,7 +156,7 @@ def test_records_enabled(self, mock_is_learner_issuance_enabled, mock_task):
mock_task.reset_mock()

# Correctly not sent
site_config.site_values['ENABLE_LEARNER_RECORDS'] = False
site_config.values['ENABLE_LEARNER_RECORDS'] = False
site_config.save()
handle_course_cert_changed(**self.signal_kwargs)
self.assertFalse(mock_task.called)
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,12 @@ def test_site_config(self, this_org_list, other_org_list, expected_message_count
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):
Expand Down
7 changes: 4 additions & 3 deletions openedx/core/djangoapps/schedules/tests/test_resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def setUp(self):
'course1'
)
def test_get_course_org_filter_equal(self, course_org_filter):
self.site_config.site_values['course_org_filter'] = course_org_filter
self.site_config.values['course_org_filter'] = course_org_filter
self.site_config.save()
mock_query = Mock()
result = self.resolver.filter_by_org(mock_query)
Expand All @@ -73,7 +73,7 @@ def test_get_course_org_filter_equal(self, course_org_filter):
(['course1', 'course2'], ['course1', 'course2'])
)
def test_get_course_org_filter_include__in(self, course_org_filter, expected_org_list):
self.site_config.site_values['course_org_filter'] = course_org_filter
self.site_config.values['course_org_filter'] = course_org_filter
self.site_config.save()
mock_query = Mock()
result = self.resolver.filter_by_org(mock_query)
Expand All @@ -88,7 +88,8 @@ def test_get_course_org_filter_include__in(self, course_org_filter, expected_org
)
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)
Expand Down
6 changes: 3 additions & 3 deletions openedx/core/djangoapps/site_configuration/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ class SiteConfigurationAdmin(admin.ModelAdmin):
"""
Admin interface for the SiteConfiguration object.
"""
list_display = ('site', 'enabled', 'site_values')
search_fields = ('site__domain', 'site_values')
list_display = ('site', 'enabled', 'values')
search_fields = ('site__domain', 'values')

class Meta(object):
"""
Expand All @@ -29,7 +29,7 @@ class SiteConfigurationHistoryAdmin(admin.ModelAdmin):
Admin interface for the SiteConfigurationHistory object.
"""
list_display = ('site', 'enabled', 'created', 'modified')
search_fields = ('site__domain', 'site_values', 'created', 'modified')
search_fields = ('site__domain', 'values', 'created', 'modified')

ordering = ['-created']

Expand Down
2 changes: 1 addition & 1 deletion openedx/core/djangoapps/site_configuration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.site_values:
if configuration and name in configuration.values:
return True
return False

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ def handle(self, *args, **options):
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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def get_site_configuration(self):
def create_fixture_site_configuration(self, enabled):
SiteConfiguration.objects.update_or_create(
site=self.site,
defaults={'enabled': enabled, 'site_values': {'ABC': 'abc', 'B': 'b'}}
defaults={'enabled': enabled, 'values': {'ABC': 'abc', 'B': 'b'}}
)

def test_command_no_args(self):
Expand Down Expand Up @@ -105,7 +105,7 @@ def test_site_configuration_created_when_non_existent(self):

call_command(self.command, *self.site_id_arg)
site_configuration = SiteConfiguration.objects.get(site=self.site)
self.assertFalse(site_configuration.site_values)
self.assertFalse(site_configuration.values)
self.assertFalse(site_configuration.enabled)

def test_both_enabled_disabled_flags(self):
Expand All @@ -126,7 +126,7 @@ def test_site_configuration_enabled_disabled(self, flag, enabled):
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.site_values)
self.assertFalse(site_configuration.values)
self.assertEqual(enabled, site_configuration.enabled)

def test_site_configuration_created_with_parameters(self):
Expand All @@ -136,7 +136,7 @@ def test_site_configuration_created_with_parameters(self):
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.site_values, self.input_configuration)
self.assertDictEqual(site_configuration.values, self.input_configuration)

def test_site_configuration_created_with_json_file_parameters(self):
"""
Expand All @@ -145,7 +145,7 @@ def test_site_configuration_created_with_json_file_parameters(self):
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.site_values, {'ABC': 123, 'XYZ': '789'})
self.assertEqual(site_configuration.values, {'ABC': 123, 'XYZ': '789'})

@ddt.data(True, False)
def test_site_configuration_updated_with_parameters(self, enabled):
Expand All @@ -156,7 +156,7 @@ def test_site_configuration_updated_with_parameters(self, enabled):
call_command(self.command, '--configuration', json.dumps(self.input_configuration), *self.site_id_arg)
site_configuration = self.get_site_configuration()
self.assertEqual(
site_configuration.site_values,
site_configuration.values,
{'ABC': 123, 'B': 'b', 'FEATURE_FLAG': True, 'SERVICE_URL': 'https://foo.bar'}
)
self.assertEqual(site_configuration.enabled, enabled)
Expand All @@ -172,5 +172,5 @@ def test_site_configuration_updated_from_json_file(self, enabled):
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.site_values, expected_site_configuration)
self.assertEqual(site_configuration.values, expected_site_configuration)
self.assertEqual(site_configuration.enabled, enabled)
Loading

0 comments on commit e6f58b6

Please sign in to comment.