Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prioritize current site setting for org value #189

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions eox_tenant/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,25 +89,35 @@ def get_microsite_for_domain(cls, domain):
return microsites[0] if microsites else None

@classmethod
def get_value_for_org(cls, org, val_name):
def get_value_for_org(cls, org, val_name, current_microsite):
"""
Filter the value by the org in the values field.
Filter the value by the org in the values field. Value from current
microsite is prioritized if org is present in multiple microsites
including the current one. Else the first valid value from an org is
returned.

Args:
org: String.
key: String.
current_microsite: String
Returns:
The value for the given key and org.
"""
results = cls.objects.filter(organizations__name=org)
first_result = None

for result in results:
value = result.values.get(val_name)
if value is None:
continue

if value:
if result.key == current_microsite:
return value

return None
if first_result is None:
first_result = value

return first_result


class TenantConfigManager(models.Manager):
Expand Down Expand Up @@ -201,25 +211,35 @@ def get_configs_for_domain(cls, domain):
objects = TenantConfigManager()

@classmethod
def get_value_for_org(cls, org, val_name):
def get_value_for_org(cls, org, val_name, current_tenant):
"""
Filter the value by the org in the lms_config field.
Filter the value by the org in the lms_config field. Value from current
tenant is prioritized if org is present in multiple tenants including
the current one. Else the first valid value from an org is returned.


Args:
org: String.
key: String.
val_name: String.
current_tenant: String
Returns:
The value for the given key and org.
"""
results = cls.objects.filter(organizations__name=org)
first_result = None

for result in results:
value = result.lms_configs.get(val_name)
if value is None:
continue

if value is not None:
if result.external_key == current_tenant:
return value

return None
if first_result is None:
first_result = value

return first_result


class Route(models.Model):
Expand Down
12 changes: 10 additions & 2 deletions eox_tenant/tenant_wise/proxies.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,18 +148,26 @@ def __get_value_for_org(cls, org, val_name, default=None):
TenantConfig or Microsite model.
"""
cache_key = f"org-value-{org}-{val_name}"

# Make use of tenant-external-key to generate unique cache_key per
# tenant. This will help to fetch the current tenant value if the org
# is configured in multiple tenants/microsites including the current
# one.
tenant_key = getattr(settings, "EDNX_TENANT_KEY", None)
if tenant_key is not None:
cache_key = f"{cache_key}-{tenant_key}"
cached_value = cache.get(cache_key)

if cached_value:
return cached_value

result = TenantConfig.get_value_for_org(org, val_name)
result = TenantConfig.get_value_for_org(org, val_name, tenant_key)

if result is not None:
cls.set_key_to_cache(cache_key, result)
return result

result = Microsite.get_value_for_org(org, val_name)
result = Microsite.get_value_for_org(org, val_name, tenant_key)
result = result if result else default
cls.set_key_to_cache(cache_key, result)

Expand Down
52 changes: 49 additions & 3 deletions eox_tenant/test/test_tenant_wise.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import json

from django.core.management import call_command
from django.test import TransactionTestCase
from django.test import TransactionTestCase, override_settings

from eox_tenant.models import Microsite, TenantConfig
from eox_tenant.tenant_wise.proxies import TenantSiteConfigProxy
Expand Down Expand Up @@ -41,7 +41,8 @@ def setUp(self):
TenantConfig.objects.create(
external_key="tenant-key1",
lms_configs={
"course_org_filter": "test4-org"
"course_org_filter": ["common-org", "test4-org"],
"lms_base": "tenant-1-base",
},
studio_configs={},
theming_configs={},
Expand All @@ -51,8 +52,9 @@ def setUp(self):
TenantConfig.objects.create(
external_key="tenant-key2",
lms_configs={
"course_org_filter": ["test5-org", "test1-org"],
"course_org_filter": ["common-org", "test5-org", "test1-org"],
"value-test": "Hello-World3",
"lms_base": "tenant-2-base",
},
studio_configs={},
theming_configs={},
Expand All @@ -70,6 +72,7 @@ def test_get_all_orgs(self):
"test3-org",
"test4-org",
"test5-org",
"common-org",
])

self.assertTrue(org_list == TenantSiteConfigProxy.get_all_orgs())
Expand Down Expand Up @@ -105,6 +108,49 @@ def test_get_value_for_org(self):
"Default",
)

# Prioritise current tenant's value if org is present
with override_settings(EDNX_TENANT_KEY="tenant-key1"):
self.assertEqual(
TenantSiteConfigProxy.get_value_for_org(
org="common-org",
val_name="lms_base",
default="tenant-1-base",
),
"tenant-1-base",
)

with override_settings(EDNX_TENANT_KEY="tenant-key2"):
self.assertEqual(
TenantSiteConfigProxy.get_value_for_org(
org="common-org",
val_name="lms_base",
default="tenant-2-base",
),
"tenant-2-base",
)

# should return the first valid value if org is not present in current tenant
with override_settings(EDNX_TENANT_KEY="tenant-key1"):
self.assertEqual(
TenantSiteConfigProxy.get_value_for_org(
org="common-org",
val_name="value-test",
default="some-value",
),
"Hello-World3",
)

# should return the first valid value if tenant config is not used by
# current site
self.assertEqual(
TenantSiteConfigProxy.get_value_for_org(
org="common-org",
val_name="value-test",
default="some-value",
),
"Hello-World3",
)

def test_create_site_configuration(self):
"""
Test that a new TenantSiteConfigProxy instance is created with
Expand Down
Loading