diff --git a/.github/workflows/check-consistent-dependencies.yml b/.github/workflows/check-consistent-dependencies.yml index 048cbe6b006b..82298af70a54 100644 --- a/.github/workflows/check-consistent-dependencies.yml +++ b/.github/workflows/check-consistent-dependencies.yml @@ -15,7 +15,7 @@ defaults: jobs: check-requirements: name: Compile requirements - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: # Only run remaining steps if there are changes to requirements/** diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index 458e00fc6b1f..d989ff9db288 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -10,7 +10,7 @@ jobs: matrix: python-version: - "3.11" - os: ["ubuntu-22.04"] + os: ["ubuntu-24.04"] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index c9d2d7ab1191..20f097d9ef65 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -26,6 +26,7 @@ jobs: uses: actions/setup-node@v4 with: node-version: ${{ matrix.node-version }} + cache: 'npm' - name: Setup npm run: npm i -g npm@10.5.x @@ -63,7 +64,9 @@ jobs: run: | make base-requirements - - uses: c-hive/gha-npm-cache@v1 + - name: Install npm + run: npm ci + - name: Run JS Tests env: TEST_SUITE: js-unit diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 624caddd5309..84e334d68872 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" # 'pinned' is used to install the latest patch version of Django @@ -126,7 +126,7 @@ jobs: if: always() needs: - check_migrations - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 8860aced7f92..9a654e09e711 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -8,7 +8,7 @@ on: jobs: run-pylint: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: @@ -75,7 +75,7 @@ jobs: if: always() needs: - run-pylint - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 84610123493c..310f9f83bf3d 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" node-version: [20] diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 4fe66e2a7778..e08b2dce8127 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -12,7 +12,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" node-version: [18, 20] diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 854677b93cff..e691e16e47f1 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -15,7 +15,7 @@ concurrency: jobs: run-tests: name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }}) - runs-on: ubuntu-22.04 + runs-on: ${{ matrix.os-version }} strategy: matrix: python-version: @@ -43,22 +43,27 @@ jobs: - "xmodule-with-cms" mongo-version: - "7.0" + os-version: + - ubuntu-24.04 - # We only need to test older versions of Mongo with modules that directly interface with Mongo (that is: xmodule.modulestore) - # This code is left here as an example for future refernce in case we need to reduce the number of shards we're - # testing but still have good confidence with older versions of mongo. We use Mongo 4.4 in the example. + # It's useful to run some subset of the tests on the older version of Ubuntu + # so that we don't spend too many resources on this but can find major issues quickly + # while we're in a situation where we support two versions. This section may be commented + # out when not in use to easily add/drop future support for any given major dependency. # - # exclude: - # - mongo-version: "4.4" - # include: - # - shard_name: "xmodule-with-cms" - # python-version: "3.11" - # django-version: "pinned" - # mongo-version: "4.4" - # - shard_name: "xmodule-with-lms" - # python-version: "3.11" - # django-version: "pinned" - # mongo-version: "4.4" + # We're testing the older version of Ubuntu and running the xmodule tests since those rely on many + # dependent complex libraries and will hopefully catch most issues quickly. + include: + - shard_name: "xmodule-with-cms" + python-version: "3.11" + django-version: "pinned" + mongo-version: "7.0" + os-version: "ubuntu-22.04" + - shard_name: "xmodule-with-lms" + python-version: "3.11" + django-version: "pinned" + mongo-version: "7.0" + os-version: "ubuntu-22.04" steps: - name: checkout repo @@ -90,19 +95,10 @@ jobs: activate = 1 EOF - - name: install mongo version - run: | - if [[ "${{ matrix.mongo-version }}" != "4.4" ]]; then - wget -qO - https://www.mongodb.org/static/pgp/server-${{ matrix.mongo-version }}.asc | sudo apt-key add - - echo "deb https://repo.mongodb.org/apt/ubuntu focal/mongodb-org/${{ matrix.mongo-version }} multiverse" | sudo tee /etc/apt/sources.list.d/mongodb-org-${{ matrix.mongo-version }}.list - sudo apt-get update && sudo apt-get install -y mongodb-org="${{ matrix.mongo-version }}.*" - fi - - - name: start mongod server for tests - run: | - sudo mkdir -p /data/db - sudo chmod -R a+rw /data/db - mongod & + - name: Start MongoDB + uses: supercharge/mongodb-github-action@1.11.0 + with: + mongodb-version: ${{ matrix.mongo-version }} - name: Setup Python uses: actions/setup-python@v5 @@ -164,7 +160,7 @@ jobs: overwrite: true collect-and-verify: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - name: Setup Python @@ -229,7 +225,7 @@ jobs: # https://github.com/orgs/community/discussions/33579 success: name: Unit tests successful - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 if: always() needs: [run-tests] steps: @@ -240,7 +236,7 @@ jobs: jobs: ${{ toJSON(needs) }} compile-warnings-report: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [run-tests] steps: - uses: actions/checkout@v4 @@ -268,7 +264,7 @@ jobs: overwrite: true merge-artifacts: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [compile-warnings-report] steps: - name: Merge Pytest Warnings JSON Artifacts @@ -288,7 +284,7 @@ jobs: # Combine and upload coverage reports. coverage: if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false)) - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [run-tests] strategy: matrix: diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8ff1f1aa39cc..b3a5900958ba 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -28,6 +28,7 @@ from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers import openedx.core.djangoapps.content_staging.api as content_staging_api import openedx.core.djangoapps.content_tagging.api as content_tagging_api +from openedx.core.djangoapps.content_staging.data import LIBRARY_SYNC_PURPOSE from .utils import reverse_course_url, reverse_library_url, reverse_usage_url @@ -261,6 +262,37 @@ class StaticFileNotices: error_files: list[str] = Factory(list) +def _insert_static_files_into_downstream_xblock( + downstream_xblock: XBlock, staged_content_id: int, request +) -> StaticFileNotices: + """ + Gets static files from staged content, and inserts them into the downstream XBlock. + """ + static_files = content_staging_api.get_staged_content_static_files(staged_content_id) + notices, substitutions = _import_files_into_course( + course_key=downstream_xblock.context_key, + staged_content_id=staged_content_id, + static_files=static_files, + usage_key=downstream_xblock.scope_ids.usage_id, + ) + + # Rewrite the OLX's static asset references to point to the new + # locations for those assets. See _import_files_into_course for more + # info on why this is necessary. + store = modulestore() + if hasattr(downstream_xblock, "data") and substitutions: + data_with_substitutions = downstream_xblock.data + for old_static_ref, new_static_ref in substitutions.items(): + data_with_substitutions = data_with_substitutions.replace( + old_static_ref, + new_static_ref, + ) + downstream_xblock.data = data_with_substitutions + if store is not None: + store.update_item(downstream_xblock, request.user.id) + return notices + + def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> tuple[XBlock | None, StaticFileNotices]: """ Import a block (along with its children and any required static assets) from @@ -298,31 +330,43 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> tags=user_clipboard.content.tags, ) - # Now handle static files that need to go into Files & Uploads. - static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id) - notices, substitutions = _import_files_into_course( - course_key=parent_key.context_key, - staged_content_id=user_clipboard.content.id, - static_files=static_files, - usage_key=new_xblock.scope_ids.usage_id, - ) - - # Rewrite the OLX's static asset references to point to the new - # locations for those assets. See _import_files_into_course for more - # info on why this is necessary. - if hasattr(new_xblock, 'data') and substitutions: - data_with_substitutions = new_xblock.data - for old_static_ref, new_static_ref in substitutions.items(): - data_with_substitutions = data_with_substitutions.replace( - old_static_ref, - new_static_ref, - ) - new_xblock.data = data_with_substitutions - store.update_item(new_xblock, request.user.id) + notices = _insert_static_files_into_downstream_xblock(new_xblock, user_clipboard.content.id, request) return new_xblock, notices +def import_static_assets_for_library_sync(downstream_xblock: XBlock, lib_block: XBlock, request) -> StaticFileNotices: + """ + Import the static assets from the library xblock to the downstream xblock + through staged content. Also updates the OLX references to point to the new + locations of those assets in the downstream course. + + Does not deal with permissions or REST stuff - do that before calling this. + + Returns a summary of changes made to static files in the destination + course. + """ + if not lib_block.runtime.get_block_assets(lib_block, fetch_asset_data=False): + return StaticFileNotices() + if not content_staging_api: + raise RuntimeError("The required content_staging app is not installed") + staged_content = content_staging_api.stage_xblock_temporarily(lib_block, request.user.id, LIBRARY_SYNC_PURPOSE) + if not staged_content: + # expired/error/loading + return StaticFileNotices() + + store = modulestore() + try: + with store.bulk_operations(downstream_xblock.context_key): + # Now handle static files that need to go into Files & Uploads. + # If the required files already exist, nothing will happen besides updating the olx. + notices = _insert_static_files_into_downstream_xblock(downstream_xblock, staged_content.id, request) + finally: + staged_content.delete() + + return notices + + def _fetch_and_set_upstream_link( copied_from_block: str, copied_from_version_num: int, @@ -452,11 +496,11 @@ def _import_xml_node_to_parent( if xblock_class.has_children and temp_xblock.children: raise NotImplementedError("We don't yet support pasting XBlocks with children") - temp_xblock.parent = parent_key if copied_from_block: _fetch_and_set_upstream_link(copied_from_block, copied_from_version_num, temp_xblock, user) # Save the XBlock into modulestore. We need to save the block and its parent for this to work: new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True) + new_xblock.parent = parent_key parent_xblock.children.append(new_xblock.location) store.update_item(parent_xblock, user.id) @@ -543,6 +587,9 @@ def _import_files_into_course( if result is True: new_files.append(file_data_obj.filename) substitutions.update(substitution_for_file) + elif substitution_for_file: + # substitutions need to be made because OLX references to these files need to be updated + substitutions.update(substitution_for_file) elif result is None: pass # This file already exists; no action needed. else: @@ -613,8 +660,8 @@ def _import_file_into_course( contentstore().save(content) return True, {clipboard_file_path: f"static/{import_path}"} elif current_file.content_digest == file_data_obj.md5_hash: - # The file already exists and matches exactly, so no action is needed - return None, {} + # The file already exists and matches exactly, so no action is needed except substitutions + return None, {clipboard_file_path: f"static/{import_path}"} else: # There is a conflict with some other file that has the same name. return False, {} diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index fa2a651f8a28..2f4eb770afad 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -50,6 +50,10 @@ class StudioHomeSerializer(serializers.Serializer): child=serializers.CharField(), allow_empty=True ) + allowed_organizations_for_libraries = serializers.ListSerializer( + child=serializers.CharField(), + allow_empty=True + ) archived_courses = CourseCommonSerializer(required=False, many=True) can_access_advanced_settings = serializers.BooleanField() can_create_organizations = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index 3de536d78092..2d18a3e0b92e 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -5,6 +5,7 @@ from rest_framework.request import Request from rest_framework.response import Response from rest_framework.views import APIView +from organizations import api as org_api from openedx.core.lib.api.view_utils import view_auth_classes from ....utils import get_home_context, get_course_context, get_library_context @@ -51,6 +52,7 @@ def get(self, request: Request): "allow_to_create_new_org": true, "allow_unicode_course_id": false, "allowed_organizations": [], + "allowed_organizations_for_libraries": [], "archived_courses": [], "can_access_advanced_settings": true, "can_create_organizations": true, @@ -80,7 +82,12 @@ def get(self, request: Request): home_context = get_home_context(request, True) home_context.update({ - 'allow_to_create_new_org': settings.FEATURES.get('ENABLE_CREATOR_GROUP', True) and request.user.is_staff, + # 'allow_to_create_new_org' is actually about auto-creating organizations + # (e.g. when creating a course or library), so we add an additional test. + 'allow_to_create_new_org': ( + home_context['can_create_organizations'] and + org_api.is_autocreate_enabled() + ), 'studio_name': settings.STUDIO_NAME, 'studio_short_name': settings.STUDIO_SHORT_NAME, 'studio_request_email': settings.FEATURES.get('STUDIO_REQUEST_EMAIL', ''), diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py index 9a3d2c24d8f5..17839a1aa6f6 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_home.py @@ -37,9 +37,10 @@ def setUp(self): self.url = reverse("cms.djangoapps.contentstore:v1:home") self.expected_response = { "allow_course_reruns": True, - "allow_to_create_new_org": False, + "allow_to_create_new_org": True, "allow_unicode_course_id": False, "allowed_organizations": [], + "allowed_organizations_for_libraries": [], "archived_courses": [], "can_access_advanced_settings": True, "can_create_organizations": True, @@ -84,6 +85,17 @@ def test_home_page_studio_with_meilisearch_enabled(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(expected_response, response.data) + @override_settings(ORGANIZATIONS_AUTOCREATE=False) + def test_home_page_studio_with_org_autocreate_disabled(self): + """Check response content when Organization autocreate is disabled""" + response = self.client.get(self.url) + + expected_response = self.expected_response + expected_response["allow_to_create_new_org"] = False + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.data) + def test_taxonomy_list_link(self): response = self.client.get(self.url) self.assertTrue(response.data['taxonomies_enabled']) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index 8c0d651910a6..46e67e87ea0c 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -56,8 +56,10 @@ "ready_to_sync": Boolean } """ + import logging +from attrs import asdict as attrs_asdict from django.contrib.auth.models import User # pylint: disable=imported-auth-user from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey @@ -71,6 +73,7 @@ UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link ) +from cms.djangoapps.contentstore.helpers import import_static_assets_for_library_sync from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access from openedx.core.lib.api.view_utils import ( DeveloperErrorViewMixin, @@ -195,7 +198,8 @@ def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respons """ downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) try: - sync_from_upstream(downstream, request.user) + upstream = sync_from_upstream(downstream, request.user) + static_file_notices = import_static_assets_for_library_sync(downstream, upstream, request) except UpstreamLinkException as exc: logger.exception( "Could not sync from upstream '%s' to downstream '%s'", @@ -206,7 +210,9 @@ def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Respons modulestore().update_item(downstream, request.user.id) # Note: We call `get_for_block` (rather than `try_get_for_block`) because if anything is wrong with the # upstream at this point, then that is completely unexpected, so it's appropriate to let the 500 happen. - return Response(UpstreamLink.get_for_block(downstream).to_json()) + response = UpstreamLink.get_for_block(downstream).to_json() + response["static_file_notices"] = attrs_asdict(static_file_notices) + return Response(response) def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index 92da28bde989..31877b9153d5 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -4,6 +4,7 @@ from unittest.mock import patch from django.conf import settings +from cms.djangoapps.contentstore.helpers import StaticFileNotices from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream from common.djangoapps.student.tests.factories import UserFactory from xmodule.modulestore.django import modulestore @@ -247,7 +248,8 @@ def call_api(self, usage_key_string): @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) @patch.object(downstreams_views, "sync_from_upstream") - def test_200(self, mock_sync_from_upstream): + @patch.object(downstreams_views, "import_static_assets_for_library_sync", return_value=StaticFileNotices()) + def test_200(self, mock_sync_from_upstream, mock_import_staged_content): """ Does the happy path work? """ @@ -255,6 +257,7 @@ def test_200(self, mock_sync_from_upstream): response = self.call_api(self.downstream_video_key) assert response.status_code == 200 assert mock_sync_from_upstream.call_count == 1 + assert mock_import_staged_content.call_count == 1 class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 3b9ba5798a01..775f08e5fa2c 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -80,6 +80,7 @@ from ..helpers import ( get_parent_xblock, import_staged_content_from_user_clipboard, + import_static_assets_for_library_sync, is_unit, xblock_embed_lms_url, xblock_lms_url, @@ -598,7 +599,7 @@ def _create_block(request): try: # Set `created_block.upstream` and then sync this with the upstream (library) version. created_block.upstream = upstream_ref - sync_from_upstream(downstream=created_block, user=request.user) + lib_block = sync_from_upstream(downstream=created_block, user=request.user) except BadUpstream as exc: _delete_item(created_block.location, request.user) log.exception( @@ -606,8 +607,10 @@ def _create_block(request): f"using provided library_content_key='{upstream_ref}'" ) return JsonResponse({"error": str(exc)}, status=400) + static_file_notices = import_static_assets_for_library_sync(created_block, lib_block, request) modulestore().update_item(created_block, request.user.id) - response['upstreamRef'] = upstream_ref + response["upstreamRef"] = upstream_ref + response["static_file_notices"] = asdict(static_file_notices) return JsonResponse(response) diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 0d95931ce29d..418b95a39fe3 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -186,7 +186,7 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: ) -def sync_from_upstream(downstream: XBlock, user: User) -> None: +def sync_from_upstream(downstream: XBlock, user: User) -> XBlock: """ Update `downstream` with content+settings from the latest available version of its linked upstream content. @@ -200,6 +200,7 @@ def sync_from_upstream(downstream: XBlock, user: User) -> None: _update_non_customizable_fields(upstream=upstream, downstream=downstream) _update_tags(upstream=upstream, downstream=downstream) downstream.upstream_version = link.version_available + return upstream def fetch_customizable_fields(*, downstream: XBlock, user: User, upstream: XBlock | None = None) -> None: diff --git a/lms/djangoapps/discussion/django_comment_client/base/tests.py b/lms/djangoapps/discussion/django_comment_client/base/tests.py index df087fdc533e..d2a4f921f28b 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/tests.py +++ b/lms/djangoapps/discussion/django_comment_client/base/tests.py @@ -812,7 +812,7 @@ def test_update_comment_basic(self, mock_is_forum_v2_enabled, mock_request): headers=ANY, params=ANY, timeout=ANY, - data={"body": updated_body} + data={"body": updated_body, "course_id": str(self.course_id)} ) def test_flag_thread_open(self, mock_is_forum_v2_enabled, mock_request): diff --git a/lms/djangoapps/discussion/django_comment_client/base/views.py b/lms/djangoapps/discussion/django_comment_client/base/views.py index 3df362bdf6d2..458b0a02857e 100644 --- a/lms/djangoapps/discussion/django_comment_client/base/views.py +++ b/lms/djangoapps/discussion/django_comment_client/base/views.py @@ -584,7 +584,7 @@ def create_thread(request, course_id, commentable_id): if follow: cc_user = cc.User.from_django_user(user) - cc_user.follow(thread) + cc_user.follow(thread, course_id) thread_followed.send(sender=None, user=user, post=thread) data = thread.to_dict() @@ -673,7 +673,7 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None): parent_id=parent_id, body=sanitize_body(post["body"]), ) - comment.save() + comment.save(params={"course_id": str(course_key)}) comment_created.send(sender=None, user=user, post=comment) @@ -715,7 +715,7 @@ def delete_thread(request, course_id, thread_id): course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) thread = cc.Thread.find(thread_id) - thread.delete() + thread.delete(course_id=course_id) thread_deleted.send(sender=None, user=request.user, post=thread) track_thread_deleted_event(request, course, thread) @@ -736,7 +736,7 @@ def update_comment(request, course_id, comment_id): if 'body' not in request.POST or not request.POST['body'].strip(): return JsonError(_("Body can't be empty")) comment.body = sanitize_body(request.POST["body"]) - comment.save() + comment.save(params={"course_id": course_id}) comment_edited.send(sender=None, user=request.user, post=comment) @@ -762,7 +762,7 @@ def endorse_comment(request, course_id, comment_id): endorsed = request.POST.get('endorsed', 'false').lower() == 'true' comment.endorsed = endorsed comment.endorsement_user_id = user.id - comment.save() + comment.save(params={"course_id": course_id}) comment_endorsed.send(sender=None, user=user, post=comment) track_forum_response_mark_event(request, course, comment, endorsed) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -781,7 +781,7 @@ def openclose_thread(request, course_id, thread_id): thread = cc.Thread.find(thread_id) close_thread = request.POST.get('closed', 'false').lower() == 'true' thread.closed = close_thread - thread.save() + thread.save(params={"course_id": course_id}) track_thread_lock_unlock_event(request, course, thread, None, close_thread) return JsonResponse({ @@ -814,7 +814,7 @@ def delete_comment(request, course_id, comment_id): course_key = CourseKey.from_string(course_id) course = get_course_with_access(request.user, 'load', course_key) comment = cc.Comment.find(comment_id) - comment.delete() + comment.delete(course_id=course_id) comment_deleted.send(sender=None, user=request.user, post=comment) track_comment_deleted_event(request, course, comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -828,12 +828,12 @@ def _vote_or_unvote(request, course_id, obj, value='up', undo_vote=False): course = get_course_with_access(request.user, 'load', course_key) user = cc.User.from_django_user(request.user) if undo_vote: - user.unvote(obj) + user.unvote(obj, course_id) # TODO(smarnach): Determine the value of the vote that is undone. Currently, you can # only cast upvotes in the user interface, so it is assumed that the vote value is 'up'. # (People could theoretically downvote by handcrafting AJAX requests.) else: - user.vote(obj, value) + user.vote(obj, value, course_id) thread_voted.send(sender=None, user=request.user, post=obj) track_voted_event(request, course, obj, value, undo_vote) return JsonResponse(prepare_content(obj.to_dict(), course_key)) @@ -899,7 +899,7 @@ def flag_abuse_for_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) - thread.flagAbuse(user, thread) + thread.flagAbuse(user, thread, course_id) track_discussion_reported_event(request, course, thread) thread_flagged.send(sender='flag_abuse_for_thread', user=request.user, post=thread) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -921,7 +921,7 @@ def un_flag_abuse_for_thread(request, course_id, thread_id): has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course) ) - thread.unFlagAbuse(user, thread, remove_all) + thread.unFlagAbuse(user, thread, remove_all, course_id) track_discussion_unreported_event(request, course, thread) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -938,7 +938,7 @@ def flag_abuse_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) course = get_course_by_id(course_key) comment = cc.Comment.find(comment_id) - comment.flagAbuse(user, comment) + comment.flagAbuse(user, comment, course_id) track_discussion_reported_event(request, course, comment) comment_flagged.send(sender='flag_abuse_for_comment', user=request.user, post=comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -960,7 +960,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): has_access(request.user, 'staff', course) ) comment = cc.Comment.find(comment_id) - comment.unFlagAbuse(user, comment, remove_all) + comment.unFlagAbuse(user, comment, remove_all, course_id) track_discussion_unreported_event(request, course, comment) return JsonResponse(prepare_content(comment.to_dict(), course_key)) @@ -976,7 +976,7 @@ def pin_thread(request, course_id, thread_id): course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.pin(user, thread_id) + thread.pin(user, thread_id, course_id) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -992,7 +992,7 @@ def un_pin_thread(request, course_id, thread_id): course_key = CourseKey.from_string(course_id) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.un_pin(user, thread_id) + thread.un_pin(user, thread_id, course_id) return JsonResponse(prepare_content(thread.to_dict(), course_key)) @@ -1005,7 +1005,7 @@ def follow_thread(request, course_id, thread_id): # lint-amnesty, pylint: disab course_key = CourseKey.from_string(course_id) course = get_course_by_id(course_key) thread = cc.Thread.find(thread_id) - user.follow(thread) + user.follow(thread, course_id=course_id) thread_followed.send(sender=None, user=request.user, post=thread) track_thread_followed_event(request, course, thread, True) return JsonResponse({}) @@ -1021,7 +1021,7 @@ def follow_commentable(request, course_id, commentable_id): # lint-amnesty, pyl """ user = cc.User.from_django_user(request.user) commentable = cc.Commentable.find(commentable_id) - user.follow(commentable) + user.follow(commentable, course_id=course_id) return JsonResponse({}) @@ -1037,7 +1037,7 @@ def unfollow_thread(request, course_id, thread_id): # lint-amnesty, pylint: dis course = get_course_by_id(course_key) user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - user.unfollow(thread) + user.unfollow(thread, course_id=course_id) thread_unfollowed.send(sender=None, user=request.user, post=thread) track_thread_followed_event(request, course, thread, False) return JsonResponse({}) @@ -1053,7 +1053,7 @@ def unfollow_commentable(request, course_id, commentable_id): # lint-amnesty, p """ user = cc.User.from_django_user(request.user) commentable = cc.Commentable.find(commentable_id) - user.unfollow(commentable) + user.unfollow(commentable, course_id=course_id) return JsonResponse({}) diff --git a/lms/djangoapps/discussion/views.py b/lms/djangoapps/discussion/views.py index bfa511a575bd..3a3c8c133471 100644 --- a/lms/djangoapps/discussion/views.py +++ b/lms/djangoapps/discussion/views.py @@ -146,7 +146,7 @@ def get_threads(request, course, user_info, discussion_id=None, per_page=THREADS # If the user clicked a sort key, update their default sort key cc_user = cc.User.from_django_user(request.user) cc_user.default_sort_key = request.GET.get('sort_key') - cc_user.save() + cc_user.save(params={"course_id": str(course.id)}) #there are 2 dimensions to consider when executing a search with respect to group id #is user a moderator @@ -218,7 +218,7 @@ def inline_discussion(request, course_key, discussion_id): with function_trace('get_course_and_user_info'): course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True) cc_user = cc.User.from_django_user(request.user) - user_info = cc_user.to_dict() + user_info = cc_user.to_dict(course_key=str(course_key)) try: with function_trace('get_threads'): @@ -356,7 +356,7 @@ def single_thread(request, course_key, discussion_id, thread_id): if request.headers.get('x-requested-with') == 'XMLHttpRequest': cc_user = cc.User.from_django_user(request.user) - user_info = cc_user.to_dict() + user_info = cc_user.to_dict(course_key=str(course_key)) is_staff = has_permission(request.user, 'openclose_thread', course.id) try: @@ -471,7 +471,7 @@ def _create_base_discussion_view_context(request, course_key): """ user = request.user cc_user = cc.User.from_django_user(user) - user_info = cc_user.to_dict() + user_info = cc_user.to_dict(course_key=str(course_key)) course = get_course_with_access(user, 'load', course_key, check_if_enrolled=True) course_settings = make_course_settings(course, user) return { diff --git a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py index 34eb7c4f34fc..aaac63375453 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py @@ -8,6 +8,7 @@ import ddt from django.contrib.auth.models import Group +from django.test import override_settings from django.test.client import Client from freezegun import freeze_time from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 @@ -140,6 +141,63 @@ def test_library_validation(self): 'slug': ['Enter a valid “slug” consisting of Unicode letters, numbers, underscores, or hyphens.'], } + def test_library_org_validation(self): + """ + Staff users can create libraries in any existing or auto-created organization. + """ + assert Organization.objects.filter(short_name='auto-created-org').count() == 0 + self._create_library(slug="auto-created-org-1", title="Library in an auto-created org", org='auto-created-org') + assert Organization.objects.filter(short_name='auto-created-org').count() == 1 + self._create_library(slug="existing-org-1", title="Library in an existing org", org="CL-TEST") + + @patch( + "openedx.core.djangoapps.content_libraries.views.user_can_create_organizations", + ) + @patch( + "openedx.core.djangoapps.content_libraries.views.get_allowed_organizations_for_libraries", + ) + @override_settings(ORGANIZATIONS_AUTOCREATE=False) + def test_library_org_no_autocreate(self, mock_get_allowed_organizations, mock_can_create_organizations): + """ + When org auto-creation is disabled, user must use one of their allowed orgs. + """ + mock_can_create_organizations.return_value = False + mock_get_allowed_organizations.return_value = ["CL-TEST"] + assert Organization.objects.filter(short_name='auto-created-org').count() == 0 + response = self._create_library( + slug="auto-created-org-2", + org="auto-created-org", + title="Library in an auto-created org", + expect_response=400, + ) + assert response == { + 'org': "No such organization 'auto-created-org' found.", + } + + Organization.objects.get_or_create( + short_name="not-allowed-org", + defaults={"name": "Content Libraries Test Org Membership"}, + ) + response = self._create_library( + slug="not-allowed-org", + org="not-allowed-org", + title="Library in an not-allowed org", + expect_response=400, + ) + assert response == { + 'org': "User not allowed to create libraries in 'not-allowed-org'.", + } + assert mock_can_create_organizations.call_count == 1 + assert mock_get_allowed_organizations.call_count == 1 + + self._create_library( + slug="allowed-org-2", + org="CL-TEST", + title="Library in an allowed org", + ) + assert mock_can_create_organizations.call_count == 2 + assert mock_get_allowed_organizations.call_count == 2 + @skip("This endpoint shouldn't support num_blocks and has_unpublished_*.") @patch("openedx.core.djangoapps.content_libraries.views.LibraryRootView.pagination_class.page_size", new=2) def test_list_library(self): diff --git a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py index 69f5cd2d797c..91d9556b01f1 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_static_assets.py @@ -63,6 +63,13 @@ def test_asset_filenames(self): file_name = "a////////b" self._set_library_block_asset(block_id, file_name, SVG_DATA, expect_response=400) + # Names with spaces are allowed but replaced with underscores + file_name_with_space = "o w o.svg" + self._set_library_block_asset(block_id, file_name_with_space, SVG_DATA) + file_name = "o_w_o.svg" + assert self._get_library_block_asset(block_id, file_name)['path'] == file_name + assert self._get_library_block_asset(block_id, file_name)['size'] == file_size + def test_video_transcripts(self): """ Test that video blocks can read transcript files out of learning core. diff --git a/openedx/core/djangoapps/content_libraries/views.py b/openedx/core/djangoapps/content_libraries/views.py index d42dbb51d8d3..27bae691de19 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -99,6 +99,10 @@ from rest_framework.views import APIView from rest_framework.viewsets import GenericViewSet +from cms.djangoapps.contentstore.views.course import ( + get_allowed_organizations_for_libraries, + user_can_create_organizations, +) from openedx.core.djangoapps.content_libraries import api, permissions from openedx.core.djangoapps.content_libraries.serializers import ( ContentLibraryBlockImportTaskCreateSerializer, @@ -271,6 +275,14 @@ def post(self, request): raise ValidationError( # lint-amnesty, pylint: disable=raise-missing-from detail={"org": f"No such organization '{org_name}' found."} ) + # Ensure the user is allowed to create libraries under this org + if not ( + user_can_create_organizations(request.user) or + org_name in get_allowed_organizations_for_libraries(request.user) + ): + raise ValidationError( # lint-amnesty, pylint: disable=raise-missing-from + detail={"org": f"User not allowed to create libraries in '{org_name}'."} + ) org = Organization.objects.get(short_name=org_name) try: @@ -788,6 +800,7 @@ def put(self, request, usage_key_str, file_path): """ Replace a static asset file belonging to this block. """ + file_path = file_path.replace(" ", "_") # Messes up url/name correspondence due to URL encoding. usage_key = LibraryUsageLocatorV2.from_string(usage_key_str) api.require_permission_for_library_key( usage_key.lib_key, request.user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY, diff --git a/openedx/core/djangoapps/content_staging/api.py b/openedx/core/djangoapps/content_staging/api.py index f0432922dcb0..a5704dc8dde8 100644 --- a/openedx/core/djangoapps/content_staging/api.py +++ b/openedx/core/djangoapps/content_staging/api.py @@ -14,29 +14,36 @@ from xblock.core import XBlock from openedx.core.lib.xblock_serializer.api import StaticFile, XBlockSerializer -from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_none from xmodule import block_metadata_utils from xmodule.contentstore.content import StaticContent from xmodule.contentstore.django import contentstore from .data import ( CLIPBOARD_PURPOSE, - StagedContentData, StagedContentFileData, StagedContentStatus, UserClipboardData + StagedContentData, + StagedContentFileData, + StagedContentStatus, + UserClipboardData, ) from .models import ( UserClipboard as _UserClipboard, StagedContent as _StagedContent, StagedContentFile as _StagedContentFile, ) -from .serializers import UserClipboardSerializer as _UserClipboardSerializer +from .serializers import ( + UserClipboardSerializer as _UserClipboardSerializer, +) from .tasks import delete_expired_clipboards log = logging.getLogger(__name__) -def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int | None = None) -> UserClipboardData: +def _save_xblock_to_staged_content( + block: XBlock, user_id: int, purpose: str, version_num: int | None = None +) -> _StagedContent: """ - Copy an XBlock's OLX to the user's clipboard. + Generic function to save an XBlock's OLX to staged content. + Used by both clipboard and library sync functionality. """ block_data = XBlockSerializer( block, @@ -49,7 +56,7 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int # Mark all of the user's existing StagedContent rows as EXPIRED to_expire = _StagedContent.objects.filter( user_id=user_id, - purpose=CLIPBOARD_PURPOSE, + purpose=purpose, ).exclude( status=StagedContentStatus.EXPIRED, ) @@ -60,7 +67,7 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int # Insert a new StagedContent row for this staged_content = _StagedContent.objects.create( user_id=user_id, - purpose=CLIPBOARD_PURPOSE, + purpose=purpose, status=StagedContentStatus.READY, block_type=usage_key.block_type, olx=block_data.olx_str, @@ -69,23 +76,16 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int tags=block_data.tags, version_num=(version_num or 0), ) - (clipboard, _created) = _UserClipboard.objects.update_or_create(user_id=user_id, defaults={ - "content": staged_content, - "source_usage_key": usage_key, - }) # Log an event so we can analyze how this feature is used: - log.info(f"Copied {usage_key.block_type} component \"{usage_key}\" to their clipboard.") + log.info(f'Saved {usage_key.block_type} component "{usage_key}" to staged content for {purpose}.') - # Try to copy the static files. If this fails, we still consider the overall copy attempt to have succeeded, - # because intra-course pasting will still work fine, and in any case users can manually resolve the file issue. + # Try to copy the static files. If this fails, we still consider the overall save attempt to have succeeded, + # because intra-course operations will still work fine, and users can manually resolve file issues. try: - _save_static_assets_to_user_clipboard(block_data.static_files, usage_key, staged_content) + _save_static_assets_to_staged_content(block_data.static_files, usage_key, staged_content) except Exception: # pylint: disable=broad-except - # Regardless of what happened, with get_asset_key_from_path or contentstore or run_filter, we don't want the - # whole "copy to clipboard" operation to fail, which would be a bad user experience. For copying and pasting - # within a single course, static assets don't even matter. So any such errors become warnings here. - log.exception(f"Unable to copy static files to clipboard for component {usage_key}") + log.exception(f"Unable to copy static files to staged content for component {usage_key}") # Enqueue a (potentially slow) task to delete the old staged content try: @@ -93,14 +93,15 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int except Exception: # pylint: disable=broad-except log.exception(f"Unable to enqueue cleanup task for StagedContents: {','.join(str(x) for x in expired_ids)}") - return _user_clipboard_model_to_data(clipboard) + return staged_content -def _save_static_assets_to_user_clipboard( +def _save_static_assets_to_staged_content( static_files: list[StaticFile], usage_key: UsageKey, staged_content: _StagedContent ): """ - Helper method for save_xblock_to_user_clipboard endpoint. This deals with copying static files into the clipboard. + Helper method for saving static files into staged content. + Used by both clipboard and library sync functionality. """ for f in static_files: source_key = ( @@ -144,6 +145,37 @@ def _save_static_assets_to_user_clipboard( log.exception(f"Unable to copy static file {f.name} to clipboard for component {usage_key}") +def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int | None = None) -> UserClipboardData: + """ + Copy an XBlock's OLX to the user's clipboard. + """ + staged_content = _save_xblock_to_staged_content(block, user_id, CLIPBOARD_PURPOSE, version_num) + usage_key = block.usage_key + + # Create/update the clipboard entry + (clipboard, _created) = _UserClipboard.objects.update_or_create( + user_id=user_id, + defaults={ + "content": staged_content, + "source_usage_key": usage_key, + }, + ) + + return _user_clipboard_model_to_data(clipboard) + + +def stage_xblock_temporarily( + block: XBlock, user_id: int, purpose: str, version_num: int | None = None, +) -> _StagedContent: + """ + "Stage" an XBlock by copying it (and its associated children + static assets) + into the content staging area. This XBlock can then be accessed and manipulated + using any of the staged content APIs, before being deleted. + """ + staged_content = _save_xblock_to_staged_content(block, user_id, purpose, version_num) + return staged_content + + def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardData | None: """ Get the details of the user's clipboard. @@ -190,28 +222,29 @@ def get_user_clipboard_json(user_id: int, request: HttpRequest | None = None): return serializer.data +def _staged_content_to_data(content: _StagedContent) -> StagedContentData: + """ + Convert a StagedContent model instance to an immutable data object. + """ + return StagedContentData( + id=content.id, + user_id=content.user_id, + created=content.created, + purpose=content.purpose, + status=content.status, + block_type=content.block_type, + display_name=content.display_name, + tags=content.tags or {}, + version_num=content.version_num, + ) + + def _user_clipboard_model_to_data(clipboard: _UserClipboard) -> UserClipboardData: """ Convert a UserClipboard model instance to an immutable data object. """ - content = clipboard.content - source_context_key = clipboard.source_usage_key.context_key - if source_context_key.is_course and (course_overview := get_course_overview_or_none(source_context_key)): - source_context_title = course_overview.display_name_with_default - else: - source_context_title = str(source_context_key) # Fall back to stringified context key as a title return UserClipboardData( - content=StagedContentData( - id=content.id, - user_id=content.user_id, - created=content.created, - purpose=content.purpose, - status=content.status, - block_type=content.block_type, - display_name=content.display_name, - tags=content.tags, - version_num=content.version_num, - ), + content=_staged_content_to_data(clipboard.content), source_usage_key=clipboard.source_usage_key, source_context_title=clipboard.get_source_context_title(), ) diff --git a/openedx/core/djangoapps/content_staging/data.py b/openedx/core/djangoapps/content_staging/data.py index d077d05a0aa4..d095f2506b17 100644 --- a/openedx/core/djangoapps/content_staging/data.py +++ b/openedx/core/djangoapps/content_staging/data.py @@ -25,6 +25,10 @@ class StagedContentStatus(TextChoices): # Value of the "purpose" field on StagedContent objects used for clipboards. CLIPBOARD_PURPOSE = "clipboard" + +# Value of the "purpose" field on StagedContent objects used for library to course sync. +LIBRARY_SYNC_PURPOSE = "library_sync" + # There may be other valid values of "purpose" which aren't defined within this app. diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/comment.py b/openedx/core/djangoapps/django_comment_common/comment_client/comment.py index 39b8680b6377..5f6348547efb 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/comment.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/comment.py @@ -63,14 +63,14 @@ def url(cls, action, params=None): else: return super().url(action, params) - def flagAbuse(self, user, voteable): + def flagAbuse(self, user, voteable, course_id=None): if voteable.type == 'thread': url = _url_for_flag_abuse_thread(voteable.id) elif voteable.type == 'comment': url = _url_for_flag_abuse_comment(voteable.id) else: raise CommentClientRequestError("Can only flag/unflag threads or comments") - course_key = get_course_key(self.attributes.get("course_id")) + course_key = get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): if voteable.type == 'thread': response = forum_api.update_thread_flag( @@ -91,14 +91,14 @@ def flagAbuse(self, user, voteable): ) voteable._update_from_response(response) - def unFlagAbuse(self, user, voteable, removeAll): + def unFlagAbuse(self, user, voteable, removeAll, course_id=None): if voteable.type == 'thread': url = _url_for_unflag_abuse_thread(voteable.id) elif voteable.type == 'comment': url = _url_for_unflag_abuse_comment(voteable.id) else: raise CommentClientRequestError("Can flag/unflag for threads or comments") - course_key = get_course_key(self.attributes.get("course_id")) + course_key = get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): if voteable.type == "thread": response = forum_api.update_thread_flag( diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/models.py b/openedx/core/djangoapps/django_comment_common/comment_client/models.py index 9b6c9ca03f3d..1812d24dec0a 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/models.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/models.py @@ -61,8 +61,8 @@ def items(self, *args, **kwargs): def get(self, *args, **kwargs): return self.attributes.get(*args, **kwargs) - def to_dict(self): - self.retrieve() + def to_dict(self, course_key=None): + self.retrieve(course_key=course_key) return self.attributes def retrieve(self, *args, **kwargs): @@ -72,7 +72,7 @@ def retrieve(self, *args, **kwargs): return self def _retrieve(self, *args, **kwargs): - course_id = self.attributes.get("course_id") or kwargs.get("course_id") + course_id = self.attributes.get("course_id") or kwargs.get("course_key") if course_id: course_key = get_course_key(course_id) use_forumv2 = is_forum_v2_enabled(course_key) @@ -175,8 +175,8 @@ def save(self, params=None): self._update_from_response(response) self.after_save(self) - def delete(self): - course_key = get_course_key(self.attributes.get("course_id")) + def delete(self, course_id=None): + course_key = get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): response = None if self.type == "comment": diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py index b0d9a00f4063..b5f227f7ccfb 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/thread.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/thread.py @@ -80,6 +80,8 @@ def search(cls, query_params): search_params.pop('commentable_id', None) response = forum_api.search_threads(**search_params) else: + if user_id := params.get('user_id'): + params['user_id'] = str(user_id) response = forum_api.get_user_threads(**params) else: response = utils.perform_request( @@ -196,12 +198,12 @@ def _retrieve(self, *args, **kwargs): ) self._update_from_response(response) - def flagAbuse(self, user, voteable): + def flagAbuse(self, user, voteable, course_id=None): if voteable.type == 'thread': url = _url_for_flag_abuse_thread(voteable.id) else: raise utils.CommentClientRequestError("Can only flag/unflag threads or comments") - course_key = utils.get_course_key(self.attributes.get("course_id")) + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): response = forum_api.update_thread_flag(voteable.id, "flag", user_id=user.id, course_id=str(course_key)) else: @@ -215,12 +217,12 @@ def flagAbuse(self, user, voteable): ) voteable._update_from_response(response) - def unFlagAbuse(self, user, voteable, removeAll): + def unFlagAbuse(self, user, voteable, removeAll, course_id=None): if voteable.type == 'thread': url = _url_for_unflag_abuse_thread(voteable.id) else: raise utils.CommentClientRequestError("Can only flag/unflag for threads or comments") - course_key = utils.get_course_key(self.attributes.get("course_id")) + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): response = forum_api.update_thread_flag( thread_id=voteable.id, @@ -244,8 +246,8 @@ def unFlagAbuse(self, user, voteable, removeAll): ) voteable._update_from_response(response) - def pin(self, user, thread_id): - course_key = utils.get_course_key(self.attributes.get("course_id")) + def pin(self, user, thread_id, course_id=None): + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): response = forum_api.pin_thread( user_id=user.id, @@ -264,8 +266,8 @@ def pin(self, user, thread_id): ) self._update_from_response(response) - def un_pin(self, user, thread_id): - course_key = utils.get_course_key(self.attributes.get("course_id")) + def un_pin(self, user, thread_id, course_id): + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): response = forum_api.unpin_thread( user_id=user.id, diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/user.py b/openedx/core/djangoapps/django_comment_common/comment_client/user.py index 2de4fbbfa95a..eaac6b408659 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/user.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/user.py @@ -50,8 +50,8 @@ def read(self, source): metric_tags=self._metric_tags + [f'target.type:{source.type}'], ) - def follow(self, source): - course_key = utils.get_course_key(self.attributes.get("course_id")) + def follow(self, source, course_id=None): + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): forum_api.create_subscription( user_id=self.id, @@ -68,8 +68,8 @@ def follow(self, source): metric_tags=self._metric_tags + [f'target.type:{source.type}'], ) - def unfollow(self, source): - course_key = utils.get_course_key(self.attributes.get("course_id")) + def unfollow(self, source, course_id=None): + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): forum_api.delete_subscription( user_id=self.id, @@ -86,14 +86,14 @@ def unfollow(self, source): metric_tags=self._metric_tags + [f'target.type:{source.type}'], ) - def vote(self, voteable, value): + def vote(self, voteable, value, course_id=None): if voteable.type == 'thread': url = _url_for_vote_thread(voteable.id) elif voteable.type == 'comment': url = _url_for_vote_comment(voteable.id) else: raise utils.CommentClientRequestError("Can only vote / unvote for threads or comments") - course_key = utils.get_course_key(self.attributes.get("course_id")) + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): if voteable.type == 'thread': response = forum_api.update_thread_votes( @@ -120,14 +120,14 @@ def vote(self, voteable, value): ) voteable._update_from_response(response) - def unvote(self, voteable): + def unvote(self, voteable, course_id=None): if voteable.type == 'thread': url = _url_for_vote_thread(voteable.id) elif voteable.type == 'comment': url = _url_for_vote_comment(voteable.id) else: raise utils.CommentClientRequestError("Can only vote / unvote for threads or comments") - course_key = utils.get_course_key(self.attributes.get("course_id")) + course_key = utils.get_course_key(self.attributes.get("course_id") or course_id) if is_forum_v2_enabled(course_key): if voteable.type == 'thread': response = forum_api.delete_thread_vote( diff --git a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py index dde2084e54cd..44dedcf42874 100644 --- a/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py +++ b/openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py @@ -6,6 +6,7 @@ import logging from collections import defaultdict from datetime import datetime, timezone +from urllib.parse import unquote from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.transaction import atomic @@ -446,9 +447,20 @@ def _lookup_asset_url(self, block: XBlock, asset_path: str) -> str | None: .get(key=f"static/{asset_path}") ) except ObjectDoesNotExist: - # This means we see a path that _looks_ like it should be a static - # asset for this Component, but that static asset doesn't really - # exist. - return None + try: + # Retry with unquoted path. We don't always unquote because it would not + # be backwards-compatible, but we need to try both. + asset_path = unquote(asset_path) + content = ( + component_version + .componentversioncontent_set + .filter(content__has_file=True) + .get(key=f"static/{asset_path}") + ) + except ObjectDoesNotExist: + # This means we see a path that _looks_ like it should be a static + # asset for this Component, but that static asset doesn't really + # exist. + return None return self._absolute_url_for_asset(component_version, asset_path) diff --git a/requirements/common_constraints.txt b/requirements/common_constraints.txt index b8166ba67540..f3cc8fc9c9e8 100644 --- a/requirements/common_constraints.txt +++ b/requirements/common_constraints.txt @@ -28,3 +28,7 @@ elasticsearch<7.14.0 # Cause: https://github.com/openedx/edx-lint/issues/458 # This can be unpinned once https://github.com/openedx/edx-lint/issues/459 has been resolved. pip<24.3 + +# Cause: https://github.com/openedx/edx-lint/issues/475 +# This can be unpinned once https://github.com/openedx/edx-lint/issues/476 has been resolved. +urllib3<2.3.0 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 969c20e114de..9a9207bd8eef 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -105,13 +105,6 @@ event-tracking==3.0.0 # https://github.com/openedx/edx-platform/issues/31616 libsass==0.10.0 -# Date: 2024-04-30 -# lxml>=5.0 introduced breaking changes related to system dependencies -# lxml==5.2.1 introduced new extra so we'll nee to rename lxml --> lxml[html-clean] -# This constraint can be removed once we upgrade to Python 3.11 -# Issue for unpinning: https://github.com/openedx/edx-platform/issues/35272 -lxml<5.0 - # Date: 2018-12-14 # markdown>=3.4.0 has failures due to internal refactorings which causes the tests to fail # pinning the version untill the issue gets resolved in the package itself @@ -192,8 +185,3 @@ social-auth-app-django<=5.4.1 # # Date: 2024-10-14 # # The edx-enterprise is currently using edx-rest-api-client==5.7.1, which needs to be updated first. # edx-rest-api-client==5.7.1 - -# Date: 2024-04-24 -# xmlsec==1.3.14 breaking tests or all builds, can be removed once a fix is available -# Issue for unpinning: https://github.com/openedx/edx-platform/issues/35264 -xmlsec<1.3.14 diff --git a/requirements/edx-sandbox/base.in b/requirements/edx-sandbox/base.in index 4c1a61bc2723..0c331ce95d62 100644 --- a/requirements/edx-sandbox/base.in +++ b/requirements/edx-sandbox/base.in @@ -2,7 +2,7 @@ chem # A helper library for chemistry calculations cryptography # Implementations of assorted cryptography algorithms -lxml # XML parser +lxml[html_clean] # XML parser matplotlib # 2D plotting library networkx # Utilities for creating, manipulating, and studying network graphs nltk # Natural language processing; used by the chem package diff --git a/requirements/edx-sandbox/base.txt b/requirements/edx-sandbox/base.txt index 991ea0efcf6a..92ee57f8b24b 100644 --- a/requirements/edx-sandbox/base.txt +++ b/requirements/edx-sandbox/base.txt @@ -26,11 +26,13 @@ joblib==1.4.2 # via nltk kiwisolver==1.4.7 # via matplotlib -lxml==4.9.4 +lxml[html-clean,html_clean]==5.3.0 # via - # -c requirements/edx-sandbox/../constraints.txt # -r requirements/edx-sandbox/base.in + # lxml-html-clean # openedx-calc +lxml-html-clean==0.3.1 + # via lxml markupsafe==3.0.2 # via # chem diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index c2aebbb6fa31..94084345ad47 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -170,7 +170,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.17 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -715,19 +715,21 @@ loremipsum==1.0.5 # via ora2 lti-consumer-xblock==9.11.3 # via -r requirements/edx/kernel.in -lxml==4.9.4 +lxml[html-clean,html_clean]==5.3.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in # edx-i18n-tools # edxval # lti-consumer-xblock + # lxml-html-clean # olxcleaner # openedx-calc # ora2 # python3-saml # xblock # xmlsec +lxml-html-clean==0.3.1 + # via lxml mailsnake==1.6.4 # via -r requirements/edx/bundled.in mako==1.3.6 @@ -836,7 +838,7 @@ openedx-filters==1.11.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # ora2 -openedx-forum==0.1.5 +openedx-forum==0.2.0 # via -r requirements/edx/kernel.in openedx-learning==0.18.1 # via @@ -1236,6 +1238,7 @@ uritemplate==4.1.1 # google-api-python-client urllib3==2.2.3 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/paver.txt # botocore # elasticsearch @@ -1300,10 +1303,8 @@ xblock-utils==4.0.0 # via # edx-sga # xblock-poll -xmlsec==1.3.13 - # via - # -c requirements/edx/../constraints.txt - # python3-saml +xmlsec==1.3.14 + # via python3-saml xss-utils==0.6.0 # via -r requirements/edx/kernel.in yarl==1.16.0 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index da46c049045b..c0667cf4959c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -337,7 +337,7 @@ distlib==0.3.9 # via # -r requirements/edx/testing.txt # virtualenv -django==4.2.17 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -1193,14 +1193,14 @@ lti-consumer-xblock==9.11.3 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt -lxml==4.9.4 +lxml[html-clean]==5.3.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-i18n-tools # edxval # lti-consumer-xblock + # lxml-html-clean # olxcleaner # openedx-calc # ora2 @@ -1208,6 +1208,11 @@ lxml==4.9.4 # python3-saml # xblock # xmlsec +lxml-html-clean==0.3.1 + # via + # -r requirements/edx/doc.txt + # -r requirements/edx/testing.txt + # lxml mailsnake==1.6.4 # via # -r requirements/edx/doc.txt @@ -1388,7 +1393,7 @@ openedx-filters==1.11.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.5 +openedx-forum==0.2.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -2180,6 +2185,7 @@ uritemplate==4.1.1 # google-api-python-client urllib3==2.2.3 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # botocore @@ -2292,9 +2298,8 @@ xblock-utils==4.0.0 # -r requirements/edx/testing.txt # edx-sga # xblock-poll -xmlsec==1.3.13 +xmlsec==1.3.14 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # python3-saml diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 4efcbcc5b5a0..a5ed7ac21f28 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -227,7 +227,7 @@ defusedxml==0.7.1 # ora2 # python3-openid # social-auth-core -django==4.2.17 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -863,19 +863,23 @@ loremipsum==1.0.5 # ora2 lti-consumer-xblock==9.11.3 # via -r requirements/edx/base.txt -lxml==4.9.4 +lxml[html-clean]==5.3.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-i18n-tools # edxval # lti-consumer-xblock + # lxml-html-clean # olxcleaner # openedx-calc # ora2 # python3-saml # xblock # xmlsec +lxml-html-clean==0.3.1 + # via + # -r requirements/edx/base.txt + # lxml mailsnake==1.6.4 # via -r requirements/edx/base.txt mako==1.3.6 @@ -1003,7 +1007,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.5 +openedx-forum==0.2.0 # via -r requirements/edx/base.txt openedx-learning==0.18.1 # via @@ -1531,6 +1535,7 @@ uritemplate==4.1.1 # google-api-python-client urllib3==2.2.3 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # botocore # elasticsearch @@ -1606,9 +1611,8 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xmlsec==1.3.13 +xmlsec==1.3.14 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # python3-saml xss-utils==0.6.0 diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index 575c621a85d7..60f49c5917e1 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -103,7 +103,7 @@ importlib_metadata # Used to access entry_points in i18n_api pl jsonfield # Django model field for validated JSON; used in several apps laboratory # Library for testing that code refactors/infrastructure changes produce identical results importlib_metadata # Used to access entry_points in i18n_api plugin -lxml # XML parser +lxml[html_clean] # XML parser lti-consumer-xblock>=7.3.0 mako # Primary template language used for server-side page rendering Markdown # Convert text markup to HTML; used in capa problems, forums, and course wikis diff --git a/requirements/edx/paver.txt b/requirements/edx/paver.txt index f3dae3b0efda..14aa08a2a6b0 100644 --- a/requirements/edx/paver.txt +++ b/requirements/edx/paver.txt @@ -58,7 +58,9 @@ stevedore==5.3.0 typing-extensions==4.12.2 # via edx-opaque-keys urllib3==2.2.3 - # via requests + # via + # -c requirements/edx/../common_constraints.txt + # requests watchdog==5.0.3 # via -r requirements/edx/paver.in wrapt==1.16.0 diff --git a/requirements/edx/semgrep.txt b/requirements/edx/semgrep.txt index 6ad814b602d6..24c3c263b5a2 100644 --- a/requirements/edx/semgrep.txt +++ b/requirements/edx/semgrep.txt @@ -126,6 +126,7 @@ typing-extensions==4.12.2 # semgrep urllib3==2.2.3 # via + # -c requirements/edx/../common_constraints.txt # requests # semgrep wcmatch==8.5.2 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 1b19a6cbf65e..309887480f96 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -253,7 +253,7 @@ dill==0.3.9 # via pylint distlib==0.3.9 # via virtualenv -django==4.2.17 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -906,13 +906,13 @@ loremipsum==1.0.5 # ora2 lti-consumer-xblock==9.11.3 # via -r requirements/edx/base.txt -lxml==4.9.4 +lxml[html-clean]==5.3.0 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # edx-i18n-tools # edxval # lti-consumer-xblock + # lxml-html-clean # olxcleaner # openedx-calc # ora2 @@ -920,6 +920,10 @@ lxml==4.9.4 # python3-saml # xblock # xmlsec +lxml-html-clean==0.3.1 + # via + # -r requirements/edx/base.txt + # lxml mailsnake==1.6.4 # via -r requirements/edx/base.txt mako==1.3.6 @@ -1048,7 +1052,7 @@ openedx-filters==1.11.0 # -r requirements/edx/base.txt # lti-consumer-xblock # ora2 -openedx-forum==0.1.5 +openedx-forum==0.2.0 # via -r requirements/edx/base.txt openedx-learning==0.18.1 # via @@ -1614,6 +1618,7 @@ uritemplate==4.1.1 # google-api-python-client urllib3==2.2.3 # via + # -c requirements/edx/../common_constraints.txt # -r requirements/edx/base.txt # botocore # elasticsearch @@ -1693,9 +1698,8 @@ xblock-utils==4.0.0 # -r requirements/edx/base.txt # edx-sga # xblock-poll -xmlsec==1.3.13 +xmlsec==1.3.14 # via - # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt # python3-saml xss-utils==0.6.0 diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index c273dee82b98..3d81950df7e7 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.txt @@ -35,7 +35,7 @@ click==8.1.6 # edx-django-utils cryptography==43.0.3 # via pyjwt -django==4.2.17 +django==4.2.20 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt @@ -77,10 +77,8 @@ jmespath==1.0.1 # via # boto3 # botocore -lxml==4.9.4 - # via - # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt - # zeep +lxml==5.3.0 + # via zeep more-itertools==10.5.0 # via simple-salesforce newrelic==10.2.0 @@ -160,6 +158,7 @@ uritemplate==4.1.1 # via google-api-python-client urllib3==1.26.20 # via + # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -r scripts/user_retirement/requirements/base.in # botocore # requests diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index bf54f6d19506..16f374375185 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -52,7 +52,7 @@ cryptography==43.0.3 # pyjwt ddt==1.7.2 # via -r scripts/user_retirement/requirements/testing.in -django==4.2.17 +django==4.2.20 # via # -r scripts/user_retirement/requirements/base.txt # django-crum @@ -116,7 +116,7 @@ jmespath==1.0.1 # -r scripts/user_retirement/requirements/base.txt # boto3 # botocore -lxml==4.9.4 +lxml==5.3.0 # via # -r scripts/user_retirement/requirements/base.txt # zeep diff --git a/scripts/xblock/requirements.txt b/scripts/xblock/requirements.txt index 920cf0cf6ac1..3e993640ea78 100644 --- a/scripts/xblock/requirements.txt +++ b/scripts/xblock/requirements.txt @@ -15,4 +15,6 @@ idna==3.10 requests==2.32.3 # via -r scripts/xblock/requirements.in urllib3==2.2.3 - # via requests + # via + # -c scripts/xblock/../../requirements/common_constraints.txt + # requests