diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index a5e997bf2f..c57eeeb250 100644 --- a/.annotation_safe_list.yml +++ b/.annotation_safe_list.yml @@ -9,13 +9,13 @@ # Via Django auth.Group: - ".. no_pii:" : "No PII" + ".. no_pii:": "No PII" auth.Permission: - ".. no_pii:" : "No PII" + ".. no_pii:": "No PII" auth.User: ".. pii:": "Contains username, password, and email address, retired in AccountRetirementView" - ".. pii_types:" : username, email_address, password - ".. pii_retirement:" : local_api + ".. pii_types:": username, email_address, password + ".. pii_retirement:": local_api contenttypes.ContentType: ".. no_pii:": "No PII" admin.LogEntry: @@ -27,6 +27,66 @@ sessions.Session: sites.Site: ".. no_pii:": "No PII" +# Automatically generated edx-platform models that can't be annotated +calendar_sync.HistoricalUserCalendarSyncConfig: + ".. no_pii:": "No PII" +certificates.HistoricalCertificateAllowlist: + ".. no_pii:": "No PII" +certificates.HistoricalCertificateDateOverride: + ".. no_pii:": "No PII" +certificates.HistoricalCertificateInvalidation: + ".. no_pii:": "No PII" +certificates.HistoricalGeneratedCertificate: + ".. pii:": "PII can exist in the generated certificate linked to in this model. Certificate data is currently retained." + ".. pii_types:": "name, username" + ".. pii_retirement:": "retained" +course_apps.HistoricalCourseAppStatus: + ".. no_pii:": "No PII" +course_goals.HistoricalCourseGoal: + ".. no_pii:": "No PII" +course_live.HistoricalCourseLiveConfiguration: + ".. no_pii:": "No PII" +course_modes.HistoricalCourseMode: + ".. no_pii:": "No PII" +course_overviews.HistoricalCourseOverview: + ".. no_pii:": "No PII" +discussions.HistoricalDiscussionsConfiguration: + ".. no_pii:": "No PII" +entitlements.HistoricalCourseEntitlement: + ".. no_pii:": "No PII" +entitlements.HistoricalCourseEntitlementSupportDetail: + ".. no_pii:": "No PII" +experiments.HistoricalExperimentKeyValue: + ".. no_pii:": "No PII" +external_user_ids.HistoricalExternalId: + ".. no_pii:": "We store external_user_id here, but do not consider that PII under OEP-30." +external_user_ids.HistoricalExternalIdType: + ".. no_pii:": "No PII" +grades.HistoricalPersistentSubsectionGradeOverride: + ".. no_pii:": "No PII" +instructor_task.HistoricalInstructorTaskSchedule: + ".. no_pii:": "No PII" +program_enrollments.HistoricalProgramCourseEnrollment: + ".. no_pii:": "No PII" +program_enrollments.HistoricalProgramEnrollment: + ".. pii:": "PII is found in the external key for a program enrollment" + ".. pii_types:": "other" + ".. pii_retirement:": "local_api" +programs.HistoricalProgramDiscussionsConfiguration: + ".. no_pii:": "No PII" +programs.HistoricalProgramLiveConfiguration: + ".. no_pii:": "No PII" +schedules.HistoricalSchedule: + ".. no_pii:": "No PII" +split_modulestore_django.HistoricalSplitModulestoreCourseIndex: + ".. no_pii:": "No PII" +student.HistoricalCourseEnrollment: + ".. no_pii:": "No PII" +student.HistoricalManualEnrollmentAudit: + ".. pii:": "Contains enrolled_email, retired in LMSAccountRetirementView" + ".. pii_types:": "email_address" + ".. pii_retirement:": "local_api" + # Automatically generated models in edx-enterprise that can't be annotated there consent.HistoricalDataSharingConsent: ".. pii:": "The username field inherited from Consent contains PII." @@ -45,7 +105,7 @@ enterprise.HistoricalEnterpriseCustomerCatalog: enterprise.HistoricalEnterpriseCustomerEntitlement: ".. no_pii:": "No PII" -# Via ORA2 +# Via edx-ora2, these can be removed once the models are annotated for real assessment.Assessment: ".. no_pii:": "No PII" assessment.AssessmentFeedback: @@ -127,10 +187,24 @@ djcelery.TaskState: djcelery.WorkerState: ".. no_pii:": "No PII" +# Via django-celery-results +django_celery_results.ChordCounter: + ".. no_pii:": "No PII" +django_celery_results.GroupResult: + ".. no_pii:": "No PII" +django_celery_results.TaskResult: + ".. no_pii:": "No PII" + # Via edx-oauth2-provider https://github.com/edx/edx-oauth2-provider edx_oauth2_provider.TrustedClient: ".. no_pii:": "No PII" +# Via edx-name-affirmation, not part of the openedx org +edx_name_affirmation.HistoricalVerifiedName: + ".. pii:": "Contains name fields." + ".. pii_types:": "name" + ".. pii_retirement:": "local_api" + # Via VAL edxval.CourseVideo: ".. no_pii:": "No PII" @@ -149,6 +223,12 @@ edxval.VideoImage: edxval.VideoTranscript: ".. no_pii:": "No PII" +# Via PyLTI1p3 +lti1p3_tool_config.LtiTool: + ".. no_pii:": "No PII" +lti1p3_tool_config.LtiToolKey: + ".. no_pii:": "No PII" + # Via Milestones milestones.CourseContentMilestone: ".. no_pii:": "No PII" @@ -190,6 +270,10 @@ oauth2_provider.Grant: ".. pii:": "Contains 3rd party authentication secrets. Retired in DeactivateLogoutView." ".. pii_types:": password, other ".. pii_retirement:": local_api +oauth2_provider.IDToken: + ".. pii:": "Contains 3rd party authentication secrets, currently this is retained until the token times out, but should be retired explicitly with the other models from this package." + ".. pii_types:": password, other + ".. pii_retirement:": retained oauth2_provider.RefreshToken: ".. pii:": "Contains 3rd party authentication secrets. Retired in DeactivateLogoutView." ".. pii_types:": password, other @@ -250,6 +334,8 @@ submissions.StudentItem: ".. no_pii:": "No PII" submissions.Submission: ".. no_pii:": "No PII" +submissions.TeamSubmission: + ".. no_pii:": "No PII" # Via sorl-thumbnail https://github.com/jazzband/sorl-thumbnail thumbnail.KVStore: diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index fc452f7acd..c41fe40b4d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -22,7 +22,7 @@ openedx/core/djangoapps/enrollments/ @openedx/2U- 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/user_authn/ @openedx/2U-infinity openedx/core/djangoapps/verified_track_content/ @openedx/2u-infinity openedx/features/course_experience/ xmodule/ diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 3f771c2c72..ca314891b2 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -32,7 +32,11 @@ // When adding an ignoreDep, please include a reason and a public link that we can use to follow up and ensure // that the ignoreDep is removed. // This can be done as a comment within the ignoreDeps list. - "ignoreDeps": [], + "ignoreDeps": [ + // karma-spec-reporter>0.20.0 does not seem compatible with our super-old 2016 Karma version (0.13.22). + // Ticket link: None, as upgrading Karma does not strike as worth the benefit. + "karma-spec-reporter" + ], "timezone": "America/New_York", "prConcurrentLimit": 3, "enabledManagers": ["npm"] diff --git a/.github/workflows/check-consistent-dependencies.yml b/.github/workflows/check-consistent-dependencies.yml index 048cbe6b00..82298af70a 100644 --- a/.github/workflows/check-consistent-dependencies.yml +++ b/.github/workflows/check-consistent-dependencies.yml @@ -15,7 +15,7 @@ defaults: jobs: check-requirements: name: Compile requirements - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: # Only run remaining steps if there are changes to requirements/** diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index 458e00fc6b..d989ff9db2 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -10,7 +10,7 @@ jobs: matrix: python-version: - "3.11" - os: ["ubuntu-22.04"] + os: ["ubuntu-24.04"] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index c9d2d7ab11..4496a4b61c 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -13,7 +13,7 @@ jobs: strategy: matrix: os: [ubuntu-latest] - node-version: [18, 20] + node-version: [20] python-version: - "3.11" @@ -28,7 +28,7 @@ jobs: node-version: ${{ matrix.node-version }} - name: Setup npm - run: npm i -g npm@10.5.x + run: npm i -g npm@10.7.x - name: Install Firefox 123.0 run: | diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 624caddd53..84e334d688 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" # 'pinned' is used to install the latest patch version of Django @@ -126,7 +126,7 @@ jobs: if: always() needs: - check_migrations - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index 8860aced7f..9a654e09e7 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -8,7 +8,7 @@ on: jobs: run-pylint: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: @@ -75,7 +75,7 @@ jobs: if: always() needs: - run-pylint - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - name: Decide whether the needed jobs succeeded or failed # uses: re-actors/alls-green@v1.2.1 diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 8461012349..310f9f83bf 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" node-version: [20] diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 4fe66e2a77..502dddce08 100644 --- a/.github/workflows/static-assets-check.yml +++ b/.github/workflows/static-assets-check.yml @@ -12,11 +12,11 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-22.04] + os: [ubuntu-24.04] python-version: - "3.11" - node-version: [18, 20] - npm-version: [10.5.x] + node-version: [20] + npm-version: [10.7.x] mongo-version: - "7.0" diff --git a/.github/workflows/unit-test-shards.json b/.github/workflows/unit-test-shards.json index 4ab126cb47..1af8af8142 100644 --- a/.github/workflows/unit-test-shards.json +++ b/.github/workflows/unit-test-shards.json @@ -239,7 +239,6 @@ "cms/djangoapps/cms_user_tasks/", "cms/djangoapps/course_creators/", "cms/djangoapps/export_course_metadata/", - "cms/djangoapps/maintenance/", "cms/djangoapps/models/", "cms/djangoapps/pipeline_js/", "cms/djangoapps/xblock_config/", diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 854677b93c..e691e16e47 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -15,7 +15,7 @@ concurrency: jobs: run-tests: name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }}) - runs-on: ubuntu-22.04 + runs-on: ${{ matrix.os-version }} strategy: matrix: python-version: @@ -43,22 +43,27 @@ jobs: - "xmodule-with-cms" mongo-version: - "7.0" + os-version: + - ubuntu-24.04 - # We only need to test older versions of Mongo with modules that directly interface with Mongo (that is: xmodule.modulestore) - # This code is left here as an example for future refernce in case we need to reduce the number of shards we're - # testing but still have good confidence with older versions of mongo. We use Mongo 4.4 in the example. + # It's useful to run some subset of the tests on the older version of Ubuntu + # so that we don't spend too many resources on this but can find major issues quickly + # while we're in a situation where we support two versions. This section may be commented + # out when not in use to easily add/drop future support for any given major dependency. # - # exclude: - # - mongo-version: "4.4" - # include: - # - shard_name: "xmodule-with-cms" - # python-version: "3.11" - # django-version: "pinned" - # mongo-version: "4.4" - # - shard_name: "xmodule-with-lms" - # python-version: "3.11" - # django-version: "pinned" - # mongo-version: "4.4" + # We're testing the older version of Ubuntu and running the xmodule tests since those rely on many + # dependent complex libraries and will hopefully catch most issues quickly. + include: + - shard_name: "xmodule-with-cms" + python-version: "3.11" + django-version: "pinned" + mongo-version: "7.0" + os-version: "ubuntu-22.04" + - shard_name: "xmodule-with-lms" + python-version: "3.11" + django-version: "pinned" + mongo-version: "7.0" + os-version: "ubuntu-22.04" steps: - name: checkout repo @@ -90,19 +95,10 @@ jobs: activate = 1 EOF - - name: install mongo version - run: | - if [[ "${{ matrix.mongo-version }}" != "4.4" ]]; then - wget -qO - https://www.mongodb.org/static/pgp/server-${{ matrix.mongo-version }}.asc | sudo apt-key add - - echo "deb https://repo.mongodb.org/apt/ubuntu focal/mongodb-org/${{ matrix.mongo-version }} multiverse" | sudo tee /etc/apt/sources.list.d/mongodb-org-${{ matrix.mongo-version }}.list - sudo apt-get update && sudo apt-get install -y mongodb-org="${{ matrix.mongo-version }}.*" - fi - - - name: start mongod server for tests - run: | - sudo mkdir -p /data/db - sudo chmod -R a+rw /data/db - mongod & + - name: Start MongoDB + uses: supercharge/mongodb-github-action@1.11.0 + with: + mongodb-version: ${{ matrix.mongo-version }} - name: Setup Python uses: actions/setup-python@v5 @@ -164,7 +160,7 @@ jobs: overwrite: true collect-and-verify: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - name: Setup Python @@ -229,7 +225,7 @@ jobs: # https://github.com/orgs/community/discussions/33579 success: name: Unit tests successful - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 if: always() needs: [run-tests] steps: @@ -240,7 +236,7 @@ jobs: jobs: ${{ toJSON(needs) }} compile-warnings-report: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [run-tests] steps: - uses: actions/checkout@v4 @@ -268,7 +264,7 @@ jobs: overwrite: true merge-artifacts: - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [compile-warnings-report] steps: - name: Merge Pytest Warnings JSON Artifacts @@ -288,7 +284,7 @@ jobs: # Combine and upload coverage reports. coverage: if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false)) - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 needs: [run-tests] strategy: matrix: diff --git a/.github/workflows/verify-dunder-init.yml b/.github/workflows/verify-dunder-init.yml index 9d920238eb..c398c50673 100644 --- a/.github/workflows/verify-dunder-init.yml +++ b/.github/workflows/verify-dunder-init.yml @@ -1,4 +1,4 @@ -name: CI +name: Verify Dunder __init__.py Files on: pull_request: diff --git a/.nvmrc b/.nvmrc index 3c032078a4..209e3ef4b6 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -18 +20 diff --git a/.readthedocs.yaml b/.readthedocs.yaml index 548ea72b3e..c0db64e5e1 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -3,7 +3,7 @@ version: 2 build: os: "ubuntu-22.04" tools: - python: "3.12" + python: "3.11" sphinx: configuration: docs/conf.py diff --git a/Dockerfile b/Dockerfile index 75a716177f..a67c46738b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -48,36 +48,36 @@ RUN echo "locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8" # Setting up ppa deadsnakes to get python 3.11 RUN apt-get update && \ - apt-get install -y software-properties-common && \ - apt-add-repository -y ppa:deadsnakes/ppa + apt-get install -y software-properties-common && \ + apt-add-repository -y ppa:deadsnakes/ppa # Install requirements that are absolutely necessary RUN apt-get update && \ apt-get -y dist-upgrade && \ apt-get -y install --no-install-recommends \ - python3-pip \ - python3.11 \ - # python3-dev: required for building mysqlclient python package - python3.11-dev \ - python3.11-venv \ - libpython3.11 \ - libpython3.11-stdlib \ - libmysqlclient21 \ - # libmysqlclient-dev: required for building mysqlclient python package - libmysqlclient-dev \ - pkg-config \ - libssl1.1 \ - libxmlsec1-openssl \ - # lynx: Required by https://github.com/openedx/edx-platform/blob/b489a4ecb122/openedx/core/lib/html_to_text.py#L16 - lynx \ - ntp \ - git \ - build-essential \ - gettext \ - gfortran \ - graphviz \ - locales \ - swig \ + python3-pip \ + python3.11 \ + # python3-dev: required for building mysqlclient python package + python3.11-dev \ + python3.11-venv \ + libpython3.11 \ + libpython3.11-stdlib \ + libmysqlclient21 \ + # libmysqlclient-dev: required for building mysqlclient python package + libmysqlclient-dev \ + pkg-config \ + libssl1.1 \ + libxmlsec1-openssl \ + # lynx: Required by https://github.com/openedx/edx-platform/blob/b489a4ecb122/openedx/core/lib/html_to_text.py#L16 + lynx \ + ntp \ + git \ + build-essential \ + gettext \ + gfortran \ + graphviz \ + locales \ + swig \ && \ apt-get clean all && \ rm -rf /var/lib/apt/* @@ -93,19 +93,19 @@ FROM minimal-system as builder-production RUN apt-get update && \ apt-get -y install --no-install-recommends \ - curl \ - libssl-dev \ - libffi-dev \ - libfreetype6-dev \ - libgeos-dev \ - libgraphviz-dev \ - libjpeg8-dev \ - liblapack-dev \ - libpng-dev \ - libsqlite3-dev \ - libxml2-dev \ - libxmlsec1-dev \ - libxslt1-dev + curl \ + libssl-dev \ + libffi-dev \ + libfreetype6-dev \ + libgeos-dev \ + libgraphviz-dev \ + libjpeg8-dev \ + liblapack-dev \ + libpng-dev \ + libsqlite3-dev \ + libxml2-dev \ + libxmlsec1-dev \ + libxslt1-dev # Setup python virtual environment # It is already 'activated' because $VIRTUAL_ENV/bin was put on $PATH @@ -118,8 +118,8 @@ RUN pip install -r requirements/pip.txt RUN pip install -r requirements/edx/base.txt # Install node and npm -RUN nodeenv /edx/app/edxapp/nodeenv --node=18.19.0 --prebuilt -RUN npm install -g npm@10.5.x +RUN nodeenv /edx/app/edxapp/nodeenv --node=20.15.1 --prebuilt +RUN npm install -g npm@10.7.x # This script is used by an npm post-install hook. # We copy it into the image now so that it will be available when we run `npm install` in the next step. @@ -178,8 +178,8 @@ FROM base as development RUN apt-get update && \ apt-get -y install --no-install-recommends \ - # wget is used in Makefile for common_constraints.txt - wget \ + # wget is used in Makefile for common_constraints.txt + wget \ && \ apt-get clean all && \ rm -rf /var/lib/apt/* diff --git a/README.rst b/README.rst index 64186fca3a..54b91363a8 100644 --- a/README.rst +++ b/README.rst @@ -70,6 +70,11 @@ complexity of Open edX configuration and deployment into their own hands. System Dependencies ------------------- +OS: +* Ubuntu 22.04 + +* Ubuntu 24.04 + Interperters/Tools: * Python 3.11 @@ -100,6 +105,17 @@ Language Packages: - ``pip install -r requirements/edx/base.txt`` (production) - ``pip install -r requirements/edx/dev.txt`` (development) + Some Python packages have system dependencies. For example, installing these packages on Debian or Ubuntu will require first running ``sudo apt install python3-dev default-libmysqlclient-dev build-essential pkg-config`` to satisfy the requirements of the ``mysqlclient`` Python package. + +Codejail Setup +-------------- + +As a part of the baremetal setup, you will need to configure your system to +work properly with codejail. See the `codejail installation steps`_ for more +details. + +.. _codejail installation steps: https://github.com/openedx/codejail?tab=readme-ov-file#installation + Build Steps ----------- diff --git a/catalog-info.yaml b/catalog-info.yaml index 885b879833..c6b87b08db 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -10,6 +10,8 @@ metadata: - url: "https://docs.openedx.org" title: "Documentation" icon: "Web" + annotations: + openedx.org/release: "master" spec: owner: group:wg-maintenance-edx-platform type: 'service' diff --git a/cms/djangoapps/contentstore/asset_storage_handlers.py b/cms/djangoapps/contentstore/asset_storage_handlers.py index 281258e03a..97fcb31257 100644 --- a/cms/djangoapps/contentstore/asset_storage_handlers.py +++ b/cms/djangoapps/contentstore/asset_storage_handlers.py @@ -25,6 +25,7 @@ from common.djangoapps.util.date_utils import get_default_time_display from common.djangoapps.util.json_request import JsonResponse from openedx.core.djangoapps.contentserver.caching import del_cached_content from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers +from openedx_filters.course_authoring.filters import LMSPageURLRequested from xmodule.contentstore.content import StaticContent # lint-amnesty, pylint: disable=wrong-import-order from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.exceptions import NotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -714,7 +715,15 @@ def get_asset_json(display_name, content_type, date, location, thumbnail_locatio Helper method for formatting the asset information to send to client. ''' asset_url = StaticContent.serialize_asset_key_with_slash(location) - external_url = urljoin(configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), asset_url) + + ## .. filter_implemented_name: LMSPageURLRequested + ## .. filter_type: org.openedx.course_authoring.lms.page.url.requested.v1 + lms_root, _ = LMSPageURLRequested.run_filter( + url=configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL), + org=location.org, + ) + + external_url = urljoin(lms_root, asset_url) portable_url = StaticContent.get_static_path_from_location(location) usage_locations = [] if usage is None else usage return { diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index 48647bf47b..d3b6f811d5 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/cms/djangoapps/contentstore/courseware_index.py @@ -256,7 +256,8 @@ class SearchIndexerBase(metaclass=ABCMeta): # Now index the content for item in structure.get_children(): prepare_item_index(item, groups_usage_info=groups_usage_info) - searcher.index(items_index, request_timeout=timeout) + if items_index: + searcher.index(items_index, request_timeout=timeout) cls.remove_deleted_items(searcher, structure_key, indexed_items) except Exception as err: # pylint: disable=broad-except # broad exception so that index operation does not prevent the rest of the application from working diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index a4ece6c85d..e40eddb6c9 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -3,14 +3,17 @@ Helper methods for Studio views. """ from __future__ import annotations import logging +import pathlib import urllib from lxml import etree from mimetypes import guess_type +import re 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.keys import CourseKey, UsageKey from opaque_keys.edx.locator import DefinitionLocator, LocalId from xblock.core import XBlock from xblock.fields import ScopeIds @@ -22,6 +25,7 @@ from xmodule.modulestore.django import modulestore from xmodule.xml_block import XmlMixin from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.lib.xblock.upstream_sync import UpstreamLink, UpstreamLinkException, 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 +34,10 @@ from .utils import reverse_course_url, reverse_library_url, reverse_usage_url 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. @@ -184,6 +192,10 @@ def xblock_type_display_name(xblock, default_display_name=None): # description like "Multiple Choice Problem", but that won't work if our 'block' argument is just the block_type # string ("problem"). return _('Problem') + elif category == 'library_v2': + return _('Library Content') + elif category == 'itembank': + return _('Problem Bank') component_class = XBlock.load_class(category) if hasattr(component_class, 'display_name') and component_class.display_name.default: return _(component_class.display_name.default) # lint-amnesty, pylint: disable=translation-of-non-string @@ -261,7 +273,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> empty, and (2) a summary of changes made to static files in the destination course. """ - from cms.djangoapps.contentstore.views.preview import _load_preview_block if not content_staging_api: @@ -271,7 +282,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) -> # Clipboard is empty or expired/error/loading return None, StaticFileNotices() olx_str = content_staging_api.get_staged_content_olx(user_clipboard.content.id) - static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id) node = etree.fromstring(olx_str) store = modulestore() with store.bulk_operations(parent_key.course_key): @@ -282,32 +292,102 @@ 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: - notices = _import_files_into_course( - course_key=parent_key.context_key, - staged_content_id=user_clipboard.content.id, - static_files=static_files, - ) + + # Now handle static files that need to go into Files & Uploads. + static_files = content_staging_api.get_staged_content_static_files(user_clipboard.content.id) + notices, substitutions = _import_files_into_course( + course_key=parent_key.context_key, + staged_content_id=user_clipboard.content.id, + static_files=static_files, + usage_key=new_xblock.scope_ids.usage_id, + ) + + # Rewrite the OLX's static asset references to point to the new + # locations for those assets. See _import_files_into_course for more + # info on why this is necessary. + if hasattr(new_xblock, 'data') and substitutions: + data_with_substitutions = new_xblock.data + for old_static_ref, new_static_ref in substitutions.items(): + data_with_substitutions = data_with_substitutions.replace( + old_static_ref, + new_static_ref, + ) + new_xblock.data = data_with_substitutions + store.update_item(new_xblock, request.user.id) return new_xblock, notices +def _fetch_and_set_upstream_link( + copied_from_block: str, + copied_from_version_num: int, + temp_xblock: XBlock, + user: User +): + """ + Fetch and set upstream link for the given xblock. This function handles following cases: + * the xblock is copied from a v2 library; the library block is set as upstream. + * the xblock is copied from a course; no upstream is set, only copied_from_block is set. + * the xblock is copied from a course where the source block was imported from a library; the original libary block + is set as upstream. + """ + # 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 UpstreamLinkException: + # 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). + + # In case if the source block was imported from a library, we need to check its upstream + # and set the same upstream link for the new block. + source_descriptor = modulestore().get_item(UsageKey.from_string(copied_from_block)) + if source_descriptor.upstream: + _fetch_and_set_upstream_link( + source_descriptor.upstream, + source_descriptor.upstream_version, + temp_xblock, + user, + ) + else: + # else 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) + + def _import_xml_node_to_parent( node, 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: @@ -315,6 +395,8 @@ def _import_xml_node_to_parent( Given an XML node representing a serialized XBlock (OLX), import it into modulestore 'store' as a child of the specified parent block. Recursively copy children as needed. """ + # pylint: disable=too-many-statements + runtime = parent_xblock.runtime parent_key = parent_xblock.scope_ids.usage_id block_type = node.tag @@ -366,6 +448,10 @@ def _import_xml_node_to_parent( temp_xblock = xblock_class.parse_xml(node_without_children, runtime, keys) child_nodes = list(node) + if issubclass(xblock_class, XmlMixin) and "x-is-pointer-node" in getattr(temp_xblock, "data", ""): + # Undo the "pointer node" hack if needed (e.g. for capa problems) + temp_xblock.data = re.sub(r'([^>]+) x-is-pointer-node="no"', r'\1', temp_xblock.data, count=1) + # Restore the original id_generator runtime.id_generator = original_id_generator @@ -373,19 +459,18 @@ 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 + _fetch_and_set_upstream_link(copied_from_block, copied_from_version_num, temp_xblock, user) # Save the XBlock into modulestore. We need to save the block and its parent for this to work: - new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True) + new_xblock = 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'): # Allow an XBlock to do anything fancy it may need to when pasted from the clipboard. # These blocks may handle their own children or parenting if needed. Let them return booleans to # let us know if we need to handle these or not. - children_handed = new_xblock.studio_post_paste(store, node) + children_handled = new_xblock.studio_post_paste(store, node) if not children_handled: for child_node in child_nodes: @@ -394,13 +479,20 @@ 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, ) # Copy content tags to the new xblock - if copied_from_block and tags: + if new_xblock.upstream: + # If this block is synced from an upstream (e.g. library content), + # copy the tags from the upstream as ready-only + content_tagging_api.copy_tags_as_read_only( + new_xblock.upstream, + new_xblock.location, + ) + elif copied_from_block and tags: object_tags = tags.get(str(copied_from_block)) if object_tags: content_tagging_api.set_all_object_tags( @@ -415,11 +507,21 @@ def _import_files_into_course( course_key: CourseKey, staged_content_id: int, static_files: list[content_staging_api.StagedContentFileData], -) -> StaticFileNotices: + usage_key: UsageKey, +) -> tuple[StaticFileNotices, dict[str, str]]: """ - For the given staged static asset files (which are in "Staged Content" such as the user's clipbaord, but which - need to end up in the course's Files & Uploads page), import them into the destination course, unless they already + For the given staged static asset files (which are in "Staged Content" such + as the user's clipbaord, but which need to end up in the course's Files & + Uploads page), import them into the destination course, unless they already exist. + + This function returns a tuple of StaticFileNotices (assets added, errors, + conflicts), and static asset path substitutions that should be made in the + OLX in order to paste this content into this course. The latter is for the + case in which we're brining content in from a v2 library, which stores + static assets locally to a Component and needs to go into a subdirectory + when pasting into a course to avoid overwriting commonly named things, e.g. + "figure1.png". """ # List of files that were newly added to the destination course new_files = [] @@ -427,17 +529,25 @@ def _import_files_into_course( conflicting_files = [] # List of files that had an error (shouldn't happen unless we have some kind of bug) error_files = [] + + # Store a mapping of asset URLs that need to be modified for the destination + # assets. This is necessary when you take something from a library and paste + # it into a course, because we need to translate Component-local static + # assets and shove them into the Course's global Files & Uploads space in a + # nested directory structure. + substitutions = {} for file_data_obj in static_files: - if not isinstance(file_data_obj.source_key, AssetKey): - # This static asset was managed by the XBlock and instead of being added to "Files & Uploads", it is stored - # using some other system. We could make it available via runtime.resources_fs during XML parsing, but it's - # not needed here. - continue # At this point, we know this is a "Files & Uploads" asset that we may need to copy into the course: try: - result = _import_file_into_course(course_key, staged_content_id, file_data_obj) + result, substitution_for_file = _import_file_into_course( + course_key, + staged_content_id, + file_data_obj, + usage_key, + ) if result is True: new_files.append(file_data_obj.filename) + substitutions.update(substitution_for_file) elif result is None: pass # This file already exists; no action needed. else: @@ -445,25 +555,45 @@ def _import_files_into_course( except Exception: # lint-amnesty, pylint: disable=broad-except error_files.append(file_data_obj.filename) log.exception(f"Failed to import Files & Uploads file {file_data_obj.filename}") - return StaticFileNotices( + + notices = StaticFileNotices( new_files=new_files, conflicting_files=conflicting_files, error_files=error_files, ) + return notices, substitutions + def _import_file_into_course( course_key: CourseKey, staged_content_id: int, file_data_obj: content_staging_api.StagedContentFileData, -) -> bool | None: + usage_key: UsageKey, +) -> tuple[bool | None, dict]: """ Import a single staged static asset file into the course, unless it already exists. Returns True if it was imported, False if there's a conflict, or None if the file already existed (no action needed). """ - filename = file_data_obj.filename - new_key = course_key.make_asset_key("asset", filename) + clipboard_file_path = file_data_obj.filename + + # We need to generate an AssetKey to add an asset to a course. The mapping + # of directories '/' -> '_' is a long-existing contentstore convention that + # we're not going to attempt to change. + if clipboard_file_path.startswith('static/'): + # If it's in this form, it came from a library and assumes component-local assets + file_path = clipboard_file_path.lstrip('static/') + import_path = f"components/{usage_key.block_type}/{usage_key.block_id}/{file_path}" + filename = pathlib.Path(file_path).name + new_key = course_key.make_asset_key("asset", import_path.replace("/", "_")) + else: + # Otherwise it came from a course... + file_path = clipboard_file_path + import_path = None + filename = pathlib.Path(file_path).name + new_key = course_key.make_asset_key("asset", file_path.replace("/", "_")) + try: current_file = contentstore().find(new_key) except NotFoundError: @@ -471,22 +601,28 @@ def _import_file_into_course( if not current_file: # This static asset should be imported into the new course: content_type = guess_type(filename)[0] - data = content_staging_api.get_staged_content_static_file_data(staged_content_id, filename) + data = content_staging_api.get_staged_content_static_file_data(staged_content_id, clipboard_file_path) if data is None: raise NotFoundError(file_data_obj.source_key) - content = StaticContent(new_key, name=filename, content_type=content_type, data=data) + content = StaticContent( + new_key, + name=filename, + content_type=content_type, + data=data, + import_path=import_path + ) # If it's an image file, also generate the thumbnail: thumbnail_content, thumbnail_location = contentstore().generate_thumbnail(content) if thumbnail_content is not None: content.thumbnail_location = thumbnail_location contentstore().save(content) - return True + return True, {clipboard_file_path: f"static/{import_path}"} elif current_file.content_digest == file_data_obj.md5_hash: # The file already exists and matches exactly, so no action is needed - return None + return None, {} else: # There is a conflict with some other file that has the same name. - return False + return False, {} def is_item_in_course_tree(item): diff --git a/cms/djangoapps/contentstore/management/commands/import.py b/cms/djangoapps/contentstore/management/commands/import.py index 4ff9e7ce8d..346f10d933 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -8,7 +8,7 @@ from django.core.management.base import BaseCommand from openedx.core.djangoapps.django_comment_common.utils import are_permissions_roles_seeded, seed_permissions_roles from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order -from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order +from xmodule.modulestore.django import SignalHandler, modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.xml_importer import import_course_from_xml # lint-amnesty, pylint: disable=wrong-import-order from xmodule.util.sandboxing import DEFAULT_PYTHON_LIB_FILENAME # lint-amnesty, pylint: disable=wrong-import-order @@ -73,6 +73,10 @@ class Command(BaseCommand): for course in course_items: course_id = course.id + # Importing is an act of publishing so send the course published signal. + SignalHandler.course_published.send_robust(sender=self, course_key=course_id) + + # Seed forum permission roles if we need to. if not are_permissions_roles_seeded(course_id): self.stdout.write(f'Seeding forum roles for course {course_id}\n') seed_permissions_roles(course_id) diff --git a/cms/djangoapps/contentstore/management/commands/reindex_course.py b/cms/djangoapps/contentstore/management/commands/reindex_course.py index accbc077c4..275eff5062 100644 --- a/cms/djangoapps/contentstore/management/commands/reindex_course.py +++ b/cms/djangoapps/contentstore/management/commands/reindex_course.py @@ -4,9 +4,10 @@ import logging from textwrap import dedent from time import time -from datetime import date +from datetime import date, datetime from django.core.management import BaseCommand, CommandError +from django.conf import settings from elasticsearch import exceptions from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -42,6 +43,10 @@ class Command(BaseCommand): parser.add_argument('--active', action='store_true', help='Reindex active courses only') + parser.add_argument('--from_inclusion_date', + action='store_true', + help='Reindex courses with a start date greater than COURSEWARE_SEARCH_INCLUSION_DATE' + ) parser.add_argument('--setup', action='store_true', help='Reindex all courses on developers stack setup') @@ -70,15 +75,17 @@ class Command(BaseCommand): course_ids = options['course_ids'] all_option = options['all'] active_option = options['active'] + inclusion_date_option = options['from_inclusion_date'] setup_option = options['setup'] readable_option = options['warning'] index_all_courses_option = all_option or setup_option - if ((not course_ids and not (index_all_courses_option or active_option)) or - (course_ids and (index_all_courses_option or active_option))): + course_option_flag_option = index_all_courses_option or active_option or inclusion_date_option + + if (not course_ids and not course_option_flag_option) or (course_ids and course_option_flag_option): raise CommandError(( "reindex_course requires one or more s" - " OR the --all, --active or --setup flags." + " OR the --all, --active, --setup, or --from_inclusion_date flags." )) store = modulestore() @@ -97,14 +104,16 @@ class Command(BaseCommand): logging.exception('Search Engine error - %s', exc) return - index_exists = searcher._es.indices.exists(index=index_name) # pylint: disable=protected-access + # Legacy Elasticsearch engine + if hasattr(searcher, '_es'): # pylint: disable=protected-access + index_exists = searcher._es.indices.exists(index=index_name) # pylint: disable=protected-access - index_mapping = searcher._es.indices.get_mapping( # pylint: disable=protected-access - index=index_name, - ) if index_exists else {} + index_mapping = searcher._es.indices.get_mapping( # pylint: disable=protected-access + index=index_name, + ) if index_exists else {} - if index_exists and index_mapping: - return + if index_exists and index_mapping: + return # if reindexing is done during devstack setup step, don't prompt the user if setup_option or query_yes_no(self.CONFIRMATION_PROMPT, default="no"): @@ -127,6 +136,20 @@ class Command(BaseCommand): course_keys = list(map(lambda course: course.id, active_courses)) logging.warning(f'Selected {len(course_keys)} active courses over a total of {len(all_courses)}.') + elif inclusion_date_option: + # in case of --from_inclusion_date, we get the list of course keys from all courses + # that are stored in modulestore and filter out courses with a start date less than + # the settings defined COURSEWARE_SEARCH_INCLUSION_DATE + all_courses = modulestore().get_courses() + + inclusion_date = datetime.strptime( + settings.FEATURES.get('COURSEWARE_SEARCH_INCLUSION_DATE', '2020-01-01'), + '%Y-%m-%d' + ) + + # We keep the courses that has a start date and the start date is greater than the inclusion date + active_courses = filter(lambda course: course.start and (course.start >= inclusion_date), all_courses) + course_keys = list(map(lambda course: course.id, active_courses)) else: # in case course keys are provided as arguments diff --git a/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py b/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py index 13d33c48f9..6d14b4a339 100644 --- a/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py +++ b/cms/djangoapps/contentstore/management/commands/tests/test_reindex_courses.py @@ -40,6 +40,9 @@ class TestReindexCourse(ModuleStoreTestCase): self.third_course = CourseFactory.create( org="test", course="course3", display_name="run1", start=None, end=None ) + self.fourth_course = CourseFactory.create( + org="test", course="course4", display_name="run1", start=datetime.min.today() - timedelta(weeks=60) + ) REINDEX_PATH_LOCATION = ( 'cms.djangoapps.contentstore.management.commands.reindex_course.CoursewareSearchIndexer.do_course_reindex' @@ -111,7 +114,9 @@ class TestReindexCourse(ModuleStoreTestCase): call_command('reindex_course', all=True) patched_yes_no.assert_called_once_with(ReindexCommand.CONFIRMATION_PROMPT, default='no') - expected_calls = self._build_calls(self.first_course, self.second_course, self.third_course) + expected_calls = self._build_calls( + self.first_course, self.second_course, self.third_course, self.fourth_course + ) self.assertCountEqual(patched_index.mock_calls, expected_calls) def test_given_all_key_prompts_and_reindexes_all_courses_cancelled(self): @@ -134,5 +139,21 @@ class TestReindexCourse(ModuleStoreTestCase): mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)): call_command('reindex_course', active=True) - expected_calls = self._build_calls(self.first_course) + expected_calls = self._build_calls(self.first_course, self.fourth_course) + self.assertCountEqual(patched_index.mock_calls, expected_calls) + + @mock.patch.dict( + 'django.conf.settings.FEATURES', + {'COURSEWARE_SEARCH_INCLUSION_DATE': (datetime.min.today() - timedelta(weeks=52)).strftime('%Y-%m-%d')} + ) + def test_given_from_inclusion_date_key_prompt(self): + """ + Test that reindexes all courses that have a start date after a defined inclusion date + when --from_inclusion_date key is given. + """ + with mock.patch(self.REINDEX_PATH_LOCATION) as patched_index, \ + mock.patch(self.MODULESTORE_PATCH_LOCATION, mock.Mock(return_value=self.store)): + call_command('reindex_course', from_inclusion_date=True) + + expected_calls = self._build_calls(self.first_course, self.second_course) self.assertCountEqual(patched_index.mock_calls, expected_calls) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py index e8d3039b0b..616204ef59 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/__init__.py @@ -6,9 +6,10 @@ from .course_details import CourseDetailsSerializer from .course_index import CourseIndexSerializer from .course_rerun import CourseRerunSerializer from .course_team import CourseTeamSerializer +from .course_waffle_flags import CourseWaffleFlagsSerializer from .grading import CourseGradingModelSerializer, CourseGradingSerializer from .group_configurations import CourseGroupConfigurationsSerializer -from .home import CourseHomeSerializer, CourseHomeTabSerializer, LibraryTabSerializer +from .home import StudioHomeSerializer, CourseHomeTabSerializer, LibraryTabSerializer from .proctoring import ( LimitedProctoredExamSettingsSerializer, ProctoredExamConfigurationSerializer, diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py new file mode 100644 index 0000000000..33dd997928 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/course_waffle_flags.py @@ -0,0 +1,154 @@ +""" +API Serializers for course waffle flags +""" + +from rest_framework import serializers + +from cms.djangoapps.contentstore import toggles + + +class CourseWaffleFlagsSerializer(serializers.Serializer): + """ + Serializer for course waffle flags + """ + use_new_home_page = serializers.SerializerMethodField() + use_new_custom_pages = serializers.SerializerMethodField() + use_new_schedule_details_page = serializers.SerializerMethodField() + use_new_advanced_settings_page = serializers.SerializerMethodField() + use_new_grading_page = serializers.SerializerMethodField() + use_new_updates_page = serializers.SerializerMethodField() + use_new_import_page = serializers.SerializerMethodField() + use_new_export_page = serializers.SerializerMethodField() + use_new_files_uploads_page = serializers.SerializerMethodField() + use_new_video_uploads_page = serializers.SerializerMethodField() + use_new_course_outline_page = serializers.SerializerMethodField() + use_new_unit_page = serializers.SerializerMethodField() + use_new_course_team_page = serializers.SerializerMethodField() + use_new_certificates_page = serializers.SerializerMethodField() + use_new_textbooks_page = serializers.SerializerMethodField() + use_new_group_configurations_page = serializers.SerializerMethodField() + enable_course_optimizer = serializers.SerializerMethodField() + + def get_course_key(self): + """ + Retrieve the course_key from the context + """ + return self.context.get("course_key") + + def get_use_new_home_page(self, obj): + """ + Method to get the use_new_home_page switch + """ + return toggles.use_new_home_page() + + def get_use_new_custom_pages(self, obj): + """ + Method to get the use_new_custom_pages switch + """ + course_key = self.get_course_key() + return toggles.use_new_custom_pages(course_key) + + def get_use_new_schedule_details_page(self, obj): + """ + Method to get the use_new_schedule_details_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_schedule_details_page(course_key) + + def get_use_new_advanced_settings_page(self, obj): + """ + Method to get the use_new_advanced_settings_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_advanced_settings_page(course_key) + + def get_use_new_grading_page(self, obj): + """ + Method to get the use_new_grading_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_grading_page(course_key) + + def get_use_new_updates_page(self, obj): + """ + Method to get the use_new_updates_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_updates_page(course_key) + + def get_use_new_import_page(self, obj): + """ + Method to get the use_new_import_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_import_page(course_key) + + def get_use_new_export_page(self, obj): + """ + Method to get the use_new_export_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_export_page(course_key) + + def get_use_new_files_uploads_page(self, obj): + """ + Method to get the use_new_files_uploads_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_files_uploads_page(course_key) + + def get_use_new_video_uploads_page(self, obj): + """ + Method to get the use_new_video_uploads_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_video_uploads_page(course_key) + + def get_use_new_course_outline_page(self, obj): + """ + Method to get the use_new_course_outline_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_course_outline_page(course_key) + + def get_use_new_unit_page(self, obj): + """ + Method to get the use_new_unit_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_unit_page(course_key) + + def get_use_new_course_team_page(self, obj): + """ + Method to get the use_new_course_team_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_course_team_page(course_key) + + def get_use_new_certificates_page(self, obj): + """ + Method to get the use_new_certificates_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_certificates_page(course_key) + + def get_use_new_textbooks_page(self, obj): + """ + Method to get the use_new_textbooks_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_textbooks_page(course_key) + + def get_use_new_group_configurations_page(self, obj): + """ + Method to get the use_new_group_configurations_page switch + """ + course_key = self.get_course_key() + return toggles.use_new_group_configurations_page(course_key) + + def get_enable_course_optimizer(self, obj): + """ + Method to get the enable_course_optimizer waffle flag + """ + course_key = self.get_course_key() + return toggles.enable_course_optimizer(course_key) diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py index 4c3e2a4321..fa2a651f8a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/home.py @@ -41,8 +41,8 @@ class LibraryTabSerializer(serializers.Serializer): libraries = LibraryViewSerializer(many=True, required=False, allow_null=True) -class CourseHomeSerializer(serializers.Serializer): - """Serializer for course home""" +class StudioHomeSerializer(serializers.Serializer): + """Serializer for Studio home""" allow_course_reruns = serializers.BooleanField() allow_to_create_new_org = serializers.BooleanField() allow_unicode_course_id = serializers.BooleanField() @@ -58,12 +58,15 @@ class CourseHomeSerializer(serializers.Serializer): in_process_course_actions = UnsucceededCourseSerializer(many=True, required=False, allow_null=True) libraries = LibraryViewSerializer(many=True, required=False, allow_null=True) libraries_enabled = serializers.BooleanField() + libraries_v1_enabled = serializers.BooleanField() + libraries_v2_enabled = serializers.BooleanField() taxonomies_enabled = serializers.BooleanField() taxonomy_list_mfe_url = serializers.CharField() optimization_enabled = serializers.BooleanField() request_course_creator_url = serializers.CharField() rerun_creator_status = serializers.BooleanField() show_new_library_button = serializers.BooleanField() + show_new_library_v2_button = serializers.BooleanField() split_studio_home = serializers.BooleanField() studio_name = serializers.CharField() studio_short_name = serializers.CharField() 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 d72707ed78..dffa23349d 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py @@ -89,6 +89,7 @@ class ContainerHandlerSerializer(serializers.Serializer): unit_block_id = serializers.CharField(source="unit.location.block_id") subsection_location = serializers.CharField(source="subsection.location") course_sequence_ids = serializers.ListField(child=serializers.CharField()) + library_content_picker_url = serializers.CharField() def get_assets_url(self, obj): """ @@ -103,6 +104,18 @@ class ContainerHandlerSerializer(serializers.Serializer): 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 +126,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/urls.py b/cms/djangoapps/contentstore/rest_api/v1/urls.py index e2afe48c96..3492186797 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -17,6 +17,7 @@ from .views import ( CourseRerunView, CourseSettingsView, CourseVideosView, + CourseWaffleFlagsView, HomePageView, HomePageCoursesView, HomePageLibrariesView, @@ -131,6 +132,11 @@ urlpatterns = [ VerticalContainerView.as_view(), name="container_vertical" ), + re_path( + fr'^course_waffle_flags(?:/{COURSE_ID_PATTERN})?$', + CourseWaffleFlagsView.as_view(), + name="course_waffle_flags" + ), # Authoring API # Do not use under v1 yet (Nov. 23). The Authoring API is still experimental and the v0 versions should be used diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py index dfba1e63f7..89d8d56eaa 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/__init__.py @@ -5,6 +5,7 @@ from .certificates import CourseCertificatesView from .course_details import CourseDetailsView from .course_index import CourseIndexView from .course_rerun import CourseRerunView +from .course_waffle_flags import CourseWaffleFlagsView from .course_team import CourseTeamView from .grading import CourseGradingView from .group_configurations import CourseGroupConfigurationsView diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py new file mode 100644 index 0000000000..ba96ff2b4a --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/course_waffle_flags.py @@ -0,0 +1,73 @@ +""" API Views for course waffle flags """ + +from opaque_keys.edx.keys import CourseKey +from rest_framework.decorators import APIView +from rest_framework.response import Response + +from openedx.core.lib.api.view_utils import view_auth_classes + +from ..serializers import CourseWaffleFlagsSerializer + + +@view_auth_classes(is_authenticated=True) +class CourseWaffleFlagsView(APIView): + """ + API view to retrieve course waffle flag settings for a specific course. + + This view provides a GET endpoint that returns the status of various waffle + flags for a given course. It requires the user to be authenticated. + """ + + def get(self, request, course_id=None): + """ + Retrieve the waffle flag settings for the specified course. + + Args: + request (HttpRequest): The HTTP request object. + course_id (str, optional): The ID of the course for which to retrieve + the waffle flag settings. If not provided, + defaults to None. + + Returns: + Response: A JSON response containing the status of various waffle flags + for the specified course. + + **Example Request** + + GET /api/contentstore/v1/course_waffle_flags + GET /api/contentstore/v1/course_waffle_flags/course-v1:test+test+test + + **Response Values** + + A JSON response containing the status of various waffle flags + for the specified course. + + **Example Response** + + ```json + { + "use_new_home_page": true, + "use_new_custom_pages": true, + "use_new_schedule_details_page": true, + "use_new_advanced_settings_page": true, + "use_new_grading_page": true, + "use_new_updates_page": true, + "use_new_import_page": true, + "use_new_export_page": true, + "use_new_files_uploads_page": true, + "use_new_video_uploads_page": false, + "use_new_course_outline_page": true, + "use_new_unit_page": false, + "use_new_course_team_page": true, + "use_new_certificates_page": true, + "use_new_textbooks_page": true, + "use_new_group_configurations_page": true + } + ``` + """ + course_key = CourseKey.from_string(course_id) if course_id else None + serializer = CourseWaffleFlagsSerializer( + context={"course_key": course_key}, data={} + ) + serializer.is_valid(raise_exception=True) + return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/home.py b/cms/djangoapps/contentstore/rest_api/v1/views/home.py index ff476090ee..3de536d780 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/home.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/home.py @@ -8,7 +8,7 @@ from rest_framework.views import APIView from openedx.core.lib.api.view_utils import view_auth_classes from ....utils import get_home_context, get_course_context, get_library_context -from ..serializers import CourseHomeSerializer, CourseHomeTabSerializer, LibraryTabSerializer +from ..serializers import StudioHomeSerializer, CourseHomeTabSerializer, LibraryTabSerializer @view_auth_classes(is_authenticated=True) @@ -24,7 +24,7 @@ class HomePageView(APIView): description="Query param to filter by course org", )], responses={ - 200: CourseHomeSerializer, + 200: StudioHomeSerializer, 401: "The requester is not authenticated.", }, ) @@ -59,10 +59,14 @@ class HomePageView(APIView): "in_process_course_actions": [], "libraries": [], "libraries_enabled": true, + "libraries_v1_enabled": true, + "libraries_v2_enabled": true, + "library_authoring_mfe_url": "//localhost:3001/course/course-v1:edX+P315+2T2023", "optimization_enabled": true, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": true, "show_new_library_button": true, + "show_new_library_v2_button": true, "split_studio_home": false, "studio_name": "Studio", "studio_short_name": "Studio", @@ -84,7 +88,7 @@ class HomePageView(APIView): 'platform_name': settings.PLATFORM_NAME, 'user_is_active': request.user.is_active, }) - serializer = CourseHomeSerializer(home_context) + serializer = StudioHomeSerializer(home_context) return Response(serializer.data) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py new file mode 100644 index 0000000000..f0332d7f28 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py @@ -0,0 +1,129 @@ +""" +Unit tests for the course waffle flags view +""" + +from django.contrib.auth import get_user_model +from django.urls import reverse +from rest_framework import status + +from cms.djangoapps.contentstore.tests.utils import CourseTestCase +from openedx.core.djangoapps.waffle_utils.models import WaffleFlagCourseOverrideModel + +User = get_user_model() + + +class CourseWaffleFlagsViewTest(CourseTestCase): + """ + Tests for the CourseWaffleFlagsView endpoint, which returns waffle flag states + for a specific course or globally if no course ID is provided. + """ + + course_waffle_flags = [ + "use_new_custom_pages", + "use_new_schedule_details_page", + "use_new_advanced_settings_page", + "use_new_grading_page", + "use_new_updates_page", + "use_new_import_page", + "use_new_export_page", + "use_new_files_uploads_page", + "use_new_video_uploads_page", + "use_new_course_outline_page", + "use_new_unit_page", + "use_new_course_team_page", + "use_new_certificates_page", + "use_new_textbooks_page", + "use_new_group_configurations_page", + ] + + other_expected_waffle_flags = ["enable_course_optimizer"] + + def setUp(self): + """ + Set up test data and state before each test method. + + This method initializes the endpoint URL and creates a set of waffle flags + for the test course, setting each flag's value to `True`. + """ + super().setUp() + self.url = reverse("cms.djangoapps.contentstore:v1:course_waffle_flags") + self.create_waffle_flags(self.course_waffle_flags) + self.create_custom_waffle_flags() + + def create_custom_waffle_flags(self, enabled=True): + """ + Helper method to create waffle flags that are not part of `course_waffle_flags` and have + a different format. + """ + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag="contentstore.enable_course_optimizer", + course_id=self.course.id, + enabled=enabled, + ) + + def create_waffle_flags(self, flags, enabled=True): + """ + Helper method to create waffle flag entries in the database for the test course. + + Args: + flags (list): A list of flag names to set up. + enabled (bool): The value to set for each flag's enabled state. + """ + for flag in flags: + WaffleFlagCourseOverrideModel.objects.create( + waffle_flag=f"contentstore.new_studio_mfe.{flag}", + course_id=self.course.id, + enabled=enabled, + ) + + def expected_response(self, enabled=False): + """ + Generate an expected response dictionary based on the enabled flag. + + Args: + enabled (bool): State to assign to each waffle flag in the response. + + Returns: + dict: A dictionary with each flag set to the value of `enabled`. + """ + res = {flag: enabled for flag in self.course_waffle_flags} + for flag in self.other_expected_waffle_flags: + res[flag] = enabled + return res + + def test_get_course_waffle_flags_with_course_id(self): + """ + Test that waffle flags for a specific course are correctly returned when + a valid course ID is provided. + + Expected Behavior: + - The response should return HTTP 200 status. + - Each flag returned should be `True` as set up in the `setUp` method. + """ + course_url = reverse( + "cms.djangoapps.contentstore:v1:course_waffle_flags", + kwargs={"course_id": self.course.id}, + ) + + expected_response = self.expected_response(enabled=True) + expected_response["use_new_home_page"] = False + + response = self.client.get(course_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.data) + + def test_get_course_waffle_flags_without_course_id(self): + """ + Test that the default waffle flag states are returned when no course ID is provided. + + Expected Behavior: + - The response should return HTTP 200 status. + - Each flag returned should default to `False`, representing the global + default state for each flag. + """ + expected_response = self.expected_response(enabled=False) + expected_response["use_new_home_page"] = False + + response = self.client.get(self.url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(expected_response, response.data) 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 c3c9652e5d..a4a6909c5d 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 @@ -2,7 +2,9 @@ Unit tests for home page view. """ import ddt +import pytz from collections import OrderedDict +from datetime import datetime, timedelta from django.conf import settings from django.test import override_settings from django.urls import reverse @@ -33,12 +35,7 @@ class HomePageViewTest(CourseTestCase): def setUp(self): super().setUp() self.url = reverse("cms.djangoapps.contentstore:v1:home") - - def test_home_page_courses_response(self): - """Check successful response content""" - response = self.client.get(self.url) - - expected_response = { + self.expected_response = { "allow_course_reruns": True, "allow_to_create_new_org": False, "allow_unicode_course_id": False, @@ -51,12 +48,15 @@ class HomePageViewTest(CourseTestCase): "in_process_course_actions": [], "libraries": [], "libraries_enabled": True, + "libraries_v1_enabled": True, + "libraries_v2_enabled": False, "taxonomies_enabled": True, "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies', "optimization_enabled": False, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": True, "show_new_library_button": True, + "show_new_library_v2_button": True, "split_studio_home": False, "studio_name": settings.STUDIO_NAME, "studio_short_name": settings.STUDIO_SHORT_NAME, @@ -66,6 +66,21 @@ class HomePageViewTest(CourseTestCase): "user_is_active": True, } + def test_home_page_studio_response(self): + """Check successful response content""" + response = self.client.get(self.url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertDictEqual(self.expected_response, response.data) + + @override_settings(MEILISEARCH_ENABLED=True) + def test_home_page_studio_with_meilisearch_enabled(self): + """Check response content when Meilisearch is enabled""" + response = self.client.get(self.url) + + expected_response = self.expected_response + expected_response["libraries_v2_enabled"] = True + self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(expected_response, response.data) @@ -88,12 +103,13 @@ class HomePageCoursesViewTest(CourseTestCase): def setUp(self): super().setUp() self.url = reverse("cms.djangoapps.contentstore:v1:courses") - CourseOverviewFactory.create( + self.course_overview = CourseOverviewFactory.create( id=self.course.id, org=self.course.org, display_name=self.course.display_name, display_number_with_default=self.course.number, ) + self.non_staff_client, _ = self.create_non_staff_authed_user_client() def test_home_page_response(self): """Check successful response content""" @@ -105,7 +121,7 @@ class HomePageCoursesViewTest(CourseTestCase): "courses": [{ "course_key": course_id, "display_name": self.course.display_name, - "lms_link": f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}', + "lms_link": f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}', "number": self.course.number, "org": self.course.org, "rerun_link": f'/course_rerun/{course_id}', @@ -119,6 +135,7 @@ class HomePageCoursesViewTest(CourseTestCase): print(response.data) self.assertDictEqual(expected_response, response.data) + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) def test_home_page_response_with_api_v2(self): """Check successful response content with api v2 modifications. @@ -132,7 +149,7 @@ class HomePageCoursesViewTest(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), ("number", self.course.number), ("org", self.course.org), ("rerun_link", f'/course_rerun/{course_id}'), @@ -143,12 +160,88 @@ class HomePageCoursesViewTest(CourseTestCase): "in_process_course_actions": [], } - with override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API): - response = self.client.get(self.url) + response = self.client.get(self.url) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(expected_response, response.data) + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) + @ddt.data( + ("active_only", "true", 2, 0), + ("archived_only", "true", 0, 1), + ("search", "sample", 1, 0), + ("search", "demo", 0, 1), + ("order", "org", 2, 1), + ("order", "display_name", 2, 1), + ("order", "number", 2, 1), + ("order", "run", 2, 1) + ) + @ddt.unpack + def test_filter_and_ordering_courses( + self, + filter_key, + filter_value, + expected_active_length, + expected_archived_length + ): + """Test home page with org filter and ordering for a staff user. + + The test creates an active/archived course, and then filters/orders them using the query parameters. + """ + archived_course_key = self.store.make_course_key("demo-org", "demo-number", "demo-run") + CourseOverviewFactory.create( + display_name="Course (Demo)", + id=archived_course_key, + org=archived_course_key.org, + end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), + ) + active_course_key = self.store.make_course_key("sample-org", "sample-number", "sample-run") + CourseOverviewFactory.create( + display_name="Course (Sample)", + id=active_course_key, + org=active_course_key.org, + ) + + response = self.client.get(self.url, {filter_key: filter_value}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["archived_courses"]), expected_archived_length) + self.assertEqual(len(response.data["courses"]), expected_active_length) + + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("order", "org"), + ) + @ddt.unpack + def test_filter_and_ordering_no_courses_staff(self, filter_key, filter_value): + """Test home page with org filter and ordering when there are no courses for a staff user.""" + self.course_overview.delete() + + response = self.client.get(self.url, {filter_key: filter_value}) + + self.assertEqual(len(response.data["courses"]), 0) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("order", "org"), + ) + @ddt.unpack + def test_home_page_response_no_courses_non_staff(self, filter_key, filter_value): + """Test home page with org filter and ordering when there are no courses for a non-staff user.""" + self.course_overview.delete() + + response = self.non_staff_client.get(self.url) + + self.assertEqual(len(response.data["courses"]), 0) + self.assertEqual(response.status_code, status.HTTP_200_OK) + @override_waffle_switch(ENABLE_GLOBAL_STAFF_OPTIMIZATION, True) def test_org_query_if_passed(self): """Test home page when org filter passed as a query param""" 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 d3fc371982..7cac074a43 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 @@ class BaseXBlockContainer(CourseTestCase): 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 @@ class ContainerVerticalViewTest(BaseXBlockContainer): "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 @@ class ContainerVerticalViewTest(BaseXBlockContainer): "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 670b94afbb..0798c341cc 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 @@ from cms.djangoapps.contentstore.rest_api.v1.serializers import ( 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 @@ class VerticalContainerView(APIView, ContainerHandlerMixin): "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 @@ class VerticalContainerView(APIView, ContainerHandlerMixin): "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 @@ class VerticalContainerView(APIView, ContainerHandlerMixin): "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 @@ class VerticalContainerView(APIView, ContainerHandlerMixin): 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 @@ class VerticalContainerView(APIView, ContainerHandlerMixin): "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 ad61cc9370..3e653d07fb 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 73ddde9844..e69de29bb2 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 0000000000..8c0d651910 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -0,0 +1,253 @@ +""" +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") + if isinstance(sync_param, str): + sync_param = sync_param.lower() + if sync_param not in ["true", "false", True, False]: + raise ValidationError({"sync": "must be 'true' or 'false'"}) + try: + if sync_param == "true" or sync_param is 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 0000000000..92da28bde9 --- /dev/null +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -0,0 +1,276 @@ +""" +Unit tests for /api/contentstore/v2/downstreams/* JSON APIs. +""" +from unittest.mock import patch +from django.conf import settings + +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_LIB_KEY = "lib:OpenedX:CSPROB3" +MOCK_UPSTREAM_REF = "lb:OpenedX:CSPROB3:html:843b4c73-1e2d-4ced-a0ff-24e503cdb3e4" +MOCK_UPSTREAM_LINK = "{mfe_url}/library/{lib_key}/components?usageKey={usage_key}".format( + mfe_url=settings.COURSE_AUTHORING_MICROFRONTEND_URL, + lib_key=MOCK_LIB_KEY, + usage_key=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 + assert response.data['upstream_link'] == MOCK_UPSTREAM_LINK + + @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 + assert response.data['upstream_link'] is None + + 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 + assert response.data['upstream_link'] is None + + +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/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index c0ffa50903..e773e7f213 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -45,6 +45,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): org=archived_course_key.org, end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), ) + self.non_staff_client, _ = self.create_non_staff_authed_user_client() def test_home_page_response(self): """Get list of courses available to the logged in user. @@ -62,7 +63,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): OrderedDict([ ("course_key", course_id), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{course_id}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{course_id}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -76,7 +77,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' + f'{settings.LMS_ROOT_URL}/courses/{archived_course_id}/jump_to/{self.archived_course.location}' ), ( "cms_link", @@ -139,7 +140,7 @@ class HomePageCoursesViewV2Test(CourseTestCase): self.assertEqual(response.data["results"]["courses"], [OrderedDict([ ("course_key", str(self.course.id)), ("display_name", self.course.display_name), - ("lms_link", f'//{settings.LMS_BASE}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), + ("lms_link", f'{settings.LMS_ROOT_URL}/courses/{str(self.course.id)}/jump_to/{self.course.location}'), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.course.id)}'), ("number", self.course.number), ("org", self.course.org), @@ -164,7 +165,11 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + '{url_root}/courses/{course_id}/jump_to/{location}'.format( + url_root=settings.LMS_ROOT_URL, + course_id=str(self.archived_course.id), + location=self.archived_course.location + ), ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), @@ -190,7 +195,11 @@ class HomePageCoursesViewV2Test(CourseTestCase): ("display_name", self.archived_course.display_name), ( "lms_link", - f'//{settings.LMS_BASE}/courses/{str(self.archived_course.id)}/jump_to/{self.archived_course.location}', + '{url_root}/courses/{course_id}/jump_to/{location}'.format( + url_root=settings.LMS_ROOT_URL, + course_id=str(self.archived_course.id), + location=self.archived_course.location + ), ), ("cms_link", f'//{settings.CMS_BASE}{reverse_course_url("course_handler", self.archived_course.id)}'), ("number", self.archived_course.number), @@ -239,3 +248,100 @@ class HomePageCoursesViewV2Test(CourseTestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) mock_modulestore().get_course_summaries.assert_called_once() mock_course_overview.get_all_courses.assert_not_called() + + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("order", "org"), + ("page", 1), + ) + @ddt.unpack + def test_if_empty_list_of_courses(self, query_param, value): + """Get list of courses when no courses are available. + + Expected result: + - An empty list of courses available to the logged in user. + """ + self.active_course.delete() + self.archived_course.delete() + + response = self.client.get(self.api_v2_url, {query_param: value}) + + self.assertEqual(len(response.data['results']['courses']), 0) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + @override_settings(FEATURES=FEATURES_WITH_HOME_PAGE_COURSE_V2_API) + @ddt.data( + ("active_only", "true", 2, 0), + ("archived_only", "true", 0, 1), + ("search", "foo", 1, 0), + ("search", "demo", 0, 1), + ("order", "org", 2, 1), + ("order", "display_name", 2, 1), + ("order", "number", 2, 1), + ("order", "run", 2, 1) + ) + @ddt.unpack + def test_filter_and_ordering_courses( + self, + filter_key, + filter_value, + expected_active_length, + expected_archived_length + ): + """Get list of courses when filter and ordering are applied. + + This test creates two courses besides the default courses created in the setUp method. + Then filters and orders them based on the filter_key and filter_value passed as query parameters. + + Expected result: + - A list of courses available to the logged in user for the specified filter and order. + """ + archived_course_key = self.store.make_course_key("demo-org", "demo-number", "demo-run") + CourseOverviewFactory.create( + display_name="Course (Demo)", + id=archived_course_key, + org=archived_course_key.org, + end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), + ) + active_course_key = self.store.make_course_key("foo-org", "foo-number", "foo-run") + CourseOverviewFactory.create( + display_name="Course (Foo)", + id=active_course_key, + org=active_course_key.org, + ) + + response = self.client.get(self.api_v2_url, {filter_key: filter_value}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + len([course for course in response.data["results"]["courses"] if course["is_active"]]), + expected_active_length + ) + self.assertEqual( + len([course for course in response.data["results"]["courses"] if not course["is_active"]]), + expected_archived_length + ) + + @ddt.data( + ("active_only", "true"), + ("archived_only", "true"), + ("search", "sample"), + ("order", "org"), + ("page", 1), + ) + @ddt.unpack + def test_if_empty_list_of_courses_non_staff(self, query_param, value): + """Get list of courses when no courses are available for non-staff users. + + Expected result: + - An empty list of courses available to the logged in user. + """ + self.active_course.delete() + self.archived_course.delete() + + response = self.non_staff_client.get(self.api_v2_url, {query_param: value}) + + self.assertEqual(len(response.data["results"]["courses"]), 0) + self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/tests/test_courseware_index.py b/cms/djangoapps/contentstore/tests/test_courseware_index.py index 98a60dce90..3ab3fa373f 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -504,6 +504,11 @@ class TestCoursewareSearchIndexer(MixedWithOptionsTestCase): """ Test for removing course from CourseAboutSearchIndexer """ self._test_delete_course_from_search_index_after_course_deletion(self.store) + def test_empty_course(self): + empty_course = CourseFactory.create(modulestore=self.store, start=datetime(2015, 3, 1, tzinfo=UTC)) + added_to_index = CoursewareSearchIndexer.do_course_reindex(self.store, empty_course.id) + assert added_to_index == 0 + @patch('django.conf.settings.SEARCH_ENGINE', 'search.tests.utils.ForceRefreshElasticSearchEngine') @ddt.ddt diff --git a/cms/djangoapps/contentstore/tests/test_filters.py b/cms/djangoapps/contentstore/tests/test_filters.py new file mode 100644 index 0000000000..13bdfa0747 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_filters.py @@ -0,0 +1,98 @@ +""" +Unit tests for the asset upload endpoint. +""" +from datetime import datetime +from urllib.parse import urljoin + +from pytz import UTC + +from django.test import override_settings +from cms.djangoapps.contentstore import asset_storage_handlers +from opaque_keys.edx.locator import CourseLocator +from openedx_filters import PipelineStep +from xmodule.contentstore.content import StaticContent +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class TestPageURLRequestedPipelineStep(PipelineStep): + """ + Utility class used when getting steps for pipeline. + """ + + def run_filter(self, url, org): # pylint: disable=arguments-differ + """Pipeline step that modifies lms url creation.""" + url = "https://lms-url-creation" + org = "org" + return { + "url": url, + "org": org, + } + + +class LMSPageURLRequestedFiltersTest(ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the lms url requested process. + This class guarantees that the following filters are triggered during the microsite render: + - LMSPageURLRequested + """ + + def setUp(self): # pylint: disable=arguments-differ + super().setUp() + self.upload_date = datetime(2013, 6, 1, 10, 30, tzinfo=UTC) + self.content_type = 'image/jpg' + self.course_key = CourseLocator('org', 'class', 'run') + self.location = self.course_key.make_asset_key('asset', 'my_file_name.jpg') + self.thumbnail_location = self.course_key.make_asset_key('thumbnail', 'my_file_name_thumb.jpg') + + self.asset_url = StaticContent.serialize_asset_key_with_slash(self.location) + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.course_authoring.lms.page.url.requested.v1": { + "pipeline": [ + "common.djangoapps.util.tests.test_filters.TestPageURLRequestedPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_lms_url_requested_filter_executed(self): + """ + Test that filter get new LMS URL for asset URL generation + based on the course organization settings for org. + Expected result: + - LMSPageURLRequested is triggered and executes TestPageURLRequestedPipelineStep. + - The arguments that the receiver gets are the arguments used by the filter. + """ + output = asset_storage_handlers.get_asset_json( + "my_file", + self.content_type, + self.upload_date, + self.location, + self.thumbnail_location, + True, + self.course_key + ) + + self.assertEqual(output.get('external_url'), urljoin('https://lms-url-creation', self.asset_url)) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}, LMS_ROOT_URL="https://lms-base") + def test_lms_url_requested_without_filter_configuration(self): + """ + Test that filter get new LMS URL for asset URL generation + based on LMS_ROOT_URL settings because OPEN_EDX_FILTERS_CONFIG is not set. + Expected result: + - Returns the asset URL with domain base LMS_ROOT_URL. + - The get process ends successfully. + """ + output = asset_storage_handlers.get_asset_json( + "my_file", + self.content_type, + self.upload_date, + self.location, + self.thumbnail_location, + True, + self.course_key + ) + + self.assertEqual(output.get('external_url'), urljoin('https://lms-base', self.asset_url)) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 9c478ddfe5..a913797970 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -960,4 +960,32 @@ class CourseUpdateNotificationTests(ModuleStoreTestCase): send_course_update_notification(self.course.id, content, self.user) assert Notification.objects.all().count() == 1 notification = Notification.objects.first() - assert notification.content == "

