Merge branch 'master' into ktyagi/primaryemail

Author: ktyagiapphelix2u

.github/workflows/add-quarterly-ticket-to-check-constraints.yml

@@ -0,0 +1,26 @@
+name: Create quarterly issues for GitHub audit
+on:
+  schedule:
+    - cron: 0 0 1 1,4,7,10 *
+  workflow_dispatch: {}
+
+env:
+  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
+jobs:
+  create_issue:
+    name: Create quarterly constraint check issue
+    runs-on: ubuntu-latest
+    permissions:
+      issues: write
+    steps:
+      - run: |
+          # Platform constraints audit
+          new_issue_url=$(gh issue create --repo "openedx/openedx-platform" \
+            --title "Quarterly audit of openedx-platform constraints" \
+            --label "code health" \
+            --body "It is time to perform the quartely audit of constrained dependencies in \`openedx-platform\`. The goal is to remove any constraints that are no longer necessary to proactively prevent version conflicts and keep us up to date with security patches. The playbook for performing the audit can be found [here](https://openedx.atlassian.net/wiki/spaces/AC/pages/6340968449/Quarterly+Platform+Constraints+Audit).")
+            echo "NEW_ISSUE_URL=$new_issue_url" >> $GITHUB_ENV
+
+      - name: Comment on issue
+        run: gh issue comment $NEW_ISSUE_URL --body "@openedx/wg-maintenance-openedx-platform-oncall heads up on this request"

README.rst

@@ -293,6 +293,11 @@ Code of Conduct
 
 Please read the `Community Code of Conduct`_ for interacting with this repository.
 
+AI Contribution Policy
+**********************
+
+Note that contributions are expected to follow the Open edX `AI Contribution Policy`_.
+
 Reporting Security Issues
 *************************
 
@@ -302,6 +307,7 @@ [email protected].
 .. _individual contributor agreement: https://openedx.org/cla
 .. _CONTRIBUTING: https://github.com/openedx/.github/blob/master/CONTRIBUTING.md
 .. _Community Code of Conduct: https://openedx.org/code-of-conduct/
+.. _AI Contribution Policy: https://github.com/openedx/.github/blob/master/AI_POLICY.md
 
 People
 ******

cms/djangoapps/contentstore/helpers.py

