Skip to content
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
2 changes: 2 additions & 0 deletions openedx/core/djangoapps/appsembler/api/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
import datetime

import factory
import factory.fuzzy

@bryanlandia bryanlandia Dec 18, 2020

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't import directly from factory. This was breaking tests

from openedx.core.djangoapps.content.course_overviews.models import (
CourseOverview,
)
Expand Down
72 changes: 70 additions & 2 deletions openedx/core/djangoapps/appsembler/sites/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
from django.test import TestCase
import unittest
from mock import patch

from openedx.core.djangoapps.appsembler.sites.utils import get_initial_page_elements
from django.core.exceptions import ImproperlyConfigured, MultipleObjectsReturned
from django.test import TestCase, override_settings
from django.test.client import RequestFactory

from openedx.core.djangoapps.appsembler.api.tests.factories import OrganizationFactory
from openedx.core.djangoapps.appsembler.sites.utils import get_current_organization, get_initial_page_elements
from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory
from organizations.models import Organization


class JSONMigrationUtilsTestCase(TestCase):
Expand All @@ -14,3 +22,63 @@ def test_initial_page_elements(self):
self.assertEqual(element['options']['text-content'], {
'en': 'Welcome to your Tahoe trial LMS site!',
})


class OrganizationByRequestTestCase(TestCase):
def setUp(self):
super(OrganizationByRequestTestCase, self).setUp()
self.siteFoo = SiteFactory.create(domain='foo.dev', name='foo.dev')
self.siteBar = SiteFactory.create(domain='bar.dev', name='bar.dev')
self.siteBaz = SiteFactory.create(domain='baz.dev', name='baz.dev')
self.organizationA = OrganizationFactory(sites=[self.siteFoo])
self.organizationB = OrganizationFactory(sites=[self.siteFoo])
self.organizationC = OrganizationFactory(sites=[self.siteBar])
self.request = RequestFactory().post('dummy_url')
self.request.session = {}
for patch_req in (
'openedx.core.djangoapps.appsembler.sites.utils.get_current_request',
'openedx.core.djangoapps.theming.helpers.get_current_request'
):
patcher = patch(patch_req)
patched_req = patcher.start()
patched_req.return_value = self.request
self.addCleanup(patcher.stop)

@unittest.skip
def test_amc_admin_user_no_org_in_request(self):
# TODO: would be good to test
pass

@patch.dict('django.conf.settings.FEATURES', {'TAHOE_ENABLE_MULTI_ORGS_PER_SITE': False})
def test_single_organization_multiorg_feature_off(self):
self.request.site = self.siteBar
current_org = get_current_organization()
self.assertEqual(current_org, self.organizationC)

@patch.dict('django.conf.settings.FEATURES', {'TAHOE_ENABLE_MULTI_ORGS_PER_SITE': False})
def test_multiple_organization_multiorg_feature_off(self):
self.request.site = self.siteFoo
# fail raising exception if more than one org found for site when feature not enabled
with self.assertRaises(MultipleObjectsReturned):
get_current_organization()

@patch.dict('django.conf.settings.FEATURES', {'TAHOE_ENABLE_MULTI_ORGS_PER_SITE': True})
def test_multiple_organizations_multiorg_feature_on(self):
self.request.site = self.siteFoo
# return one org from Site's org relations
current_org = get_current_organization()
self.assertIn(current_org, (self.organizationA, self.organizationB))

def test_no_org_for_site(self):
self.request.site = self.siteBaz
with self.assertRaises(Organization.DoesNotExist):
get_current_organization()

@patch.dict('django.conf.settings.FEATURES', {
'TAHOE_ENABLE_MULTI_ORGS_PER_SITE': True,
'APPSEMBLER_MULTI_TENANT_EMAILS': True
})
def test_raises_if_multiorg_feature_and_multitenant_email_feature_on(self):
self.request.site = self.siteFoo
with self.assertRaises(ImproperlyConfigured):
get_current_organization()
21 changes: 17 additions & 4 deletions openedx/core/djangoapps/appsembler/sites/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.conf import settings
from django.contrib.auth.models import User
from django.contrib.sites.models import Site
from django.core.exceptions import ImproperlyConfigured
from django.db.models.query import Q
from provider.oauth2.models import AccessToken, RefreshToken, Client
from django.utils.text import slugify
Expand Down Expand Up @@ -233,10 +234,22 @@ def get_current_organization(failure_return_none=False):
)
else:
try:
# TODO: Using `get` is expected to fail when multiple-orgs found for a site.
# Maybe catch MultipleObjectsReturned?
current_org = current_site.organizations.get()
except Organization.DoesNotExist:
if settings.FEATURES.get('TAHOE_ENABLE_MULTI_ORGS_PER_SITE', False):
if settings.FEATURES.get('APPSEMBLER_MULTI_TENANT_EMAILS', False):
raise ImproperlyConfigured(
'TAHOE_ENABLE_MULTI_ORGS_PER_SITE and '
'APPSEMBLER_MULTI_TENANT_EMAILS are incompatible as '
'we are not able to determine the exact Org when more than one '
'is associated with a Site.')
current_org = current_site.organizations.first()
if not current_org:
raise Organization.DoesNotExist(
'TAHOE_ENABLE_MULTI_ORGS_PER_SITE: Could not find current '
'organization for site `{}`'.format(repr(current_site))
)
else:
current_org = current_site.organizations.get()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to note that this will fail with a MultipleObjectsReturned if there happen to be multiple orgs for a site in the deployment that has TAHOE_ENABLE_MULTI_ORGS_PER_SITE=True So we probably want to log this somewhere. Would it get automatically thrown to Sentry?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean if TAHOE_ENABLE_MULTI_ORGS_PER_SITE=False. Agreed, would probably be good to catch that and generally log the exceptions.

except (Organization.DoesNotExist, ImproperlyConfigured):
if not failure_return_none:
raise # Re-raise the exception
else:
Expand Down