From cff28539aa5a18d40ae74e204699ec1d578dcb22 Mon Sep 17 00:00:00 2001 From: Saleem Latif Date: Fri, 29 Jul 2016 16:50:42 +0500 Subject: [PATCH] Update configuration helpers to take into account microsite configurations --- .../djangoapps/site_configuration/helpers.py | 28 ++++++++------- .../djangoapps/site_configuration/models.py | 13 ++++++- .../site_configuration/tests/test_helpers.py | 35 +++++++++++++++++-- .../site_configuration/tests/test_util.py | 16 ++++++--- 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/openedx/core/djangoapps/site_configuration/helpers.py b/openedx/core/djangoapps/site_configuration/helpers.py index 8671793cbd..2ffc3bbb7b 100644 --- a/openedx/core/djangoapps/site_configuration/helpers.py +++ b/openedx/core/djangoapps/site_configuration/helpers.py @@ -168,19 +168,21 @@ def has_override_value(name): def get_value_for_org(org, val_name, default=None): """ - This returns a configuration value for a site configuration which has an org_filter that matches - what is passed in. + This returns a configuration value for a site configuration or microsite configuration + which has an org_filter that matches with the argument. Args: - org (str): Course ord filter, this value will be used to filter out the correct site configuration. + org (str): Course org filter, this value will be used to filter out the correct site configuration. name (str): Name of the key for which to return configuration value. - default: default value tp return if key is not found in the configuration + default: default value to return if key is not present in the configuration Returns: Configuration value for the given key. """ - if is_site_configuration_enabled(): + # Here we first look for the asked org inside site configuration, and if org is not present in site configuration + # then we go ahead and look it inside microsite configuration. + if SiteConfiguration.has_org(org): return SiteConfiguration.get_value_for_org(org, val_name, default) else: return microsite.get_value_for_org(org, val_name, default) @@ -188,16 +190,16 @@ def get_value_for_org(org, val_name, default=None): def get_all_orgs(): """ - This returns all of the orgs that are considered in site configurations, This can be used, - for example, to do filtering. + This returns all of the orgs that are considered in site configurations or microsite configuration, + This can be used, for example, to do filtering. - Returns: - Configuration value for the given key. + Returns: + A list of all organizations present in either microsite configuration or site configuration. """ - if is_site_configuration_enabled(): - return SiteConfiguration.get_all_orgs() - else: - return microsite.get_all_orgs() + site_configuration_orgs = SiteConfiguration.get_all_orgs() + microsite_orgs = microsite.get_all_orgs() + + return site_configuration_orgs.union(microsite_orgs) def page_title_breadcrumbs(*crumbs, **kwargs): diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index 1a8ccd1f74..61c05eb044 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -89,15 +89,26 @@ class SiteConfiguration(models.Model): for example, to do filtering. Returns: - Configuration value for the given key. + A list of all organizations present in site configuration. """ org_filter_set = set() + for configuration in cls.objects.filter(values__contains='course_org_filter', enabled=True).all(): org_filter = configuration.get_value('course_org_filter', None) if org_filter: org_filter_set.add(org_filter) return org_filter_set + @classmethod + def has_org(cls, org): + """ + Check if the given organization is present in any of the site configuration. + + Returns: + True if given organization is present in site configurations otherwise False. + """ + return org in cls.get_all_orgs() + class SiteConfigurationHistory(TimeStampedModel): """ diff --git a/openedx/core/djangoapps/site_configuration/tests/test_helpers.py b/openedx/core/djangoapps/site_configuration/tests/test_helpers.py index 4edad72b5d..305d2fe046 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_helpers.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_helpers.py @@ -148,11 +148,42 @@ class TestHelpers(TestCase): "default for non existent" ) + def test_get_value_for_org_2(self): + """ + Test that get_value_for_org returns correct value for any given key. + """ + test_org = test_config['course_org_filter'] + with with_site_configuration_context(configuration=test_config): + + # Make sure if ORG is not present in site configuration then microsite configuration is used instead + self.assertEqual( + configuration_helpers.get_value_for_org("TestSiteX", "email_from_address"), + "test_site@edx.org" + ) + # Make sure 'default' is returned if org is present but key is not + self.assertEqual( + configuration_helpers.get_value_for_org(test_org, "email_from_address"), + None + ) + # Make sure if ORG is not present in site configuration then microsite configuration is used instead + self.assertEqual( + configuration_helpers.get_value_for_org("LogistrationX", "email_from_address"), + "test_site@edx.org" + ) + + # This test must come after the above test + with with_site_configuration_context(configuration={"course_org_filter": "TestSiteX", "university": "Test"}): + # Make sure site configuration gets preference over microsite configuration + self.assertEqual( + configuration_helpers.get_value_for_org("TestSiteX", "university"), + "Test" + ) + def test_get_all_orgs(self): """ - Test that get_all_orgs returns correct values. + Test that get_all_orgs returns organizations defined in both site configuration and microsite configuration. """ - test_orgs = [test_config['course_org_filter']] + test_orgs = [test_config['course_org_filter'], "LogistrationX", "TestSiteX"] with with_site_configuration_context(configuration=test_config): self.assertItemsEqual( list(configuration_helpers.get_all_orgs()), diff --git a/openedx/core/djangoapps/site_configuration/tests/test_util.py b/openedx/core/djangoapps/site_configuration/tests/test_util.py index ad4f7efd55..ace9bccaf5 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_util.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_util.py @@ -26,9 +26,13 @@ def with_site_configuration(domain="test.localhost", configuration=None): def _decorated(*args, **kwargs): # pylint: disable=missing-docstring # make a domain name out of directory name site, __ = Site.objects.get_or_create(domain=domain, name=domain) - site_configuration, __ = SiteConfiguration.objects.get_or_create( - site=site, enabled=True, values=configuration, + site_configuration, created = SiteConfiguration.objects.get_or_create( + site=site, + defaults={"enabled": True, "values": configuration}, ) + if not created: + site_configuration.values = configuration + site_configuration.save() with patch('openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration', return_value=site_configuration): @@ -48,9 +52,13 @@ def with_site_configuration_context(domain="test.localhost", configuration=None) configuration (dict): configuration to use for the test site. """ site, __ = Site.objects.get_or_create(domain=domain, name=domain) - site_configuration, __ = SiteConfiguration.objects.get_or_create( - site=site, enabled=True, values=configuration, + site_configuration, created = SiteConfiguration.objects.get_or_create( + site=site, + defaults={"enabled": True, "values": configuration}, ) + if not created: + site_configuration.values = configuration + site_configuration.save() with patch('openedx.core.djangoapps.site_configuration.helpers.get_current_site_configuration', return_value=site_configuration):