diff --git a/.annotation_safe_list.yml b/.annotation_safe_list.yml index a5e997bf2f4a..e91fe39cd613 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: @@ -82,8 +142,6 @@ workflow.AssessmentWorkflowStep: # Via edx-celeryutils celery_utils.ChordData: ".. no_pii:": "No PII" -celery_utils.FailedTask: - ".. no_pii:": "No PII" # Via completion XBlock completion.BlockCompletion: @@ -127,10 +185,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 +221,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 +268,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 +332,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/.eslintignore b/.eslintignore index 9044a0cc711f..b7754944dca5 100644 --- a/.eslintignore +++ b/.eslintignore @@ -20,9 +20,21 @@ test_root/staticfiles common/static/xmodule -# Symlinks into xmodule/js +# Various intra-repo symlinks that we've added over the years to duct-tape the JS build together. +# Ignore them so that we're not double-counting these violations. +cms/static/edx-ui-toolkit cms/static/xmodule_js +lms/static/common +lms/static/course_bookmarks +lms/static/course_experience +lms/static/course_search +lms/static/discussion +lms/static/edx-ui-toolkit +lms/static/learner_profile +lms/static/support +lms/static/teams lms/static/xmodule_js +xmodule/js/common_static # Mako templates that generate .js files diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 3f9abcc671fb..c41fe40b4d27 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -15,12 +15,15 @@ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_task/ lms/djangoapps/mobile_api/ +openedx/core/djangoapps/commerce/ @openedx/2u-infinity openedx/core/djangoapps/credentials @openedx/2U-aperture openedx/core/djangoapps/credit @openedx/2U-aperture +openedx/core/djangoapps/enrollments/ @openedx/2U-aperture openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/oauth_dispatch openedx/core/djangoapps/user_api/ @openedx/2U-aperture -openedx/core/djangoapps/user_authn/ @openedx/2U-vanguards +openedx/core/djangoapps/user_authn/ @openedx/2U-infinity +openedx/core/djangoapps/verified_track_content/ @openedx/2u-infinity openedx/features/course_experience/ xmodule/ @@ -35,15 +38,18 @@ common/djangoapps/track/ lms/djangoapps/certificates/ @openedx/2U-aperture # Discovery -common/djangoapps/course_modes/ +common/djangoapps/course_modes/ @openedx/2U-aperture common/djangoapps/enrollment/ -lms/djangoapps/commerce/ -lms/djangoapps/experiments/ -lms/djangoapps/learner_dashboard/ @openedx/2U-aperture -lms/djangoapps/learner_home/ @openedx/2U-aperture -openedx/features/content_type_gating/ +common/djangoapps/entitlements/ @openedx/2U-aperture +lms/djangoapps/branding/ @openedx/2U-aperture +lms/djangoapps/commerce/ @openedx/2u-infinity +lms/djangoapps/experiments/ @openedx/2u-infinity +lms/djangoapps/gating/ @openedx/2u-infinity +lms/djangoapps/learner_dashboard/ @openedx/2U-aperture +lms/djangoapps/learner_home/ @openedx/2U-aperture +openedx/features/content_type_gating/ @openedx/2u-infinity openedx/features/course_duration_limits/ -openedx/features/discounts/ +openedx/features/discounts/ @openedx/2u-infinity # Ping Axim On-call if someone uses the QuickStart # https://docs.openedx.org/en/latest/developers/quickstarts/first_openedx_pr.html diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 3f771c2c727f..ca314891b21d 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 048cbe6b006b..82298af70a54 100644 --- a/.github/workflows/check-consistent-dependencies.yml +++ b/.github/workflows/check-consistent-dependencies.yml @@ -15,7 +15,7 @@ defaults: jobs: check-requirements: name: Compile requirements - runs-on: ubuntu-22.04 + runs-on: ubuntu-24.04 steps: # Only run remaining steps if there are changes to requirements/** diff --git a/.github/workflows/ci-static-analysis.yml b/.github/workflows/ci-static-analysis.yml index 7e768a456463..d989ff9db288 100644 --- a/.github/workflows/ci-static-analysis.yml +++ b/.github/workflows/ci-static-analysis.yml @@ -10,7 +10,7 @@ jobs: matrix: python-version: - "3.11" - os: ["ubuntu-20.04"] + os: ["ubuntu-24.04"] steps: - uses: actions/checkout@v4 diff --git a/.github/workflows/compile-python-requirements.yml b/.github/workflows/compile-python-requirements.yml index 0ff99b9c685a..21cb80083f1d 100644 --- a/.github/workflows/compile-python-requirements.yml +++ b/.github/workflows/compile-python-requirements.yml @@ -15,7 +15,7 @@ defaults: jobs: recompile-python-dependencies: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out target branch diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml deleted file mode 100644 index 6831e3563d81..000000000000 --- a/.github/workflows/docker-publish.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Push Docker Images - -on: - push: - branches: - - master - -jobs: - # Push image to GitHub Packages. - # See also https://docs.docker.com/docker-hub/builds/ - push: - runs-on: ubuntu-latest - if: github.event_name == 'push' - - strategy: - matrix: - variant: - - "lms_dev" - - "cms_dev" - - "cms" - - "lms" - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - - name: Login to DockerHub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_PASSWORD }} - - - name: Build and push lms/cms base docker images - env: - DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }} - DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} - run: make docker_tag_build_push_${{matrix.variant}} diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index 4d025e540163..463352e1c552 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -12,8 +12,8 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] - node-version: [18, 20] + os: [ubuntu-latest] + 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: | @@ -64,13 +64,13 @@ jobs: make base-requirements - uses: c-hive/gha-npm-cache@v1 + + - name: Install npm + run: npm ci + - name: Run JS Tests - env: - TEST_SUITE: js-unit - SCRIPT_TO_RUN: ./scripts/generic-ci-tests.sh run: | - npm install -g jest - xvfb-run --auto-servernum ./scripts/all-tests.sh + npm run test - name: Save Job Artifacts uses: actions/upload-artifact@v4 diff --git a/.github/workflows/lint-imports.yml b/.github/workflows/lint-imports.yml index 8ead8396bf39..e3c59ec09304 100644 --- a/.github/workflows/lint-imports.yml +++ b/.github/workflows/lint-imports.yml @@ -9,7 +9,7 @@ on: jobs: lint-imports: name: Lint Python Imports - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out branch diff --git a/.github/workflows/migrations-check.yml b/.github/workflows/migrations-check.yml index 183b90effa29..84e334d68872 100644 --- a/.github/workflows/migrations-check.yml +++ b/.github/workflows/migrations-check.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] + os: [ubuntu-24.04] python-version: - "3.11" # 'pinned' is used to install the latest patch version of Django @@ -52,7 +52,7 @@ jobs: steps: - name: Setup mongodb user run: | - mongosh edxapp --eval ' + docker exec ${{ job.services.mongo.id }} mongosh edxapp --eval ' db.createUser( { user: "edxapp", @@ -67,7 +67,7 @@ jobs: - name: Verify mongo and mysql db credentials run: | mysql -h 127.0.0.1 -uedxapp001 -ppassword -e "select 1;" edxapp - mongosh --host 127.0.0.1 --username edxapp --password password --eval 'use edxapp; db.adminCommand("ping");' edxapp + docker exec ${{ job.services.mongo.id }} mongosh --host 127.0.0.1 --username edxapp --password password --eval 'use edxapp; db.adminCommand("ping");' edxapp - name: Checkout repo uses: actions/checkout@v4 @@ -126,7 +126,7 @@ jobs: if: always() needs: - check_migrations - runs-on: ubuntu-latest + runs-on: ubuntu-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/publish-ci-docker-image.yml b/.github/workflows/publish-ci-docker-image.yml deleted file mode 100644 index 0a9f50f6daf9..000000000000 --- a/.github/workflows/publish-ci-docker-image.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Push CI Runner Docker Image - -on: - workflow_dispatch: - schedule: - - cron: "0 1 * * 3" - -jobs: - push: - runs-on: ubuntu-20.04 - - steps: - - name: Checkout - uses: actions/checkout@v4 - - # This has to happen after checkout in order for gh to work. - - name: "Cancel scheduled job on forks" - if: github.repository != 'openedx/edx-platform' && github.event_name == 'schedule' - env: - GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" - run: | - gh run cancel "${{ github.run_id }}" - gh run watch "${{ github.run_id }}" - - - name: Configure AWS Credentials - uses: aws-actions/configure-aws-credentials@v4 - with: - aws-access-key-id: ${{ secrets.TOOLS_EDX_ECR_USER_AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.TOOLS_EDX_ECR_USER_AWS_SECRET_ACCESS_KEY }} - aws-region: us-east-1 - - - name: Log in to ECR - id: login-ecr - uses: aws-actions/amazon-ecr-login@v2 - - - name: Build, tag, and push image to Amazon ECR - env: - ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} - ECR_REPOSITORY: actions-runner - IMAGE_TAG: latest - run: | - docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG -f scripts/ci-runner.Dockerfile . - docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG diff --git a/.github/workflows/pylint-checks.yml b/.github/workflows/pylint-checks.yml index eeb53c24ed98..1afa032b6ad1 100644 --- a/.github/workflows/pylint-checks.yml +++ b/.github/workflows/pylint-checks.yml @@ -8,25 +8,25 @@ on: jobs: run-pylint: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 strategy: fail-fast: false matrix: include: - module-name: lms-1 - path: "--django-settings-module=lms.envs.test lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/" + path: "lms/djangoapps/badges/ lms/djangoapps/branding/ lms/djangoapps/bulk_email/ lms/djangoapps/bulk_enroll/ lms/djangoapps/bulk_user_retirement/ lms/djangoapps/ccx/ lms/djangoapps/certificates/ lms/djangoapps/commerce/ lms/djangoapps/course_api/ lms/djangoapps/course_blocks/ lms/djangoapps/course_home_api/ lms/djangoapps/course_wiki/ lms/djangoapps/coursewarehistoryextended/ lms/djangoapps/debug/ lms/djangoapps/courseware/ lms/djangoapps/course_goals/ lms/djangoapps/rss_proxy/" - module-name: lms-2 - path: "--django-settings-module=lms.envs.test lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py" + path: "lms/djangoapps/gating/ lms/djangoapps/grades/ lms/djangoapps/instructor/ lms/djangoapps/instructor_analytics/ lms/djangoapps/discussion/ lms/djangoapps/edxnotes/ lms/djangoapps/email_marketing/ lms/djangoapps/experiments/ lms/djangoapps/instructor_task/ lms/djangoapps/learner_dashboard/ lms/djangoapps/learner_home/ lms/djangoapps/lms_initialization/ lms/djangoapps/lms_xblock/ lms/djangoapps/lti_provider/ lms/djangoapps/mailing/ lms/djangoapps/mobile_api/ lms/djangoapps/monitoring/ lms/djangoapps/ora_staff_grader/ lms/djangoapps/program_enrollments/ lms/djangoapps/rss_proxy lms/djangoapps/static_template_view/ lms/djangoapps/staticbook/ lms/djangoapps/support/ lms/djangoapps/survey/ lms/djangoapps/teams/ lms/djangoapps/tests/ lms/djangoapps/user_tours/ lms/djangoapps/verify_student/ lms/djangoapps/mfe_config_api/ lms/envs/ lms/lib/ lms/tests.py" - module-name: openedx-1 - path: "--django-settings-module=lms.envs.test openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/" + path: "openedx/core/types/ openedx/core/djangoapps/ace_common/ openedx/core/djangoapps/agreements/ openedx/core/djangoapps/api_admin/ openedx/core/djangoapps/auth_exchange/ openedx/core/djangoapps/bookmarks/ openedx/core/djangoapps/cache_toolbox/ openedx/core/djangoapps/catalog/ openedx/core/djangoapps/ccxcon/ openedx/core/djangoapps/commerce/ openedx/core/djangoapps/common_initialization/ openedx/core/djangoapps/common_views/ openedx/core/djangoapps/config_model_utils/ openedx/core/djangoapps/content/ openedx/core/djangoapps/content_libraries/ openedx/core/djangoapps/content_staging/ openedx/core/djangoapps/contentserver/ openedx/core/djangoapps/cookie_metadata/ openedx/core/djangoapps/cors_csrf/ openedx/core/djangoapps/course_apps/ openedx/core/djangoapps/course_date_signals/ openedx/core/djangoapps/course_groups/ openedx/core/djangoapps/courseware_api/ openedx/core/djangoapps/crawlers/ openedx/core/djangoapps/credentials/ openedx/core/djangoapps/credit/ openedx/core/djangoapps/dark_lang/ openedx/core/djangoapps/debug/ openedx/core/djangoapps/discussions/ openedx/core/djangoapps/django_comment_common/ openedx/core/djangoapps/embargo/ openedx/core/djangoapps/enrollments/ openedx/core/djangoapps/external_user_ids/ openedx/core/djangoapps/zendesk_proxy/ openedx/core/djangolib/ openedx/core/lib/ openedx/core/tests/ openedx/core/djangoapps/course_live/" - module-name: openedx-2 - path: "--django-settings-module=lms.envs.test openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/learner_pathway/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/" + path: "openedx/core/djangoapps/geoinfo/ openedx/core/djangoapps/header_control/ openedx/core/djangoapps/heartbeat/ openedx/core/djangoapps/lang_pref/ openedx/core/djangoapps/models/ openedx/core/djangoapps/monkey_patch/ openedx/core/djangoapps/oauth_dispatch/ openedx/core/djangoapps/olx_rest_api/ openedx/core/djangoapps/password_policy/ openedx/core/djangoapps/plugin_api/ openedx/core/djangoapps/plugins/ openedx/core/djangoapps/profile_images/ openedx/core/djangoapps/programs/ openedx/core/djangoapps/safe_sessions/ openedx/core/djangoapps/schedules/ openedx/core/djangoapps/service_status/ openedx/core/djangoapps/session_inactivity_timeout/ openedx/core/djangoapps/signals/ openedx/core/djangoapps/site_configuration/ openedx/core/djangoapps/system_wide_roles/ openedx/core/djangoapps/theming/ openedx/core/djangoapps/user_api/ openedx/core/djangoapps/user_authn/ openedx/core/djangoapps/util/ openedx/core/djangoapps/verified_track_content/ openedx/core/djangoapps/video_config/ openedx/core/djangoapps/video_pipeline/ openedx/core/djangoapps/waffle_utils/ openedx/core/djangoapps/xblock/ openedx/core/djangoapps/xmodule_django/ openedx/core/tests/ openedx/features/ openedx/testing/ openedx/tests/ openedx/core/djangoapps/notifications/ openedx/core/djangoapps/staticfiles/ openedx/core/djangoapps/content_tagging/" - module-name: common - path: "--django-settings-module=lms.envs.test common pavelib" + path: "common" - module-name: cms - path: "--django-settings-module=cms.envs.test cms" + path: "cms" - module-name: xmodule - path: "--django-settings-module=lms.envs.test xmodule" + path: "xmodule" name: pylint ${{ matrix.module-name }} steps: @@ -75,7 +75,7 @@ jobs: if: always() needs: - run-pylint - runs-on: ubuntu-latest + runs-on: ubuntu-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 cf8ffd5d2910..2452f54da14b 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -13,7 +13,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-20.04] + os: [ubuntu-24.04] python-version: - "3.11" node-version: [20] @@ -60,16 +60,29 @@ jobs: PIP_SRC: ${{ runner.temp }} run: | make test-requirements - + + - name: Install npm + env: + PIP_SRC: ${{ runner.temp }} + run: npm ci + + - name: Install python packages + env: + PIP_SRC: ${{ runner.temp }} + run: | + pip install -e . + - name: Run Quality Tests env: - TEST_SUITE: quality - SCRIPT_TO_RUN: ./scripts/generic-ci-tests.sh PIP_SRC: ${{ runner.temp }} TARGET_BRANCH: ${{ github.base_ref }} run: | - ./scripts/all-tests.sh - + make pycodestyle + npm run lint + make xsslint + make pii_check + make check_keywords + - name: Save Job Artifacts if: always() uses: actions/upload-artifact@v4 diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index 7f2b4925af8e..d880d7351766 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -17,7 +17,7 @@ jobs: runs-on: "${{ matrix.os }}" strategy: matrix: - os: ["ubuntu-20.04"] + os: ["ubuntu-latest"] python-version: - "3.11" diff --git a/.github/workflows/static-assets-check.yml b/.github/workflows/static-assets-check.yml index 7bbfd3369b6b..502dddce08ad 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-20.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" @@ -72,9 +72,6 @@ jobs: run: | pip install -r requirements/edx/assets.txt - - name: Initiate Mongo DB Service - run: sudo systemctl start mongod - - name: Add node_modules bin to $Path run: echo $GITHUB_WORKSPACE/node_modules/.bin >> $GITHUB_PATH diff --git a/.github/workflows/unit-test-shards.json b/.github/workflows/unit-test-shards.json index 4ab126cb4715..4709930493ce 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/", @@ -256,15 +255,13 @@ "common-with-lms": { "settings": "lms.envs.test", "paths": [ - "common/djangoapps/", - "pavelib/" + "common/djangoapps/" ] }, "common-with-cms": { "settings": "cms.envs.test", "paths": [ - "common/djangoapps/", - "pavelib/" + "common/djangoapps/" ] }, "xmodule-with-lms": { diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 3e442b75d4e7..d399d38770b9 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -15,7 +15,7 @@ concurrency: jobs: run-tests: name: ${{ matrix.shard_name }}(py=${{ matrix.python-version }},dj=${{ matrix.django-version }},mongo=${{ matrix.mongo-version }}) - runs-on: ubuntu-20.04 + runs-on: ${{ 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 @@ -68,19 +73,20 @@ jobs: run: | sudo apt-get update && sudo apt-get install libmysqlclient-dev libxmlsec1-dev lynx - - 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 + # Try to log into DockerHub so that we're less likely to be rate-limited when pulling certain images. + # This will fail on any edx-platform fork which doesn't explicitly define its own DockerHub creds. + # That's OK--if we fail to log in, we'll proceed anonymously, and hope we don't get rate-limited. + - name: Try to log into Docker Hub + uses: docker/login-action@v3.3.0 + continue-on-error: true + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_PASSWORD }} - - 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 @@ -142,7 +148,7 @@ jobs: overwrite: true collect-and-verify: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - name: Setup Python @@ -162,7 +168,7 @@ jobs: shell: bash run: | echo "root_cms_unit_tests_count=$(pytest --disable-warnings --collect-only --ds=cms.envs.test cms/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - echo "root_lms_unit_tests_count=$(pytest --disable-warnings --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ xmodule/ pavelib/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV + echo "root_lms_unit_tests_count=$(pytest --disable-warnings --collect-only --ds=lms.envs.test lms/ openedx/ common/djangoapps/ xmodule/ -q | head -n -2 | wc -l)" >> $GITHUB_ENV - name: get GHA unit test paths shell: bash @@ -201,13 +207,12 @@ jobs: to add any missing apps and match the count. for more details please take a look at scripts/gha-shards-readme.md" exit 1 - # This job aggregates test results. It's the required check for branch protection. # https://github.com/marketplace/actions/alls-green#why # https://github.com/orgs/community/discussions/33579 success: name: Unit tests successful - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 if: always() needs: [run-tests] steps: @@ -218,7 +223,7 @@ jobs: jobs: ${{ toJSON(needs) }} compile-warnings-report: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 needs: [run-tests] steps: - uses: actions/checkout@v4 @@ -246,7 +251,7 @@ jobs: overwrite: true merge-artifacts: - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 needs: [compile-warnings-report] steps: - name: Merge Pytest Warnings JSON Artifacts @@ -266,7 +271,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-20.04 + runs-on: ubuntu-24.04 needs: [run-tests] strategy: matrix: diff --git a/.github/workflows/upgrade-one-python-dependency.yml b/.github/workflows/upgrade-one-python-dependency.yml index 6ca5dfcb355e..84a00266e99f 100644 --- a/.github/workflows/upgrade-one-python-dependency.yml +++ b/.github/workflows/upgrade-one-python-dependency.yml @@ -28,7 +28,7 @@ defaults: jobs: upgrade-one-python-dependency: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out target branch diff --git a/.github/workflows/verify-dunder-init.yml b/.github/workflows/verify-dunder-init.yml index 611fc0afc6e3..c398c506730b 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: @@ -8,7 +8,7 @@ on: jobs: verify_dunder_init: name: Verify __init__.py Files - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out branch diff --git a/.nvmrc b/.nvmrc index 3c032078a4a2..209e3ef4b624 100644 --- a/.nvmrc +++ b/.nvmrc @@ -1 +1 @@ -18 +20 diff --git a/.pii_annotations.yml b/.pii_annotations.yml index 328520738f10..9000115a253e 100644 --- a/.pii_annotations.yml +++ b/.pii_annotations.yml @@ -1,7 +1,7 @@ source_path: ./ report_path: pii_report safelist_path: .annotation_safe_list.yml -coverage_target: 94.5 +coverage_target: 85.3 # See OEP-30 for more information on these values and what they mean: # https://open-edx-proposals.readthedocs.io/en/latest/oep-0030-arch-pii-markup-and-auditing.html#docstring-annotations annotations: diff --git a/.readthedocs.yaml b/.readthedocs.yaml index 56d794e3567c..c0db64e5e188 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -3,7 +3,7 @@ version: 2 build: os: "ubuntu-22.04" tools: - python: "3.8" + python: "3.11" sphinx: configuration: docs/conf.py diff --git a/.stylelintignore b/.stylelintignore deleted file mode 100644 index cd53bacf3cf9..000000000000 --- a/.stylelintignore +++ /dev/null @@ -1,5 +0,0 @@ -xmodule/css -common/static/sass/bourbon -common/static/xmodule/modules/css -common/test/test-theme -lms/static/sass/vendor diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 75a716177fdc..000000000000 --- a/Dockerfile +++ /dev/null @@ -1,200 +0,0 @@ -FROM ubuntu:focal as minimal-system - -# Warning: This file is experimental. -# -# Short-term goals: -# * Be a suitable replacement for the `edxops/edxapp` image in devstack (in progress). -# * Take advantage of Docker caching layers: aim to put commands in order of -# increasing cache-busting frequency. -# * Related to ^, use no Ansible or Paver. -# Long-term goal: -# * Be a suitable base for production LMS and CMS images (THIS IS NOT YET THE CASE!). - -ARG DEBIAN_FRONTEND=noninteractive -ARG SERVICE_VARIANT -ARG SERVICE_PORT - -# Env vars: paver -# We intentionally don't use paver in this Dockerfile, but Devstack may invoke paver commands -# during provisioning. Enabling NO_PREREQ_INSTALL tells paver not to re-install Python -# requirements for every paver command, potentially saving a lot of developer time. -ARG NO_PREREQ_INSTALL='1' - -# Env vars: locale -ENV LANG='en_US.UTF-8' -ENV LANGUAGE='en_US:en' -ENV LC_ALL='en_US.UTF-8' - -# Env vars: configuration -ENV CONFIG_ROOT='/edx/etc' -ENV LMS_CFG="$CONFIG_ROOT/lms.yml" -ENV CMS_CFG="$CONFIG_ROOT/cms.yml" - -# Env vars: path -ENV VIRTUAL_ENV="/edx/app/edxapp/venvs/edxapp" -ENV PATH="${VIRTUAL_ENV}/bin:${PATH}" -ENV PATH="/edx/app/edxapp/edx-platform/node_modules/.bin:${PATH}" -ENV PATH="/edx/app/edxapp/edx-platform/bin:${PATH}" -ENV PATH="/edx/app/edxapp/nodeenv/bin:${PATH}" - -WORKDIR /edx/app/edxapp/edx-platform - -# Create user before assigning any directory ownership to it. -RUN useradd -m --shell /bin/false app - -# Use debconf to set locales to be generated when the locales apt package is installed later. -RUN echo "locales locales/default_environment_locale select en_US.UTF-8" | debconf-set-selections -RUN echo "locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8" | debconf-set-selections - -# 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 - -# 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 \ - && \ - apt-get clean all && \ - rm -rf /var/lib/apt/* - -RUN mkdir -p /edx/var/edxapp -RUN mkdir -p /edx/etc -RUN chown app:app /edx/var/edxapp - -# The builder-production stage is a temporary stage that installs required packages and builds the python virtualenv, -# installs nodejs and node_modules. -# The built artifacts from this stage are then copied to the base stage. -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 - -# Setup python virtual environment -# It is already 'activated' because $VIRTUAL_ENV/bin was put on $PATH -RUN python3.11 -m venv "${VIRTUAL_ENV}" - -# Install python requirements -# Requires copying over requirements files, but not entire repository -COPY requirements requirements -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 - -# 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. -# The script itself will copy certain modules into some uber-legacy parts of edx-platform which still use RequireJS. -COPY scripts/copy-node-modules.sh scripts/copy-node-modules.sh - -# Install node modules -COPY package.json package.json -COPY package-lock.json package-lock.json -RUN npm set progress=false && npm ci - -# The builder-development stage is a temporary stage that installs python modules required for development purposes -# The built artifacts from this stage are then copied to the development stage. -FROM builder-production as builder-development - -RUN pip install -r requirements/edx/development.txt - -# base stage -FROM minimal-system as base - -# Copy python virtual environment, nodejs and node_modules -COPY --from=builder-production /edx/app/edxapp/venvs/edxapp /edx/app/edxapp/venvs/edxapp -COPY --from=builder-production /edx/app/edxapp/nodeenv /edx/app/edxapp/nodeenv -COPY --from=builder-production /edx/app/edxapp/edx-platform/node_modules /edx/app/edxapp/edx-platform/node_modules - -# Copy over remaining parts of repository (including all code) -COPY . . - -# Install Python requirements again in order to capture local projects -RUN pip install -e . - -# Setting edx-platform directory as safe for git commands -RUN git config --global --add safe.directory /edx/app/edxapp/edx-platform - -# Production target -FROM base as production - -USER app - -ENV EDX_PLATFORM_SETTINGS='docker-production' -ENV SERVICE_VARIANT="${SERVICE_VARIANT}" -ENV SERVICE_PORT="${SERVICE_PORT}" -ENV DJANGO_SETTINGS_MODULE="${SERVICE_VARIANT}.envs.$EDX_PLATFORM_SETTINGS" -EXPOSE ${SERVICE_PORT} - -CMD gunicorn \ - -c /edx/app/edxapp/edx-platform/${SERVICE_VARIANT}/docker_${SERVICE_VARIANT}_gunicorn.py \ - --name ${SERVICE_VARIANT} \ - --bind=0.0.0.0:${SERVICE_PORT} \ - --max-requests=1000 \ - --access-logfile \ - - ${SERVICE_VARIANT}.wsgi:application - -# Development target -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 \ - && \ - apt-get clean all && \ - rm -rf /var/lib/apt/* - -COPY --from=builder-development /edx/app/edxapp/venvs/edxapp /edx/app/edxapp/venvs/edxapp - -RUN ln -s "$(pwd)/lms/envs/devstack-experimental.yml" "$LMS_CFG" -RUN ln -s "$(pwd)/cms/envs/devstack-experimental.yml" "$CMS_CFG" -# Temporary compatibility hack while devstack is supporting both the old `edxops/edxapp` image and this image. -# * Add in a dummy ../edxapp_env file -# * devstack sets /edx/etc/studio.yml as CMS_CFG. -RUN ln -s "$(pwd)/cms/envs/devstack-experimental.yml" "/edx/etc/studio.yml" -RUN touch ../edxapp_env - -ENV EDX_PLATFORM_SETTINGS='devstack_docker' -ENV SERVICE_VARIANT="${SERVICE_VARIANT}" -EXPOSE ${SERVICE_PORT} -CMD ./manage.py ${SERVICE_VARIANT} runserver 0.0.0.0:${SERVICE_PORT} diff --git a/Makefile b/Makefile index 15bab5df67a9..08b6f14893cc 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,7 @@ # Do things in edx-platform .PHONY: base-requirements check-types clean \ compile-requirements detect_changed_source_translations dev-requirements \ - docker_auth docker_build docker_tag_build_push_lms docker_tag_build_push_lms_dev \ - docker_tag_build_push_cms docker_tag_build_push_cms_dev docs extract_translations \ + docs extract_translations \ guides help lint-imports local-requirements migrate migrate-lms migrate-cms \ pre-requirements pull pull_xblock_translations pull_translations push_translations \ requirements shell swagger \ @@ -67,9 +66,6 @@ pull_translations: clean_translations ## pull translations via atlas detect_changed_source_translations: ## check if translation files are up-to-date i18n_tool changed -pull: ## update the Docker image used by "make shell" - docker pull edxops/edxapp:latest - pre-requirements: ## install Python requirements for running pip-tools pip install -r requirements/pip.txt pip install -r requirements/pip-tools.txt @@ -94,17 +90,9 @@ test-requirements: pre-requirements requirements: dev-requirements ## install development environment requirements -shell: ## launch a bash shell in a Docker container with all edx-platform dependencies installed - docker run -it -e "NO_PYTHON_UNINSTALL=1" -e "PIP_INDEX_URL=https://pypi.python.org/simple" -e TERM \ - -v `pwd`:/edx/app/edxapp/edx-platform:cached \ - -v edxapp_lms_assets:/edx/var/edxapp/staticfiles/ \ - -v edxapp_node_modules:/edx/app/edxapp/edx-platform/node_modules \ - edxops/edxapp:latest /edx/app/edxapp/devstack.sh open - # Order is very important in this list: files must appear after everything they include! REQ_FILES = \ requirements/edx/coverage \ - requirements/edx/paver \ requirements/edx-sandbox/base \ requirements/edx/base \ requirements/edx/doc \ @@ -164,27 +152,6 @@ upgrade-package: ## update just one package to the latest usable release check-types: ## run static type-checking tests mypy -docker_auth: - echo "$$DOCKERHUB_PASSWORD" | docker login -u "$$DOCKERHUB_USERNAME" --password-stdin - -docker_build: docker_auth - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target development -t openedx/lms-dev - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target production -t openedx/lms - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target development -t openedx/cms-dev - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target production -t openedx/cms - -docker_tag_build_push_lms: docker_auth - docker buildx build -t openedx/lms:latest -t openedx/lms:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target production --push . - -docker_tag_build_push_lms_dev: docker_auth - docker buildx build -t openedx/lms-dev:latest -t openedx/lms-dev:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target development --push . - -docker_tag_build_push_cms: docker_auth - docker buildx build -t openedx/cms:latest -t openedx/cms:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target production --push . - -docker_tag_build_push_cms_dev: docker_auth - docker buildx build -t openedx/cms-dev:latest -t openedx/cms-dev:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target development --push . - lint-imports: lint-imports @@ -204,3 +171,37 @@ migrate: migrate-lms migrate-cms # Part of https://github.com/openedx/wg-developer-experience/issues/136 ubuntu-requirements: ## Install ubuntu 22.04 system packages needed for `pip install` to work on ubuntu. sudo apt install libmysqlclient-dev libxmlsec1-dev + +xsslint: ## check xss for quality issuest + python scripts/xsslint/xss_linter.py \ + --rule-totals \ + --config=scripts.xsslint_config \ + --thresholds=scripts/xsslint_thresholds.json + +pycodestyle: ## check python files for quality issues + pycodestyle . + +## Re-enable --lint flag when this issue https://github.com/openedx/edx-platform/issues/35775 is resolved +pii_check: ## check django models for pii annotations + DJANGO_SETTINGS_MODULE=cms.envs.test \ + code_annotations django_find_annotations \ + --config_file .pii_annotations.yml \ + --app_name cms \ + --coverage \ + --lint + + DJANGO_SETTINGS_MODULE=lms.envs.test \ + code_annotations django_find_annotations \ + --config_file .pii_annotations.yml \ + --app_name lms \ + --coverage \ + --lint + +check_keywords: ## check django models for reserve keywords + DJANGO_SETTINGS_MODULE=cms.envs.test \ + python manage.py cms check_reserved_keywords \ + --override_file db_keyword_overrides.yml + + DJANGO_SETTINGS_MODULE=lms.envs.test \ + python manage.py lms check_reserved_keywords \ + --override_file db_keyword_overrides.yml diff --git a/README.rst b/README.rst index e74176faf91e..54b91363a85f 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 ----------- @@ -124,6 +140,35 @@ sites):: ./manage.py lms collectstatic ./manage.py cms collectstatic +Set up CMS SSO (for Development):: + + ./manage.py lms manage_user studio_worker example@example.com --unusable-password + # DO NOT DO THIS IN PRODUCTION. It will make your auth insecure. + ./manage.py lms create_dot_application studio-sso-id studio_worker \ + --grant-type authorization-code \ + --skip-authorization \ + --redirect-uris 'http://localhost:18010/complete/edx-oauth2/' \ + --scopes user_id \ + --client-id 'studio-sso-id' \ + --client-secret 'studio-sso-secret' + +Set up CMS SSO (for Production): + +* Create the CMS user and the OAuth application:: + + ./manage.py lms manage_user studio_worker --unusable-password + ./manage.py lms create_dot_application studio-sso-id studio_worker \ + --grant-type authorization-code \ + --skip-authorization \ + --redirect-uris 'http://localhost:18010/complete/edx-oauth2/' \ + --scopes user_id + +* Log into Django admin (eg. http://localhost:18000/admin/oauth2_provider/application/), + click into the application you created above (``studio-sso-id``), and copy its "Client secret". +* In your private LMS_CFG yaml file or your private Django settings module: + + * Set ``SOCIAL_AUTH_EDX_OAUTH2_KEY`` to the client ID (``studio-sso-id``). + * Set ``SOCIAL_AUTH_EDX_OAUTH2_SECRET`` to the client secret (which you copied). Run the Platform ---------------- @@ -131,11 +176,11 @@ First, ensure MySQL, Mongo, and Memcached are running. Start the LMS:: - ./manage.py lms runserver + ./manage.py lms runserver 18000 Start the CMS:: - ./manage.py cms runserver + ./manage.py cms runserver 18010 This will give you a mostly-headless Open edX platform. Most frontends have been migrated to "Micro-Frontends (MFEs)" which need to be installed and run diff --git a/catalog-info.yaml b/catalog-info.yaml index 885b879833d4..c6b87b08db82 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 281258e03a8d..97fcb3125745 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.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/config/waffle.py b/cms/djangoapps/contentstore/config/waffle.py index f84290ba83ae..ae0e6ea467b5 100644 --- a/cms/djangoapps/contentstore/config/waffle.py +++ b/cms/djangoapps/contentstore/config/waffle.py @@ -4,7 +4,7 @@ """ -from edx_toggles.toggles import WaffleFlag, WaffleSwitch +from edx_toggles.toggles import WaffleSwitch from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag @@ -26,20 +26,6 @@ f'{WAFFLE_NAMESPACE}.show_review_rules', __name__, LOG_PREFIX ) -# Waffle flag to redirect to the library authoring MFE. -# .. toggle_name: contentstore.library_authoring_mfe -# .. toggle_implementation: WaffleFlag -# .. toggle_default: False -# .. toggle_description: Toggles the new micro-frontend-based implementation of the library authoring experience. -# .. toggle_use_cases: temporary, open_edx -# .. toggle_creation_date: 2020-08-03 -# .. toggle_target_removal_date: 2020-12-31 -# .. toggle_warning: Also set settings.LIBRARY_AUTHORING_MICROFRONTEND_URL and ENABLE_LIBRARY_AUTHORING_MICROFRONTEND. -# .. toggle_tickets: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4106944527/Libraries+Relaunch+Proposal+For+Product+Review -REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND = WaffleFlag( - f'{WAFFLE_NAMESPACE}.library_authoring_mfe', __name__, LOG_PREFIX -) - # .. toggle_name: studio.custom_relative_dates # .. toggle_implementation: CourseWaffleFlag diff --git a/cms/djangoapps/contentstore/courseware_index.py b/cms/djangoapps/contentstore/courseware_index.py index 48647bf47bc6..d3b6f811d5f6 100644 --- a/cms/djangoapps/contentstore/courseware_index.py +++ b/cms/djangoapps/contentstore/courseware_index.py @@ -256,7 +256,8 @@ def prepare_item_index(item, skip_index=False, groups_usage_info=None): # 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 a4ece6c85d59..e40eddb6c99e 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -3,14 +3,17 @@ """ 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.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 @@ 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 4ff9e7ce8d43..346f10d933b1 100644 --- a/cms/djangoapps/contentstore/management/commands/import.py +++ b/cms/djangoapps/contentstore/management/commands/import.py @@ -8,7 +8,7 @@ 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 @@ def handle(self, *args, **options): 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 accbc077c4fc..275eff50626d 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 @@ def add_arguments(self, parser): 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 @@ def handle(self, *args, **options): # pylint: disable=too-many-statements 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 @@ def handle(self, *args, **options): # pylint: disable=too-many-statements 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 @@ def handle(self, *args, **options): # pylint: disable=too-many-statements 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 13d33c48f92a..6d14b4a339f2 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 @@ def setUp(self): 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 @@ def test_given_all_key_prompts_and_reindexes_all_courses(self): 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 @@ def test_given_active_key_prompt(self): 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 e8d3039b0b29..616204ef59c7 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_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 000000000000..33dd99792882 --- /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 1afc51ed77af..fa2a651f8a28 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() @@ -51,20 +51,22 @@ class CourseHomeSerializer(serializers.Serializer): allow_empty=True ) archived_courses = CourseCommonSerializer(required=False, many=True) + can_access_advanced_settings = serializers.BooleanField() can_create_organizations = serializers.BooleanField() course_creator_status = serializers.CharField() courses = CourseCommonSerializer(required=False, many=True) 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() - library_authoring_mfe_url = serializers.CharField() taxonomy_list_mfe_url = serializers.CharField() optimization_enabled = serializers.BooleanField() - redirect_to_library_authoring_mfe = serializers.BooleanField() request_course_creator_url = serializers.CharField() rerun_creator_status = serializers.BooleanField() show_new_library_button = serializers.BooleanField() + 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/settings.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py index 1c9f9f608478..b1a02fff1f39 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/serializers/settings.py @@ -31,4 +31,3 @@ class CourseSettingsSerializer(serializers.Serializer): show_min_grade_warning = serializers.BooleanField() sidebar_html_enabled = serializers.BooleanField() upgrade_deadline = serializers.DateTimeField(allow_null=True) - use_v2_cert_display_settings = serializers.BooleanField() diff --git a/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py index d72707ed7836..dffa23349d92 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 @@ def get_assets_url(self, obj): return None +class UpstreamLinkSerializer(serializers.Serializer): + """ + Serializer holding info for syncing a block with its upstream (eg, a library block). + """ + upstream_ref = serializers.CharField() + version_synced = serializers.IntegerField() + version_available = serializers.IntegerField(allow_null=True) + version_declined = serializers.IntegerField(allow_null=True) + error_message = serializers.CharField(allow_null=True) + ready_to_sync = serializers.BooleanField() + + class ChildVerticalContainerSerializer(serializers.Serializer): """ Serializer for representing a xblock child of vertical container. @@ -113,6 +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 e2afe48c96a4..349218679709 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v1/urls.py @@ -17,6 +17,7 @@ CourseRerunView, CourseSettingsView, CourseVideosView, + CourseWaffleFlagsView, HomePageView, HomePageCoursesView, HomePageLibrariesView, @@ -131,6 +132,11 @@ 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 dfba1e63f72f..89d8d56eaa11 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 .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 000000000000..ba96ff2b4a6c --- /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 d41ceb2647c5..3de536d78092 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 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.", }, ) @@ -52,18 +52,21 @@ def get(self, request: Request): "allow_unicode_course_id": false, "allowed_organizations": [], "archived_courses": [], + "can_access_advanced_settings": true, "can_create_organizations": true, "course_creator_status": "granted", "courses": [], "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, - "redirect_to_library_authoring_mfe": 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": "Studio", "studio_short_name": "Studio", @@ -85,7 +88,7 @@ def get(self, request: Request): '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/settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py index e921ac60398b..fbb05cba4de2 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/settings.py @@ -95,7 +95,6 @@ def get(self, request: Request, course_id: str): "show_min_grade_warning": false, "sidebar_html_enabled": true, "upgrade_deadline": null, - "use_v2_cert_display_settings": false } ``` """ @@ -112,7 +111,6 @@ def get(self, request: Request, course_id: str): 'course_display_name_with_default': course_block.display_name_with_default, 'platform_name': settings.PLATFORM_NAME, 'licensing_enabled': settings.FEATURES.get("LICENSING", False), - 'use_v2_cert_display_settings': settings.FEATURES.get("ENABLE_V2_CERT_DISPLAY_SETTINGS", False), }) serializer = CourseSettingsSerializer(settings_context) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py index eafc2b37aa0c..189f2496a427 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_index.py @@ -1,6 +1,7 @@ """ Unit tests for course index outline. """ +from django.conf import settings from django.test import RequestFactory from django.urls import reverse from rest_framework import status @@ -62,7 +63,7 @@ def test_course_index_response(self): "advance_settings_url": f"/settings/advanced/{self.course.id}" }, "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL, "is_custom_relative_dates_active": True, "initial_state": None, "initial_user_clipboard": { @@ -103,7 +104,7 @@ def test_course_index_response_with_show_locators(self): "advance_settings_url": f"/settings/advanced/{self.course.id}" }, "discussions_incontext_feedback_url": "", - "discussions_incontext_learnmore_url": "", + "discussions_incontext_learnmore_url": settings.DISCUSSIONS_INCONTEXT_LEARNMORE_URL, "is_custom_relative_dates_active": False, "initial_state": { "expanded_locators": [ diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_course_waffle_flags.py new file mode 100644 index 000000000000..f0332d7f2870 --- /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 1b8bfaa84728..a4a6909c5dcb 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,31 +35,28 @@ 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, "allowed_organizations": [], "archived_courses": [], + "can_access_advanced_settings": True, "can_create_organizations": True, "course_creator_status": "granted", "courses": [], "in_process_course_actions": [], "libraries": [], "libraries_enabled": True, + "libraries_v1_enabled": True, + "libraries_v2_enabled": False, "taxonomies_enabled": True, - "library_authoring_mfe_url": settings.LIBRARY_AUTHORING_MICROFRONTEND_URL, "taxonomy_list_mfe_url": 'http://course-authoring-mfe/taxonomies', "optimization_enabled": False, - "redirect_to_library_authoring_mfe": False, "request_course_creator_url": "/request_course_creator", "rerun_creator_status": True, "show_new_library_button": True, + "show_new_library_v2_button": True, "split_studio_home": False, "studio_name": settings.STUDIO_NAME, "studio_short_name": settings.STUDIO_SHORT_NAME, @@ -67,6 +66,21 @@ def test_home_page_courses_response(self): "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) @@ -89,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""" @@ -106,7 +121,7 @@ def test_home_page_response(self): "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}', @@ -120,6 +135,7 @@ def test_home_page_response(self): 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. @@ -133,7 +149,7 @@ def test_home_page_response_with_api_v2(self): 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}'), @@ -144,12 +160,88 @@ def test_home_page_response_with_api_v2(self): "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_proctoring.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py index 66b5f46128c8..8e220a334c06 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_proctoring.py @@ -277,18 +277,14 @@ def test_update_exam_settings_invalid_value(self): # response is correct assert response.status_code == status.HTTP_400_BAD_REQUEST - self.assertDictEqual( - response.data, + self.assertIn( { - "detail": [ - { - "proctoring_provider": ( - "The selected proctoring provider, notvalidprovider, is not a valid provider. " - "Please select from one of ['test_proctoring_provider']." - ) - } - ] + "proctoring_provider": ( + "The selected proctoring provider, notvalidprovider, is not a valid provider. " + "Please select from one of ['test_proctoring_provider']." + ) }, + response.data['detail'], ) # course settings have been updated @@ -408,18 +404,14 @@ def test_400_for_disabled_lti(self): # response is correct assert response.status_code == status.HTTP_400_BAD_REQUEST - self.assertDictEqual( - response.data, + self.assertIn( { - "detail": [ - { - "proctoring_provider": ( - "The selected proctoring provider, lti_external, is not a valid provider. " - "Please select from one of ['null']." - ) - } - ] + "proctoring_provider": ( + "The selected proctoring provider, lti_external, is not a valid provider. " + "Please select from one of ['null']." + ) }, + response.data['detail'], ) # course settings have been updated diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py index 5365443c47d9..15b0992fdf1a 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_settings.py @@ -55,7 +55,6 @@ def test_course_settings_response(self): "show_min_grade_warning": False, "upgrade_deadline": None, "licensing_enabled": False, - "use_v2_cert_display_settings": False, } self.assertEqual(response.status_code, status.HTTP_200_OK) diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py index d3fc37198213..7cac074a433f 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/tests/test_vertical_block.py @@ -70,6 +70,8 @@ def setup_xblock(self): parent=self.vertical.location, category="html", display_name="Html Content 2", + upstream="lb:FakeOrg:FakeLib:html:FakeBlock", + upstream_version=5, ) def create_block(self, parent, category, display_name, **kwargs): @@ -193,6 +195,7 @@ def test_children_content(self): "name": self.html_unit_first.display_name_with_default, "block_id": str(self.html_unit_first.location), "block_type": self.html_unit_first.location.block_type, + "upstream_link": None, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "actions": { @@ -218,12 +221,21 @@ def test_children_content(self): "can_delete": True, "can_manage_tags": True, }, + "upstream_link": { + "upstream_ref": "lb:FakeOrg:FakeLib:html:FakeBlock", + "version_synced": 5, + "version_available": None, + "version_declined": None, + "error_message": "Linked library item was not found in the system", + "ready_to_sync": False, + }, "user_partition_info": expected_user_partition_info, "user_partitions": expected_user_partitions, "validation_messages": [], "render_error": "", }, ] + self.maxDiff = None self.assertEqual(response.data["children"], expected_response) def test_not_valid_usage_key_string(self): diff --git a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py index 670b94afbbe0..0798c341cc1c 100644 --- a/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py +++ b/cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py @@ -20,6 +20,7 @@ ContainerHandlerSerializer, VerticalContainerSerializer, ) +from cms.lib.xblock.upstream_sync import UpstreamLink from openedx.core.lib.api.view_utils import view_auth_classes from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order @@ -198,6 +199,7 @@ def get(self, request: Request, usage_key_string: str): "block_type": "drag-and-drop-v2", "user_partition_info": {}, "user_partitions": {} + "upstream_link": null, "actions": { "can_copy": true, "can_duplicate": true, @@ -215,6 +217,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "video", "user_partition_info": {}, "user_partitions": {} + "upstream_link": { + "upstream_ref": "lb:org:mylib:video:404", + "version_synced": 16 + "version_available": null, + "error_message": "Linked library item not found: lb:org:mylib:video:404", + "ready_to_sync": false, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -232,6 +241,13 @@ def get(self, request: Request, usage_key_string: str): "block_type": "html", "user_partition_info": {}, "user_partitions": {}, + "upstream_link": { + "upstream_ref": "lb:org:mylib:html:abcd", + "version_synced": 43, + "version_available": 49, + "error_message": null, + "ready_to_sync": true, + }, "actions": { "can_copy": true, "can_duplicate": true, @@ -267,6 +283,7 @@ def get(self, request: Request, usage_key_string: str): child_info = modulestore().get_item(child) user_partition_info = get_visibility_partition_info(child_info, course=course) user_partitions = get_user_partition_info(child_info, course=course) + upstream_link = UpstreamLink.try_get_for_block(child_info) validation_messages = get_xblock_validation_messages(child_info) render_error = get_xblock_render_error(request, child_info) @@ -277,6 +294,12 @@ def get(self, request: Request, usage_key_string: str): "block_type": child_info.location.block_type, "user_partition_info": user_partition_info, "user_partitions": user_partitions, + "upstream_link": ( + # If the block isn't linked to an upstream (which is by far the most common case) then just + # make this field null, which communicates the same info, but with less noise. + upstream_link.to_json() if upstream_link.upstream_ref + else None + ), "validation_messages": validation_messages, "render_error": render_error, }) diff --git a/cms/djangoapps/contentstore/rest_api/v2/urls.py b/cms/djangoapps/contentstore/rest_api/v2/urls.py index ad61cc937015..3e653d07fbcf 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/urls.py +++ b/cms/djangoapps/contentstore/rest_api/v2/urls.py @@ -1,15 +1,31 @@ """Contenstore API v2 URLs.""" -from django.urls import path - -from cms.djangoapps.contentstore.rest_api.v2.views import HomePageCoursesViewV2 +from django.conf import settings +from django.urls import path, re_path +from cms.djangoapps.contentstore.rest_api.v2.views import home, downstreams app_name = "v2" urlpatterns = [ path( "home/courses", - HomePageCoursesViewV2.as_view(), + home.HomePageCoursesViewV2.as_view(), name="courses", ), + # TODO: Potential future path. + # re_path( + # fr'^downstreams/$', + # downstreams.DownstreamsListView.as_view(), + # name="downstreams_list", + # ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}$', + downstreams.DownstreamView.as_view(), + name="downstream" + ), + re_path( + fr'^downstreams/{settings.USAGE_KEY_PATTERN}/sync$', + downstreams.SyncFromUpstreamView.as_view(), + name="sync_from_upstream" + ), ] diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py index 73ddde98440c..e69de29bb2d1 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/__init__.py @@ -1,3 +0,0 @@ -"""Module for v2 views.""" - -from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesViewV2 diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py new file mode 100644 index 000000000000..8c0d651910a6 --- /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 000000000000..92da28bde989 --- /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 c0ffa50903cd..e773e7f213c6 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 @@ def setUp(self): 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 @@ def test_home_page_response(self): 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 @@ def test_home_page_response(self): ("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 @@ def test_active_only_query_if_passed(self): 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 @@ def test_archived_only_query_if_passed(self): ("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 @@ def test_search_query_if_passed(self): ("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 @@ def test_api_v2_is_disabled(self, mock_modulestore, mock_course_overview): 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 98a60dce901f..3ab3fa373f81 100644 --- a/cms/djangoapps/contentstore/tests/test_courseware_index.py +++ b/cms/djangoapps/contentstore/tests/test_courseware_index.py @@ -504,6 +504,11 @@ def test_delete_course_from_search_index_after_course_deletion(self): """ 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 000000000000..13bdfa07473e --- /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 9c478ddfe5d7..a91379797060 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -960,4 +960,32 @@ def test_course_update_notification_sent(self): 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 7c3a369fed62..39b793f479a2 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 17e94112bafe..7023bcaefaf7 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -3,12 +3,13 @@ """ 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 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,30 +101,7 @@ 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 LibraryToolsService +from xmodule.library_tools import LegacyLibraryToolsService from xmodule.course_block import DEFAULT_START_DATE # lint-amnesty, pylint: disable=wrong-import-order from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order @@ -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. @@ -1265,7 +1279,7 @@ def load_services_for_studio(runtime, user): "settings": SettingsService(), "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag), "teams_configuration": TeamsConfigurationService(), - "library_tools": LibraryToolsService(modulestore(), user.id) + "library_tools": LegacyLibraryToolsService(modulestore(), user.id) } runtime._services.update(services) # lint-amnesty, pylint: disable=protected-access @@ -1536,10 +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], } @@ -1549,8 +1566,9 @@ def get_library_context(request, request_is_json=False): **data, 'in_process_course_actions': [], 'courses': [], - 'libraries_enabled': LIBRARIES_ENABLED, - 'show_new_library_button': LIBRARIES_ENABLED and request.user.is_active, + '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), @@ -1670,11 +1688,11 @@ def get_home_context(request, no_course=False): ENABLE_GLOBAL_STAFF_OPTIMIZATION, ) from cms.djangoapps.contentstore.views.library import ( - LIBRARY_AUTHORING_MICROFRONTEND_URL, - LIBRARIES_ENABLED, - should_redirect_to_library_authoring_mfe, user_can_view_create_library_button, ) + from openedx.core.djangoapps.content_libraries.api import ( + user_can_create_library, + ) active_courses = [] archived_courses = [] @@ -1688,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 = { @@ -1696,14 +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(), - 'redirect_to_library_authoring_mfe': should_redirect_to_library_authoring_mfe(), - 'library_authoring_mfe_url': LIBRARY_AUTHORING_MICROFRONTEND_URL, 'taxonomy_list_mfe_url': get_taxonomy_list_url(), 'libraries': libraries, - 'show_new_library_button': user_can_view_create_library_button(user) - and not should_redirect_to_library_authoring_mfe(), + 'show_new_library_button': user_can_view_create_library_button(user), + '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), @@ -1715,6 +1733,7 @@ def get_home_context(request, no_course=False): 'allowed_organizations': get_allowed_organizations(user), 'allowed_organizations_for_libraries': get_allowed_organizations_for_libraries(user), 'can_create_organizations': user_can_create_organizations(user), + 'can_access_advanced_settings': auth.has_studio_advanced_settings_access(user), } return home_context @@ -2044,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 @@ -2200,7 +2220,7 @@ class StudioPermissionsService: Deprecated. To be replaced by a more general authorization service. - Only used by LibraryContentBlock (and library_tools.py). + Only used by LegacyLibraryContentBlock (and library_tools.py). """ def __init__(self, user): @@ -2246,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 6827b295b0a8..4cc5c738b5dc 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 4d6c17838e57..e6b41dc261d2 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 ba767df78dc9..914c07884665 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -11,6 +11,7 @@ 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.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 @@ __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 @@ "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 create_support_legend_dict(): '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 create_support_legend_dict(): 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 create_support_legend_dict(): 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 create_support_legend_dict(): "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 create_support_legend_dict(): 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 9f6cfb7c430e..244804c3062b 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -15,6 +15,7 @@ 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 filter_ccx(course_access): 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 filter_ccx(course_access): 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 870c192653d2..92e4329c2f94 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -41,8 +41,8 @@ ) from common.djangoapps.util.json_request import JsonResponse, JsonResponseBadRequest, expect_json -from ..config.waffle import REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND from ..utils import add_instructor, reverse_library_url +from ..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 @@ -51,52 +51,11 @@ log = logging.getLogger(__name__) -LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) -ENABLE_LIBRARY_AUTHORING_MICROFRONTEND = settings.FEATURES.get('ENABLE_LIBRARY_AUTHORING_MICROFRONTEND', False) -LIBRARY_AUTHORING_MICROFRONTEND_URL = settings.LIBRARY_AUTHORING_MICROFRONTEND_URL - -def should_redirect_to_library_authoring_mfe(): - """ - Boolean helper method, returns whether or not to redirect to the Library - Authoring MFE based on settings and flags. - """ - - return ( - ENABLE_LIBRARY_AUTHORING_MICROFRONTEND and - LIBRARY_AUTHORING_MICROFRONTEND_URL and - REDIRECT_TO_LIBRARY_AUTHORING_MICROFRONTEND.is_enabled() - ) - - -def user_can_view_create_library_button(user): - """ - Helper method for displaying the visibilty of the create_library_button. - """ - if not LIBRARIES_ENABLED: - return False - elif user.is_staff: - return True - elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): - is_course_creator = get_course_creator_status(user) == 'granted' - has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).exists() - has_course_staff_role = UserBasedRole(user=user, role=CourseStaffRole.ROLE).courses_with_role().exists() - has_course_admin_role = UserBasedRole(user=user, role=CourseInstructorRole.ROLE).courses_with_role().exists() - return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role - else: - # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. - disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) - disable_course_creation = settings.FEATURES.get('DISABLE_COURSE_CREATION', False) - if disable_library_creation is not None: - return not disable_library_creation - else: - return not disable_course_creation - - -def user_can_create_library(user, org): +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. @@ -109,29 +68,29 @@ def user_can_create_library(user, org): 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 org is None: - return False - if not LIBRARIES_ENABLED: + if not libraries_v1_enabled(): return False elif user.is_staff: return True - if settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + elif settings.FEATURES.get('ENABLE_CREATOR_GROUP', False): + org_filter_params = {} + if org: + org_filter_params['org'] = org is_course_creator = get_course_creator_status(user) == 'granted' - has_org_staff_role = org in OrgStaffRole().get_orgs_for_user(user) + has_org_staff_role = OrgStaffRole().get_orgs_for_user(user).filter(**org_filter_params).exists() has_course_staff_role = ( UserBasedRole(user=user, role=CourseStaffRole.ROLE) .courses_with_role() - .filter(org=org) + .filter(**org_filter_params) .exists() ) has_course_admin_role = ( UserBasedRole(user=user, role=CourseInstructorRole.ROLE) .courses_with_role() - .filter(org=org) + .filter(**org_filter_params) .exists() ) return is_course_creator or has_org_staff_role or has_course_staff_role or has_course_admin_role - else: # EDUCATOR-1924: DISABLE_LIBRARY_CREATION overrides DISABLE_COURSE_CREATION, if present. disable_library_creation = settings.FEATURES.get('DISABLE_LIBRARY_CREATION', None) @@ -142,6 +101,22 @@ def user_can_create_library(user, org): return not disable_course_creation +def user_can_view_create_library_button(user): + """ + Helper method for displaying the visibilty of the create_library_button. + """ + return _user_can_create_library_for_org(user) + + +def user_can_create_library(user, org): + """ + Helper method for to check if user can create library for given org. + """ + if org is None: + return False + return _user_can_create_library_for_org(user, org) + + @login_required @ensure_csrf_cookie @require_http_methods(('GET', 'POST')) @@ -149,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 acc5fc95dfe3..aa7421bb87b3 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 fc119c7edd58..1190eb239518 100644 --- a/cms/djangoapps/contentstore/views/tests/test_block.py +++ b/cms/djangoapps/contentstore/views/tests/test_block.py @@ -312,12 +312,12 @@ def test_split_test(self): 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): """ @@ -982,7 +982,7 @@ def test_shallow_duplicate(self): def test_duplicate_library_content_block(self): # pylint: disable=too-many-statements """ - Test the LibraryContentBlock's special duplication process. + Test the LegacyLibraryContentBlock's special duplication process. """ store = modulestore() @@ -3184,13 +3184,13 @@ def _verify_advanced_xblocks(self, expected_xblocks, expected_support_levels): 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) @@ -3674,14 +3674,15 @@ def test_special_exam_xblock_info( @patch_does_backend_support_onboarding @patch_get_exam_by_content_id_success @ddt.data( - ("lti_external", False), - ("other_proctoring_backend", True), + ("lti_external", False, None), + ("other_proctoring_backend", True, "test_url"), ) @ddt.unpack - def test_support_onboarding_is_correct_depending_on_lti_external( + def test_proctoring_values_correct_depending_on_lti_external( self, external_id, - expected_value, + expected_supports_onboarding_value, + expected_proctoring_link, mock_get_exam_by_content_id, mock_does_backend_support_onboarding, _mock_get_exam_configuration_dashboard_url, @@ -3691,8 +3692,9 @@ def test_support_onboarding_is_correct_depending_on_lti_external( category="sequential", display_name="Test Lesson 1", user_id=self.user.id, - is_proctored_enabled=False, - is_time_limited=False, + is_proctored_enabled=True, + is_time_limited=True, + default_time_limit_minutes=100, is_onboarding_exam=False, ) @@ -3709,7 +3711,8 @@ def test_support_onboarding_is_correct_depending_on_lti_external( include_children_predicate=ALWAYS, course=self.course, ) - assert xblock_info["supports_onboarding"] is expected_value + assert xblock_info["supports_onboarding"] is expected_supports_onboarding_value + assert xblock_info["proctoring_exam_configuration_link"] == expected_proctoring_link @patch_get_exam_configuration_dashboard_url @patch_does_backend_support_onboarding @@ -3773,6 +3776,42 @@ def test_xblock_was_never_proctortrack_proctored_exam( assert xblock_info["was_exam_ever_linked_with_external"] is False assert mock_get_exam_by_content_id.call_count == 1 + @patch_get_exam_configuration_dashboard_url + @patch_does_backend_support_onboarding + @patch_get_exam_by_content_id_success + def test_special_exam_xblock_info_get_dashboard_error( + self, + mock_get_exam_by_content_id, + _mock_does_backend_support_onboarding, + mock_get_exam_configuration_dashboard_url, + ): + sequential = BlockFactory.create( + parent_location=self.chapter.location, + category="sequential", + display_name="Test Lesson 1", + user_id=self.user.id, + is_proctored_enabled=True, + is_time_limited=True, + default_time_limit_minutes=100, + is_onboarding_exam=False, + ) + sequential = modulestore().get_item(sequential.location) + mock_get_exam_configuration_dashboard_url.side_effect = Exception("proctoring error") + xblock_info = create_xblock_info( + sequential, + include_child_info=True, + include_children_predicate=ALWAYS, + ) + + # no errors should be raised and proctoring_exam_configuration_link is None + assert xblock_info["is_proctored_exam"] is True + assert xblock_info["was_exam_ever_linked_with_external"] is True + assert xblock_info["is_time_limited"] is True + assert xblock_info["default_time_limit_minutes"] == 100 + assert xblock_info["proctoring_exam_configuration_link"] is None + assert xblock_info["supports_onboarding"] is True + assert xblock_info["is_onboarding_exam"] is False + class TestLibraryXBlockInfo(ModuleStoreTestCase): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py index 76dba57f1d67..9244ffa989b6 100644 --- a/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py +++ b/cms/djangoapps/contentstore/views/tests/test_clipboard_paste.py @@ -10,7 +10,7 @@ from organizations.models import Organization from xmodule.modulestore.django import contentstore, modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, upload_file_to_course -from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory, ToyCourseFactory, LibraryFactory from cms.djangoapps.contentstore.utils import reverse_usage_url from openedx.core.djangoapps.content_libraries import api as library_api @@ -165,12 +165,12 @@ def _setup_tagged_content(self, course_key) -> dict: publish_item=True, ).location - library = ClipboardLibraryContentPasteTestCase.setup_library() + library = ClipboardPasteFromV1LibraryTestCase.setup_library() with self.store.bulk_operations(course_key): library_content_block_key = BlockFactory.create( parent=self.store.get_item(unit_key), category="library_content", - source_library_id=str(library.key), + source_library_id=str(library.context_key), display_name="LC Block", publish_item=True, ).location @@ -184,7 +184,7 @@ def _setup_tagged_content(self, course_key) -> dict: 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, @@ -393,9 +393,9 @@ def test_paste_with_assets(self): assert source_pic2_hash != dest_pic2_hash # Because there was a conflict, this file was unchanged. -class ClipboardLibraryContentPasteTestCase(ModuleStoreTestCase): +class ClipboardPasteFromV2LibraryTestCase(ModuleStoreTestCase): """ - Test Clipboard Paste functionality with library content + Test Clipboard Paste functionality with a "new" (as of Sumac) library """ def setUp(self): @@ -406,14 +406,185 @@ def setUp(self): self.client = APIClient() self.client.login(username=self.user.username, password=self.user_password) self.store = modulestore() - library = self.setup_library() + + self.library = library_api.create_library( + 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 + """ + + def setUp(self): + """ + Set up a v1 Content Library and a library content block + """ + super().setUp() + self.client = APIClient() + self.client.login(username=self.user.username, password=self.user_password) + self.store = modulestore() + self.library = self.setup_library() # Create a library content block (lc), point it out our library, and sync it. self.course = CourseFactory.create(display_name='Course') self.orig_lc_block = BlockFactory.create( parent=self.course, category="library_content", - source_library_id=str(library.key), + source_library_id=str(self.library.context_key), display_name="LC Block", publish_item=False, ) @@ -426,18 +597,15 @@ def setUp(self): @classmethod def setup_library(cls): """ - Creates and returns a content library. + Creates and returns a legacy content library with 1 problem """ - library = library_api.create_library( - library_type=library_api.COMPLEX, - org=Organization.objects.create(name="Test Org", short_name="CL-TEST"), - slug="lib", - title="Library", - ) - # Populate it with a problem: - problem_key = library_api.create_library_block(library.key, "problem", "p1").usage_key - library_api.set_library_block_olx(problem_key, """ - + library = LibraryFactory.create(display_name='Library') + lib_block = BlockFactory.create( + parent_location=library.usage_key, + category="problem", + display_name="MCQ", + max_attempts=1, + data=""" @@ -445,9 +613,9 @@ def setup_library(cls): Right - - """) - library_api.publish_changes(library.key) + """, + publish_item=False, + ) return library def test_paste_library_content_block(self): diff --git a/cms/djangoapps/contentstore/views/tests/test_container_page.py b/cms/djangoapps/contentstore/views/tests/test_container_page.py index 1d5b52905357..426477e23408 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 @@ def test_container_page_with_valid_and_invalid_usage_key_string(self): 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=( + '