content

" + assert notification.content == "

content

" + + def test_if_content_is_plain_text(self): + """ + Test that the course_update notification is sent. + """ + user = UserFactory() + CourseEnrollment.enroll(user=user, course_key=self.course.id) + assert Notification.objects.all().count() == 0 + content = "

content

Sub content

heading

" + send_course_update_notification(self.course.id, content, self.user) + assert Notification.objects.all().count() == 1 + notification = Notification.objects.first() + assert notification.content == "

content Sub content heading

" + + def test_if_html_unescapes(self): + """ + Tests if html unescapes when creating content of course update notification + """ + user = UserFactory() + CourseEnrollment.enroll(user=user, course_key=self.course.id) + assert Notification.objects.all().count() == 0 + content = "

<p> &nbsp;</p>
"\ + "<p>abcd</p>
"\ + "<p>&nbsp;</p>

" + send_course_update_notification(self.course.id, content, self.user) + assert Notification.objects.all().count() == 1 + notification = Notification.objects.first() + assert notification.content == "

abcd

" diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py index 7c3a369fed..39b793f479 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -2,6 +2,7 @@ CMS feature toggles. """ from edx_toggles.toggles import SettingDictToggle, WaffleFlag +from openedx.core.djangoapps.content.search import api as search_api from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag # .. toggle_name: FEATURES['ENABLE_EXPORT_GIT'] @@ -593,3 +594,96 @@ def default_enable_flexible_peer_openassessments(course_key): level to opt in/out of rolling forward this feature. """ return DEFAULT_ENABLE_FLEXIBLE_PEER_OPENASSESSMENTS.is_enabled(course_key) + + +# .. toggle_name: FEATURES['ENABLE_CONTENT_LIBRARIES'] +# .. toggle_implementation: SettingDictToggle +# .. toggle_default: True +# .. toggle_description: Enables use of the legacy and v2 libraries waffle flags. +# Note that legacy content libraries are only supported in courses using split mongo. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2015-03-06 +# .. toggle_target_removal_date: 2025-04-09 +# .. toggle_warning: This flag is deprecated in Sumac, and will be removed in favor of the disable_legacy_libraries and +# disable_new_libraries waffle flags. +ENABLE_CONTENT_LIBRARIES = SettingDictToggle( + "FEATURES", "ENABLE_CONTENT_LIBRARIES", default=True, module_name=__name__ +) + +# .. toggle_name: contentstore.new_studio_mfe.disable_legacy_libraries +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Hides legacy (v1) Libraries tab in Authoring MFE. +# This toggle interacts with ENABLE_CONTENT_LIBRARIES toggle: if this is disabled, then legacy libraries are also +# disabled. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2024-10-02 +# .. toggle_target_removal_date: 2025-04-09 +# .. toggle_tickets: https://github.com/openedx/frontend-app-authoring/issues/1334 +# .. toggle_warning: Legacy libraries are deprecated in Sumac, cf https://github.com/openedx/edx-platform/issues/32457 +DISABLE_LEGACY_LIBRARIES = WaffleFlag( + f'{CONTENTSTORE_NAMESPACE}.new_studio_mfe.disable_legacy_libraries', + __name__, + CONTENTSTORE_LOG_PREFIX, +) + + +def libraries_v1_enabled(): + """ + Returns a boolean if Libraries V2 is enabled in the new Studio Home. + """ + return ( + ENABLE_CONTENT_LIBRARIES.is_enabled() and + not DISABLE_LEGACY_LIBRARIES.is_enabled() + ) + + +# .. toggle_name: contentstore.new_studio_mfe.disable_new_libraries +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Hides new Libraries v2 tab in Authoring MFE. +# This toggle interacts with settings.MEILISEARCH_ENABLED and ENABLE_CONTENT_LIBRARIES toggle: if these flags are +# False, then v2 libraries are also disabled. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2024-10-02 +# .. toggle_target_removal_date: 2025-04-09 +# .. toggle_tickets: https://github.com/openedx/frontend-app-authoring/issues/1334 +# .. toggle_warning: Libraries v2 are in beta for Sumac, will be fully supported in Teak. +DISABLE_NEW_LIBRARIES = WaffleFlag( + f'{CONTENTSTORE_NAMESPACE}.new_studio_mfe.disable_new_libraries', + __name__, + CONTENTSTORE_LOG_PREFIX, +) + + +def libraries_v2_enabled(): + """ + Returns a boolean if Libraries V2 is enabled in the new Studio Home. + + Requires the ENABLE_CONTENT_LIBRARIES feature flag to be enabled, plus Meilisearch. + """ + return ( + ENABLE_CONTENT_LIBRARIES.is_enabled() and + search_api.is_meilisearch_enabled() and + not DISABLE_NEW_LIBRARIES.is_enabled() + ) + + +# .. toggle_name: contentstore.enable_course_optimizer +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: This flag enables the use of unique anonymous_user_id during studio preview +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2022-05-04 +# .. toggle_target_removal_date: 2022-05-30 +# .. toggle_tickets: MST-1455 +ENABLE_COURSE_OPTIMIZER = CourseWaffleFlag( + f'{CONTENTSTORE_NAMESPACE}.enable_course_optimizer', __name__ +) + + +def enable_course_optimizer(course_id): + """ + Returns a boolean if individualized anonymous_user_id is enabled on the course + """ + return ENABLE_COURSE_OPTIMIZER.is_enabled(course_id) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 035e44d5ce..7023bcaefa 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -3,12 +3,13 @@ Common utility functions useful throughout the contentstore """ from __future__ import annotations import configparser +import html import logging import re from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone -from urllib.parse import quote_plus +from urllib.parse import quote_plus, urlencode, urlunparse, urlparse from uuid import uuid4 from bs4 import BeautifulSoup @@ -34,7 +35,32 @@ from milestones import api as milestones_api from pytz import UTC from xblock.fields import Scope -from cms.djangoapps.contentstore.toggles import exam_setting_view_enabled +from cms.djangoapps.contentstore.toggles import ( + exam_setting_view_enabled, + libraries_v1_enabled, + libraries_v2_enabled, + split_library_view_on_dashboard, + use_new_advanced_settings_page, + use_new_course_outline_page, + use_new_certificates_page, + use_new_export_page, + use_new_files_uploads_page, + use_new_grading_page, + use_new_group_configurations_page, + use_new_course_team_page, + use_new_home_page, + use_new_import_page, + use_new_schedule_details_page, + use_new_text_editor, + use_new_textbooks_page, + use_new_unit_page, + use_new_updates_page, + use_new_video_editor, + use_new_video_uploads_page, + use_new_custom_pages, +) +from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.djangoapps.models.settings.course_metadata import CourseMetadata from common.djangoapps.course_action_state.models import CourseRerunUIStateManager, CourseRerunState from common.djangoapps.course_action_state.managers import CourseActionStateItemNotFoundError from common.djangoapps.course_modes.models import CourseMode @@ -75,29 +101,6 @@ from openedx.core.lib.html_to_text import html_to_text from openedx.features.content_type_gating.models import ContentTypeGatingConfig from openedx.features.content_type_gating.partitions import CONTENT_TYPE_GATING_SCHEME from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML -from cms.djangoapps.contentstore.toggles import ( - split_library_view_on_dashboard, - use_new_advanced_settings_page, - use_new_course_outline_page, - use_new_certificates_page, - use_new_export_page, - use_new_files_uploads_page, - use_new_grading_page, - use_new_group_configurations_page, - use_new_course_team_page, - use_new_home_page, - use_new_import_page, - use_new_schedule_details_page, - use_new_text_editor, - use_new_textbooks_page, - use_new_unit_page, - use_new_updates_page, - use_new_video_editor, - use_new_video_uploads_page, - use_new_custom_pages, -) -from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from cms.djangoapps.models.settings.course_metadata import CourseMetadata 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 @@ -190,31 +193,30 @@ def get_lms_link_for_item(location, preview=False): """ assert isinstance(location, UsageKey) - # checks LMS_BASE value in site configuration for the given course_org_filter(org) - # if not found returns settings.LMS_BASE + # checks LMS_ROOT_URL value in site configuration for the given course_org_filter(org) + # if not found returns settings.LMS_ROOT_URL lms_base = SiteConfiguration.get_value_for_org( location.org, - "LMS_BASE", - settings.LMS_BASE + "LMS_ROOT_URL", + settings.LMS_ROOT_URL ) + query_string = '' if lms_base is None: return None if preview: - # checks PREVIEW_LMS_BASE value in site configuration for the given course_org_filter(org) - # if not found returns settings.FEATURES.get('PREVIEW_LMS_BASE') - lms_base = SiteConfiguration.get_value_for_org( - location.org, - "PREVIEW_LMS_BASE", - settings.FEATURES.get('PREVIEW_LMS_BASE') - ) + params = {'preview': '1'} + query_string = urlencode(params) - return "//{lms_base}/courses/{course_key}/jump_to/{location}".format( - lms_base=lms_base, + url_parts = list(urlparse(lms_base)) + url_parts[2] = '/courses/{course_key}/jump_to/{location}'.format( course_key=str(location.course_key), location=str(location), ) + url_parts[4] = query_string + + return urlunparse(url_parts) def get_lms_link_for_certificate_web_view(course_key, mode): @@ -427,6 +429,18 @@ def get_course_outline_url(course_locator) -> str: return course_outline_url +def get_library_content_picker_url(course_locator) -> str: + """ + Gets course authoring microfrontend library content picker URL for the given parent block. + """ + content_picker_url = None + if libraries_v2_enabled(): + mfe_base_url = get_course_authoring_url(course_locator) + content_picker_url = f'{mfe_base_url}/component-picker?variant=published' + + return content_picker_url + + def get_unit_url(course_locator, unit_locator) -> str: """ Gets course authoring microfrontend URL for unit page view. @@ -1536,11 +1550,13 @@ def get_library_context(request, request_is_json=False): _format_library_for_view, ) from cms.djangoapps.contentstore.views.library import ( - LIBRARIES_ENABLED, user_can_view_create_library_button, ) + from openedx.core.djangoapps.content_libraries.api import ( + user_can_create_library, + ) - libraries = _accessible_libraries_iter(request.user) if LIBRARIES_ENABLED else [] + libraries = _accessible_libraries_iter(request.user) if libraries_v1_enabled() else [] data = { 'libraries': [_format_library_for_view(lib, request) for lib in libraries], } @@ -1550,8 +1566,9 @@ def get_library_context(request, request_is_json=False): **data, 'in_process_course_actions': [], 'courses': [], - 'libraries_enabled': LIBRARIES_ENABLED, + 'libraries_enabled': libraries_v1_enabled(), 'show_new_library_button': user_can_view_create_library_button(request.user) and request.user.is_active, + 'show_new_library_v2_button': user_can_create_library(request.user), 'user': request.user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), @@ -1671,9 +1688,11 @@ def get_home_context(request, no_course=False): ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) from cms.djangoapps.contentstore.views.library import ( - LIBRARIES_ENABLED, user_can_view_create_library_button, ) + from openedx.core.djangoapps.content_libraries.api import ( + user_can_create_library, + ) active_courses = [] archived_courses = [] @@ -1687,7 +1706,7 @@ def get_home_context(request, no_course=False): if not no_course: active_courses, archived_courses, in_process_course_actions = get_course_context(request) - if not split_library_view_on_dashboard() and LIBRARIES_ENABLED and not no_course: + if not split_library_view_on_dashboard() and libraries_v1_enabled() and not no_course: libraries = get_library_context(request, True)['libraries'] home_context = { @@ -1695,11 +1714,14 @@ def get_home_context(request, no_course=False): 'split_studio_home': split_library_view_on_dashboard(), 'archived_courses': archived_courses, 'in_process_course_actions': in_process_course_actions, - 'libraries_enabled': LIBRARIES_ENABLED, + 'libraries_enabled': libraries_v1_enabled(), + 'libraries_v1_enabled': libraries_v1_enabled(), + 'libraries_v2_enabled': libraries_v2_enabled(), 'taxonomies_enabled': not is_tagging_feature_disabled(), 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, 'show_new_library_button': user_can_view_create_library_button(user), + 'show_new_library_v2_button': user_can_create_library(user), 'user': user, 'request_course_creator_url': reverse('request_course_creator'), 'course_creator_status': _get_course_creator_status(user), @@ -2041,6 +2063,7 @@ def get_container_handler_context(request, usage_key, course, xblock): # pylint 'user_clipboard': user_clipboard, 'is_fullwidth_content': is_library_xblock, 'course_sequence_ids': course_sequence_ids, + 'library_content_picker_url': get_library_content_picker_url(course.id), } return context @@ -2243,23 +2266,11 @@ def clean_html_body(html_body): """ Get html body, remove tags and limit to 500 characters """ + html_body = html.unescape(html_body).strip() html_body = BeautifulSoup(Truncator(html_body).chars(500, html=True), 'html.parser') - - tags_to_remove = [ - "a", "link", # Link Tags - "img", "picture", "source", # Image Tags - "video", "track", # Video Tags - "audio", # Audio Tags - "embed", "object", "iframe", # Embedded Content - "script" - ] - - # Remove the specified tags while keeping their content - for tag in tags_to_remove: - for match in html_body.find_all(tag): - match.unwrap() - - return str(html_body) + text_content = html_body.get_text(separator=" ").strip() + text_content = text_content.replace('\n', '').replace('\r', '') + return text_content def send_course_update_notification(course_key, content, user): diff --git a/cms/djangoapps/contentstore/video_storage_handlers.py b/cms/djangoapps/contentstore/video_storage_handlers.py index 6827b295b0..4cc5c738b5 100644 --- a/cms/djangoapps/contentstore/video_storage_handlers.py +++ b/cms/djangoapps/contentstore/video_storage_handlers.py @@ -967,26 +967,38 @@ def get_course_youtube_edx_video_ids(course_id): """ Get a list of youtube edx_video_ids """ - error_msg = "Invalid course_key: '%s'." % course_id - try: + invalid_key_error_msg = "Invalid course_key: '%s'." % course_id + unexpected_error_msg = "Unexpected error occurred for course_id: '%s'." % course_id + + try: # lint-amnesty, pylint: disable=too-many-nested-blocks course_key = CourseKey.from_string(course_id) course = modulestore().get_course(course_key) - except InvalidKeyError: - return JsonResponse({'error': error_msg}, status=500) - blocks = [] - block_yt_field = 'youtube_id_1_0' - block_edx_id_field = 'edx_video_id' - if hasattr(course, 'get_children'): - for section in course.get_children(): - for subsection in section.get_children(): - for vertical in subsection.get_children(): - for block in vertical.get_children(): - blocks.append(block) - edx_video_ids = [] - for block in blocks: - if hasattr(block, block_yt_field) and getattr(block, block_yt_field): - if getattr(block, block_edx_id_field): - edx_video_ids.append(getattr(block, block_edx_id_field)) + blocks = [] + block_yt_field = 'youtube_id_1_0' + block_edx_id_field = 'edx_video_id' + if hasattr(course, 'get_children'): + for section in course.get_children(): + for subsection in section.get_children(): + for vertical in subsection.get_children(): + for block in vertical.get_children(): + blocks.append(block) + + edx_video_ids = [] + for block in blocks: + if hasattr(block, block_yt_field) and getattr(block, block_yt_field): + if getattr(block, block_edx_id_field): + edx_video_ids.append(getattr(block, block_edx_id_field)) + + except InvalidKeyError as error: + LOGGER.exception( + f"InvalidKeyError occurred while getting YouTube video IDs for course_id: {course_id}: {error}" + ) + return JsonResponse({'error': invalid_key_error_msg}, status=500) + except Exception as error: + LOGGER.exception( + f"Unexpected error occurred while getting YouTube video IDs for course_id: {course_id}: {error}" + ) + return JsonResponse({'error': unexpected_error_msg}, status=500) return JsonResponse({'edx_video_ids': edx_video_ids}, status=200) diff --git a/cms/djangoapps/contentstore/views/block.py b/cms/djangoapps/contentstore/views/block.py index 4d6c17838e..e6b41dc261 100644 --- a/cms/djangoapps/contentstore/views/block.py +++ b/cms/djangoapps/contentstore/views/block.py @@ -135,6 +135,8 @@ def xblock_handler(request, usage_key_string=None): if duplicate_source_locator is not present :staged_content: use "clipboard" to paste from the OLX user's clipboard. (Incompatible with all other fields except parent_locator) + :library_content_key: the key of the library content to add. (Incompatible with + all other fields except parent_locator) The locator (unicode representation of a UsageKey) for the created xblock (minus children) is returned. """ return handle_xblock(request, usage_key_string) diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index ba767df78d..914c078846 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -11,6 +11,7 @@ from django.core.exceptions import PermissionDenied from django.http import Http404, HttpResponseBadRequest from django.shortcuts import redirect from django.utils.translation import gettext as _ +from django.views.decorators.clickjacking import xframe_options_exempt from django.views.decorators.http import require_GET from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import UsageKey @@ -25,7 +26,12 @@ from common.djangoapps.student.auth import has_course_author_access from common.djangoapps.xblock_django.api import authorable_xblocks, disabled_xblocks from common.djangoapps.xblock_django.models import XBlockStudioConfigurationFlag from cms.djangoapps.contentstore.helpers import is_unit -from cms.djangoapps.contentstore.toggles import use_new_problem_editor, use_new_unit_page +from cms.djangoapps.contentstore.toggles import ( + libraries_v1_enabled, + libraries_v2_enabled, + use_new_problem_editor, + use_new_unit_page, +) from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import load_services_for_studio from openedx.core.lib.xblock_utils import get_aside_from_xblock, is_xblock_aside from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration @@ -35,13 +41,26 @@ from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, py __all__ = [ 'container_handler', - 'component_handler' + 'component_handler', + 'container_embed_handler', ] log = logging.getLogger(__name__) # NOTE: This list is disjoint from ADVANCED_COMPONENT_TYPES -COMPONENT_TYPES = ['discussion', 'library', 'html', 'openassessment', 'problem', 'video', 'drag-and-drop-v2'] +COMPONENT_TYPES = [ + 'html', + 'video', + 'problem', + 'itembank', + 'library_v2', # Not an XBlock + 'library', + 'discussion', + 'openassessment', + 'drag-and-drop-v2', +] + +BETA_COMPONENT_TYPES = ['library_v2', 'itembank'] ADVANCED_COMPONENT_TYPES = sorted({name for name, class_ in XBlock.load_classes()} - set(COMPONENT_TYPES)) @@ -56,7 +75,7 @@ CONTAINER_TEMPLATES = [ "add-xblock-component-support-legend", "add-xblock-component-support-level", "add-xblock-component-menu-problem", "xblock-string-field-editor", "xblock-access-editor", "publish-xblock", "publish-history", "tag-list", "unit-outline", "container-message", "container-access", "license-selector", "copy-clipboard-button", - "edit-title-button", + "edit-title-button", "edit-upstream-alert", ] @@ -95,6 +114,10 @@ def _load_mixed_class(category): """ Load an XBlock by category name, and apply all defined mixins """ + # Libraries v2 content doesn't have an XBlock. + if category == 'library_v2': + return None + component_class = XBlock.load_class(category) mixologist = Mixologist(settings.XBLOCK_MIXINS) return mixologist.mix(component_class) @@ -141,6 +164,36 @@ def container_handler(request, usage_key_string): # pylint: disable=too-many-st return HttpResponseBadRequest("Only supports HTML requests") +@require_GET +@login_required +@xframe_options_exempt +def container_embed_handler(request, usage_key_string): # pylint: disable=too-many-statements + """ + Returns an HttpResponse with HTML content for the container XBlock. + The returned HTML is a chromeless rendering of the XBlock. + + GET + html: returns the HTML page for editing a container + json: not currently supported + """ + + # Avoiding a circular dependency + from ..utils import get_container_handler_context + + try: + usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError: # Raise Http404 on invalid 'usage_key_string' + return HttpResponseBadRequest() + with modulestore().bulk_operations(usage_key.course_key): + try: + course, xblock, lms_link, preview_lms_link = _get_item_in_course(request, usage_key) + except ItemNotFoundError: + raise Http404 # lint-amnesty, pylint: disable=raise-missing-from + + container_handler_context = get_container_handler_context(request, usage_key, course, xblock) + return render_to_response('container_chromeless.html', container_handler_context) + + def get_component_templates(courselike, library=False): # lint-amnesty, pylint: disable=too-many-statements """ Returns the applicable component templates that can be used by the specified course or library. @@ -215,7 +268,9 @@ def get_component_templates(courselike, library=False): # lint-amnesty, pylint: 'problem': _("Problem"), 'video': _("Video"), 'openassessment': _("Open Response"), - 'library': _("Library Content"), + 'library': _("Legacy Library"), + 'library_v2': _("Library Content"), + 'itembank': _("Problem Bank"), 'drag-and-drop-v2': _("Drag and Drop"), } @@ -226,7 +281,14 @@ def get_component_templates(courselike, library=False): # lint-amnesty, pylint: component_types = COMPONENT_TYPES[:] # Libraries do not support discussions, drag-and-drop, and openassessment and other libraries - component_not_supported_by_library = ['discussion', 'library', 'openassessment', 'drag-and-drop-v2'] + component_not_supported_by_library = [ + 'discussion', + 'library', + 'openassessment', + 'drag-and-drop-v2', + 'library_v2', + 'itembank', + ] if library: component_types = [component for component in component_types if component not in set(component_not_supported_by_library)] @@ -245,7 +307,7 @@ def get_component_templates(courselike, library=False): # lint-amnesty, pylint: templates_for_category = [] component_class = _load_mixed_class(category) - if support_level_without_template and category != 'library': + if support_level_without_template and category not in ['library']: # add the default template with localized display name # TODO: Once mixins are defined per-application, rather than per-runtime, # this should use a cms mixed-in class. (cpennington) @@ -366,7 +428,8 @@ def get_component_templates(courselike, library=False): # lint-amnesty, pylint: "type": category, "templates": templates_for_category, "display_name": component_display_names[category], - "support_legend": create_support_legend_dict() + "support_legend": create_support_legend_dict(), + "beta": category in BETA_COMPONENT_TYPES, }) # Libraries do not support advanced components at this time. @@ -416,7 +479,7 @@ def get_component_templates(courselike, library=False): # lint-amnesty, pylint: course_advanced_keys ) if advanced_component_templates['templates']: - component_templates.insert(0, advanced_component_templates) + component_templates.append(advanced_component_templates) return component_templates @@ -440,6 +503,11 @@ def _filter_disabled_blocks(all_blocks): Filter out disabled xblocks from the provided list of xblock names. """ disabled_block_names = [block.name for block in disabled_xblocks()] + if not libraries_v1_enabled(): + disabled_block_names.append('library') + if not libraries_v2_enabled(): + disabled_block_names.append('library_v2') + disabled_block_names.append('itembank') return [block_name for block_name in all_blocks if block_name not in disabled_block_names] diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 9f6cfb7c43..244804c306 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -15,6 +15,7 @@ from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.core.exceptions import FieldError, PermissionDenied, ValidationError as DjangoValidationError +from django.db.models import QuerySet from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpResponseNotFound from django.shortcuts import redirect from django.urls import reverse @@ -575,6 +576,10 @@ def _accessible_courses_list_from_groups(request): if course_keys: courses_list = CourseOverview.get_all_courses(filter_={'id__in': course_keys}) + else: + # If no course keys are found for the current user, then return without filtering + # or ordering the courses list. + return courses_list, [] search_query, order, active_only, archived_only = get_query_params_if_present(request) courses_list = get_filtered_and_ordered_courses( @@ -588,7 +593,11 @@ def _accessible_courses_list_from_groups(request): return courses_list, [] -def get_courses_by_status(active_only, archived_only, course_overviews): +def get_courses_by_status( + active_only: bool, + archived_only: bool, + course_overviews: QuerySet[CourseOverview] +) -> QuerySet[CourseOverview]: """ Return course overviews based on a base queryset filtered by a status. @@ -602,7 +611,10 @@ def get_courses_by_status(active_only, archived_only, course_overviews): return CourseOverview.get_courses_by_status(active_only, archived_only, course_overviews) -def get_courses_by_search_query(search_query, course_overviews): +def get_courses_by_search_query( + search_query: str | None, + course_overviews: QuerySet[CourseOverview] +) -> QuerySet[CourseOverview]: """Return course overviews based on a base queryset filtered by a search query. Args: @@ -614,7 +626,10 @@ def get_courses_by_search_query(search_query, course_overviews): return CourseOverview.get_courses_matching_query(search_query, course_overviews=course_overviews) -def get_courses_order_by(order_query, course_overviews): +def get_courses_order_by( + order_query: str | None, + course_overviews: QuerySet[CourseOverview] +) -> QuerySet[CourseOverview]: """Return course overviews based on a base queryset ordered by a query. Args: diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index 340cadb4e2..92e4329c2f 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -42,6 +42,7 @@ from common.djangoapps.student.roles import ( from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json from ..utils import add_instructor, reverse_library_url +from ..toggles import libraries_v1_enabled from .component import CONTAINER_TEMPLATES, get_component_templates from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import create_xblock_info from .user import user_with_role @@ -50,13 +51,11 @@ __all__ = ['library_handler', 'manage_library_users'] log = logging.getLogger(__name__) -LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) - def _user_can_create_library_for_org(user, org=None): """ Helper method for returning the library creation status for a particular user, - taking into account the value LIBRARIES_ENABLED. + taking into account the libraries_v1_enabled toggle. if the ENABLE_CREATOR_GROUP value is False, then any user can create a library (in any org), if library creation is enabled. @@ -69,7 +68,7 @@ def _user_can_create_library_for_org(user, org=None): Course Staff: Can make libraries in the organization which has courses of which they are staff. Course Admin: Can make libraries in the organization which has courses of which they are Admin. """ - if not LIBRARIES_ENABLED: + if not libraries_v1_enabled(): return False elif user.is_staff: return True @@ -125,7 +124,7 @@ def library_handler(request, library_key_string=None): """ RESTful interface to most content library related functionality. """ - if not LIBRARIES_ENABLED: + if not libraries_v1_enabled(): log.exception("Attempted to use the content library API when the libraries feature is disabled.") raise Http404 # Should never happen because we test the feature in urls.py also diff --git a/cms/djangoapps/contentstore/views/preview.py b/cms/djangoapps/contentstore/views/preview.py index acc5fc95df..aa7421bb87 100644 --- a/cms/djangoapps/contentstore/views/preview.py +++ b/cms/djangoapps/contentstore/views/preview.py @@ -300,8 +300,9 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): selected_groups_label = _('Access restricted to: {list_of_groups}').format(list_of_groups=selected_groups_label) # lint-amnesty, pylint: disable=line-too-long course = modulestore().get_course(xblock.location.course_key) can_edit = context.get('can_edit', True) + can_add = context.get('can_add', True) # Is this a course or a library? - is_course = xblock.scope_ids.usage_id.context_key.is_course + is_course = xblock.context_key.is_course tags_count_map = context.get('tags_count_map') tags_count = 0 if tags_count_map: @@ -320,7 +321,10 @@ def _studio_wrap_xblock(xblock, view, frag, context, display_name_only=False): 'is_selected': context.get('is_selected', False), 'selectable': context.get('selectable', False), 'selected_groups_label': selected_groups_label, - 'can_add': context.get('can_add', True), + 'can_add': can_add, + # Generally speaking, "if you can add, you can delete". One exception is itembank (Problem Bank) + # which has its own separate "add" workflow but uses the normal delete workflow for its child blocks. + 'can_delete': can_add or (root_xblock and root_xblock.scope_ids.block_type == "itembank" and can_edit), 'can_move': context.get('can_move', is_course), 'language': getattr(course, 'language', None), 'is_course': is_course, diff --git a/cms/djangoapps/contentstore/views/tests/test_block.py b/cms/djangoapps/contentstore/views/tests/test_block.py index f3e20b45b2..1190eb2395 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -312,12 +312,12 @@ class GetItemTest(ItemTest): resp = self.create_xblock( parent_usage_key=split_test_usage_key, category="html", - boilerplate="zooming_image.yaml", + boilerplate="latex_html.yaml", ) self.assertEqual(resp.status_code, 200) html, __ = self._get_container_preview(split_test_usage_key) self.assertIn("Announcement", html) - self.assertIn("Zooming", html) + self.assertIn("LaTeX", html) def test_split_test_edited(self): """ @@ -3184,13 +3184,13 @@ class TestComponentTemplates(CourseTestCase): templates = get_component_templates(self.course) button_names = [template["display_name"] for template in templates] self.assertIn("Advanced", button_names) - self.assertEqual(len(templates[0]["templates"]), len(expected_xblocks)) + self.assertEqual(len(templates[-1]["templates"]), len(expected_xblocks)) template_display_names = [ - template["display_name"] for template in templates[0]["templates"] + template["display_name"] for template in templates[-1]["templates"] ] self.assertEqual(template_display_names, expected_xblocks) template_support_levels = [ - template["support_level"] for template in templates[0]["templates"] + template["support_level"] for template in templates[-1]["templates"] ] self.assertEqual(template_support_levels, expected_support_levels) diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 7979a422a3..9244ffa989 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -7,11 +7,13 @@ import ddt from opaque_keys.edx.keys import UsageKey from rest_framework.test import APIClient from openedx_tagging.core.tagging.models import Tag +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, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url +from openedx.core.djangoapps.content_libraries import api as library_api from openedx.core.djangoapps.content_tagging import api as tagging_api CLIPBOARD_ENDPOINT = "/api/content-staging/v1/clipboard/" @@ -182,7 +184,7 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): tagging_api.set_taxonomy_orgs(taxonomy_all_org, all_orgs=True) for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4', 'tag_5', 'tag_6', 'tag_7'): - Tag.objects.create(taxonomy=taxonomy_all_org, value=tag_value) + tagging_api.add_tag_to_taxonomy(taxonomy_all_org, tag_value) tagging_api.tag_object( object_id=str(unit_key), taxonomy=taxonomy_all_org, @@ -391,6 +393,177 @@ class ClipboardPasteTestCase(ModuleStoreTestCase): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. +class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): + """ + Test Clipboard Paste functionality with a "new" (as of Sumac) library + """ + + def setUp(self): + """ + Set up a v2 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 = library_api.create_library( + 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') + + taxonomy_all_org = tagging_api.create_taxonomy( + "test_taxonomy", + "Test Taxonomy", + export_id="ALL_ORGS", + ) + tagging_api.set_taxonomy_orgs(taxonomy_all_org, all_orgs=True) + for tag_value in ('tag_1', 'tag_2', 'tag_3', 'tag_4', 'tag_5', 'tag_6', 'tag_7'): + tagging_api.add_tag_to_taxonomy(taxonomy_all_org, tag_value) + + self.lib_block_tags = ['tag_1', 'tag_5'] + tagging_api.tag_object(str(self.lib_block_key), taxonomy_all_org, self.lib_block_tags) + + def test_paste_from_library_read_only_tags(self): + """ + When we copy a v2 lib block into a course, the dest block should have read-only copied tags. + """ + + 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 = paste_response.json()["locator"] + + object_tags = tagging_api.get_object_tags(new_block_key) + assert len(object_tags) == len(self.lib_block_tags) + for object_tag in object_tags: + assert object_tag.value in self.lib_block_tags + assert object_tag.is_copied + + def test_paste_from_library_copies_asset(self): + """ + Assets from a library component copied into a subdir of Files & Uploads. + """ + # This is the binary for a real, 1px webp file – we need actual image + # data because contentstore will try to make a thumbnail and grab + # metadata. + webp_raw_data = b'RIFF\x16\x00\x00\x00WEBPVP8L\n\x00\x00\x00/\x00\x00\x00\x00E\xff#\xfa\x1f' + + # First add the asset. + library_api.add_library_block_static_asset_file( + self.lib_block_key, + "static/1px.webp", + webp_raw_data, + ) # v==4 + + # Now add the reference to the asset + library_api.set_library_block_olx(self.lib_block_key, """ + +

