From dba51e48bb2b2ee0d769a781fb11d66a634531b1 Mon Sep 17 00:00:00 2001 From: Muhammad Faraz Maqsood Date: Mon, 16 Sep 2024 16:44:34 +0500 Subject: [PATCH 1/7] feat: Integrate Forum V2 into edx-platform This commit introduces the new Forum V2 application, allowing users to choose between the legacy Forum V1 and the new Forum V2 at the course level. Key Changes: - Added waffle flag `forum_v2.enable_forum_v2` to enable Forum V2 for selected courses, allowing coexistence with Forum V1. - Default data storage for Forum V2 is set to MongoDB, with an option to switch to MySQL using the waffle flag `forum_v2.enable_mysql_backend`. - Introduced management command `forum_migrate_course_from_mongodb_to_mysql` for per-course data migration from MongoDB to MySQL. Note: This PR does not include all unit tests for the Forum V2 native API due to ongoing migration efforts. Further updates will follow to ensure full test coverage before final release. Co-authored-by: [Muhammad Faraz Maqsood] Co-authored-by: [Ali Salman] --- .github/CODEOWNERS | 20 +- .github/workflows/ci-static-analysis.yml | 2 +- .../workflows/compile-python-requirements.yml | 2 +- .github/workflows/js-tests.yml | 2 +- .github/workflows/lint-imports.yml | 2 +- .github/workflows/migrations-check.yml | 8 +- .github/workflows/publish-ci-docker-image.yml | 2 +- .github/workflows/pylint-checks.yml | 18 +- .github/workflows/quality-checks.yml | 2 +- .github/workflows/semgrep.yml | 2 +- .github/workflows/static-assets-check.yml | 5 +- .github/workflows/unit-tests.yml | 36 +- .../upgrade-one-python-dependency.yml | 2 +- .github/workflows/verify-dunder-init.yml | 2 +- .readthedocs.yaml | 2 +- README.rst | 33 +- cms/djangoapps/contentstore/config/waffle.py | 16 +- cms/djangoapps/contentstore/helpers.py | 47 +- .../rest_api/v1/serializers/home.py | 2 - .../rest_api/v1/serializers/settings.py | 1 - .../rest_api/v1/serializers/vertical_block.py | 13 + .../contentstore/rest_api/v1/views/home.py | 2 - .../rest_api/v1/views/settings.py | 2 - .../v1/views/tests/test_course_index.py | 5 +- .../rest_api/v1/views/tests/test_home.py | 2 - .../rest_api/v1/views/tests/test_settings.py | 1 - .../v1/views/tests/test_vertical_block.py | 12 + .../rest_api/v1/views/vertical_block.py | 23 + .../contentstore/rest_api/v2/urls.py | 24 +- .../rest_api/v2/views/__init__.py | 3 - .../rest_api/v2/views/downstreams.py | 251 +++++++++ .../v2/views/tests/test_downstreams.py | 266 +++++++++ cms/djangoapps/contentstore/utils.py | 13 +- cms/djangoapps/contentstore/views/library.py | 16 - .../contentstore/views/tests/test_block.py | 55 +- .../views/tests/test_clipboard_paste.py | 111 +++- .../contentstore/views/transcripts_ajax.py | 3 + .../xblock_storage_handlers/view_handlers.py | 20 +- cms/envs/common.py | 43 +- cms/envs/devstack.py | 6 +- cms/envs/production.py | 7 + cms/envs/test.py | 10 + cms/lib/xblock/test/test_upstream_sync.py | 337 ++++++++++++ cms/lib/xblock/upstream_sync.py | 451 ++++++++++++++++ cms/templates/course_outline.html | 6 +- .../course_creator_admin_user_pending.txt | 2 +- cms/templates/index.html | 4 - cms/templates/settings.html | 68 +-- cms/templates/studio_xblock_wrapper.html | 32 +- .../course_modes/tests/test_views.py | 44 -- common/djangoapps/course_modes/views.py | 2 +- common/djangoapps/student/helpers.py | 21 +- common/djangoapps/third_party_auth/README.rst | 11 + common/djangoapps/third_party_auth/admin.py | 22 +- .../tests/test_pipeline_integration.py | 2 +- .../static/data/geoip/GeoLite2-Country.mmdb | Bin 7570372 -> 7594092 bytes .../templates/xblock_v2}/xblock_iframe.html | 2 +- docs/conf.py | 19 +- docs/decisions/0020-upstream-downstream.rst | 407 ++++++++++++++ docs/docs_settings.py | 1 - docs/index.rst | 14 + lms/djangoapps/branding/tests/test_page.py | 24 + lms/djangoapps/bulk_email/message_types.py | 1 + lms/djangoapps/bulk_email/signals.py | 30 +- lms/djangoapps/bulk_email/tasks.py | 3 +- lms/djangoapps/ccx/api/v0/tests/test_views.py | 4 +- lms/djangoapps/ccx/api/v0/views.py | 8 +- lms/djangoapps/ccx/utils.py | 26 +- lms/djangoapps/ccx/views.py | 4 +- lms/djangoapps/certificates/api.py | 15 +- .../docs/diagrams/certificate_generation.dsl | 4 +- lms/djangoapps/certificates/signals.py | 29 +- lms/djangoapps/certificates/tests/test_api.py | 26 +- .../certificates/tests/test_filters.py | 6 +- .../certificates/tests/test_signals.py | 19 +- .../certificates/tests/test_utils.py | 36 +- lms/djangoapps/certificates/utils.py | 37 +- lms/djangoapps/certificates/views/webview.py | 36 +- lms/djangoapps/commerce/tests/test_utils.py | 23 - lms/djangoapps/commerce/utils.py | 92 +--- lms/djangoapps/commerce/waffle.py | 49 -- .../transformers/library_content.py | 8 +- .../commands/goal_reminder_email.py | 15 +- .../tests/test_goal_reminder_email.py | 33 +- lms/djangoapps/courseware/block_render.py | 4 +- lms/djangoapps/courseware/plugins.py | 11 +- lms/djangoapps/courseware/toggles.py | 1 - lms/djangoapps/courseware/views/views.py | 11 +- .../django_comment_client/base/tests.py | 398 ++++++++++---- .../django_comment_client/base/views.py | 1 - .../django_comment_client/tests/group_id.py | 147 +++-- lms/djangoapps/discussion/rest_api/api.py | 7 +- .../rest_api/discussions_notifications.py | 57 +- .../discussion/rest_api/serializers.py | 2 +- .../discussion/rest_api/tests/test_api.py | 84 +++ .../tests/test_discussions_notifications.py | 70 ++- .../rest_api/tests/test_serializers.py | 19 + .../discussion/rest_api/tests/test_tasks.py | 63 ++- .../discussion/rest_api/tests/test_views.py | 130 +++++ lms/djangoapps/discussion/signals/handlers.py | 2 + lms/djangoapps/discussion/tasks.py | 71 ++- .../edx_ace/commentnotification/push/body.txt | 3 + .../commentnotification/push/title.txt | 2 + .../responsenotification/push/body.txt | 2 + .../responsenotification/push/title.txt | 2 + lms/djangoapps/discussion/tests/test_tasks.py | 39 +- lms/djangoapps/discussion/tests/test_views.py | 270 +++++++++- lms/djangoapps/discussion/toggles.py | 25 +- .../grades/rest_api/v1/tests/test_views.py | 8 +- lms/djangoapps/grades/rest_api/v1/views.py | 4 +- lms/djangoapps/grades/scores.py | 4 +- .../grades/tests/test_course_grade_factory.py | 8 +- .../grades/tests/test_subsection_grade.py | 2 +- lms/djangoapps/instructor/access.py | 4 +- lms/djangoapps/instructor/enrollment.py | 101 ++-- lms/djangoapps/instructor/tests/test_api.py | 46 +- lms/djangoapps/instructor/views/api.py | 508 ++++++++++-------- lms/djangoapps/instructor/views/api_urls.py | 15 +- lms/djangoapps/instructor/views/serializer.py | 72 ++- lms/djangoapps/learner_home/serializers.py | 39 +- .../learner_home/test_serializers.py | 23 +- .../mobile_api/notifications}/__init__.py | 0 .../mobile_api/notifications/urls.py | 12 + .../mobile_api/notifications/views.py | 53 ++ lms/djangoapps/mobile_api/urls.py | 1 + lms/djangoapps/verify_student/admin.py | 14 +- lms/djangoapps/verify_student/api.py | 53 +- lms/djangoapps/verify_student/apps.py | 2 +- .../commands/approve_id_verifications.py | 34 ++ .../tests/test_approve_id_verifications.py | 59 ++ ...sso_verifications_for_old_account_links.py | 2 +- .../test_retry_failed_photo_verifications.py | 2 +- ...curephotoverifications_post_save_signal.py | 2 +- ...ve_verificationattempt_created_and_more.py | 50 ++ lms/djangoapps/verify_student/models.py | 57 +- lms/djangoapps/verify_student/services.py | 59 +- .../verify_student/signals}/__init__.py | 0 .../{signals.py => signals/handlers.py} | 14 +- .../verify_student/signals/signals.py | 109 ++++ .../verify_student/tests/__init__.py | 11 +- .../verify_student/tests/test_api.py | 49 +- .../{test_signals.py => test_handlers.py} | 26 +- .../verify_student/tests/test_models.py | 12 + .../verify_student/tests/test_services.py | 184 ++++--- lms/envs/common.py | 29 +- lms/envs/devstack.py | 1 + lms/envs/minimal.yml | 3 + lms/envs/production.py | 7 + lms/static/completion/js/ViewedEvent.js | 46 +- .../instructor_dashboard.js | 8 +- .../course_modes/track_selection.html | 10 +- .../_dashboard_certificate_information.html | 11 +- .../edx_ace/addbetatester/push/body.txt | 5 + .../edx_ace/addbetatester/push/title.txt | 4 + .../edx_ace/allowedenroll/push/body.txt | 4 + .../edx_ace/allowedenroll/push/title.txt | 4 + .../edx_ace/allowedunenroll/push/body.txt | 4 + .../edx_ace/allowedunenroll/push/title.txt | 4 + .../edx_ace/enrolledunenroll/push/body.txt | 5 + .../edx_ace/enrolledunenroll/push/title.txt | 4 + .../edx_ace/enrollenrolled/push/body.txt | 5 + .../edx_ace/enrollenrolled/push/title.txt | 4 + .../edx_ace/removebetatester/push/body.txt | 5 + .../edx_ace/removebetatester/push/title.txt | 4 + lms/templates/xblock_v2/xblock_iframe.html | 1 - mypy.ini | 4 +- .../djangoapps/ace_common/settings/common.py | 30 ++ .../ace_common/settings/production.py | 24 + openedx/core/djangoapps/ace_common/utils.py | 21 + openedx/core/djangoapps/content/search/api.py | 146 ++++- .../djangoapps/content/search/documents.py | 132 +++-- .../djangoapps/content/search/handlers.py | 51 +- .../core/djangoapps/content/search/tasks.py | 15 +- .../content/search/tests/test_api.py | 163 +++++- .../content/search/tests/test_documents.py | 46 +- .../content/search/tests/test_handlers.py | 2 +- .../core/djangoapps/content_libraries/api.py | 391 +++++++++----- .../content_libraries/library_context.py | 71 +-- .../content_libraries/permissions.py | 15 +- .../content_libraries/serializers.py | 25 + .../content_libraries/signal_handlers.py | 154 +++++- .../djangoapps/content_libraries/tasks.py | 195 +------ .../content_libraries/tests/base.py | 28 + .../tests/fields_test_block.py | 60 +++ .../content_libraries/tests/test_api.py | 107 +++- .../tests/test_content_libraries.py | 127 +++-- .../tests/test_embed_block.py | 231 ++++++++ .../content_libraries/tests/test_runtime.py | 82 +++ .../tests/test_static_assets.py | 92 +++- .../tests/test_views_collections.py | 27 +- .../core/djangoapps/content_libraries/urls.py | 7 + .../djangoapps/content_libraries/views.py | 124 ++++- .../content_libraries/views_collections.py | 30 +- .../core/djangoapps/content_staging/api.py | 4 +- .../core/djangoapps/content_staging/data.py | 1 + .../0005_stagedcontent_version_num.py | 18 + .../core/djangoapps/content_staging/models.py | 2 + .../content_staging/tests/test_clipboard.py | 2 +- .../core/djangoapps/content_staging/views.py | 4 +- .../tests/test_objecttag_export_helpers.py | 5 +- .../contentserver/test/test_contentserver.py | 21 +- .../core/djangoapps/contentserver/views.py | 502 +++++++++-------- .../comment_client/comment.py | 80 ++- .../comment_client/course.py | 77 +-- .../comment_client/models.py | 223 ++++++-- .../comment_client/subscriptions.py | 29 +- .../comment_client/thread.py | 175 ++++-- .../comment_client/user.py | 291 ++++++---- .../comment_client/utils.py | 7 + .../core/djangoapps/learner_pathway/admin.py | 3 - .../core/djangoapps/learner_pathway/apps.py | 12 - .../core/djangoapps/learner_pathway/models.py | 3 - .../learner_pathway/tests/__init__.py | 0 .../core/djangoapps/learner_pathway/views.py | 3 - .../core/djangoapps/models/course_details.py | 4 - .../models/tests/test_course_details.py | 26 +- .../notifications/base_notification.py | 66 ++- .../djangoapps/notifications/config/waffle.py | 31 ++ .../djangoapps/notifications/email/utils.py | 4 + .../core/djangoapps/notifications/events.py | 5 +- .../notifications/grouping_notifications.py | 121 +++++ .../core/djangoapps/notifications/handlers.py | 8 +- .../core/djangoapps/notifications/models.py | 2 +- .../notifications/notification_content.py | 36 +- .../core/djangoapps/notifications/policies.py | 41 ++ .../core/djangoapps/notifications/tasks.py | 45 +- .../tests/test_notification_grouping.py | 159 ++++++ .../notifications/tests/test_tasks.py | 49 +- .../notifications/tests/test_views.py | 9 +- .../core/djangoapps/notifications/utils.py | 9 +- .../core/djangoapps/notifications/views.py | 8 +- openedx/core/djangoapps/programs/tasks.py | 58 +- .../djangoapps/programs/tests/test_tasks.py | 12 +- openedx/core/djangoapps/signals/signals.py | 11 +- .../core/djangoapps/user_api/accounts/api.py | 7 +- .../user_api/accounts/tests/test_api.py | 31 +- .../accounts/tests/test_settings_views.py | 4 +- .../djangoapps/user_api/tests/test_views.py | 2 +- openedx/core/djangoapps/user_api/urls.py | 6 - .../verification_api/tests/test_views.py | 49 -- .../user_api/verification_api/views.py | 19 - .../djangoapps/user_authn/config/waffle.py | 1 - .../user_authn/views/registration_form.py | 14 +- .../user_authn/views/tests/test_register.py | 46 +- .../core/djangoapps/user_authn/views/utils.py | 17 + .../core/djangoapps/waffle_utils/__init__.py | 24 +- openedx/core/djangoapps/xblock/api.py | 103 ++-- openedx/core/djangoapps/xblock/apps.py | 16 +- openedx/core/djangoapps/xblock/data.py | 36 ++ .../learning_context/learning_context.py | 25 +- .../core/djangoapps/xblock/rest_api/urls.py | 2 +- .../core/djangoapps/xblock/rest_api/views.py | 87 ++- .../xblock/runtime/learning_core_runtime.py | 183 ++++++- .../core/djangoapps/xblock/runtime/runtime.py | 128 ++--- .../lib/xblock_serializer/block_serializer.py | 39 +- openedx/core/lib/xblock_utils/__init__.py | 2 +- .../features/course_experience/__init__.py | 10 + .../enterprise_support/tests/test_utils.py | 6 +- openedx/features/enterprise_support/utils.py | 2 +- .../completion_integration/test_services.py | 4 +- package-lock.json | 8 +- package.json | 2 +- pylint_django_settings.py | 83 ++- pylintrc | 9 +- pylintrc_tweaks | 2 +- requirements.txt | 10 - requirements/constraints.txt | 200 ++++--- requirements/edx-sandbox/base.txt | 24 +- requirements/edx-sandbox/releases/quince.txt | 2 +- requirements/edx-sandbox/releases/redwood.txt | 2 +- requirements/edx/base.txt | 211 ++++---- requirements/edx/coverage.txt | 6 +- requirements/edx/development.txt | 263 ++++----- requirements/edx/doc.txt | 214 ++++---- requirements/edx/github.in | 2 + requirements/edx/paver.txt | 16 +- requirements/edx/semgrep.txt | 14 +- requirements/edx/testing.txt | 249 +++++---- requirements/pip-tools.txt | 4 +- requirements/pip.txt | 2 +- .../structures_pruning/requirements/base.txt | 8 +- .../requirements/testing.txt | 10 +- scripts/user_retirement/requirements/base.txt | 58 +- .../user_retirement/requirements/testing.txt | 68 ++- scripts/xblock/requirements.txt | 6 +- setup.cfg | 3 +- setup.py | 5 +- webpack.builtinblocks.config.js | 9 +- .../public/js/library_content_edit_helpers.js | 54 -- xmodule/capa/tests/test_responsetypes.py | 14 + xmodule/capa_block.py | 6 +- xmodule/course_block.py | 3 - .../0003-library-content-block-schema.rst | 4 +- xmodule/graders.py | 6 +- xmodule/library_content_block.py | 77 +-- xmodule/library_tools.py | 75 +-- xmodule/mako_block.py | 9 +- xmodule/modulestore/mixed.py | 2 +- .../split_mongo/caching_descriptor_system.py | 4 +- xmodule/modulestore/split_mongo/split.py | 6 +- xmodule/partitions/tests/test_partitions.py | 15 - xmodule/tests/test_capa_block.py | 57 +- xmodule/tests/test_library_content.py | 63 +-- xmodule/tests/test_library_tools.py | 73 +-- xmodule/tests/test_randomize_block.py | 2 +- xmodule/vertical_block.py | 2 +- xmodule/video_block/transcripts_utils.py | 115 +++- xmodule/video_block/video_block.py | 18 +- 308 files changed, 9927 insertions(+), 3892 deletions(-) create mode 100644 cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py create mode 100644 cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py create mode 100644 cms/lib/xblock/test/test_upstream_sync.py create mode 100644 cms/lib/xblock/upstream_sync.py create mode 100644 common/djangoapps/third_party_auth/README.rst rename {cms/templates/content_libraries => common/templates/xblock_v2}/xblock_iframe.html (99%) create mode 100644 docs/decisions/0020-upstream-downstream.rst delete mode 100644 lms/djangoapps/commerce/waffle.py create mode 100644 lms/djangoapps/discussion/templates/discussion/edx_ace/commentnotification/push/body.txt create mode 100644 lms/djangoapps/discussion/templates/discussion/edx_ace/commentnotification/push/title.txt create mode 100644 lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/push/body.txt create mode 100644 lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/push/title.txt rename {openedx/core/djangoapps/learner_pathway => lms/djangoapps/mobile_api/notifications}/__init__.py (100%) create mode 100644 lms/djangoapps/mobile_api/notifications/urls.py create mode 100644 lms/djangoapps/mobile_api/notifications/views.py create mode 100644 lms/djangoapps/verify_student/migrations/0016_remove_verificationattempt_created_and_more.py rename {openedx/core/djangoapps/learner_pathway/migrations => lms/djangoapps/verify_student/signals}/__init__.py (100%) rename lms/djangoapps/verify_student/{signals.py => signals/handlers.py} (90%) create mode 100644 lms/djangoapps/verify_student/signals/signals.py rename lms/djangoapps/verify_student/tests/{test_signals.py => test_handlers.py} (88%) create mode 100644 lms/templates/instructor/edx_ace/addbetatester/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/addbetatester/push/title.txt create mode 100644 lms/templates/instructor/edx_ace/allowedenroll/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/allowedenroll/push/title.txt create mode 100644 lms/templates/instructor/edx_ace/allowedunenroll/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/allowedunenroll/push/title.txt create mode 100644 lms/templates/instructor/edx_ace/enrolledunenroll/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/enrolledunenroll/push/title.txt create mode 100644 lms/templates/instructor/edx_ace/enrollenrolled/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/enrollenrolled/push/title.txt create mode 100644 lms/templates/instructor/edx_ace/removebetatester/push/body.txt create mode 100644 lms/templates/instructor/edx_ace/removebetatester/push/title.txt delete mode 120000 lms/templates/xblock_v2/xblock_iframe.html create mode 100644 openedx/core/djangoapps/ace_common/utils.py create mode 100644 openedx/core/djangoapps/content_libraries/tests/fields_test_block.py create mode 100644 openedx/core/djangoapps/content_libraries/tests/test_embed_block.py create mode 100644 openedx/core/djangoapps/content_staging/migrations/0005_stagedcontent_version_num.py delete mode 100644 openedx/core/djangoapps/learner_pathway/admin.py delete mode 100644 openedx/core/djangoapps/learner_pathway/apps.py delete mode 100644 openedx/core/djangoapps/learner_pathway/models.py delete mode 100644 openedx/core/djangoapps/learner_pathway/tests/__init__.py delete mode 100644 openedx/core/djangoapps/learner_pathway/views.py create mode 100644 openedx/core/djangoapps/notifications/grouping_notifications.py create mode 100644 openedx/core/djangoapps/notifications/policies.py create mode 100644 openedx/core/djangoapps/notifications/tests/test_notification_grouping.py delete mode 100644 requirements.txt delete mode 100644 xmodule/assets/library_content/public/js/library_content_edit_helpers.js diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3f9abcc671fb..fc452f7acde7 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -15,12 +15,15 @@ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_task/ lms/djangoapps/mobile_api/ +openedx/core/djangoapps/commerce/ @openedx/2u-infinity openedx/core/djangoapps/credentials @openedx/2U-aperture openedx/core/djangoapps/credit @openedx/2U-aperture +openedx/core/djangoapps/enrollments/ @openedx/2U-aperture openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/oauth_dispatch openedx/core/djangoapps/user_api/ @openedx/2U-aperture openedx/core/djangoapps/user_authn/ @openedx/2U-vanguards +openedx/core/djangoapps/verified_track_content/ @openedx/2u-infinity openedx/features/course_experience/ xmodule/ @@ -35,15 +38,18 @@ common/djangoapps/track/ lms/djangoapps/certificates/ @openedx/2U-aperture # Discovery -common/djangoapps/course_modes/ +common/djangoapps/course_modes/ @openedx/2U-aperture common/djangoapps/enrollment/ -lms/djangoapps/commerce/ -lms/djangoapps/experiments/ -lms/djangoapps/learner_dashboard/ @openedx/2U-aperture -lms/djangoapps/learner_home/ @openedx/2U-aperture -openedx/features/content_type_gating/ +common/djangoapps/entitlements/ @openedx/2U-aperture +lms/djangoapps/branding/ @openedx/2U-aperture +lms/djangoapps/commerce/ @openedx/2u-infinity +lms/djangoapps/experiments/ @openedx/2u-infinity +lms/djangoapps/gating/ @openedx/2u-infinity +lms/djangoapps/learner_dashboard/ @openedx/2U-aperture +lms/djangoapps/learner_home/ @openedx/2U-aperture +openedx/features/content_type_gating/ @openedx/2u-infinity openedx/features/course_duration_limits/ -openedx/features/discounts/ +openedx/features/discounts/ @openedx/2u-infinity # Ping Axim On-call if someone uses the QuickStart # https://docs.openedx.org/en/latest/developers/quickstarts/first_openedx_pr.html diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index 7e768a456463..458e00fc6b1f 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-20.04"] + os: ["ubuntu-22.04"] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/compile-python-requirements.yml b/.github/workflows/compile-python-requirements.yml index 0ff99b9c685a..21cb80083f1d 100644 --- a/.github/workflows/compile-python-requirements.yml +++ b/.github/workflows/compile-python-requirements.yml @@ -15,7 +15,7 @@ defaults: jobs: recompile-python-dependencies: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out target branch diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index 4d025e540163..c9d2d7ab1191 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -12,7 +12,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] + os: [ubuntu-latest] node-version: [18, 20] python-version: - "3.11" diff --git a/.github/workflows/lint-imports.yml b/.github/workflows/lint-imports.yml index 8ead8396bf39..e3c59ec09304 100644 --- a/.github/workflows/lint-imports.yml +++ b/.github/workflows/lint-imports.yml @@ -9,7 +9,7 @@ on: jobs: lint-imports: name: Lint Python Imports - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out branch diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 183b90effa29..624caddd5309 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-20.04] + os: [ubuntu-22.04] python-version: - "3.11" # 'pinned' is used to install the latest patch version of Django @@ -52,7 +52,7 @@ jobs: steps: - name: Setup mongodb user run: | - mongosh edxapp --eval ' + docker exec ${{ job.services.mongo.id }} mongosh edxapp --eval ' db.createUser( { user: "edxapp", @@ -67,7 +67,7 @@ jobs: - name: Verify mongo and mysql db credentials run: | mysql -h 127.0.0.1 -uedxapp001 -ppassword -e "select 1;" edxapp - mongosh --host 127.0.0.1 --username edxapp --password password --eval 'use edxapp; db.adminCommand("ping");' edxapp + docker exec ${{ job.services.mongo.id }} mongosh --host 127.0.0.1 --username edxapp --password password --eval 'use edxapp; db.adminCommand("ping");' edxapp - name: Checkout repo uses: actions/checkout@v4 @@ -126,7 +126,7 @@ jobs: if: always() needs: - check_migrations - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/publish-ci-docker-image.yml b/.github/workflows/publish-ci-docker-image.yml index 0a9f50f6daf9..6a0f3768b7e6 100644 --- a/.github/workflows/publish-ci-docker-image.yml +++ b/.github/workflows/publish-ci-docker-image.yml @@ -7,7 +7,7 @@ on: jobs: push: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Checkout diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index eeb53c24ed98..8860aced7f92 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -8,25 +8,25 @@ on: jobs: run-pylint: - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 strategy: fail-fast: false matrix: include: - module-name: lms-1 - path: "--django-settings-module=lms.envs.test lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/" + path: "lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/" - module-name: lms-2 - path: "--django-settings-module=lms.envs.test lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py" + path: "lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py" - module-name: openedx-1 - path: "--django-settings-module=lms.envs.test openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/" + path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/" - module-name: openedx-2 - path: "--django-settings-module=lms.envs.test openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/learner_pathway/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/" + path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/" - module-name: common - path: "--django-settings-module=lms.envs.test common pavelib" + path: "common pavelib" - module-name: cms - path: "--django-settings-module=cms.envs.test cms" + path: "cms" - module-name: xmodule - path: "--django-settings-module=lms.envs.test xmodule" + path: "xmodule" name: pylint ${{ matrix.module-name }} steps: @@ -75,7 +75,7 @@ jobs: if: always() needs: - run-pylint - runs-on: ubuntu-latest + runs-on: ubuntu-22.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 cf8ffd5d2910..84610123493c 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-20.04] + os: [ubuntu-22.04] python-version: - "3.11" node-version: [20] diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index 7f2b4925af8e..d880d7351766 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -17,7 +17,7 @@ jobs: runs-on: "${{ matrix.os }}" strategy: matrix: - os: ["ubuntu-20.04"] + os: ["ubuntu-latest"] python-version: - "3.11" diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 7bbfd3369b6b..4fe66e2a7778 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-20.04] + os: [ubuntu-22.04] python-version: - "3.11" node-version: [18, 20] @@ -72,9 +72,6 @@ jobs: run: | pip install -r requirements/edx/assets.txt - - name: Initiate Mongo DB Service - run: sudo systemctl start mongod - - name: Add node_modules bin to $Path run: echo $GITHUB_WORKSPACE/node_modules/.bin >> $GITHUB_PATH diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 3e442b75d4e7..854677b93cff 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-20.04 + runs-on: ubuntu-22.04 strategy: matrix: python-version: @@ -66,7 +66,29 @@ jobs: - name: install system requirements run: | - sudo apt-get update && sudo apt-get install libmysqlclient-dev libxmlsec1-dev lynx + sudo apt-get update && sudo apt-get install libmysqlclient-dev libxmlsec1-dev lynx openssl + + # This is needed until the ENABLE_BLAKE2B_HASHING can be removed and we + # can stop using MD4 by default. + - name: enable md4 hashing in libssl + run: | + cat < --unusable-password + ./manage.py lms create_dot_application studio-sso-id studio_worker \ + --grant-type authorization-code \ + --skip-authorization \ + --redirect-uris 'http://localhost:18010/complete/edx-oauth2/' \ + --scopes user_id + +* Log into Django admin (eg. http://localhost:18000/admin/oauth2_provider/application/), + click into the application you created above (``studio-sso-id``), and copy its "Client secret". +* In your private LMS_CFG yaml file or your private Django settings module: + + * Set ``SOCIAL_AUTH_EDX_OAUTH2_KEY`` to the client ID (``studio-sso-id``). + * Set ``SOCIAL_AUTH_EDX_OAUTH2_SECRET`` to the client secret (which you copied). Run the Platform ---------------- @@ -131,11 +160,11 @@ First, ensure MySQL, Mongo, and Memcached are running. Start the LMS:: - ./manage.py lms runserver + ./manage.py lms runserver 18000 Start the CMS:: - ./manage.py cms runserver + ./manage.py cms runserver 18010 This will give you a mostly-headless Open edX platform. Most frontends have been migrated to "Micro-Frontends (MFEs)" which need to be installed and run diff --git a/cms/djangoapps/contentstore/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index f84290ba83ae..ae0e6ea467b5 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -4,7 +4,7 @@ """ -from edx_toggles.toggles import WaffleFlag, WaffleSwitch +from edx_toggles.toggles import WaffleSwitch from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag @@ -26,20 +26,6 @@ f'{WAFFLE_NAMESPACE}.show_review_rules', __name__, LOG_PREFIX ) -# Waffle flag to redirect to the library authoring MFE. -# .. toggle_name: contentstore.library_authoring_mfe -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Toggles the new micro-frontend-based implementation of the library authoring experience. -# .. toggle_use_cases: temporary, open_edx -# .. toggle_creation_date: 2020-08-03 -# .. toggle_target_removal_date: 2020-12-31 -# .. toggle_warning: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and ENABLE_LIBRARY_AUTHORING_MICROFRONTEND. -# .. toggle_tickets: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4106944527/Libraries+Relaunch+Proposal+For+Product+Review -REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND = WaffleFlag( - f'{WAFFLE_NAMESPACE}.library_authoring_mfe', __name__, LOG_PREFIX -) - # .. toggle_name: studio.custom_relative_dates # .. toggle_implementation: CourseWaffleFlag diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index a4ece6c85d59..e9f599772d3e 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -9,6 +9,7 @@ from attrs import frozen, Factory from django.conf import settings +from django.contrib.auth import get_user_model from django.utils.translation import gettext as _ from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey from opaque_keys.edx.locator import DefinitionLocator, LocalId @@ -22,6 +23,7 @@ from xmodule.xml_block import XmlMixin from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream, BadDownstream, fetch_customizable_fields 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 @@ -30,6 +32,10 @@ log = logging.getLogger(__name__) + +User = get_user_model() + + # Note: Grader types are used throughout the platform but most usages are simply in-line # strings. In addition, new grader types can be defined on the fly anytime one is needed # (because they're just strings). This dict is an attempt to constrain the sprawl in Studio. @@ -282,9 +288,10 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> node, parent_xblock, store, - user_id=request.user.id, + user=request.user, slug_hint=user_clipboard.source_usage_key.block_id, copied_from_block=str(user_clipboard.source_usage_key), + copied_from_version_num=user_clipboard.content.version_num, tags=user_clipboard.content.tags, ) # Now handle static files that need to go into Files & Uploads: @@ -293,7 +300,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> staged_content_id=user_clipboard.content.id, static_files=static_files, ) - return new_xblock, notices @@ -302,12 +308,15 @@ def _import_xml_node_to_parent( parent_xblock: XBlock, # The modulestore we're using store, - # The ID of the user who is performing this operation - user_id: int, + # The user who is performing this operation + user: User, # Hint to use as usage ID (block_id) for the new XBlock slug_hint: str | None = None, # UsageKey of the XBlock that this one is a copy of copied_from_block: str | None = None, + # Positive int version of source block, if applicable (e.g., library block). + # Zero if not applicable (e.g., course block). + copied_from_version_num: int = 0, # Content tags applied to the source XBlock(s) tags: dict[str, str] | None = None, ) -> XBlock: @@ -373,12 +382,32 @@ def _import_xml_node_to_parent( raise NotImplementedError("We don't yet support pasting XBlocks with children") temp_xblock.parent = parent_key if copied_from_block: - # Store a reference to where this block was copied from, in the 'copied_from_block' field (AuthoringMixin) - temp_xblock.copied_from_block = copied_from_block + # Try to link the pasted block (downstream) to the copied block (upstream). + temp_xblock.upstream = copied_from_block + try: + UpstreamLink.get_for_block(temp_xblock) + except (BadDownstream, BadUpstream): + # Usually this will fail. For example, if the copied block is a modulestore course block, it can't be an + # upstream. That's fine! Instead, we store a reference to where this block was copied from, in the + # 'copied_from_block' field (from AuthoringMixin). + temp_xblock.upstream = None + temp_xblock.copied_from_block = copied_from_block + else: + # But if it doesn't fail, then populate the `upstream_version` field based on what was copied. Note that + # this could be the latest published version, or it could be an an even newer draft version. + temp_xblock.upstream_version = copied_from_version_num + # Also, fetch upstream values (`upstream_display_name`, etc.). + # Recall that the copied block could be a draft. So, rather than fetching from the published upstream (which + # could be older), fetch from the copied block itself. That way, if an author customizes a field, but then + # later wants to restore it, it will restore to the value that the field had when the block was pasted. Of + # course, if the author later syncs updates from a *future* published upstream version, then that will fetch + # new values from the published upstream content. + fetch_customizable_fields(upstream=temp_xblock, downstream=temp_xblock, user=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 = store.update_item(temp_xblock, user.id, allow_not_found=True) parent_xblock.children.append(new_xblock.location) - store.update_item(parent_xblock, user_id) + store.update_item(parent_xblock, user.id) children_handled = False if hasattr(new_xblock, 'studio_post_paste'): @@ -394,7 +423,7 @@ def _import_xml_node_to_parent( child_node, new_xblock, store, - user_id=user_id, + user=user, copied_from_block=str(child_copied_from), tags=tags, ) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index 0aa06d8b8dcc..4c3e2a4321d3 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -59,10 +59,8 @@ class CourseHomeSerializer(serializers.Serializer): libraries = LibraryViewSerializer(many=True, required=False, allow_null=True) libraries_enabled = serializers.BooleanField() taxonomies_enabled = serializers.BooleanField() - library_authoring_mfe_url = serializers.CharField() taxonomy_list_mfe_url = serializers.CharField() optimization_enabled = serializers.BooleanField() - redirect_to_library_authoring_mfe = serializers.BooleanField() request_course_creator_url = serializers.CharField() rerun_creator_status = serializers.BooleanField() show_new_library_button = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py index 1c9f9f608478..b1a02fff1f39 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py @@ -31,4 +31,3 @@ class CourseSettingsSerializer(serializers.Serializer): show_min_grade_warning = serializers.BooleanField() sidebar_html_enabled = serializers.BooleanField() upgrade_deadline = serializers.DateTimeField(allow_null=True) - use_v2_cert_display_settings = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py index d72707ed7836..f2e8b6ef431b 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -103,6 +103,18 @@ def get_assets_url(self, obj): return None +class UpstreamLinkSerializer(serializers.Serializer): + """ + Serializer holding info for syncing a block with its upstream (eg, a library block). + """ + upstream_ref = serializers.CharField() + version_synced = serializers.IntegerField() + version_available = serializers.IntegerField(allow_null=True) + version_declined = serializers.IntegerField(allow_null=True) + error_message = serializers.CharField(allow_null=True) + ready_to_sync = serializers.BooleanField() + + class ChildVerticalContainerSerializer(serializers.Serializer): """ Serializer for representing a xblock child of vertical container. @@ -113,6 +125,7 @@ class ChildVerticalContainerSerializer(serializers.Serializer): block_type = serializers.CharField() user_partition_info = serializers.DictField() user_partitions = serializers.ListField() + upstream_link = UpstreamLinkSerializer(allow_null=True) actions = serializers.SerializerMethodField() validation_messages = MessageValidation(many=True) render_error = serializers.CharField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index d72042cff611..ff476090ee7a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -59,9 +59,7 @@ def get(self, request: Request): "in_process_course_actions": [], "libraries": [], "libraries_enabled": true, - "library_authoring_mfe_url": "//localhost:3001/course/course-v1:edX+P315+2T2023", "optimization_enabled": true, - "redirect_to_library_authoring_mfe": false, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": true, "show_new_library_button": true, diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py index e921ac60398b..fbb05cba4de2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py @@ -95,7 +95,6 @@ def get(self, request: Request, course_id: str): "show_min_grade_warning": false, "sidebar_html_enabled": true, "upgrade_deadline": null, - "use_v2_cert_display_settings": false } ``` """ @@ -112,7 +111,6 @@ def get(self, request: Request, course_id: str): 'course_display_name_with_default': course_block.display_name_with_default, 'platform_name': settings.PLATFORM_NAME, 'licensing_enabled': settings.FEATURES.get("LICENSING", False), - 'use_v2_cert_display_settings': settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False), }) serializer = CourseSettingsSerializer(settings_context) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py index eafc2b37aa0c..189f2496a427 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py @@ -1,6 +1,7 @@ """ Unit tests for course index outline. """ +from django.conf import settings from django.test import RequestFactory from django.urls import reverse from rest_framework import status @@ -62,7 +63,7 @@ def test_course_index_response(self): "advance_settings_url": f"/settings/advanced/{self.course.id}" }, "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL, "is_custom_relative_dates_active": True, "initial_state": None, "initial_user_clipboard": { @@ -103,7 +104,7 @@ def test_course_index_response_with_show_locators(self): "advance_settings_url": f"/settings/advanced/{self.course.id}" }, "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL, "is_custom_relative_dates_active": False, "initial_state": { "expanded_locators": [ 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 a8b4cf5e3933..c3c9652e5d9e 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 @@ -52,10 +52,8 @@ def test_home_page_courses_response(self): "libraries": [], "libraries_enabled": True, "taxonomies_enabled": True, - "library_authoring_mfe_url": settings.LIBRARY_AUTHORING_MICROFRONTEND_URL, "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies', "optimization_enabled": False, - "redirect_to_library_authoring_mfe": False, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": True, "show_new_library_button": True, diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py index 5365443c47d9..15b0992fdf1a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py @@ -55,7 +55,6 @@ def test_course_settings_response(self): "show_min_grade_warning": False, "upgrade_deadline": None, "licensing_enabled": False, - "use_v2_cert_display_settings": False, } self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index d3fc37198213..7cac074a433f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -70,6 +70,8 @@ def setup_xblock(self): parent=self.vertical.location, category="html", display_name="Html Content 2", + upstream="lb:FakeOrg:FakeLib:html:FakeBlock", + upstream_version=5, ) def create_block(self, parent, category, display_name, **kwargs): @@ -193,6 +195,7 @@ def test_children_content(self): "name": self.html_unit_first.display_name_with_default, "block_id": str(self.html_unit_first.location), "block_type": self.html_unit_first.location.block_type, + "upstream_link": None, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "actions": { @@ -218,12 +221,21 @@ def test_children_content(self): "can_delete": True, "can_manage_tags": True, }, + "upstream_link": { + "upstream_ref": "lb:FakeOrg:FakeLib:html:FakeBlock", + "version_synced": 5, + "version_available": None, + "version_declined": None, + "error_message": "Linked library item was not found in the system", + "ready_to_sync": False, + }, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "validation_messages": [], "render_error": "", }, ] + self.maxDiff = None self.assertEqual(response.data["children"], expected_response) def test_not_valid_usage_key_string(self): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py index 670b94afbbe0..0798c341cc1c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py @@ -20,6 +20,7 @@ ContainerHandlerSerializer, VerticalContainerSerializer, ) +from cms.lib.xblock.upstream_sync import UpstreamLink from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -198,6 +199,7 @@ def get(self, request: Request, usage_key_string: str): "block_type": "drag-and-drop-v2", "user_partition_info": {}, "user_partitions": {} + "upstream_link": null, "actions": { "can_copy": true, "can_duplicate": true, @@ -215,6 +217,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "video", "user_partition_info": {}, "user_partitions": {} + "upstream_link": { + "upstream_ref": "lb:org:mylib:video:404", + "version_synced": 16 + "version_available": null, + "error_message": "Linked library item not found: lb:org:mylib:video:404", + "ready_to_sync": false, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -232,6 +241,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "html", "user_partition_info": {}, "user_partitions": {}, + "upstream_link": { + "upstream_ref": "lb:org:mylib:html:abcd", + "version_synced": 43, + "version_available": 49, + "error_message": null, + "ready_to_sync": true, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -267,6 +283,7 @@ def get(self, request: Request, usage_key_string: str): child_info = modulestore().get_item(child) user_partition_info = get_visibility_partition_info(child_info, course=course) user_partitions = get_user_partition_info(child_info, course=course) + upstream_link = UpstreamLink.try_get_for_block(child_info) validation_messages = get_xblock_validation_messages(child_info) render_error = get_xblock_render_error(request, child_info) @@ -277,6 +294,12 @@ def get(self, request: Request, usage_key_string: str): "block_type": child_info.location.block_type, "user_partition_info": user_partition_info, "user_partitions": user_partitions, + "upstream_link": ( + # If the block isn't linked to an upstream (which is by far the most common case) then just + # make this field null, which communicates the same info, but with less noise. + upstream_link.to_json() if upstream_link.upstream_ref + else None + ), "validation_messages": validation_messages, "render_error": render_error, }) diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index ad61cc937015..3e653d07fbcf 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -1,15 +1,31 @@ """Contenstore API v2 URLs.""" -from django.urls import path - -from cms.djangoapps.contentstore.rest_api.v2.views import HomePageCoursesViewV2 +from django.conf import settings +from django.urls import path, re_path +from cms.djangoapps.contentstore.rest_api.v2.views import home, downstreams app_name = "v2" urlpatterns = [ path( "home/courses", - HomePageCoursesViewV2.as_view(), + home.HomePageCoursesViewV2.as_view(), name="courses", ), + # TODO: Potential future path. + # re_path( + # fr'^downstreams/$', + # downstreams.DownstreamsListView.as_view(), + # name="downstreams_list", + # ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}$', + downstreams.DownstreamView.as_view(), + name="downstream" + ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}/sync$', + downstreams.SyncFromUpstreamView.as_view(), + name="sync_from_upstream" + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py index 73ddde98440c..e69de29bb2d1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py @@ -1,3 +0,0 @@ -"""Module for v2 views.""" - -from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesViewV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py new file mode 100644 index 000000000000..5079698082be --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -0,0 +1,251 @@ +""" +API Views for managing & syncing links between upstream & downstream content + +API paths (We will move these into proper api_doc_tools annotations soon +https://github.com/openedx/edx-platform/issues/35653): + + /api/contentstore/v2/downstreams/{usage_key_string} + + GET: Inspect a single downstream block's link to upstream content. + 200: Upstream link details successfully fetched. Returns UpstreamLink (may contain an error_message). + 404: Downstream block not found or user lacks permission to edit it. + + DELETE: Sever a single downstream block's link to upstream content. + 204: Block successfully unlinked (or it wasn't linked in the first place). No response body. + 404: Downstream block not found or user lacks permission to edit it. + + PUT: Establish or modify a single downstream block's link to upstream content. An authoring client could use this + endpoint to add library content in a two-step process, specifically: (1) add a blank block to a course, then + (2) link it to a content library with ?sync=True. + REQUEST BODY: { + "upstream_ref": str, // reference to upstream block (eg, library block usage key) + "sync": bool, // whether to sync in upstream content (False by default) + } + 200: Downstream block's upstream link successfully edited (and synced, if requested). Returns UpstreamLink. + 400: upstream_ref is malformed, missing, or inaccessible. + 400: Content at upstream_ref does not support syncing. + 404: Downstream block not found or user lacks permission to edit it. + + /api/contentstore/v2/downstreams/{usage_key_string}/sync + + POST: Sync a downstream block with upstream content. + 200: Downstream block successfully synced with upstream content. + 400: Downstream block is not linked to upstream content. + 400: Upstream is malformed, missing, or inaccessible. + 400: Upstream block does not support syncing. + 404: Downstream block not found or user lacks permission to edit it. + + DELETE: Decline an available sync for a downstream block. + 204: Sync successfuly dismissed. No response body. + 400: Downstream block is not linked to upstream content. + 404: Downstream block not found or user lacks permission to edit it. + + # NOT YET IMPLEMENTED -- Will be needed for full Libraries Relaunch in ~Teak. + /api/contentstore/v2/downstreams + /api/contentstore/v2/downstreams?course_id=course-v1:A+B+C&ready_to_sync=true + GET: List downstream blocks that can be synced, filterable by course or sync-readiness. + 200: A paginated list of applicable & accessible downstream blocks. Entries are UpstreamLinks. + +UpstreamLink response schema: + { + "upstream_ref": string? + "version_synced": string?, + "version_available": string?, + "version_declined": string?, + "error_message": string?, + "ready_to_sync": Boolean + } +""" +import logging + +from django.contrib.auth.models import User # pylint: disable=imported-auth-user +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import UsageKey +from rest_framework.exceptions import NotFound, ValidationError +from rest_framework.request import Request +from rest_framework.response import Response +from rest_framework.views import APIView +from xblock.core import XBlock + +from cms.lib.xblock.upstream_sync import ( + UpstreamLink, UpstreamLinkException, NoUpstream, BadUpstream, BadDownstream, + fetch_customizable_fields, sync_from_upstream, decline_sync, sever_upstream_link +) +from common.djangoapps.student.auth import has_studio_write_access, has_studio_read_access +from openedx.core.lib.api.view_utils import ( + DeveloperErrorViewMixin, + view_auth_classes, +) +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.exceptions import ItemNotFoundError + + +logger = logging.getLogger(__name__) + + +class _AuthenticatedRequest(Request): + """ + Alias for the `Request` class which tells mypy to assume that `.user` is not an AnonymousUser. + + Using this does NOT ensure the request is actually authenticated-- + you will some other way to ensure that, such as `@view_auth_classes(is_authenticated=True)`. + """ + user: User + + +# TODO: Potential future view. +# @view_auth_classes(is_authenticated=True) +# class DownstreamListView(DeveloperErrorViewMixin, APIView): +# """ +# List all blocks which are linked to upstream content, with optional filtering. +# """ +# def get(self, request: _AuthenticatedRequest) -> Response: +# """ +# Handle the request. +# """ +# course_key_string = request.GET['course_id'] +# syncable = request.GET['ready_to_sync'] +# ... + + +@view_auth_classes(is_authenticated=True) +class DownstreamView(DeveloperErrorViewMixin, APIView): + """ + Inspect or manage an XBlock's link to upstream content. + """ + def get(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Inspect an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=False) + return Response(UpstreamLink.try_get_for_block(downstream).to_json()) + + def put(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Edit an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + new_upstream_ref = request.data.get("upstream_ref") + + # Set `downstream.upstream` so that we can try to sync and/or fetch. + # Note that, if this fails and we raise a 4XX, then we will not call modulstore().update_item, + # thus preserving the former value of `downstream.upstream`. + downstream.upstream = new_upstream_ref + sync_param = request.data.get("sync", "false").lower() + if sync_param not in ["true", "false"]: + raise ValidationError({"sync": "must be 'true' or 'false'"}) + try: + if sync_param == "true": + sync_from_upstream(downstream=downstream, user=request.user) + else: + # Even if we're not syncing (i.e., updating the downstream's values with the upstream's), we still need + # to fetch the upstream's customizable values and store them as hidden fields on the downstream. This + # ensures that downstream authors can restore defaults based on the upstream. + fetch_customizable_fields(downstream=downstream, user=request.user) + except BadDownstream as exc: + logger.exception( + "'%s' is an invalid downstream; refusing to set its upstream to '%s'", + usage_key_string, + new_upstream_ref, + ) + raise ValidationError(str(exc)) from exc + except BadUpstream as exc: + logger.exception( + "'%s' is an invalid upstream reference; refusing to set it as upstream of '%s'", + new_upstream_ref, + usage_key_string, + ) + raise ValidationError({"upstream_ref": str(exc)}) from exc + except NoUpstream as exc: + raise ValidationError({"upstream_ref": "value missing"}) from exc + 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()) + + def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Sever an XBlock's link to upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + sever_upstream_link(downstream) + except NoUpstream as exc: + logger.exception( + "Tried to DELETE upstream link of '%s', but it wasn't linked to anything in the first place. " + "Will do nothing. ", + usage_key_string, + ) + else: + modulestore().update_item(downstream, request.user.id) + return Response(status=204) + + +@view_auth_classes(is_authenticated=True) +class SyncFromUpstreamView(DeveloperErrorViewMixin, APIView): + """ + Accept or decline an opportunity to sync a downstream block from its upstream content. + """ + + def post(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Pull latest updates to the block at {usage_key_string} from its linked upstream content. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + sync_from_upstream(downstream, request.user) + except UpstreamLinkException as exc: + logger.exception( + "Could not sync from upstream '%s' to downstream '%s'", + downstream.upstream, + usage_key_string, + ) + raise ValidationError(detail=str(exc)) from exc + 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()) + + def delete(self, request: _AuthenticatedRequest, usage_key_string: str) -> Response: + """ + Decline the latest updates to the block at {usage_key_string}. + """ + downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True) + try: + decline_sync(downstream) + except (NoUpstream, BadUpstream, BadDownstream) as exc: + # This is somewhat unexpected. If the upstream link is missing or invalid, then the downstream author + # shouldn't have been prompted to accept/decline a sync in the first place. Of course, they could have just + # hit the HTTP API anyway, or they could be viewing a Studio page which hasn't been refreshed in a while. + # So, it's a 400, not a 500. + logger.exception( + "Tried to decline a sync to downstream '%s', but the upstream link '%s' is invalid.", + usage_key_string, + downstream.upstream, + ) + raise ValidationError(str(exc)) from exc + modulestore().update_item(downstream, request.user.id) + return Response(status=204) + + +def _load_accessible_block(user: User, usage_key_string: str, *, require_write_access: bool) -> XBlock: + """ + Given a logged in-user and a serialized usage key of an upstream-linked XBlock, load it from the ModuleStore, + raising a DRF-friendly exception if anything goes wrong. + + Raises NotFound if usage key is malformed, if the user lacks access, or if the block doesn't exist. + """ + not_found = NotFound(detail=f"Block not found or not accessible: {usage_key_string}") + try: + usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError as exc: + raise ValidationError(detail=f"Malformed block usage key: {usage_key_string}") from exc + if require_write_access and not has_studio_write_access(user, usage_key.context_key): + raise not_found + if not has_studio_read_access(user, usage_key.context_key): + raise not_found + try: + block = modulestore().get_item(usage_key) + except ItemNotFoundError as exc: + raise not_found from exc + return block 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 new file mode 100644 index 000000000000..616035473e7e --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -0,0 +1,266 @@ +""" +Unit tests for /api/contentstore/v2/downstreams/* JSON APIs. +""" +from unittest.mock import patch + +from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream +from common.djangoapps.student.tests.factories import UserFactory +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + +from .. import downstreams as downstreams_views + + +MOCK_UPSTREAM_REF = "mock-upstream-ref" +MOCK_UPSTREAM_ERROR = "your LibraryGPT subscription has expired" + + +def _get_upstream_link_good_and_syncable(downstream): + return UpstreamLink( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=(downstream.upstream_version or 0) + 1, + version_declined=downstream.upstream_version_declined, + error_message=None, + ) + + +def _get_upstream_link_bad(_downstream): + raise BadUpstream(MOCK_UPSTREAM_ERROR) + + +class _DownstreamViewTestMixin: + """ + Shared data and error test cases. + """ + + def setUp(self): + """ + Create a simple course with one unit and two videos, one of which is linked to an "upstream". + """ + super().setUp() + self.course = CourseFactory.create() + chapter = BlockFactory.create(category='chapter', parent=self.course) + sequential = BlockFactory.create(category='sequential', parent=chapter) + unit = BlockFactory.create(category='vertical', parent=sequential) + self.regular_video_key = BlockFactory.create(category='video', parent=unit).usage_key + self.downstream_video_key = BlockFactory.create( + category='video', parent=unit, upstream=MOCK_UPSTREAM_REF, upstream_version=123, + ).usage_key + self.fake_video_key = self.course.id.make_usage_key("video", "NoSuchVideo") + self.superuser = UserFactory(username="superuser", password="password", is_staff=True, is_superuser=True) + self.learner = UserFactory(username="learner", password="password") + + def call_api(self, usage_key_string): + raise NotImplementedError + + def test_404_downstream_not_found(self): + """ + Do we raise 404 if the specified downstream block could not be loaded? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.fake_video_key) + assert response.status_code == 404 + assert "not found" in response.data["developer_message"] + + def test_404_downstream_not_accessible(self): + """ + Do we raise 404 if the user lacks read access on the specified downstream block? + """ + self.client.login(username="learner", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 404 + assert "not found" in response.data["developer_message"] + + +class GetDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `GET /api/v2/contentstore/downstreams/...` inspects a downstream's link to an upstream. + """ + def call_api(self, usage_key_string): + return self.client.get(f"/api/contentstore/v2/downstreams/{usage_key_string}") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_good_upstream(self): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF + assert response.data['error_message'] is None + assert response.data['ready_to_sync'] is True + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_bad) + def test_200_bad_upstream(self): + """ + If the upstream link is broken, do we still return 200, but with an error message in body? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] == MOCK_UPSTREAM_REF + assert response.data['error_message'] == MOCK_UPSTREAM_ERROR + assert response.data['ready_to_sync'] is False + + def test_200_no_upstream(self): + """ + If the upstream link is missing, do we still return 200, but with an error message in body? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 200 + assert response.data['upstream_ref'] is None + assert "is not linked" in response.data['error_message'] + assert response.data['ready_to_sync'] is False + + +class PutDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `PUT /api/v2/contentstore/downstreams/...` edits a downstream's link to an upstream. + """ + def call_api(self, usage_key_string, sync: str | None = None): + return self.client.put( + f"/api/contentstore/v2/downstreams/{usage_key_string}", + data={ + "upstream_ref": MOCK_UPSTREAM_REF, + **({"sync": sync} if sync else {}), + }, + content_type="application/json", + ) + + @patch.object(downstreams_views, "fetch_customizable_fields") + @patch.object(downstreams_views, "sync_from_upstream") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_with_sync(self, mock_sync, mock_fetch): + """ + Does the happy path work (with sync=True)? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key, sync='true') + assert response.status_code == 200 + video_after = modulestore().get_item(self.regular_video_key) + assert mock_sync.call_count == 1 + assert mock_fetch.call_count == 0 + assert video_after.upstream == MOCK_UPSTREAM_REF + + @patch.object(downstreams_views, "fetch_customizable_fields") + @patch.object(downstreams_views, "sync_from_upstream") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_no_sync(self, mock_sync, mock_fetch): + """ + Does the happy path work (with sync=False)? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key, sync='false') + assert response.status_code == 200 + video_after = modulestore().get_item(self.regular_video_key) + assert mock_sync.call_count == 0 + assert mock_fetch.call_count == 1 + assert video_after.upstream == MOCK_UPSTREAM_REF + + @patch.object(downstreams_views, "fetch_customizable_fields", side_effect=BadUpstream(MOCK_UPSTREAM_ERROR)) + def test_400(self, sync: str): + """ + Do we raise a 400 if the provided upstream reference is malformed or not accessible? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 400 + assert MOCK_UPSTREAM_ERROR in response.data['developer_message']['upstream_ref'] + video_after = modulestore().get_item(self.regular_video_key) + assert video_after.upstream is None + + +class DeleteDownstreamViewTest(_DownstreamViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `DELETE /api/v2/contentstore/downstreams/...` severs a downstream's link to an upstream. + """ + def call_api(self, usage_key_string): + return self.client.delete(f"/api/contentstore/v2/downstreams/{usage_key_string}") + + @patch.object(downstreams_views, "sever_upstream_link") + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_204(self, mock_sever): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 204 + assert mock_sever.call_count == 1 + + @patch.object(downstreams_views, "sever_upstream_link") + def test_204_no_upstream(self, mock_sever): + """ + If there's no upsream, do we still happily return 204? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 204 + assert mock_sever.call_count == 1 + + +class _DownstreamSyncViewTestMixin(_DownstreamViewTestMixin): + """ + Shared tests between the /api/contentstore/v2/downstreams/.../sync endpoints. + """ + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_bad) + def test_400_bad_upstream(self): + """ + If the upstream link is bad, do we get a 400? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 400 + assert MOCK_UPSTREAM_ERROR in response.data["developer_message"][0] + + def test_400_no_upstream(self): + """ + If the upstream link is missing, do we get a 400? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.regular_video_key) + assert response.status_code == 400 + assert "is not linked" in response.data["developer_message"][0] + + +class PostDownstreamSyncViewTest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `POST /api/v2/contentstore/downstreams/.../sync` initiates a sync from the linked upstream. + """ + def call_api(self, usage_key_string): + return self.client.post(f"/api/contentstore/v2/downstreams/{usage_key_string}/sync") + + @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): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 200 + assert mock_sync_from_upstream.call_count == 1 + + +class DeleteDownstreamSyncViewtest(_DownstreamSyncViewTestMixin, SharedModuleStoreTestCase): + """ + Test that `DELETE /api/v2/contentstore/downstreams/.../sync` declines a sync from the linked upstream. + """ + def call_api(self, usage_key_string): + return self.client.delete(f"/api/contentstore/v2/downstreams/{usage_key_string}/sync") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + @patch.object(downstreams_views, "decline_sync") + def test_204(self, mock_decline_sync): + """ + Does the happy path work? + """ + self.client.login(username="superuser", password="password") + response = self.call_api(self.downstream_video_key) + assert response.status_code == 204 + assert mock_decline_sync.call_count == 1 diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 214193918eb4..035e44d5ce31 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -98,7 +98,7 @@ ) from cms.djangoapps.models.settings.course_grading import CourseGradingModel from cms.djangoapps.models.settings.course_metadata import CourseMetadata -from xmodule.library_tools import LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -1265,7 +1265,7 @@ def load_services_for_studio(runtime, user): "settings": SettingsService(), "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) + "library_tools": LegacyLibraryToolsService(modulestore(), user.id) } runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access @@ -1671,9 +1671,7 @@ def get_home_context(request, no_course=False): ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) from cms.djangoapps.contentstore.views.library import ( - LIBRARY_AUTHORING_MICROFRONTEND_URL, LIBRARIES_ENABLED, - should_redirect_to_library_authoring_mfe, user_can_view_create_library_button, ) @@ -1699,12 +1697,9 @@ def get_home_context(request, no_course=False): 'in_process_course_actions': in_process_course_actions, 'libraries_enabled': LIBRARIES_ENABLED, 'taxonomies_enabled': not is_tagging_feature_disabled(), - 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), - 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_view_create_library_button(user) - and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), @@ -2202,7 +2197,7 @@ class StudioPermissionsService: Deprecated. To be replaced by a more general authorization service. - Only used by LibraryContentBlock (and library_tools.py). + Only used by LegacyLibraryContentBlock (and library_tools.py). """ def __init__(self, user): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 8c314caa6697..340cadb4e244 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -41,7 +41,6 @@ ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url from .component import CONTAINER_TEMPLATES, get_component_templates from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info @@ -52,21 +51,6 @@ log = logging.getLogger(__name__) LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) -ENABLE_LIBRARY_AUTHORING_MICROFRONTEND = settings.FEATURES.get('ENABLE_LIBRARY_AUTHORING_MICROFRONTEND', False) -LIBRARY_AUTHORING_MICROFRONTEND_URL = settings.LIBRARY_AUTHORING_MICROFRONTEND_URL - - -def should_redirect_to_library_authoring_mfe(): - """ - Boolean helper method, returns whether or not to redirect to the Library - Authoring MFE based on settings and flags. - """ - - return ( - ENABLE_LIBRARY_AUTHORING_MICROFRONTEND and - LIBRARY_AUTHORING_MICROFRONTEND_URL and - REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() - ) def _user_can_create_library_for_org(user, org=None): diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index fc119c7edd58..f3e20b45b2ea 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -982,7 +982,7 @@ def test_shallow_duplicate(self): def test_duplicate_library_content_block(self): # pylint: disable=too-many-statements """ - Test the LibraryContentBlock's special duplication process. + Test the LegacyLibraryContentBlock's special duplication process. """ store = modulestore() @@ -3674,14 +3674,15 @@ def test_special_exam_xblock_info( @patch_does_backend_support_onboarding @patch_get_exam_by_content_id_success @ddt.data( - ("lti_external", False), - ("other_proctoring_backend", True), + ("lti_external", False, None), + ("other_proctoring_backend", True, "test_url"), ) @ddt.unpack - def test_support_onboarding_is_correct_depending_on_lti_external( + def test_proctoring_values_correct_depending_on_lti_external( self, external_id, - expected_value, + expected_supports_onboarding_value, + expected_proctoring_link, mock_get_exam_by_content_id, mock_does_backend_support_onboarding, _mock_get_exam_configuration_dashboard_url, @@ -3691,8 +3692,9 @@ def test_support_onboarding_is_correct_depending_on_lti_external( category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_enabled=False, - is_time_limited=False, + is_proctored_enabled=True, + is_time_limited=True, + default_time_limit_minutes=100, is_onboarding_exam=False, ) @@ -3709,7 +3711,8 @@ def test_support_onboarding_is_correct_depending_on_lti_external( include_children_predicate=ALWAYS, course=self.course, ) - assert xblock_info["supports_onboarding"] is expected_value + assert xblock_info["supports_onboarding"] is expected_supports_onboarding_value + assert xblock_info["proctoring_exam_configuration_link"] == expected_proctoring_link @patch_get_exam_configuration_dashboard_url @patch_does_backend_support_onboarding @@ -3773,6 +3776,42 @@ def test_xblock_was_never_proctortrack_proctored_exam( assert xblock_info["was_exam_ever_linked_with_external"] is False assert mock_get_exam_by_content_id.call_count == 1 + @patch_get_exam_configuration_dashboard_url + @patch_does_backend_support_onboarding + @patch_get_exam_by_content_id_success + def test_special_exam_xblock_info_get_dashboard_error( + self, + mock_get_exam_by_content_id, + _mock_does_backend_support_onboarding, + mock_get_exam_configuration_dashboard_url, + ): + sequential = BlockFactory.create( + parent_location=self.chapter.location, + category="sequential", + display_name="Test Lesson 1", + user_id=self.user.id, + is_proctored_enabled=True, + is_time_limited=True, + default_time_limit_minutes=100, + is_onboarding_exam=False, + ) + sequential = modulestore().get_item(sequential.location) + mock_get_exam_configuration_dashboard_url.side_effect = Exception("proctoring error") + xblock_info = create_xblock_info( + sequential, + include_child_info=True, + include_children_predicate=ALWAYS, + ) + + # no errors should be raised and proctoring_exam_configuration_link is None + assert xblock_info["is_proctored_exam"] is True + assert xblock_info["was_exam_ever_linked_with_external"] is True + assert xblock_info["is_time_limited"] is True + assert xblock_info["default_time_limit_minutes"] == 100 + assert xblock_info["proctoring_exam_configuration_link"] is None + assert xblock_info["supports_onboarding"] is True + assert xblock_info["is_onboarding_exam"] is False + class TestLibraryXBlockInfo(ModuleStoreTestCase): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 76dba57f1d67..5706b44e2cec 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -10,7 +10,7 @@ from organizations.models import Organization from xmodule.modulestore.django import contentstore, modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course -from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url from openedx.core.djangoapps.content_libraries import api as library_api @@ -165,12 +165,12 @@ def _setup_tagged_content(self, course_key) -> dict: publish_item=True, ).location - library = ClipboardLibraryContentPasteTestCase.setup_library() + library = ClipboardPasteFromV1LibraryTestCase.setup_library() with self.store.bulk_operations(course_key): library_content_block_key = BlockFactory.create( parent=self.store.get_item(unit_key), category="library_content", - source_library_id=str(library.key), + source_library_id=str(library.context_key), display_name="LC Block", publish_item=True, ).location @@ -393,9 +393,9 @@ def test_paste_with_assets(self): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. -class ClipboardLibraryContentPasteTestCase(ModuleStoreTestCase): +class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): """ - Test Clipboard Paste functionality with library content + Test Clipboard Paste functionality with a "new" (as of Sumac) library """ def setUp(self): @@ -406,14 +406,86 @@ def setUp(self): self.client = APIClient() self.client.login(username=self.user.username, password=self.user_password) self.store = modulestore() - library = self.setup_library() + + self.library = library_api.create_library( + library_type=library_api.COMPLEX, + org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), + slug="lib", + title="Library", + ) + + self.lib_block_key = library_api.create_library_block(self.library.key, "problem", "p1").usage_key # v==1 + library_api.set_library_block_olx(self.lib_block_key, """ + + + + + Wrong + Right + + + + """) # v==2 + library_api.publish_changes(self.library.key) + library_api.set_library_block_olx(self.lib_block_key, """ + + + + + Wrong + Right + + + + """) # v==3 + lib_block_meta = library_api.get_library_block(self.lib_block_key) + assert lib_block_meta.published_version_num == 2 + assert lib_block_meta.draft_version_num == 3 + + self.course = CourseFactory.create(display_name='Course') + + def test_paste_from_library_creates_link(self): + """ + When we copy a v2 lib block into a course, the dest block should be linked up to the lib block. + """ + copy_response = self.client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(self.lib_block_key)}, format="json") + assert copy_response.status_code == 200 + + paste_response = self.client.post(XBLOCK_ENDPOINT, { + "parent_locator": str(self.course.usage_key), + "staged_content": "clipboard", + }, format="json") + assert paste_response.status_code == 200 + + new_block_key = UsageKey.from_string(paste_response.json()["locator"]) + new_block = modulestore().get_item(new_block_key) + assert new_block.upstream == str(self.lib_block_key) + assert new_block.upstream_version == 3 + assert new_block.upstream_display_name == "MCQ-draft" + assert new_block.upstream_max_attempts == 5 + + +class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): + """ + Test Clipboard Paste functionality with legacy (v1) library content + """ + + def setUp(self): + """ + Set up a v1 Content Library and a library content block + """ + super().setUp() + self.client = APIClient() + self.client.login(username=self.user.username, password=self.user_password) + self.store = modulestore() + self.library = self.setup_library() # Create a library content block (lc), point it out our library, and sync it. self.course = CourseFactory.create(display_name='Course') self.orig_lc_block = BlockFactory.create( parent=self.course, category="library_content", - source_library_id=str(library.key), + source_library_id=str(self.library.context_key), display_name="LC Block", publish_item=False, ) @@ -426,18 +498,15 @@ def setUp(self): @classmethod def setup_library(cls): """ - Creates and returns a content library. + Creates and returns a legacy content library with 1 problem """ - library = library_api.create_library( - library_type=library_api.COMPLEX, - org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), - slug="lib", - title="Library", - ) - # Populate it with a problem: - problem_key = library_api.create_library_block(library.key, "problem", "p1").usage_key - library_api.set_library_block_olx(problem_key, """ - + library = LibraryFactory.create(display_name='Library') + lib_block = BlockFactory.create( + parent_location=library.usage_key, + category="problem", + display_name="MCQ", + max_attempts=1, + data=""" @@ -445,9 +514,9 @@ def setup_library(cls): Right - - """) - library_api.publish_changes(library.key) + """, + publish_item=False, + ) return library def test_paste_library_content_block(self): diff --git a/cms/djangoapps/contentstore/views/transcripts_ajax.py b/cms/djangoapps/contentstore/views/transcripts_ajax.py index 892b76caae72..8cb7f455013b 100644 --- a/cms/djangoapps/contentstore/views/transcripts_ajax.py +++ b/cms/djangoapps/contentstore/views/transcripts_ajax.py @@ -649,6 +649,9 @@ def _get_item(request, data): Returns the item. """ usage_key = UsageKey.from_string(data.get('locator')) + if not usage_key.context_key.is_course: + # TODO: implement transcript support for learning core / content libraries. + raise TranscriptsRequestValidationException(_('Transcripts are not yet supported in content libraries.')) # This is placed before has_course_author_access() to validate the location, # because has_course_author_access() raises r if location is invalid. item = modulestore().get_item(usage_key) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index e7dbec01f8e0..e4d37f942331 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -539,7 +539,8 @@ def _create_block(request): # Paste from the user's clipboard (content_staging app clipboard, not browser clipboard) into 'usage_key': try: created_xblock, notices = import_staged_content_from_user_clipboard( - parent_key=usage_key, request=request + parent_key=usage_key, + request=request, ) except Exception: # pylint: disable=broad-except log.exception( @@ -1159,12 +1160,19 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements supports_onboarding = False proctoring_exam_configuration_link = None - if xblock.is_proctored_exam: - proctoring_exam_configuration_link = ( - get_exam_configuration_dashboard_url( - course.id, xblock_info["id"] + + # only call get_exam_configuration_dashboard_url if not using an LTI proctoring provider + if xblock.is_proctored_exam and (course.proctoring_provider != 'lti_external'): + try: + proctoring_exam_configuration_link = ( + get_exam_configuration_dashboard_url( + course.id, xblock_info["id"] + ) + ) + except Exception as e: # pylint: disable=broad-except + log.error( + f"Error while getting proctoring exam configuration link: {e}" ) - ) if course.proctoring_provider == "proctortrack": show_review_rules = SHOW_REVIEW_RULES_FLAG.is_enabled( diff --git a/cms/envs/common.py b/cms/envs/common.py index 34dd8503f35e..7297adae8354 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -127,6 +127,7 @@ from lms.djangoapps.lms_xblock.mixin import LmsBlockMixin from cms.lib.xblock.authoring_mixin import AuthoringMixin +from cms.lib.xblock.upstream_sync import UpstreamSyncMixin from xmodule.modulestore.edit_info import EditInfoMixin from openedx.core.djangoapps.theming.helpers_dirs import ( get_themes_unchecked, @@ -434,18 +435,6 @@ # .. toggle_tickets: https://openedx.atlassian.net/browse/DEPR-58 'DEPRECATE_OLD_COURSE_KEYS_IN_STUDIO': True, - # .. toggle_name: FEATURES['ENABLE_LIBRARY_AUTHORING_MICROFRONTEND'] - # .. toggle_implementation: DjangoSetting - # .. toggle_default: False - # .. toggle_description: Set to True to enable the Library Authoring MFE - # .. toggle_use_cases: temporary - # .. toggle_creation_date: 2020-06-20 - # .. toggle_target_removal_date: 2020-12-31 - # .. toggle_tickets: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4106944527/Libraries+Relaunch+Proposal+For+Product+Review - # .. toggle_warning: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and see - # REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND for rollout. - 'ENABLE_LIBRARY_AUTHORING_MICROFRONTEND': False, - # .. toggle_name: FEATURES['DISABLE_COURSE_CREATION'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -468,17 +457,6 @@ # .. toggle_tickets: https://github.com/openedx/edx-platform/pull/26106 'ENABLE_HELP_LINK': True, - # .. toggle_name: FEATURES['ENABLE_V2_CERT_DISPLAY_SETTINGS'] - # .. toggle_implementation: DjangoSetting - # .. toggle_default: False - # .. toggle_description: Whether to use the reimagined certificates_display_behavior and certificate_available_date - # .. settings. Will eventually become the default. - # .. toggle_use_cases: temporary - # .. toggle_creation_date: 2021-07-26 - # .. toggle_target_removal_date: 2021-10-01 - # .. toggle_tickets: 'https://openedx.atlassian.net/browse/MICROBA-1405' - 'ENABLE_V2_CERT_DISPLAY_SETTINGS': False, - # .. toggle_name: FEATURES['ENABLE_INTEGRITY_SIGNATURE'] # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -612,7 +590,6 @@ COURSE_AUTHORING_MICROFRONTEND_URL = None DISCUSSIONS_MICROFRONTEND_URL = None DISCUSSIONS_MFE_FEEDBACK_URL = None -LIBRARY_AUTHORING_MICROFRONTEND_URL = None # .. toggle_name: ENABLE_AUTHN_RESET_PASSWORD_HIBP_POLICY # .. toggle_implementation: DjangoSetting # .. toggle_default: False @@ -1019,6 +996,7 @@ XModuleMixin, EditInfoMixin, AuthoringMixin, + UpstreamSyncMixin, ) # .. setting_name: XBLOCK_EXTRA_MIXINS @@ -2790,6 +2768,7 @@ CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#adding-custom-pages" COURSE_LIVE_HELP_URL = "https://edx.readthedocs.io/projects/edx-partner-course-staff/en/latest/course_assets/course_live.html" ORA_SETTINGS_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_assets/pages.html#configuring-course-level-open-response-assessment-settings" +# pylint: enable=line-too-long # keys for big blue button live provider COURSE_LIVE_GLOBAL_CREDENTIALS = {} @@ -2813,8 +2792,15 @@ BRAZE_COURSE_ENROLLMENT_CANVAS_ID = '' +######################## Discussion Forum settings ######################## + +# Feedback link in upgraded discussion notification alert DISCUSSIONS_INCONTEXT_FEEDBACK_URL = '' -DISCUSSIONS_INCONTEXT_LEARNMORE_URL = '' + +# Learn More link in upgraded discussion notification alert +# pylint: disable=line-too-long +DISCUSSIONS_INCONTEXT_LEARNMORE_URL = "https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/manage_discussions/discussions.html" +# pylint: enable=line-too-long #### django-simple-history## # disable indexing on date field its coming django-simple-history. @@ -2936,3 +2922,10 @@ def _should_send_learning_badge_events(settings): # See https://www.meilisearch.com/docs/learn/security/tenant_tokens MEILISEARCH_INDEX_PREFIX = "" MEILISEARCH_API_KEY = "devkey" + +# .. setting_name: DISABLED_COUNTRIES +# .. setting_default: [] +# .. setting_description: List of country codes that should be disabled +# .. for now it wil impact country listing in auth flow and user profile. +# .. eg ['US', 'CA'] +DISABLED_COUNTRIES = [] diff --git a/cms/envs/devstack.py b/cms/envs/devstack.py index e944d67eda1b..1200a61b0617 100644 --- a/cms/envs/devstack.py +++ b/cms/envs/devstack.py @@ -174,9 +174,6 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing ################### FRONTEND APPLICATION PUBLISHER URL ################### FEATURES['FRONTEND_APP_PUBLISHER_URL'] = 'http://localhost:18400' -################### FRONTEND APPLICATION LIBRARY AUTHORING ################### -LIBRARY_AUTHORING_MICROFRONTEND_URL = 'http://localhost:3001' - ################### FRONTEND APPLICATION COURSE AUTHORING ################### COURSE_AUTHORING_MICROFRONTEND_URL = 'http://localhost:2001' @@ -267,7 +264,8 @@ def should_show_debug_toolbar(request): # lint-amnesty, pylint: disable=missing ################ Using LMS SSO for login to Studio ################ SOCIAL_AUTH_EDX_OAUTH2_KEY = 'studio-sso-key' SOCIAL_AUTH_EDX_OAUTH2_SECRET = 'studio-sso-secret' # in stage, prod would be high-entropy secret -SOCIAL_AUTH_EDX_OAUTH2_URL_ROOT = 'http://edx.devstack.lms:18000' # routed internally server-to-server +# routed internally server-to-server +SOCIAL_AUTH_EDX_OAUTH2_URL_ROOT = ENV_TOKENS.get('SOCIAL_AUTH_EDX_OAUTH2_URL_ROOT', 'http://edx.devstack.lms:18000') SOCIAL_AUTH_EDX_OAUTH2_PUBLIC_URL_ROOT = 'http://localhost:18000' # used in browser redirect # Don't form the return redirect URL with HTTPS on devstack diff --git a/cms/envs/production.py b/cms/envs/production.py index 50519b55229b..ad7667772f9a 100644 --- a/cms/envs/production.py +++ b/cms/envs/production.py @@ -689,3 +689,10 @@ def get_env_setting(setting): } BEAMER_PRODUCT_ID = ENV_TOKENS.get('BEAMER_PRODUCT_ID', BEAMER_PRODUCT_ID) + +# .. setting_name: DISABLED_COUNTRIES +# .. setting_default: [] +# .. setting_description: List of country codes that should be disabled +# .. for now it wil impact country listing in auth flow and user profile. +# .. eg ['US', 'CA'] +DISABLED_COUNTRIES = ENV_TOKENS.get('DISABLED_COUNTRIES', []) diff --git a/cms/envs/test.py b/cms/envs/test.py index 38b7c7817149..49db50608858 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -333,3 +333,13 @@ "SECRET": "***", "URL": "***", } + +############## openedx-learning (Learning Core) config ############## +OPENEDX_LEARNING = { + 'MEDIA': { + 'BACKEND': 'django.core.files.storage.InMemoryStorage', + 'OPTIONS': { + 'location': MEDIA_ROOT + "_private" + } + } +} diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py new file mode 100644 index 000000000000..5db020393eab --- /dev/null +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -0,0 +1,337 @@ +""" +Test CMS's upstream->downstream syncing system +""" +import ddt + +from organizations.api import ensure_organization +from organizations.models import Organization + +from cms.lib.xblock.upstream_sync import ( + UpstreamLink, + sync_from_upstream, decline_sync, fetch_customizable_fields, sever_upstream_link, + NoUpstream, BadUpstream, BadDownstream, +) +from common.djangoapps.student.tests.factories import UserFactory +from openedx.core.djangoapps.content_libraries import api as libs +from openedx.core.djangoapps.xblock import api as xblock +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory + + +@ddt.ddt +class UpstreamTestCase(ModuleStoreTestCase): + """ + Tests the upstream_sync mixin, data object, and Python APIs. + """ + + def setUp(self): + """ + Create a simple course with one unit, and simple V2 library with two blocks. + """ + super().setUp() + course = CourseFactory.create() + chapter = BlockFactory.create(category='chapter', parent=course) + sequential = BlockFactory.create(category='sequential', parent=chapter) + self.unit = BlockFactory.create(category='vertical', parent=sequential) + + ensure_organization("TestX") + self.library = libs.create_library( + org=Organization.objects.get(short_name="TestX"), + slug="TestLib", + title="Test Upstream Library", + ) + self.upstream_key = libs.create_library_block(self.library.key, "html", "test-upstream").usage_key + libs.create_library_block(self.library.key, "video", "video-upstream") + + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V2" + upstream.data = "Upstream content V2" + upstream.save() + + def test_sync_bad_downstream(self): + """ + Syncing into an unsupported downstream (such as a another Content Library block) raises BadDownstream, but + doesn't affect the block. + """ + downstream_lib_block_key = libs.create_library_block(self.library.key, "html", "bad-downstream").usage_key + downstream_lib_block = xblock.load_block(downstream_lib_block_key, self.user) + downstream_lib_block.display_name = "Another lib block" + downstream_lib_block.data = "another lib block" + downstream_lib_block.upstream = str(self.upstream_key) + downstream_lib_block.save() + + with self.assertRaises(BadDownstream): + sync_from_upstream(downstream_lib_block, self.user) + + assert downstream_lib_block.display_name == "Another lib block" + assert downstream_lib_block.data == "another lib block" + + def test_sync_no_upstream(self): + """ + Trivial case: Syncing a block with no upstream is a no-op + """ + block = BlockFactory.create(category='html', parent=self.unit) + block.display_name = "Block Title" + block.data = "Block content" + + with self.assertRaises(NoUpstream): + sync_from_upstream(block, self.user) + + assert block.display_name == "Block Title" + assert block.data == "Block content" + assert not block.upstream_display_name + + @ddt.data( + ("not-a-key-at-all", ".*is malformed.*"), + ("course-v1:Oops+ItsA+CourseKey", ".*is malformed.*"), + ("block-v1:The+Wrong+KindOfUsageKey+type@html+block@nope", ".*is malformed.*"), + ("lb:TestX:NoSuchLib:html:block-id", ".*not found in the system.*"), + ("lb:TestX:TestLib:video:should-be-html-but-is-a-video", ".*type mismatch.*"), + ("lb:TestX:TestLib:html:no-such-html", ".*not found in the system.*"), + ) + @ddt.unpack + def test_sync_bad_upstream(self, upstream, message_regex): + """ + Syncing with a bad upstream raises BadUpstream, but doesn't affect the block + """ + block = BlockFactory.create(category='html', parent=self.unit, upstream=upstream) + block.display_name = "Block Title" + block.data = "Block content" + + with self.assertRaisesRegex(BadUpstream, message_regex): + sync_from_upstream(block, self.user) + + assert block.display_name == "Block Title" + assert block.data == "Block content" + assert not block.upstream_display_name + + def test_sync_not_accessible(self): + """ + Syncing with an block that exists, but is inaccessible, raises BadUpstream + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + user_who_cannot_read_upstream = UserFactory.create(username="rando", is_staff=False, is_superuser=False) + with self.assertRaisesRegex(BadUpstream, ".*could not be loaded.*") as exc: + sync_from_upstream(downstream, user_who_cannot_read_upstream) + + def test_sync_updates_happy_path(self): + """ + Can we sync updates from a content library block to a linked out-of-date course block? + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + + # Initial sync + sync_from_upstream(downstream, self.user) + assert downstream.upstream_version == 2 # Library blocks start at version 2 (v1 is the empty new block) + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # Upstream updates + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V3" + upstream.data = "Upstream content V3" + upstream.save() + + # Follow-up sync. Assert that updates are pulled into downstream. + sync_from_upstream(downstream, self.user) + assert downstream.upstream_version == 3 + assert downstream.upstream_display_name == "Upstream Title V3" + assert downstream.display_name == "Upstream Title V3" + assert downstream.data == "Upstream content V3" + + def test_sync_updates_to_modified_content(self): + """ + If we sync to modified content, will it preserve customizable fields, but overwrite the rest? + """ + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + + # Initial sync + sync_from_upstream(downstream, self.user) + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # Upstream updates + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.display_name = "Upstream Title V3" + upstream.data = "Upstream content V3" + upstream.save() + + # Downstream modifications + downstream.display_name = "Downstream Title Override" # "safe" customization + downstream.data = "Downstream content override" # "unsafe" override + downstream.save() + + # Follow-up sync. Assert that updates are pulled into downstream, but customizations are saved. + sync_from_upstream(downstream, self.user) + assert downstream.upstream_display_name == "Upstream Title V3" + assert downstream.display_name == "Downstream Title Override" # "safe" customization survives + assert downstream.data == "Upstream content V3" # "unsafe" override is gone + + # For the Content Libraries Relaunch Beta, we do not yet need to support this edge case. + # See "PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM DEFAULTS" in cms/lib/xblock/upstream_sync.py. + # + # def test_sync_to_downstream_with_subtle_customization(self): + # """ + # Edge case: If our downstream customizes a field, but then the upstream is changed to match the + # customization do we still remember that the downstream field is customized? That is, + # if the upstream later changes again, do we retain the downstream customization (rather than + # following the upstream update?) + # """ + # # Start with an uncustomized downstream block. + # downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + # sync_from_upstream(downstream, self.user) + # assert downstream.downstream_customized == [] + # assert downstream.display_name == downstream.upstream_display_name == "Upstream Title V2" + # + # # Then, customize our downstream title. + # downstream.display_name = "Title V3" + # downstream.save() + # assert downstream.downstream_customized == ["display_name"] + # + # # Syncing should retain the customization. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 2 + # assert downstream.upstream_display_name == "Upstream Title V2" + # assert downstream.display_name == "Title V3" + # + # # Whoa, look at that, the upstream has updated itself to the exact same title... + # upstream = xblock.load_block(self.upstream_key, self.user) + # upstream.display_name = "Title V3" + # upstream.save() + # + # # ...which is reflected when we sync. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 3 + # assert downstream.upstream_display_name == downstream.display_name == "Title V3" + # + # # But! Our downstream knows that its title is still customized. + # assert downstream.downstream_customized == ["display_name"] + # # So, if the upstream title changes again... + # upstream.display_name = "Title V4" + # upstream.save() + # + # # ...then the downstream title should remain put. + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 4 + # assert downstream.upstream_display_name == "Title V4" + # assert downstream.display_name == "Title V3" + # + # # Finally, if we "de-customize" the display_name field, then it should go back to syncing normally. + # downstream.downstream_customized = [] + # upstream.display_name = "Title V5" + # upstream.save() + # sync_from_upstream(downstream, self.user) + # assert downstream.upstream_version == 5 + # assert downstream.upstream_display_name == downstream.display_name == "Title V5" + + @ddt.data(None, "Title From Some Other Upstream Version") + def test_fetch_customizable_fields(self, initial_upstream_display_name): + """ + Can we fetch a block's upstream field values without syncing it? + + Test both with and without a pre-"fetched" upstrema values on the downstream. + """ + downstream = BlockFactory.create(category='html', parent=self.unit) + downstream.upstream_display_name = initial_upstream_display_name + downstream.display_name = "Some Title" + downstream.data = "Some content" + + # Note that we're not linked to any upstream. fetch_customizable_fields shouldn't care. + assert not downstream.upstream + assert not downstream.upstream_version + + # fetch! + upstream = xblock.load_block(self.upstream_key, self.user) + fetch_customizable_fields(upstream=upstream, downstream=downstream, user=self.user) + + # Ensure: fetching doesn't affect the upstream link (or lack thereof). + assert not downstream.upstream + assert not downstream.upstream_version + + # Ensure: fetching doesn't affect actual content or settings. + assert downstream.display_name == "Some Title" + assert downstream.data == "Some content" + + # Ensure: fetching DOES set the upstream_* fields. + assert downstream.upstream_display_name == "Upstream Title V2" + + def test_prompt_and_decline_sync(self): + """ + Is the user prompted for sync when it's available? Does declining remove the prompt until a new sync is ready? + """ + # Initial conditions (pre-sync) + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced is None + assert link.version_declined is None + assert link.version_available == 2 # Library block with content starts at version 2 + assert link.ready_to_sync is True + + # Initial sync to V2 + sync_from_upstream(downstream, self.user) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined is None + assert link.version_available == 2 + assert link.ready_to_sync is False + + # Upstream updated to V3 + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.data = "Upstream content V3" + upstream.save() + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined is None + assert link.version_available == 3 + assert link.ready_to_sync is True + + # Decline to sync to V3 -- ready_to_sync becomes False. + decline_sync(downstream) + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined == 3 + assert link.version_available == 3 + assert link.ready_to_sync is False + + # Upstream updated to V4 -- ready_to_sync becomes True again. + upstream = xblock.load_block(self.upstream_key, self.user) + upstream.data = "Upstream content V4" + upstream.save() + link = UpstreamLink.get_for_block(downstream) + assert link.version_synced == 2 + assert link.version_declined == 3 + assert link.version_available == 4 + assert link.ready_to_sync is True + + def test_sever_upstream_link(self): + """ + Does sever_upstream_link correctly disconnect a block from its upstream? + """ + # Start with a course block that is linked+synced to a content library block. + downstream = BlockFactory.create(category='html', parent=self.unit, upstream=str(self.upstream_key)) + sync_from_upstream(downstream, self.user) + + # (sanity checks) + assert downstream.upstream == str(self.upstream_key) + assert downstream.upstream_version == 2 + assert downstream.upstream_display_name == "Upstream Title V2" + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + assert downstream.copied_from_block is None + + # Now, disconnect the course block. + sever_upstream_link(downstream) + + # All upstream metadata has been wiped out. + assert downstream.upstream is None + assert downstream.upstream_version is None + assert downstream.upstream_display_name is None + + # BUT, the content which was synced into the upstream remains. + assert downstream.display_name == "Upstream Title V2" + assert downstream.data == "Upstream content V2" + + # AND, we have recorded the old upstream as our copied_from_block. + assert downstream.copied_from_block == str(self.upstream_key) diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py new file mode 100644 index 000000000000..785cc7dc7e36 --- /dev/null +++ b/cms/lib/xblock/upstream_sync.py @@ -0,0 +1,451 @@ +""" +Synchronize content and settings from upstream blocks to their downstream usages. + +At the time of writing, we assume that for any upstream-downstream linkage: +* The upstream is a Component from a Learning Core-backed Content Library. +* The downstream is a block of matching type in a SplitModuleStore-backed Course. +* They are both on the same Open edX instance. + +HOWEVER, those assumptions may loosen in the future. So, we consider these to be INTERNAL ASSUMPIONS that should not be +exposed through this module's public Python interface. +""" +from __future__ import annotations + +import logging +import typing as t +from dataclasses import dataclass, asdict + +from django.core.exceptions import PermissionDenied +from django.utils.translation import gettext_lazy as _ +from rest_framework.exceptions import NotFound +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryUsageLocatorV2 +from xblock.exceptions import XBlockNotFoundError +from xblock.fields import Scope, String, Integer +from xblock.core import XBlockMixin, XBlock + +if t.TYPE_CHECKING: + from django.contrib.auth.models import User # pylint: disable=imported-auth-user + + +logger = logging.getLogger(__name__) + + +class UpstreamLinkException(Exception): + """ + Raised whenever we try to inspect, sync-from, fetch-from, or delete a block's link to upstream content. + + There are three flavors (defined below): BadDownstream, BadUpstream, NoUpstream. + + Should be constructed with a human-friendly, localized, PII-free message, suitable for API responses and UI display. + For now, at least, the message can assume that upstreams are Content Library blocks and downstreams are Course + blocks, although that may need to change (see module docstring). + """ + + +class BadDownstream(UpstreamLinkException): + """ + Downstream content does not support sync. + """ + + +class BadUpstream(UpstreamLinkException): + """ + Reference to upstream content is malformed, invalid, and/or inaccessible. + """ + + +class NoUpstream(UpstreamLinkException): + """ + The downstream content does not have an upstream link at all (...as is the case for most XBlocks usages). + + (This isn't so much an "error" like the other two-- it's just a case that needs to be handled exceptionally, + usually by logging a message and then doing nothing.) + """ + def __init__(self): + super().__init__(_("Content is not linked to a Content Library.")) + + +@dataclass(frozen=True) +class UpstreamLink: + """ + Metadata about some downstream content's relationship with its linked upstream content. + """ + upstream_ref: str | None # Reference to the upstream content, e.g., a serialized library block usage key. + version_synced: int | None # Version of the upstream to which the downstream was last synced. + version_available: int | None # Latest version of the upstream that's available, or None if it couldn't be loaded. + version_declined: int | None # Latest version which the user has declined to sync with, if any. + error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message. + + @property + def ready_to_sync(self) -> bool: + """ + Should we invite the downstream's authors to sync the latest upstream updates? + """ + return bool( + self.upstream_ref and + self.version_available and + self.version_available > (self.version_synced or 0) and + self.version_available > (self.version_declined or 0) + ) + + def to_json(self) -> dict[str, t.Any]: + """ + Get an JSON-API-friendly representation of this upstream link. + """ + return { + **asdict(self), + "ready_to_sync": self.ready_to_sync, + } + + @classmethod + def try_get_for_block(cls, downstream: XBlock) -> t.Self: + """ + Same as `get_for_block`, but upon failure, sets `.error_message` instead of raising an exception. + """ + try: + return cls.get_for_block(downstream) + except UpstreamLinkException as exc: + logger.exception( + "Tried to inspect an unsupported, broken, or missing downstream->upstream link: '%s'->'%s'", + downstream.usage_key, + downstream.upstream, + ) + return cls( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=None, + version_declined=None, + error_message=str(exc), + ) + + @classmethod + def get_for_block(cls, downstream: XBlock) -> t.Self: + """ + Get info on a block's relationship with its linked upstream content (without actually loading the content). + + Currently, the only supported upstreams are LC-backed Library Components. This may change in the future (see + module docstring). + + If link exists, is supported, and is followable, returns UpstreamLink. + Otherwise, raises an UpstreamLinkException. + """ + if not downstream.upstream: + raise NoUpstream() + if not isinstance(downstream.usage_key.context_key, CourseKey): + raise BadDownstream(_("Cannot update content because it does not belong to a course.")) + if downstream.has_children: + raise BadDownstream(_("Updating content with children is not yet supported.")) + try: + upstream_key = LibraryUsageLocatorV2.from_string(downstream.upstream) + except InvalidKeyError as exc: + raise BadUpstream(_("Reference to linked library item is malformed")) from exc + downstream_type = downstream.usage_key.block_type + if upstream_key.block_type != downstream_type: + # Note: Currently, we strictly enforce that the downstream and upstream block_types must exactly match. + # It could be reasonable to relax this requirement in the future if there's product need for it. + # For example, there's no reason that a StaticTabBlock couldn't take updates from an HtmlBlock. + raise BadUpstream( + _("Content type mismatch: {downstream_type} cannot be linked to {upstream_type}.").format( + downstream_type=downstream_type, upstream_type=upstream_key.block_type + ) + ) from TypeError( + f"downstream block '{downstream.usage_key}' is linked to " + f"upstream block of different type '{upstream_key}'" + ) + # We import this here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. + from openedx.core.djangoapps.content_libraries.api import ( + get_library_block # pylint: disable=wrong-import-order + ) + try: + lib_meta = get_library_block(upstream_key) + except XBlockNotFoundError as exc: + raise BadUpstream(_("Linked library item was not found in the system")) from exc + return cls( + upstream_ref=downstream.upstream, + version_synced=downstream.upstream_version, + version_available=(lib_meta.draft_version_num if lib_meta else None), + # TODO: Previous line is wrong. It should use the published version instead, but the + # LearningCoreXBlockRuntime APIs do not yet support published content yet. + # Will be fixed in a follow-up task: https://github.com/openedx/edx-platform/issues/35582 + # version_available=(lib_meta.published_version_num if lib_meta else None), + version_declined=downstream.upstream_version_declined, + error_message=None, + ) + + +def sync_from_upstream(downstream: XBlock, user: User) -> None: + """ + Update `downstream` with content+settings from the latest available version of its linked upstream content. + + Preserves overrides to customizable fields; overwrites overrides to other fields. + Does not save `downstream` to the store. That is left up to the caller. + + If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException. + """ + link, upstream = _load_upstream_link_and_block(downstream, user) + _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=False) + _update_non_customizable_fields(upstream=upstream, downstream=downstream) + downstream.upstream_version = link.version_available + + +def fetch_customizable_fields(*, downstream: XBlock, user: User, upstream: XBlock | None = None) -> None: + """ + Fetch upstream-defined value of customizable fields and save them on the downstream. + + If `upstream` is provided, use that block as the upstream. + Otherwise, load the block specified by `downstream.upstream`, which may raise an UpstreamLinkException. + """ + if not upstream: + _link, upstream = _load_upstream_link_and_block(downstream, user) + _update_customizable_fields(upstream=upstream, downstream=downstream, only_fetch=True) + + +def _load_upstream_link_and_block(downstream: XBlock, user: User) -> tuple[UpstreamLink, XBlock]: + """ + Load the upstream metadata and content for a downstream block. + + Assumes that the upstream content is an XBlock in an LC-backed content libraries. This assumption may need to be + relaxed in the future (see module docstring). + + If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException. + """ + link = UpstreamLink.get_for_block(downstream) # can raise UpstreamLinkException + # We import load_block here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. + from openedx.core.djangoapps.xblock.api import load_block # pylint: disable=wrong-import-order + try: + lib_block: XBlock = load_block(LibraryUsageLocatorV2.from_string(downstream.upstream), user) + except (NotFound, PermissionDenied) as exc: + raise BadUpstream(_("Linked library item could not be loaded: {}").format(downstream.upstream)) from exc + return link, lib_block + + +def _update_customizable_fields(*, upstream: XBlock, downstream: XBlock, only_fetch: bool) -> None: + """ + For each customizable field: + * Save the upstream value to a hidden field on the downstream ("FETCH"). + * If `not only_fetch`, and if the field *isn't* customized on the downstream, then: + * Update it the downstream field's value from the upstream field ("SYNC"). + + Concrete example: Imagine `lib_problem` is our upstream and `course_problem` is our downstream. + + * Say that the customizable fields are [display_name, max_attempts]. + + * Set `course_problem.upstream_display_name = lib_problem.display_name` ("fetch"). + * If `not only_fetch`, and `course_problem.display_name` wasn't customized, then: + * Set `course_problem.display_name = lib_problem.display_name` ("sync"). + + * Set `course_problem.upstream_max_attempts = lib_problem.max_attempts` ("fetch"). + * If `not only_fetch`, and `course_problem.max_attempts` wasn't customized, then: + * Set `course_problem.max_attempts = lib_problem.max_attempts` ("sync"). + """ + syncable_field_names = _get_synchronizable_fields(upstream, downstream) + + for field_name, fetch_field_name in downstream.get_customizable_fields().items(): + + if field_name not in syncable_field_names: + continue + + # FETCH the upstream's value and save it on the downstream (ie, `downstream.upstream_$FIELD`). + old_upstream_value = getattr(downstream, fetch_field_name) + new_upstream_value = getattr(upstream, field_name) + setattr(downstream, fetch_field_name, new_upstream_value) + + if only_fetch: + continue + + # Okay, now for the nuanced part... + # We need to update the downstream field *iff it has not been customized**. + # Determining whether a field has been customized will differ in Beta vs Future release. + # (See "PRESERVING DOWNSTREAM CUSTOMIZATIONS" comment below for details.) + + ## FUTURE BEHAVIOR: field is "customized" iff we have noticed that the user edited it. + # if field_name in downstream.downstream_customized: + # continue + + ## BETA BEHAVIOR: field is "customized" iff we have the prev upstream value, but field doesn't match it. + downstream_value = getattr(downstream, field_name) + if old_upstream_value and downstream_value != old_upstream_value: + continue # Field has been customized. Don't touch it. Move on. + + # Field isn't customized -- SYNC it! + setattr(downstream, field_name, new_upstream_value) + + +def _update_non_customizable_fields(*, upstream: XBlock, downstream: XBlock) -> None: + """ + For each field `downstream.blah` that isn't customizable: set it to `upstream.blah`. + """ + syncable_fields = _get_synchronizable_fields(upstream, downstream) + customizable_fields = set(downstream.get_customizable_fields().keys()) + for field_name in syncable_fields - customizable_fields: + new_upstream_value = getattr(upstream, field_name) + setattr(downstream, field_name, new_upstream_value) + + +def _get_synchronizable_fields(upstream: XBlock, downstream: XBlock) -> set[str]: + """ + The syncable fields are the ones which are content- or settings-scoped AND are defined on both (up,down)stream. + """ + return set.intersection(*[ + set( + field_name + for (field_name, field) in block.__class__.fields.items() + if field.scope in [Scope.settings, Scope.content] + ) + for block in [upstream, downstream] + ]) + + +def decline_sync(downstream: XBlock) -> None: + """ + Given an XBlock that is linked to upstream content, mark the latest available update as 'declined' so that its + authors are not prompted (until another upstream version becomes available). + + Does not save `downstream` to the store. That is left up to the caller. + + If `downstream` lacks a valid+supported upstream link, this raises an UpstreamLinkException. + """ + upstream_link = UpstreamLink.get_for_block(downstream) # Can raise UpstreamLinkException + downstream.upstream_version_declined = upstream_link.version_available + + +def sever_upstream_link(downstream: XBlock) -> None: + """ + Given an XBlock that is linked to upstream content, disconnect the link, such that authors are never again prompted + to sync upstream updates. Erase all `.upstream*` fields from the downtream block. + + However, before nulling out the `.upstream` field, we copy its value over to `.copied_from_block`. This makes sense, + because once a downstream block has been de-linked from source (e.g., a Content Library block), it is no different + than if the block had just been copy-pasted in the first place. + + Does not save `downstream` to the store. That is left up to the caller. + + If `downstream` lacks a link, then this raises NoUpstream (though it is reasonable for callers to handle such + exception and ignore it, as the end result is the same: `downstream.upstream is None`). + """ + if not downstream.upstream: + raise NoUpstream() + downstream.copied_from_block = downstream.upstream + downstream.upstream = None + downstream.upstream_version = None + for _, fetched_upstream_field in downstream.get_customizable_fields().items(): + setattr(downstream, fetched_upstream_field, None) # Null out upstream_display_name, et al. + + +class UpstreamSyncMixin(XBlockMixin): + """ + Allows an XBlock in the CMS to be associated & synced with an upstream. + + Mixed into CMS's XBLOCK_MIXINS, but not LMS's. + """ + + # Upstream synchronization metadata fields + upstream = String( + help=( + "The usage key of a block (generally within a content library) which serves as a source of upstream " + "updates for this block, or None if there is no such upstream. Please note: It is valid for this " + "field to hold a usage key for an upstream block that does not exist (or does not *yet* exist) on " + "this instance, particularly if this downstream block was imported from a different instance." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True + ) + upstream_version = Integer( + help=( + "Record of the upstream block's version number at the time this block was created from it. If this " + "upstream_version is smaller than the upstream block's latest published version, then the author will be " + "invited to sync updates into this downstream block, presuming that they have not already declined to sync " + "said version." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + upstream_version_declined = Integer( + help=( + "Record of the latest upstream version for which the author declined to sync updates, or None if they have " + "never declined an update." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + + # Store the fetched upstream values for customizable fields. + upstream_display_name = String( + help=("The value of display_name on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + upstream_max_attempts = Integer( + help=("The value of max_attempts on the linked upstream block."), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + + @classmethod + def get_customizable_fields(cls) -> dict[str, str]: + """ + Mapping from each customizable field to the field which can be used to restore its upstream value. + + XBlocks outside of edx-platform can override this in order to set up their own customizable fields. + """ + return { + "display_name": "upstream_display_name", + "max_attempts": "upstream_max_attempts", + } + + # PRESERVING DOWNSTREAM CUSTOMIZATIONS and RESTORING UPSTREAM VALUES + # + # For the full Content Libraries Relaunch, we would like to keep track of which customizable fields the user has + # actually customized. The idea is: once an author has customized a customizable field.... + # + # - future upstream syncs will NOT blow away the customization, + # - but future upstream syncs WILL fetch the upstream values and tuck them away in a hidden field, + # - and the author can can revert back to said fetched upstream value at any point. + # + # Now, whether field is "customized" (and thus "revertible") is dependent on whether they have ever edited it. + # To instrument this, we need to keep track of which customizable fields have been edited using a new XBlock field: + # `downstream_customized` + # + # Implementing `downstream_customized` has proven difficult, because there is no simple way to keep it up-to-date + # with the many different ways XBlock fields can change. The `.save()` and `.editor_saved()` methods are promising, + # but we need to do more due diligence to be sure that they cover all cases, including API edits, import/export, + # copy/paste, etc. We will figure this out in time for the full Content Libraries Relaunch (related ticket: + # https://github.com/openedx/frontend-app-authoring/issues/1317). But, for the Beta realease, we're going to + # implement something simpler: + # + # - We fetch upstream values for customizable fields and tuck them away in a hidden field (same as above). + # - If a customizable field DOES match the fetched upstream value, then future upstream syncs DO update it. + # - If a customizable field does NOT the fetched upstream value, then future upstream syncs DO NOT update it. + # - There is no UI option for explicitly reverting back to the fetched upstream value. + # + # For future reference, here is a partial implementation of what we are thinking for the full Content Libraries + # Relaunch:: + # + # downstream_customized = List( + # help=( + # "Names of the fields which have values set on the upstream block yet have been explicitly " + # "overridden on this downstream block. Unless explicitly cleared by the user, these customizations " + # "will persist even when updates are synced from the upstream." + # ), + # default=[], scope=Scope.settings, hidden=True, enforce_type=True, + # ) + # + # def save(self, *args, **kwargs): + # """ + # Update `downstream_customized` when a customizable field is modified. + # + # NOTE: This does not work, because save() isn't actually called in all the cases that we'd want it to be. + # """ + # super().save(*args, **kwargs) + # customizable_fields = self.get_customizable_fields() + # + # # Loop through all the fields that are potentially cutomizable. + # for field_name, restore_field_name in self.get_customizable_fields(): + # + # # If the field is already marked as customized, then move on so that we don't + # # unneccessarily query the block for its current value. + # if field_name in self.downstream_customized: + # continue + # + # # If this field's value doesn't match the synced upstream value, then mark the field + # # as customized so that we don't clobber it later when syncing. + # # NOTE: Need to consider the performance impact of all these field lookups. + # if getattr(self, field_name) != getattr(self, restore_field_name): + # self.downstream_customized.append(field_name) diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 16d9ccbd4ca5..61f524123b48 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -76,14 +76,18 @@

