Dmh/rebase release ulmo community

Makefile

@@ -116,7 +116,7 @@ $(COMMON_CONSTRAINTS_TXT):
 	printf "$(COMMON_CONSTRAINTS_TEMP_COMMENT)" | cat - $(@) > temp && mv temp $(@)
 
 compile-requirements: export CUSTOM_COMPILE_COMMAND=make upgrade
-compile-requirements: pre-requirements $(COMMON_CONSTRAINTS_TXT) ## Re-compile *.in requirements to *.txt
+compile-requirements: pre-requirements ## Re-compile *.in requirements to *.txt
 	@# Bootstrapping: Rebuild pip and pip-tools first, and then install them
 	@# so that if there are any failures we'll know now, rather than the next
 	@# time someone tries to use the outputs.
@@ -139,7 +139,7 @@ compile-requirements: pre-requirements $(COMMON_CONSTRAINTS_TXT) ## Re-compile *
 		export REBUILD=''; \
 	done
 
-upgrade:  ## update the pip requirements files to use the latest releases satisfying our constraints
+upgrade:  $(COMMON_CONSTRAINTS_TXT) ## update the pip requirements files to use the latest releases satisfying our constraints
 	$(MAKE) compile-requirements COMPILE_OPTS="--upgrade"
 
 upgrade-package: ## update just one package to the latest usable release

cms/djangoapps/contentstore/rest_api/v1/serializers/home.py

@@ -28,26 +28,11 @@ class LibraryViewSerializer(serializers.Serializer):
     org = serializers.CharField()
     number = serializers.CharField()
     can_edit = serializers.BooleanField()
-    is_migrated = serializers.SerializerMethodField()
-    migrated_to_title = serializers.CharField(
-        source="migrations__target__title",
-        required=False
-    )
-    migrated_to_key = serializers.CharField(
-        source="migrations__target__key",
-        required=False
-    )
-    migrated_to_collection_key = serializers.CharField(
-        source="migrations__target_collection__key",
-        required=False
-    )
-    migrated_to_collection_title = serializers.CharField(
-        source="migrations__target_collection__title",
-        required=False
-    )
-
-    def get_is_migrated(self, obj):
-        return "migrations__target__key" in obj
+    is_migrated = serializers.BooleanField()
+    migrated_to_title = serializers.CharField(required=False)
+    migrated_to_key = serializers.CharField(required=False)
+    migrated_to_collection_key = serializers.CharField(required=False)
+    migrated_to_collection_title = serializers.CharField(required=False)
 
 
 class CourseHomeTabSerializer(serializers.Serializer):

cms/djangoapps/contentstore/rest_api/v1/views/home.py

@@ -236,7 +236,7 @@ def get(self, request: Request):
                 "number": "CPSPR",
                 "can_edit": true
                 }