@@ -509,8 +509,18 @@ def _fetch_and_set_upstream_link(
             # temp_xblock.display_name == temp_xblock.upstream_display_name
             # temp_xblock.data == temp_xblock.upstream_data   # for html blocks
             # Even then we want to set `downstream_customized` value to avoid overriding user customisations on sync
-            downstream_customized = temp_xblock.xml_attributes.get("downstream_customized", '[]')
-            temp_xblock.downstream_customized = json.loads(downstream_customized)
+            downstream_customized = getattr(temp_xblock, "downstream_customized", [])
+            # XmlMixin blocks expose raw XML attrs on `xml_attributes`; other blocks (e.g. DnD)
+            # may not have this attribute, but still have parsed downstream_customized field.
+            xml_attributes = getattr(temp_xblock, "xml_attributes", None)
+            if isinstance(xml_attributes, dict):
+                raw_downstream_customized = xml_attributes.get("downstream_customized")
+                if isinstance(raw_downstream_customized, str):
+                    downstream_customized = json.loads(raw_downstream_customized)
+                elif isinstance(raw_downstream_customized, list):
+                    downstream_customized = raw_downstream_customized
+            if hasattr(temp_xblock, "downstream_customized"):
+                temp_xblock.downstream_customized = downstream_customized
 
 
 def _import_xml_node_to_parent(

cms/djangoapps/contentstore/models.py

@@ -17,7 +17,7 @@
 from opaque_keys.edx.locator import LibraryContainerLocator
 from openedx_content.api import get_published_version
 from openedx_content.models_api import Component, Container
-from openedx_django_lib.fields import immutable_uuid_field, key_field, manual_date_time_field
+from openedx_django_lib.fields import immutable_uuid_field, manual_date_time_field, ref_field
 
 logger = logging.getLogger(__name__)
 
@@ -87,7 +87,7 @@ class EntityLinkBase(models.Model):
     """
     uuid = immutable_uuid_field()
     # Search by library/upstream context key
-    upstream_context_key = key_field(
+    upstream_context_key = ref_field(
         help_text=_("Upstream context key i.e., learning_package/library key"),
         db_index=True,
     )

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

@@ -274,7 +274,7 @@ def setUp(self):
         collection_key = "test-collection"
         content_api.create_collection(
             learning_package_id=learning_package.id,
-            key=collection_key,
+            collection_code=collection_key,
             title="Test Collection",
             created_by=self.user.id,
         )
@@ -329,7 +329,7 @@ def test_home_page_libraries_response(self):
                     'can_edit': True,
                     'is_migrated': True,
                     'migrated_to_title': 'Test Library',
-                    'migrated_to_key': 'lib:name0:test-key',
+                    'migrated_to_key': str(self.lib_key_v2),
                     'migrated_to_collection_key': 'test-collection',
                     'migrated_to_collection_title': 'Test Collection',
                 },
@@ -364,7 +364,7 @@ def test_home_page_libraries_response(self):
                     'can_edit': True,
                     'is_migrated': True,
                     'migrated_to_title': 'Test Library',
-                    'migrated_to_key': 'lib:name0:test-key',
+                    'migrated_to_key': str(self.lib_key_v2),
                     'migrated_to_collection_key': 'test-collection',
                     'migrated_to_collection_title': 'Test Collection',
                 }

cms/djangoapps/contentstore/views/block.py

@@ -12,6 +12,7 @@
 from django.views.decorators.clickjacking import xframe_options_exempt
 from django.views.decorators.http import require_http_methods
 from opaque_keys.edx.keys import CourseKey
+from openedx_authz.constants.permissions import COURSES_VIEW_COURSE
 from web_fragments.fragment import Fragment
 
 from cms.djangoapps.contentstore.utils import load_services_for_studio
@@ -27,6 +28,8 @@
 from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
 from common.djangoapps.student.auth import has_studio_read_access, has_studio_write_access
 from common.djangoapps.util.json_request import JsonResponse, expect_json
+from openedx.core.djangoapps.authz.constants import LegacyAuthoringPermission
+from openedx.core.djangoapps.authz.decorators import user_has_course_permission
 from openedx.core.djangoapps.content_tagging.toggles import is_tagging_feature_disabled
 from openedx.core.lib.xblock_utils import hash_resource, request_token, wrap_xblock, wrap_xblock_aside
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
@@ -329,7 +332,12 @@ def xblock_outline_handler(request, usage_key_string):
     a course.
     """
     usage_key = usage_key_with_run(usage_key_string)
-    if not has_studio_read_access(request.user, usage_key.course_key):
+    if not user_has_course_permission(
+        request.user,
+        COURSES_VIEW_COURSE.identifier,
+        usage_key.course_key,
+        LegacyAuthoringPermission.READ,
+    ):
         raise PermissionDenied()
 
     response_format = request.GET.get("format", "html")

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

@@ -19,6 +19,7 @@
 from opaque_keys.edx.asides import AsideUsageKeyV2
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
+from openedx_authz.constants.roles import COURSE_ADMIN, COURSE_AUDITOR, COURSE_EDITOR, COURSE_STAFF
 from openedx_events.content_authoring.data import DuplicatedXBlockData
 from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED
 from openedx_events.testing import OpenEdxEventsTestMixin
@@ -54,6 +55,7 @@
 from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
 from common.test.utils import assert_dict_contains_subset
 from lms.djangoapps.lms_xblock.mixin import NONSENSICAL_ACCESS_RESTRICTION
+from openedx.core.djangoapps.authz.tests.mixins import CourseAuthoringAuthzTestMixin
 from openedx.core.djangoapps.content_tagging import api as tagging_api
 from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
 from openedx.core.djangoapps.video_config.toggles import PUBLIC_VIDEO_SHARE
@@ -3486,6 +3488,145 @@ def validate_xblock_info_consistency(
             self.assertIsNone(xblock_info.get("child_info", None))  # noqa: PT009
 
 
+@ddt.ddt
+class TestXBlockOutlineHandlerAuthz(CourseAuthoringAuthzTestMixin, ItemTest):
+    """
+    Unit tests for xblock_outline_handler authorization functionality.
+    """
+
+    def setUp(self):
+        super().setUp()
+        user_id = self.user.id
+        self.chapter = BlockFactory.create(
+            parent_location=self.course.location,
+            category="chapter",
+            display_name="Week 1",
+            user_id=user_id,
+        )
+        self.sequential = BlockFactory.create(
+            parent_location=self.chapter.location,
+            category="sequential",
+            display_name="Lesson 1",
+            user_id=user_id,
+        )
+        self.vertical = BlockFactory.create(
+            parent_location=self.sequential.location,
+            category="vertical",
+            display_name="Unit 1",
+            user_id=user_id,
+        )
+        # Assign COURSE_STAFF role to authorized_user for the course
+        self.add_user_to_role_in_course(
+            self.authorized_user,
+            COURSE_STAFF.external_key,
+            self.course.id
+        )
+
+    def test_authorized_user_gets_json_response(self):
+        """
+        Test that authorized user gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.authorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    @ddt.data(
+        COURSE_ADMIN.external_key,
+        COURSE_AUDITOR.external_key,
+        COURSE_EDITOR.external_key,
+    )
+    def test_other_course_roles_can_view_outline(self, role_key):
+        """
+        Test that course_admin, course_auditor, and course_editor roles
+        can access the outline (all have COURSES_VIEW_COURSE).
+        """
+        role_user = UserFactory(password=self.password)
+        self.add_user_to_role_in_course(role_user, role_key, self.course.id)
+
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+        self.client.login(username=role_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+
+    def test_unauthorized_user_gets_permission_denied(self):
+        """
+        Test that unauthorized user gets 403 response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.unauthorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 403
+
+    def test_superuser_gets_json_response(self):
+        """
+        Test that superuser gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.super_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    def test_staff_user_gets_json_response(self):
+        """
+        Test that staff user gets JSON response from xblock_outline_handler.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.usage_key)
+
+        self.client.login(username=self.staff_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert "id" in json_response
+        assert "display_name" in json_response
+        assert "child_info" in json_response
+
+    def test_authorized_chapter_outline(self):
+        """
+        Test that authorized user can access chapter-level outline.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
+
+        self.client.login(username=self.authorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 200
+        json_response = json.loads(resp.content.decode("utf-8"))
+        assert json_response["display_name"] == "Week 1"
+        assert "child_info" in json_response
+        # Verify that children are included (should have the sequential)
+        children = json_response["child_info"]["children"]
+        assert len(children) > 0
+        assert children[0]["display_name"] == "Lesson 1"
+
+    def test_unauthorized_chapter_outline(self):
+        """
+        Test that unauthorized user cannot access chapter-level outline.
+        """
+        outline_url = reverse_usage_url("xblock_outline_handler", self.chapter.location)
+
+        self.client.login(username=self.unauthorized_user.username, password=self.password)
+        resp = self.client.get(outline_url, HTTP_ACCEPT="application/json")
+
+        assert resp.status_code == 403
+
+
 class TestGetMetadataWithProblemDefaults(ModuleStoreTestCase):
     """
     Unit tests for _get_metadata_with_problem_defaults.

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

@@ -5,6 +5,7 @@
 """
 import ddt
 from opaque_keys.edx.keys import UsageKey
+from openedx_content.api import signals as content_signals
 from openedx_events.content_authoring.signals import (
     LIBRARY_BLOCK_DELETED,
     XBLOCK_CREATED,
@@ -405,6 +406,7 @@ class ClipboardPasteFromV2LibraryTestCase(OpenEdxEventsTestMixin, ImmediateOnCom
     Test Clipboard Paste functionality with a "new" (as of Sumac) library
     """
     ENABLED_OPENEDX_EVENTS = [
+        content_signals.ENTITIES_DRAFT_CHANGED.event_type,  # Required for library events to work
         LIBRARY_BLOCK_DELETED.event_type,
         XBLOCK_CREATED.event_type,
         XBLOCK_DELETED.event_type,
@@ -491,7 +493,8 @@ def test_paste_from_library_read_only_tags(self):
             assert object_tag.is_copied
 
         # If we delete the upstream library block...
-        library_api.delete_library_block(self.lib_block_key)
+        with self.captureOnCommitCallbacks(execute=True):  # make event handlers fire now, within TestCase transaction
+            library_api.delete_library_block(self.lib_block_key)
 
         # ...the copied tags remain, but should no longer be marked as "copied"
         object_tags = tagging_api.get_object_tags(new_block_key)

cms/djangoapps/modulestore_migrator/api/read_api.py

@@ -135,9 +135,9 @@ def get_migrations(
     if source_key:
         migrations = migrations.filter(source__key=source_key)
     if target_key:
-        migrations = migrations.filter(target__key=str(target_key))
+        migrations = migrations.filter(target__package_ref=str(target_key))
     if target_collection_slug:
-        migrations = migrations.filter(target_collection__key=target_collection_slug)
+        migrations = migrations.filter(target_collection__collection_code=target_collection_slug)
     if task_uuid:
         migrations = migrations.filter(task_status__uuid=task_uuid)
     if is_failed is not None:
@@ -176,9 +176,9 @@ def _migration(m: models.ModulestoreMigration) -> ModulestoreMigration:
     return ModulestoreMigration(
         pk=m.id,
         source_key=m.source.key,
-        target_key=LibraryLocatorV2.from_string(m.target.key),
+        target_key=LibraryLocatorV2.from_string(m.target.package_ref),
         target_title=m.target.title,
-        target_collection_slug=(m.target_collection.key if m.target_collection else None),
+        target_collection_slug=(m.target_collection.collection_code if m.target_collection else None),
         target_collection_title=(m.target_collection.title if m.target_collection else None),
         is_failed=m.is_failed,
         task_uuid=m.task_status.uuid,
@@ -209,7 +209,7 @@ def _block_migration_success(
     """
     Build an instance of the migration success dataclass
     """
-    target_library_key = LibraryLocatorV2.from_string(target.learning_package.key)
+    target_library_key = LibraryLocatorV2.from_string(target.learning_package.package_ref)
     target_key: LibraryUsageLocatorV2 | LibraryContainerLocator
     if hasattr(target, "component"):
         target_key = library_component_usage_key(target_library_key, target.component)

cms/djangoapps/modulestore_migrator/rest_api/v1/serializers.py

@@ -18,6 +18,10 @@ class LibraryMigrationCollectionSerializer(serializers.ModelSerializer):
     """
     Serializer for the target collection of a library migration.
     """
+    # Expose Collection.collection_code as "key" to preserve the REST API field name.
+    # This is temporary: https://github.com/openedx/openedx-platform/issues/38406
+    key = serializers.CharField(source='collection_code')
+
     class Meta:
         model = Collection
         fields = ["key", "title"]