Merge pull request #13127 from edx/saleem-latif/MAYN-207

MAYN-207: Update configuration helpers to take into account microsite configurations

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

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

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

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