Including this totally real image:

+ + + + Wrong + Right + + + + """) # v==5 + + 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) + + # Check that the substitution worked. + expected_import_path = f"components/{new_block_key.block_type}/{new_block_key.block_id}/1px.webp" + assert f"/static/{expected_import_path}" in new_block.data + + # Check that the asset was copied over properly + image_asset = contentstore().find( + self.course.id.make_asset_key("asset", expected_import_path.replace('/', '_')) + ) + assert image_asset.import_path == expected_import_path + assert image_asset.name == "1px.webp" + assert image_asset.length == len(webp_raw_data) + + def test_paste_from_course_block_imported_from_library_creates_link(self): + """ + When we copy a course xblock which was imported or copied from v2 lib block into a course, + the dest block should be linked up to the original lib block. + """ + def _copy_paste_and_assert_link(key_to_copy): + copy_response = self.client.post(CLIPBOARD_ENDPOINT, {"usage_key": str(key_to_copy)}, 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 + return new_block_key + + # first verify link for copied block from library + new_block_key = _copy_paste_and_assert_link(self.lib_block_key) + # next verify link for copied block from the pasted block + _copy_paste_and_assert_link(new_block_key) + + class ClipboardPasteFromV1LibraryTestCase(ModuleStoreTestCase): """ Test Clipboard Paste functionality with legacy (v1) library content diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 1d5b529053..426477e234 100644 --- a/cms/djangoapps/contentstore/views/tests/test_container_page.py +++ b/cms/djangoapps/contentstore/views/tests/test_container_page.py @@ -242,3 +242,61 @@ class ContainerPageTestCase(StudioPageTestCase, LibraryTestCase): usage_key_string=str(self.vertical.location) ) self.assertEqual(response.status_code, 200) + + +class ContainerEmbedPageTestCase(ContainerPageTestCase): # lint-amnesty, pylint: disable=test-inherits-tests + """ + Unit tests for the container embed page. + """ + + def test_container_html(self): + assets_url = reverse( + 'assets_handler', kwargs={'course_key_string': str(self.child_container.location.course_key)} + ) + self._test_html_content( + self.child_container, + expected_section_tag=( + '