-            ],        }
+            ],
         ```
         """
 

cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py

@@ -18,7 +18,6 @@
 from cms.djangoapps.contentstore.tests.utils import CourseTestCase
 from cms.djangoapps.modulestore_migrator import api as migrator_api
 from cms.djangoapps.modulestore_migrator.data import CompositionLevel, RepeatHandlingStrategy
-from cms.djangoapps.modulestore_migrator.tests.factories import ModulestoreSourceFactory
 from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory
 from openedx.core.djangoapps.content_libraries import api as lib_api
 
@@ -253,8 +252,9 @@ class HomePageLibrariesViewTest(LibraryTestCase):
 
     def setUp(self):
         super().setUp()
-        # Create an additional legacy library
+        # Create an two additional legacy libaries
         self.lib_key_1 = self._create_library(library="lib1")
+        self.lib_key_2 = self._create_library(library="lib2")
         self.organization = OrganizationFactory()
 
         # Create a new v2 library
@@ -269,7 +269,6 @@ def setUp(self):
         library = lib_api.ContentLibrary.objects.get(slug=self.lib_key_v2.slug)
         learning_package = library.learning_package
         # Create a migration source for the legacy library
-        self.source = ModulestoreSourceFactory(key=self.lib_key_1)
         self.url = reverse("cms.djangoapps.contentstore:v1:libraries")
         # Create a collection to migrate this library to
         collection_key = "test-collection"
@@ -280,20 +279,32 @@ def setUp(self):
             created_by=self.user.id,
         )
 
-        # Migrate self.lib_key_1 to self.lib_key_v2
+        # Migrate both lib_key_1 and lib_key_2 to v2
+        # Only make lib_key_1 a "forwarding" migration.
         migrator_api.start_migration_to_library(
             user=self.user,
-            source_key=self.source.key,
+            source_key=self.lib_key_1,
             target_library_key=self.lib_key_v2,
             target_collection_slug=collection_key,
-            composition_level=CompositionLevel.Component.value,
-            repeat_handling_strategy=RepeatHandlingStrategy.Skip.value,
+            composition_level=CompositionLevel.Component,
+            repeat_handling_strategy=RepeatHandlingStrategy.Skip,
+            preserve_url_slugs=True,
+            forward_source_to_target=True,
+        )
+        migrator_api.start_migration_to_library(
+            user=self.user,
+            source_key=self.lib_key_2,
+            target_library_key=self.lib_key_v2,
+            target_collection_slug=collection_key,
+            composition_level=CompositionLevel.Component,
+            repeat_handling_strategy=RepeatHandlingStrategy.Skip,
             preserve_url_slugs=True,
             forward_source_to_target=False,
         )
 
     def test_home_page_libraries_response(self):
-        """Check successful response content"""
+        """Check sucessful response content"""
+        self.maxDiff = None
         response = self.client.get(self.url)
 
         expected_response = {
@@ -322,6 +333,17 @@ def test_home_page_libraries_response(self):
                     'migrated_to_collection_key': 'test-collection',
                     'migrated_to_collection_title': 'Test Collection',
                 },
+                # Third library was migrated, but not with forwarding.
+                # So, it appears just like the unmigrated library.
+                {
+                    'display_name': 'Test Library',
+                    'library_key': 'library-v1:org+lib2',
+                    'url': '/library/library-v1:org+lib2',
+                    'org': 'org',
+                    'number': 'lib2',
+                    'can_edit': True,
+                    'is_migrated': False,
+                },
             ]
         }
 
@@ -366,6 +388,15 @@ def test_home_page_libraries_response(self):
                     'can_edit': True,
                     'is_migrated': False,
                 },
+                {
+                    'display_name': 'Test Library',
+                    'library_key': 'library-v1:org+lib2',
+                    'url': '/library/library-v1:org+lib2',
+                    'org': 'org',
+                    'number': 'lib2',
+                    'can_edit': True,
+                    'is_migrated': False,
+                },
             ],
         }
 

cms/djangoapps/contentstore/tests/test_course_listing.py

@@ -21,8 +21,10 @@
     get_courses_accessible_to_user
 )
 from common.djangoapps.course_action_state.models import CourseRerunState
+from common.djangoapps.student.models.user import CourseAccessRole
 from common.djangoapps.student.roles import (
     CourseInstructorRole,
+    CourseLimitedStaffRole,
     CourseStaffRole,
     GlobalStaff,
     OrgInstructorRole,
@@ -176,6 +178,48 @@ def test_staff_course_listing(self):
         with self.assertNumQueries(2):
             list(_accessible_courses_summary_iter(self.request))
 
+    def test_course_limited_staff_course_listing(self):
+        # Setup a new course
+        course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run')
+        CourseFactory.create(
+            org=course_location.org,
+            number=course_location.course,
+            run=course_location.run
+        )
+        course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
+
+        # Add the user as a course_limited_staff on the course
+        CourseLimitedStaffRole(course.id).add_users(self.user)
+        self.assertTrue(CourseLimitedStaffRole(course.id).has_user(self.user))
+
+        # Fetch accessible courses list & verify their count
+        courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
+
+        # Limited Course Staff should not be able to list courses in Studio
+        assert len(list(courses_list_by_staff)) == 0
+
+    def test_org_limited_staff_course_listing(self):
+
+        # Setup a new course
+        course_location = self.store.make_course_key('Org', 'CreatedCourse', 'Run')
+        CourseFactory.create(
+            org=course_location.org,
+            number=course_location.course,
+            run=course_location.run
+        )
+        course = CourseOverviewFactory.create(id=course_location, org=course_location.org)
+
+        # Add a user as course_limited_staff on the org
+        # This is not possible using the course roles classes but is possible via Django admin so we
+        # insert a row into the model directly to test that scenario.
+        CourseAccessRole.objects.create(user=self.user, org=course_location.org, role=CourseLimitedStaffRole.ROLE)
+
+        # Fetch accessible courses list & verify their count
+        courses_list_by_staff, __ = get_courses_accessible_to_user(self.request)
+
+        # Limited Course Staff should not be able to list courses in Studio
+        assert len(list(courses_list_by_staff)) == 0
+
     def test_get_course_list_with_invalid_course_location(self):
         """
         Test getting courses with invalid course location (course deleted from modulestore).

cms/djangoapps/contentstore/utils.py

@@ -57,7 +57,8 @@
 )
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
-from cms.djangoapps.modulestore_migrator.api import get_migration_info
+from cms.djangoapps.modulestore_migrator import api as migrator_api
+from cms.djangoapps.modulestore_migrator.data import ModulestoreMigration
 from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError
 from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager
 from common.djangoapps.course_modes.models import CourseMode
@@ -1578,13 +1579,12 @@ def request_response_format_is_json(request, response_format):
 
 def get_library_context(request, request_is_json=False):
     """
-    Utils is used to get context of course home library tab.
-    It is used for both DRF and django views.
+    Utils is used to get context of course home library tab. Returned in DRF view.
     """
     from cms.djangoapps.contentstore.views.course import (
         _accessible_libraries_iter,
-        _format_library_for_view,
         _get_course_creator_status,
+        format_library_for_view,
         get_allowed_organizations,
         get_allowed_organizations_for_libraries,
         user_can_create_organizations,
@@ -1596,21 +1596,25 @@ def get_library_context(request, request_is_json=False):
         user_can_create_library,
     )
 
+    is_migrated: bool | None  # None means: do not filter on is_migrated
+    if (is_migrated_param := request.GET.get('is_migrated')) is not None:
+        is_migrated = BooleanField().to_internal_value(is_migrated_param)
+    else:
+        is_migrated = None
     libraries = list(_accessible_libraries_iter(request.user) if libraries_v1_enabled() else [])
-    library_keys = [lib.location.library_key for lib in libraries]
-    migration_info = get_migration_info(library_keys)
-    is_migrated_filter = request.GET.get('is_migrated', None)
+    migration_info: dict[LibraryLocator, ModulestoreMigration | None] = {
+        lib.id: migrator_api.get_forwarding(lib.id)
+        for lib in libraries
+    }
     data = {
         'libraries': [
-            _format_library_for_view(
+            format_library_for_view(
                 lib,
                 request,
-                migrated_to=migration_info.get(lib.location.library_key)
+                migration=migration_info[lib.id],
             )
             for lib in libraries
-            if is_migrated_filter is None or (
-                BooleanField().to_internal_value(is_migrated_filter) == (lib.location.library_key in migration_info)
-            )
+            if is_migrated is None or is_migrated == bool(migration_info[lib.id])
         ]
     }
 
@@ -1719,8 +1723,7 @@ def format_in_process_course_view(uca):
 
 def get_home_context(request, no_course=False):
     """
-    Utils is used to get context of course home.
-    It is used for both DRF and django views.
+    Utils is used to get context of course home. Returned by DRF view.
     """
 
     from cms.djangoapps.contentstore.views.course import (

cms/djangoapps/contentstore/views/course.py

@@ -7,7 +7,7 @@
 import random
 import re
 import string
-from typing import Dict, NamedTuple, Optional
+from typing import Dict
 
 import django.utils
 from ccx_keys.locator import CCXLocator
@@ -44,6 +44,7 @@
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
 from cms.djangoapps.models.settings.encoder import CourseSettingsEncoder
+from cms.djangoapps.modulestore_migrator.data import ModulestoreMigration
 from cms.djangoapps.contentstore.api.views.utils import get_bool_param
 from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError
 from common.djangoapps.course_action_state.models import CourseRerunState, CourseRerunUIStateManager
@@ -61,6 +62,7 @@
     GlobalStaff,
     UserBasedRole,
     OrgStaffRole,
+    strict_role_checking,
 )
 from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json
 from common.djangoapps.util.string_utils import _has_non_ascii_characters
@@ -536,7 +538,9 @@ def filter_ccx(course_access):
         return not isinstance(course_access.course_id, CCXLocator)
 
     instructor_courses = UserBasedRole(request.user, CourseInstructorRole.ROLE).courses_with_role()
-    staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role()
+    with strict_role_checking():
+        staff_courses = UserBasedRole(request.user, CourseStaffRole.ROLE).courses_with_role()
+
     all_courses = list(filter(filter_ccx, instructor_courses | staff_courses))
     courses_list = []
     course_keys = {}
@@ -671,19 +675,27 @@ def library_listing(request):
     )
 
 
-def _format_library_for_view(library, request, migrated_to: Optional[NamedTuple]):
+def format_library_for_view(library, request, migration: ModulestoreMigration | None):
     """
     Return a dict of the data which the view requires for each library
     """
-
+    migration_info = {}
+    if migration:
+        migration_info = {
+            'migrated_to_key': migration.target_key,
+            'migrated_to_title': migration.target_title,
+            'migrated_to_collection_key': migration.target_collection_slug,
+            'migrated_to_collection_title': migration.target_collection_title,
+        }
     return {
         'display_name': library.display_name,
         'library_key': str(library.location.library_key),
         'url': reverse_library_url('library_handler', str(library.location.library_key)),
         'org': library.display_org_with_default,
         'number': library.display_number_with_default,
         'can_edit': has_studio_write_access(request.user, library.location.library_key),
-        **(migrated_to._asdict() if migrated_to is not None else {}),
+        'is_migrated': migration is not None,
+        **migration_info,
     }
 
 
@@ -1840,12 +1852,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):