${_("This course was created as a re-run. Some manual ${_("This course run is using an upgraded version of edx discussion forum. In order to display the discussions sidebar, discussions xBlocks will no longer be visible to learners.")}
+ %if settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL: ${_(" Learn more")} + %endif + %if settings.DISCUSSIONS_INCONTEXT_FEEDBACK_URL: ${_("Share feedback")} + %endif
@@ -162,7 +166,7 @@

${_("This course has proctored exam settings that are % if mfe_proctored_exam_settings_url: <% url_encoded_course_id = quote(str(context_course.id).encode('utf-8'), safe='') %> ${Text(_("To update these settings go to the {link_start}Proctored Exam Settings page{link_end}.")).format( - link_start=HTML('').format( + link_start=HTML('').format( mfe_proctored_exam_settings_url=mfe_proctored_exam_settings_url ), link_end=HTML("") diff --git a/cms/templates/emails/course_creator_admin_user_pending.txt b/cms/templates/emails/course_creator_admin_user_pending.txt index 7a7d41744dc8..b8eed75e20da 100644 --- a/cms/templates/emails/course_creator_admin_user_pending.txt +++ b/cms/templates/emails/course_creator_admin_user_pending.txt @@ -1,5 +1,5 @@ <%! from django.utils.translation import gettext as _ %> -${_("User '{user}' with e-mail {email} has requested {studio_name} course creator privileges on edge.").format( +${_("User '{user}' with e-mail {email} has requested {studio_name} course creator privileges.").format( user=user_name, email=user_email, studio_name=settings.STUDIO_SHORT_NAME, )} ${_("To grant or deny this request, use the course creator admin table.")} diff --git a/cms/templates/index.html b/cms/templates/index.html index 766d68da780c..e55859730729 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -348,9 +348,6 @@

${course_info['display_name']}

% endif % if libraries_enabled: - % if redirect_to_library_authoring_mfe: -
  • ${_("Libraries")}
    • - %else:
  • % if split_studio_home: ${_("Libraries")} @@ -358,7 +355,6 @@

    ${course_info['display_name']}

    ${_("Libraries")} % endif
    • - % endif % endif % if taxonomies_enabled:
  • ${_("Taxonomies")}
    • diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 15d259b8b7e6..df64bcc39361 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -43,7 +43,6 @@ ${show_min_grade_warning | n, dump_js_escaped_json}, ${can_show_certificate_available_date_field(context_course) | n, dump_js_escaped_json}, "${upgrade_deadline | n, js_escaped_string}", - ${settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS") | n, dump_js_escaped_json} ); }); @@ -251,58 +250,45 @@

    ${_('Course Schedule')}

    - <% - use_v2_cert_display_settings = settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False) - %> % if can_show_certificate_available_date_field(context_course):
    1. - % if use_v2_cert_display_settings: - - % else: - - % endif + ${_("Certificates are awarded at the end of a course run")} - % if use_v2_cert_display_settings: - -
      -