fix: allow library creation by course creators

Prior to this, if ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES
was enabled, we would not return the orgs that someone had course
creator rights on, even if ENABLE_CREATOR_GROUP was enabled. (For the
moment, we are conflating "can create courses" with "can create
libraries" for a given org, even though we should probably eventually
split those apart.)

cms/djangoapps/contentstore/views/course.py

@@ -1832,12 +1832,20 @@ def get_allowed_organizations_for_libraries(user):
     """
     Helper method for returning the list of organizations for which the user is allowed to create libraries.
     """
+    organizations_set = set()
+
+    # This allows org-level staff to create libraries. We should re-evaluate
+    # whether this is necessary and try to normalize course and library creation
+    # authorization behavior.
     if settings.FEATURES.get('ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES', False):
-        return get_organizations_for_non_course_creators(user)
-    elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
-        return get_organizations(user)
-    else:
-        return []
+        organizations_set.update(get_organizations_for_non_course_creators(user))
+
+    # This allows people in the course creator group for an org to create
+    # libraries, which mimics course behavior.
+    if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False):
+        organizations_set.update(get_organizations(user))
+
+    return sorted(organizations_set)
 
 
 def user_can_create_organizations(user):

cms/djangoapps/contentstore/views/tests/test_organizations.py

@@ -3,12 +3,18 @@
 
 import json
 
+from django.conf import settings
 from django.test import TestCase
+from django.test.utils import override_settings
 from django.urls import reverse
 from organizations.api import add_organization
 
+from cms.djangoapps.course_creators.models import CourseCreator
+from common.djangoapps.student.roles import OrgStaffRole
 from common.djangoapps.student.tests.factories import UserFactory
 
+from ..course import get_allowed_organizations_for_libraries
+
 
 class TestOrganizationListing(TestCase):
     """Verify Organization listing behavior."""
@@ -32,3 +38,96 @@ class TestOrganizationListing(TestCase):
         self.assertEqual(response.status_code, 200)
         org_names = json.loads(response.content.decode('utf-8'))
         self.assertEqual(org_names, self.org_short_names)
+
+
+class TestOrganizationsForLibraries(TestCase):
+    """
+    Verify who is allowed to create Libraries.
+
+    This uses some low-level implementation details to set up course creator and
+    org staff data, which should be replaced by API calls.
+
+    The behavior of this call depends on two FEATURES toggles:
+
+    * ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES
+    * ENABLE_CREATOR_GROUP
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        cls.library_author = UserFactory(is_staff=False)
+        cls.org_short_names = ["OrgStaffOrg", "CreatorOrg", "RandomOrg"]
+        cls.orgs = {}
+        for index, short_name in enumerate(cls.org_short_names):
+            cls.orgs[short_name] = add_organization(organization_data={
+                'name': 'Test Organization %s' % index,
+                'short_name': short_name,
+                'description': 'Testing Organization %s Description' % index,
+            })
+
+        # Our user is an org staff for OrgStaffOrg
+        OrgStaffRole("OrgStaffOrg").add_users(cls.library_author)
+
+        # Our user is also a CourseCreator in CreatorOrg
+        creator = CourseCreator.objects.create(
+            user=cls.library_author,
+            state=CourseCreator.GRANTED,
+            all_organizations=False,
+        )
+        # The following is because course_creators app logic assumes that all
+        # updates to CourseCreator go through the CourseCreatorAdmin.
+        # Specifically, CourseCreatorAdmin.save_model() attaches the current
+        # request.user to the model instance's .admin field, and then the
+        # course_creator_organizations_changed_callback() signal handler assumes
+        # creator.admin is present. I think that code could use some judicious
+        # refactoring, but I'm just writing this test as part of a last-minute
+        # Ulmo bug fix, and I don't want to add risk by refactoring something as
+        # critical-path as course_creators as part of this work.
+        creator.admin = UserFactory(is_staff=True)
+        creator.organizations.add(
+            cls.orgs["CreatorOrg"]['id']
+        )
+
+    @override_settings(
+        FEATURES={
+            **settings.FEATURES,
+            'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': False,
+            'ENABLE_CREATOR_GROUP': False,
+        }
+    )
+    def test_both_toggles_disabled(self):
+        allowed_orgs = get_allowed_organizations_for_libraries(self.library_author)
+        assert allowed_orgs == []
+
+    @override_settings(
+        FEATURES={
+            **settings.FEATURES,
+            'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True,
+            'ENABLE_CREATOR_GROUP': True,
+        }
+    )
+    def test_both_toggles_enabled(self):
+        allowed_orgs = get_allowed_organizations_for_libraries(self.library_author)
+        assert allowed_orgs == ["CreatorOrg", "OrgStaffOrg"]
+
+    @override_settings(
+        FEATURES={
+            **settings.FEATURES,
+            'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': True,
+            'ENABLE_CREATOR_GROUP': False,
+        }
+    )
+    def test_org_staff_enabled(self):
+        allowed_orgs = get_allowed_organizations_for_libraries(self.library_author)
+        assert allowed_orgs == ["OrgStaffOrg"]
+
+    @override_settings(
+        FEATURES={
+            **settings.FEATURES,
+            'ENABLE_ORGANIZATION_STAFF_ACCESS_FOR_CONTENT_LIBRARIES': False,
+            'ENABLE_CREATOR_GROUP': True,
+        }
+    )
+    def test_creator_group_enabled(self):
+        allowed_orgs = get_allowed_organizations_for_libraries(self.library_author)
+        assert allowed_orgs == ["CreatorOrg"]