diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 1711368197..a3f28b56cd 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -286,14 +286,14 @@ def reverification_info(statuses): return reverifications -def get_course_enrollments(user, org_to_include, orgs_to_exclude): +def get_course_enrollments(user, orgs_to_include, orgs_to_exclude): """ Given a user, return a filtered set of his or her course enrollments. Arguments: user (User): the user in question. - org_to_include (str): If not None, ONLY courses of this org will be returned. - orgs_to_exclude (list[str]): If org_to_include is not None, this + orgs_to_include (list[str]): If not None, ONLY courses of these orgs will be returned. + orgs_to_exclude (list[str]): If orgs_to_include is not None, this argument is ignored. Else, courses of this org will be excluded. Returns: @@ -312,8 +312,8 @@ def get_course_enrollments(user, org_to_include, orgs_to_exclude): ) continue - # Filter out anything that is not attributed to the current ORG. - if org_to_include and course_overview.location.org != org_to_include: + # Filter out anything that is not attributed to the orgs to include. + if orgs_to_include and course_overview.location.org not in orgs_to_include: continue # Conversely, filter out any enrollments with courses attributed to current ORG. @@ -600,17 +600,16 @@ def dashboard(request): settings.FEATURES.get('DISPLAY_COURSE_MODES_ON_DASHBOARD', True) ) - # we want to filter and only show enrollments for courses within - # the 'ORG' defined in configuration. - course_org_filter = configuration_helpers.get_value('course_org_filter') - # Let's filter out any courses in an "org" that has been declared to be # in a configuration org_filter_out_set = configuration_helpers.get_all_orgs() - # remove our current org from the "filter out" list, if applicable + # Remove current site orgs from the "filter out" list, if applicable. + # We want to filter and only show enrollments for courses within + # the organizations defined in configuration for the current site. + course_org_filter = configuration_helpers.get_current_site_orgs() if course_org_filter: - org_filter_out_set.remove(course_org_filter) + org_filter_out_set = org_filter_out_set - set(course_org_filter) # Build our (course, enrollment) list for the user, but ignore any courses that no # longer exist (because the course IDs have changed). Still, we don't delete those diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py index 2c7045b540..9602cb79ef 100644 --- a/common/djangoapps/util/views.py +++ b/common/djangoapps/util/views.py @@ -295,11 +295,11 @@ def _record_feedback_in_zendesk( # Tag all issues with LMS to distinguish channel in Zendesk; requested by student support team zendesk_tags = list(tags.values()) + ["LMS"] - # Per edX support, we would like to be able to route white label feedback items - # via tagging - white_label_org = configuration_helpers.get_value('course_org_filter') - if white_label_org: - zendesk_tags = zendesk_tags + ["whitelabel_{org}".format(org=white_label_org)] + # Per edX support, we would like to be able to route feedback items by site via tagging + current_site_orgs = configuration_helpers.get_current_site_orgs() + if current_site_orgs: + for org in current_site_orgs: + zendesk_tags.append("whitelabel_{org}".format(org=org)) new_ticket = { "ticket": { diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py index 22a16cb7ae..cd603b03a7 100644 --- a/lms/djangoapps/branding/__init__.py +++ b/lms/djangoapps/branding/__init__.py @@ -27,25 +27,23 @@ def get_visible_courses(org=None, filter_=None): filter_ (dict): Optional parameter that allows custom filtering by fields on the course. """ - current_site_org = configuration_helpers.get_value('course_org_filter') + courses = [] + current_site_orgs = configuration_helpers.get_current_site_orgs() - if org and current_site_org: - # Return an empty result if the org passed by the caller does not match the designated site org. - courses = CourseOverview.get_all_courses( - org=org, - filter_=filter_, - ) if org == current_site_org else [] + if org: + # Check the current site's orgs to make sure the org's courses should be displayed + if not current_site_orgs or org in current_site_orgs: + courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_) + elif current_site_orgs: + # Only display courses that should be displayed on this site + courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_) else: - # We only make it to this point if one of org or current_site_org is defined. - # If both org and current_site_org were defined, the code would have fallen into the - # first branch of the conditional above, wherein an equality check is performed. - target_org = org or current_site_org - courses = CourseOverview.get_all_courses(org=target_org, filter_=filter_) + courses = CourseOverview.get_all_courses(filter_=filter_) courses = sorted(courses, key=lambda course: course.number) # Filtering can stop here. - if current_site_org: + if current_site_orgs: return courses # See if we have filtered course listings in this domain diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index c32b931648..34ad6d0d12 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -14,7 +14,6 @@ from django.contrib.staticfiles.storage import staticfiles_storage from edxmako.shortcuts import render_to_response import student.views -from student.models import CourseEnrollment import courseware.views.views from edxmako.shortcuts import marketing_link from util.cache import cache_if_anonymous @@ -25,24 +24,6 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_ log = logging.getLogger(__name__) -def get_course_enrollments(user): - """ - Returns the course enrollments for the passed in user within the context of current org, that - is filtered by course_org_filter - """ - enrollments = CourseEnrollment.enrollments_for_user(user) - course_org = configuration_helpers.get_value('course_org_filter') - if course_org: - site_enrollments = [ - enrollment for enrollment in enrollments if enrollment.course_id.org == course_org - ] - else: - site_enrollments = [ - enrollment for enrollment in enrollments - ] - return site_enrollments - - @ensure_csrf_cookie @cache_if_anonymous() def index(request): diff --git a/lms/djangoapps/shoppingcart/api.py b/lms/djangoapps/shoppingcart/api.py index 5c7145df55..65f2b95508 100644 --- a/lms/djangoapps/shoppingcart/api.py +++ b/lms/djangoapps/shoppingcart/api.py @@ -11,8 +11,8 @@ def order_history(user, **kwargs): Returns the list of previously purchased orders for a user. Only the orders with PaidCourseRegistration and CourseRegCodeItem are returned. Params: - course_org_filter: Current Microsite's ORG. - org_filter_out_set: A list of all other Microsites' ORGs. + course_org_filter: A list of the current Site's orgs. + org_filter_out_set: A list of all other Sites' orgs. """ course_org_filter = kwargs['course_org_filter'] if 'course_org_filter' in kwargs else None org_filter_out_set = kwargs['org_filter_out_set'] if 'org_filter_out_set' in kwargs else [] @@ -27,7 +27,7 @@ def order_history(user, **kwargs): # not attributed (by ORG) to any Microsite. order_item_course_id = getattr(order_item, 'course_id', None) if order_item_course_id: - if (course_org_filter and course_org_filter == order_item_course_id.org) or \ + if (course_org_filter and order_item_course_id.org in course_org_filter) or \ (course_org_filter is None and order_item_course_id.org not in org_filter_out_set): order_history_list.append({ 'order_id': order_item.order.id, diff --git a/lms/lib/courseware_search/lms_filter_generator.py b/lms/lib/courseware_search/lms_filter_generator.py index 50503e26cd..1af0713696 100644 --- a/lms/lib/courseware_search/lms_filter_generator.py +++ b/lms/lib/courseware_search/lms_filter_generator.py @@ -34,7 +34,7 @@ class LmsSearchFilterGenerator(SearchFilterGenerator): field_dictionary['course'] = [unicode(enrollment.course_id) for enrollment in user_enrollments] # if we have an org filter, only include results for this org filter - course_org_filter = configuration_helpers.get_value('course_org_filter') + course_org_filter = configuration_helpers.get_current_site_orgs() if course_org_filter: field_dictionary['org'] = course_org_filter @@ -45,7 +45,7 @@ class LmsSearchFilterGenerator(SearchFilterGenerator): Exclude any courses defined outside the current org. """ exclude_dictionary = super(LmsSearchFilterGenerator, self).exclude_dictionary(**kwargs) - course_org_filter = configuration_helpers.get_value('course_org_filter') + course_org_filter = configuration_helpers.get_current_site_orgs() # If we have a course filter we are ensuring that we only get those courses above if not course_org_filter: org_filter_out_set = configuration_helpers.get_all_orgs() diff --git a/lms/lib/courseware_search/test/test_lms_filter_generator.py b/lms/lib/courseware_search/test/test_lms_filter_generator.py index 42c6700d48..3f062c87d8 100644 --- a/lms/lib/courseware_search/test/test_lms_filter_generator.py +++ b/lms/lib/courseware_search/test/test_lms_filter_generator.py @@ -109,7 +109,7 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): field_dictionary, _, exclude_dictionary = LmsSearchFilterGenerator.generate_field_filters(user=self.user) self.assertNotIn('org', exclude_dictionary) self.assertIn('org', field_dictionary) - self.assertEqual('TestSiteX', field_dictionary['org']) + self.assertEqual(['TestSiteX'], field_dictionary['org']) @patch( 'openedx.core.djangoapps.site_configuration.helpers.get_all_orgs', @@ -134,4 +134,4 @@ class LmsSearchFilterGeneratorTestCase(ModuleStoreTestCase): field_dictionary, _, exclude_dictionary = LmsSearchFilterGenerator.generate_field_filters(user=self.user) self.assertNotIn('org', exclude_dictionary) self.assertIn('org', field_dictionary) - self.assertEqual('TestSite3', field_dictionary['org']) + self.assertEqual(['TestSite3'], field_dictionary['org']) diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 82adac6150..ccd4698db8 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -446,12 +446,12 @@ class CourseOverview(TimeStampedModel): return course_overviews @classmethod - def get_all_courses(cls, org=None, filter_=None): + def get_all_courses(cls, orgs=None, filter_=None): """ Returns all CourseOverview objects in the database. Arguments: - org (string): Optional parameter that allows case-insensitive + orgs (list[string]): Optional parameter that allows case-insensitive filtering by organization. filter_ (dict): Optional parameter that allows custom filtering. """ @@ -460,11 +460,11 @@ class CourseOverview(TimeStampedModel): # created. For tests using CourseFactory, use emit_signals=True. course_overviews = CourseOverview.objects.all() - if org: + if orgs: # In rare cases, courses belonging to the same org may be accidentally assigned # an org code with a different casing (e.g., Harvardx as opposed to HarvardX). - # Case-insensitive exact matching allows us to deal with this kind of dirty data. - course_overviews = course_overviews.filter(org__iexact=org) + # Case-insensitive matching allows us to deal with this kind of dirty data. + course_overviews = course_overviews.filter(org__iregex=r'(' + '|'.join(orgs) + ')') if filter_: course_overviews = course_overviews.filter(**filter_) diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py index bd60498f7e..99c9918f5b 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests.py +++ b/openedx/core/djangoapps/content/course_overviews/tests.py @@ -470,7 +470,7 @@ class CourseOverviewTestCase(ModuleStoreTestCase): def test_get_all_courses_by_org(self): org_courses = [] # list of lists of courses - for index in range(2): + for index in range(3): org_courses.append([ CourseFactory.create(org='test_org_' + unicode(index), emit_signals=True) for __ in range(3) @@ -478,18 +478,18 @@ class CourseOverviewTestCase(ModuleStoreTestCase): self.assertEqual( {c.id for c in CourseOverview.get_all_courses()}, - {c.id for c in org_courses[0] + org_courses[1]}, + {c.id for c in org_courses[0] + org_courses[1] + org_courses[2]}, ) self.assertEqual( - {c.id for c in CourseOverview.get_all_courses(org='test_org_1')}, - {c.id for c in org_courses[1]}, + {c.id for c in CourseOverview.get_all_courses(orgs=['test_org_1', 'test_org_2'])}, + {c.id for c in org_courses[1] + org_courses[2]}, ) # Test case-insensitivity. self.assertEqual( - {c.id for c in CourseOverview.get_all_courses(org='TEST_ORG_1')}, - {c.id for c in org_courses[1]}, + {c.id for c in CourseOverview.get_all_courses(orgs=['TEST_ORG_1', 'TEST_ORG_2'])}, + {c.id for c in org_courses[1] + org_courses[2]}, ) def test_get_all_courses_by_mobile_available(self): diff --git a/openedx/core/djangoapps/site_configuration/helpers.py b/openedx/core/djangoapps/site_configuration/helpers.py index d13083d81f..8cc61d1652 100644 --- a/openedx/core/djangoapps/site_configuration/helpers.py +++ b/openedx/core/djangoapps/site_configuration/helpers.py @@ -188,6 +188,21 @@ def get_value_for_org(org, val_name, default=None): return microsite.get_value_for_org(org, val_name, default) +def get_current_site_orgs(): + """ + This returns the orgs configured in site configuration or microsite configuration for the current site. + + Returns: + list: A list of organization names. + """ + course_org_filter = get_value('course_org_filter') + # Make sure we have a list + if course_org_filter and not isinstance(course_org_filter, list): + course_org_filter = [course_org_filter] + + return course_org_filter + + def get_all_orgs(): """ This returns all of the orgs that are considered in site configurations or microsite configuration, diff --git a/openedx/core/djangoapps/site_configuration/models.py b/openedx/core/djangoapps/site_configuration/models.py index 61c05eb044..9456c3aa36 100644 --- a/openedx/core/djangoapps/site_configuration/models.py +++ b/openedx/core/djangoapps/site_configuration/models.py @@ -77,8 +77,8 @@ class SiteConfiguration(models.Model): Configuration value for the given key. """ for configuration in cls.objects.filter(values__contains=org, enabled=True).all(): - org_filter = configuration.get_value('course_org_filter', None) - if org_filter == org: + course_org_filter = configuration.get_value('course_org_filter', None) + if org in course_org_filter: return configuration.get_value(name, default) return default @@ -94,9 +94,10 @@ class SiteConfiguration(models.Model): 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) + course_org_filter = configuration.get_value('course_org_filter', []) + if not isinstance(course_org_filter, list): + course_org_filter = [course_org_filter] + org_filter_set.update(course_org_filter) return org_filter_set @classmethod diff --git a/openedx/core/djangoapps/site_configuration/tests/test_helpers.py b/openedx/core/djangoapps/site_configuration/tests/test_helpers.py index 305d2fe046..146015660c 100644 --- a/openedx/core/djangoapps/site_configuration/tests/test_helpers.py +++ b/openedx/core/djangoapps/site_configuration/tests/test_helpers.py @@ -36,6 +36,10 @@ test_config = { # pylint: disable=invalid-name }, } +test_config_multi_org = { # pylint: disable=invalid-name + "course_org_filter": ["FooOrg", "BarOrg", "FooBarOrg"] +} + class TestHelpers(TestCase): """ @@ -189,3 +193,11 @@ class TestHelpers(TestCase): list(configuration_helpers.get_all_orgs()), test_orgs, ) + + @with_site_configuration(configuration=test_config_multi_org) + def test_get_current_site_orgs(self): + test_orgs = test_config_multi_org['course_org_filter'] + self.assertItemsEqual( + list(configuration_helpers.get_current_site_orgs()), + test_orgs + )