From 59cb8f006a3603a20047a4eb434537ce1b8cf383 Mon Sep 17 00:00:00 2001 From: Daniel Valenzuela Date: Tue, 11 Feb 2025 23:08:43 -0300 Subject: [PATCH 1/6] fix: static assets used in problem bank and library content block (#36240) * fix: render library v2 assets with whitespace (#35974) Assets that contain whitespace fail to be rendered when uploaded to library v2 * fix: static assets used in problem bank and library content block (#36173) Static assets were not being copied into the course when using library content via Problem Bank or "Add Library Content" workflows. --- cms/djangoapps/contentstore/helpers.py | 93 +++++++++++---- .../rest_api/v2/views/downstreams.py | 10 +- .../v2/views/tests/test_downstreams.py | 5 +- .../xblock_storage_handlers/view_handlers.py | 7 +- cms/lib/xblock/upstream_sync.py | 3 +- .../tests/test_static_assets.py | 7 ++ .../djangoapps/content_libraries/views.py | 1 + .../core/djangoapps/content_staging/api.py | 111 ++++++++++++------ .../core/djangoapps/content_staging/data.py | 4 + .../xblock/runtime/learning_core_runtime.py | 20 +++- 10 files changed, 189 insertions(+), 72 deletions(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8ff1f1aa39cc..8250d70486cd 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, @@ -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/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/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..48fe49144194 100644 --- a/openedx/core/djangoapps/content_libraries/views.py +++ b/openedx/core/djangoapps/content_libraries/views.py @@ -788,6 +788,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/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) From 94d5c88d5f7a7bd9261831804c2b4a3197eeac2e Mon Sep 17 00:00:00 2001 From: Maga Giorgianni Date: Wed, 12 Feb 2025 08:40:05 -0400 Subject: [PATCH 2/6] chore: update Django to 4.2.19 for Sumac - Security Patch (#36234) * chore: upgrade Django to 4.2.19 * chore: compile requirements --- requirements/common_constraints.txt | 4 ++++ requirements/edx/base.txt | 3 ++- requirements/edx/development.txt | 3 ++- requirements/edx/doc.txt | 3 ++- requirements/edx/paver.txt | 4 +++- requirements/edx/semgrep.txt | 1 + requirements/edx/testing.txt | 3 ++- scripts/user_retirement/requirements/base.txt | 3 ++- scripts/user_retirement/requirements/testing.txt | 2 +- scripts/xblock/requirements.txt | 4 +++- 10 files changed, 22 insertions(+), 8 deletions(-) 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/edx/base.txt b/requirements/edx/base.txt index c2aebbb6fa31..4d782e2d84ba 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.19 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -1236,6 +1236,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 diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index da46c049045b..3b4676c93d38 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.19 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -2180,6 +2180,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 diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 4efcbcc5b5a0..1f41c03dd199 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.19 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -1531,6 +1531,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 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..f97463a12c97 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.19 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt @@ -1614,6 +1614,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 diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index c273dee82b98..0eaa9afd1923 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.19 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt @@ -160,6 +160,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..1d144fecb4eb 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.19 # via # -r scripts/user_retirement/requirements/base.txt # django-crum 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 From 866c1fae20db7b61f2affafa85724450953c49c2 Mon Sep 17 00:00:00 2001 From: Jillian Date: Thu, 27 Feb 2025 08:34:38 +1030 Subject: [PATCH 3/6] fix: allow_to_create_new_org checks org autocreate [FC-0076] (#36094) (#36288) Updates the StudioHome API's allow_to_create_new_org to require both organization-creation permissions and ORGANIZATION_AUTOCREATE to be enabled. It also adds the list of "allowed organizations for libraries" to the Studio Home API so that the Authoring MFE can use it. (cherry picked from commit b96a3bf249ecc104b4882e67738d06499c1a0245) --- .../rest_api/v1/serializers/home.py | 4 ++ .../contentstore/rest_api/v1/views/home.py | 9 ++- .../rest_api/v1/views/tests/test_home.py | 14 ++++- .../tests/test_content_libraries.py | 58 +++++++++++++++++++ .../djangoapps/content_libraries/views.py | 12 ++++ 5 files changed, 95 insertions(+), 2 deletions(-) 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/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/views.py b/openedx/core/djangoapps/content_libraries/views.py index 48fe49144194..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: From b709dab8ac127a3ab1b6aed530b0953c7e5d6ae3 Mon Sep 17 00:00:00 2001 From: magajh Date: Thu, 6 Mar 2025 18:53:55 -0400 Subject: [PATCH 4/6] chore: upgrade Django to 4.2.20 --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- scripts/user_retirement/requirements/base.txt | 2 +- scripts/user_retirement/requirements/testing.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 4d782e2d84ba..b665260a7910 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.19 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 3b4676c93d38..5f30322f02f1 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.19 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 1f41c03dd199..26fbe0615b44 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.19 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index f97463a12c97..f282363707c4 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.19 +django==4.2.20 # via # -c requirements/edx/../common_constraints.txt # -c requirements/edx/../constraints.txt diff --git a/scripts/user_retirement/requirements/base.txt b/scripts/user_retirement/requirements/base.txt index 0eaa9afd1923..46f18b2415f6 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.19 +django==4.2.20 # via # -c scripts/user_retirement/requirements/../../../requirements/common_constraints.txt # -c scripts/user_retirement/requirements/../../../requirements/constraints.txt diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index 1d144fecb4eb..ab56b97801ed 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.19 +django==4.2.20 # via # -r scripts/user_retirement/requirements/base.txt # django-crum From 80fa4a8c32fa93efcd1bec51427d0f5dc74be7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=B4mulo=20Penido?= Date: Tue, 11 Mar 2025 13:51:49 -0300 Subject: [PATCH 5/6] fix: pasted component search index document missing breadcrumbs [sumac] [FC-0076] --- cms/djangoapps/contentstore/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8250d70486cd..b3a5900958ba 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -496,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) From c1804f58765a554fbeeb251cacb1b5c670b54354 Mon Sep 17 00:00:00 2001 From: Abdul-Muqadim-Arbisoft <139064778+Abdul-Muqadim-Arbisoft@users.noreply.github.com> Date: Mon, 28 Apr 2025 19:04:52 +0500 Subject: [PATCH 6/6] Fix/forum patches sumac (#36606) * fix: discussion xblock not compatible with forum v2 (#36315) fix all endpoints that were currently breaking with the discussion xblock. Co-authored-by: Taimoor Ahmed * fix: legacy discussion issues (#36433) Explicitly passed course_id to all views * fix: legacy forum issues (#36470) Co-authored-by: Taimoor Ahmed * build: Switch off deprecated C-Hive NPM cache (#36502) JS tests are failing because we are using a discontinued GHA caching service: https://github.blog/changelog/2025-03-20-notification-of-upcoming-breaking-changes-in-github-actions/#decommissioned-cache-service-brownouts This service is used by the unsupported C-Hive caching action which we are relying on: https://github.com/c-hive/gha-npm-cache We are switching to the supported caching mechanims which is provided by setup-node: https://github.com/actions/setup-node?tab=readme-ov-file#caching-global-packages-data * Merge pull request #35713 from openedx/feanil/ubuntu-24.04 feanil/ubuntu 24.04 --------- Co-authored-by: Taimoor Ahmed <68893403+taimoor-ahmed-1@users.noreply.github.com> Co-authored-by: Taimoor Ahmed Co-authored-by: Ali Salman <88362079+Ali-Salman29@users.noreply.github.com> Co-authored-by: Kyle McCormick Co-authored-by: Feanil Patel --- .../check-consistent-dependencies.yml | 2 +- .github/workflows/ci-static-analysis.yml | 2 +- .github/workflows/js-tests.yml | 5 +- .github/workflows/migrations-check.yml | 4 +- .github/workflows/pylint-checks.yml | 4 +- .github/workflows/quality-checks.yml | 2 +- .github/workflows/static-assets-check.yml | 2 +- .github/workflows/unit-tests.yml | 62 +++++++++---------- .../django_comment_client/base/tests.py | 2 +- .../django_comment_client/base/views.py | 38 ++++++------ lms/djangoapps/discussion/views.py | 8 +-- .../comment_client/comment.py | 8 +-- .../comment_client/models.py | 10 +-- .../comment_client/thread.py | 18 +++--- .../comment_client/user.py | 16 ++--- requirements/constraints.txt | 12 ---- requirements/edx-sandbox/base.in | 2 +- requirements/edx-sandbox/base.txt | 6 +- requirements/edx/base.txt | 14 ++--- requirements/edx/development.txt | 14 +++-- requirements/edx/doc.txt | 13 ++-- requirements/edx/kernel.in | 2 +- requirements/edx/testing.txt | 13 ++-- scripts/user_retirement/requirements/base.txt | 6 +- .../user_retirement/requirements/testing.txt | 2 +- 25 files changed, 133 insertions(+), 134 deletions(-) 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/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/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/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 b665260a7910..94084345ad47 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.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 @@ -1301,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 5f30322f02f1..c0667cf4959c 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.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 @@ -2293,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 26fbe0615b44..a5ed7ac21f28 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.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 @@ -1607,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/testing.txt b/requirements/edx/testing.txt index f282363707c4..309887480f96 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.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 @@ -1694,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 46f18b2415f6..3d81950df7e7 100644 --- a/scripts/user_retirement/requirements/base.txt +++ b/scripts/user_retirement/requirements/base.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 diff --git a/scripts/user_retirement/requirements/testing.txt b/scripts/user_retirement/requirements/testing.txt index ab56b97801ed..16f374375185 100644 --- a/scripts/user_retirement/requirements/testing.txt +++ b/scripts/user_retirement/requirements/testing.txt @@ -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