diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index e7d9e0c1fa8e..ff73e14fce53 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,14 +9,35 @@ jobs: # See also https://docs.docker.com/docker-hub/builds/ push: runs-on: ubuntu-latest - if: github.event_name == 'push' + if: github.event_name == 'push' + + strategy: + matrix: + variant: + - "lms_dev" + - "cms_dev" + - "cms" + - "lms" steps: - name: Checkout uses: actions/checkout@v2 - - name: Build and Push docker image + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v2 + + - name: Set up QEMU + uses: docker/setup-qemu-action@v2 + + - name: Login to DockerHub + uses: docker/login-action@v2 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_PASSWORD }} + + - name: Build and push lms base docker image env: DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }} - DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} - run : make docker_push + DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} + run : make docker_tag_build_push_${{matrix.variant}} + \ No newline at end of file diff --git a/Makefile b/Makefile index e328da0dc3ab..32314eef18f0 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,8 @@ # Do things in edx-platform .PHONY: base-requirements check-types clean \ compile-requirements detect_changed_source_translations dev-requirements \ - docker_auth docker_build docker_push docker_tag docs extract_translations \ + 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 \ guides help lint-imports local-requirements migrate migrate-lms migrate-cms \ pre-requirements pull pull_translations push_translations requirements shell swagger \ technical-docs test-requirements ubuntu-requirements upgrade-package upgrade @@ -100,6 +101,7 @@ REQ_FILES = \ requirements/edx/doc \ requirements/edx/testing \ requirements/edx/development \ + requirements/edx/assets \ scripts/xblock/requirements define COMMON_CONSTRAINTS_TEMP_COMMENT @@ -144,30 +146,26 @@ upgrade-package: ## update just one package to the latest usable release check-types: ## run static type-checking tests mypy -docker_build: +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: docker_build - docker tag openedx/lms openedx/lms:${GITHUB_SHA} - docker tag openedx/lms-dev openedx/lms-dev:${GITHUB_SHA} - docker tag openedx/cms openedx/cms:${GITHUB_SHA} - docker tag openedx/cms-dev openedx/cms-dev:${GITHUB_SHA} +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_auth: - echo "$$DOCKERHUB_PASSWORD" | docker login -u "$$DOCKERHUB_USERNAME" --password-stdin +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_push: docker_tag docker_auth ## push to docker hub - docker push "openedx/lms:latest" - docker push "openedx/lms:${GITHUB_SHA}" - docker push "openedx/lms-dev:latest" - docker push "openedx/lms-dev:${GITHUB_SHA}" - docker push "openedx/cms:latest" - docker push "openedx/cms:${GITHUB_SHA}" - docker push "openedx/cms-dev:latest" - docker push "openedx/cms-dev:${GITHUB_SHA}" +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 diff --git a/cms/djangoapps/contentstore/helpers.py b/cms/djangoapps/contentstore/helpers.py index 8b5ad7030228..706a20273784 100644 --- a/cms/djangoapps/contentstore/helpers.py +++ b/cms/djangoapps/contentstore/helpers.py @@ -20,8 +20,6 @@ from xmodule.modulestore.django import modulestore from cms.djangoapps.models.settings.course_grading import CourseGradingModel -from common.djangoapps.student import auth -from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole import openedx.core.djangoapps.content_staging.api as content_staging_api from .utils import reverse_course_url, reverse_library_url, reverse_usage_url @@ -377,15 +375,3 @@ def is_item_in_course_tree(item): ancestor = ancestor.get_parent() return ancestor is not None - - -def is_content_creator(user, org): - """ - Check if the user has the role to create content. - - This function checks if the User has role to create content - or if the org is supplied, it checks for Org level course content - creator. - """ - return (auth.user_has_role(user, CourseCreatorRole()) or - auth.user_has_role(user, OrgContentCreatorRole(org=org))) diff --git a/cms/djangoapps/contentstore/toggles.py b/cms/djangoapps/contentstore/toggles.py index 438f99f97b01..4f84e7bf9e32 100644 --- a/cms/djangoapps/contentstore/toggles.py +++ b/cms/djangoapps/contentstore/toggles.py @@ -159,6 +159,25 @@ def use_new_problem_editor(): return ENABLE_NEW_PROBLEM_EDITOR_FLAG.is_enabled() +# .. toggle_name: new_editors.add_game_block_button +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: This flag enables the creation of the new games block +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2023-07-26 +# .. toggle_target_removal_date: 2023-09-31 +# .. toggle_tickets: TNL-10924 +# .. toggle_warning: +ENABLE_ADD_GAME_BLOCK_FLAG = WaffleFlag('new_editors.add_game_block_button', __name__) + + +def use_add_game_block(): + """ + Returns a boolean if add game block button is enabled + """ + return ENABLE_ADD_GAME_BLOCK_FLAG.is_enabled() + + # .. toggle_name: contentstore.individualize_anonymous_user_id # .. toggle_implementation: CourseWaffleFlag # .. toggle_default: False diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 20c6237ed704..6f2835887d11 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -972,13 +972,12 @@ def get_subsections_by_assignment_type(course_key): return subsections_by_assignment_type -def update_course_discussions_settings(course_key): +def update_course_discussions_settings(course): """ Updates course provider_type when new course is created """ - provider = DiscussionsConfiguration.get(context_key=course_key).provider_type + provider = DiscussionsConfiguration.get(context_key=course.id).provider_type store = modulestore() - course = store.get_course(course_key) course.discussions_settings['provider_type'] = provider store.update_item(course, course.published_by) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 50f5fc298a40..96743daf5116 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -45,7 +45,8 @@ has_course_author_access, has_studio_read_access, has_studio_write_access, - has_studio_advanced_settings_access + has_studio_advanced_settings_access, + is_content_creator, ) from common.djangoapps.student.roles import ( CourseInstructorRole, @@ -118,7 +119,6 @@ update_course_discussions_settings, ) from .component import ADVANCED_COMPONENT_TYPES -from ..helpers import is_content_creator from cms.djangoapps.contentstore.xblock_storage_handlers.view_handlers import ( create_xblock_info, ) @@ -996,7 +996,7 @@ def create_new_course(user, org, number, run, fields): store_for_new_course = modulestore().default_modulestore.get_modulestore_type() new_course = create_new_course_in_store(store_for_new_course, user, org, number, run, fields) add_organization_course(org_data, new_course.id) - update_course_discussions_settings(new_course.id) + update_course_discussions_settings(new_course) # Enable certain fields rolling forward, where configured if default_enable_flexible_peer_openassessments(new_course.id): diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py index b1285a632539..2d1501d42014 100644 --- a/cms/djangoapps/contentstore/views/library.py +++ b/cms/djangoapps/contentstore/views/library.py @@ -30,7 +30,7 @@ STUDIO_VIEW_USERS, get_user_permissions, has_studio_read_access, - has_studio_write_access + has_studio_write_access, ) from common.djangoapps.student.roles import ( CourseInstructorRole, diff --git a/cms/envs/bok_choy.env.json b/cms/envs/bok_choy.env.json index 6f14115ab751..5301b8ada84b 100644 --- a/cms/envs/bok_choy.env.json +++ b/cms/envs/bok_choy.env.json @@ -3,7 +3,7 @@ "BULK_EMAIL_DEFAULT_FROM_EMAIL": "no-reply@example.com", "CACHES": { "celery": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_celery", "LOCATION": [ @@ -11,7 +11,7 @@ ] }, "default": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "sandbox_default", "LOCATION": [ @@ -19,7 +19,7 @@ ] }, "general": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "sandbox_general", "LOCATION": [ @@ -27,7 +27,7 @@ ] }, "mongo_metadata_inheritance": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_mongo_metadata_inheritance", "LOCATION": [ @@ -35,7 +35,7 @@ ] }, "staticfiles": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_static_files", "LOCATION": [ diff --git a/cms/envs/bok_choy.yml b/cms/envs/bok_choy.yml index d5314b557efb..705d1a472fe4 100644 --- a/cms/envs/bok_choy.yml +++ b/cms/envs/bok_choy.yml @@ -6,27 +6,27 @@ BUGS_EMAIL: bugs@example.com BULK_EMAIL_DEFAULT_FROM_EMAIL: no-reply@example.com CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_celery LOCATION: ['localhost:11211'] default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_default LOCATION: ['localhost:11211'] general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_general LOCATION: ['localhost:11211'] mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_mongo_metadata_inheritance LOCATION: ['localhost:11211'] staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_static_files LOCATION: ['localhost:11211'] @@ -128,4 +128,3 @@ XQUEUE_INTERFACE: url: http://localhost:18040 ZENDESK_API_KEY: '' ZENDESK_USER: '' - diff --git a/cms/envs/bok_choy_docker.env.json b/cms/envs/bok_choy_docker.env.json index 6ab1fb3d6647..d6b01a061e1d 100644 --- a/cms/envs/bok_choy_docker.env.json +++ b/cms/envs/bok_choy_docker.env.json @@ -3,7 +3,7 @@ "BULK_EMAIL_DEFAULT_FROM_EMAIL": "no-reply@example.com", "CACHES": { "celery": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_celery", "LOCATION": [ @@ -11,7 +11,7 @@ ] }, "default": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "sandbox_default", "LOCATION": [ @@ -19,7 +19,7 @@ ] }, "general": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "sandbox_general", "LOCATION": [ @@ -27,7 +27,7 @@ ] }, "mongo_metadata_inheritance": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_mongo_metadata_inheritance", "LOCATION": [ @@ -35,7 +35,7 @@ ] }, "staticfiles": { - "BACKEND": "django.core.cache.backends.memcached.MemcachedCache", + "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache", "KEY_FUNCTION": "common.djangoapps.util.memcache.safe_key", "KEY_PREFIX": "integration_static_files", "LOCATION": [ diff --git a/cms/envs/bok_choy_docker.yml b/cms/envs/bok_choy_docker.yml index 747c9342bbb6..b5f0990b6e28 100644 --- a/cms/envs/bok_choy_docker.yml +++ b/cms/envs/bok_choy_docker.yml @@ -6,27 +6,27 @@ BUGS_EMAIL: bugs@example.com BULK_EMAIL_DEFAULT_FROM_EMAIL: no-reply@example.com CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_celery LOCATION: ['edx.devstack.memcached:11211'] default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_default LOCATION: ['edx.devstack.memcached:11211'] general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_general LOCATION: ['edx.devstack.memcached:11211'] mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_mongo_metadata_inheritance LOCATION: ['edx.devstack.memcached:11211'] staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_static_files LOCATION: ['edx.devstack.memcached:11211'] @@ -125,4 +125,3 @@ XQUEUE_INTERFACE: url: http://localhost:18040 ZENDESK_API_KEY: '' ZENDESK_USER: '' - diff --git a/cms/envs/common.py b/cms/envs/common.py index 57d1f49f4c4e..0fa7130b9f9a 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1748,6 +1748,10 @@ # API Documentation 'drf_yasg', + # Tagging + 'openedx_tagging.core.tagging.apps.TaggingConfig', + 'openedx.features.content_tagging', + 'openedx.features.course_duration_limits', 'openedx.features.content_type_gating', 'openedx.features.discounts', @@ -2171,53 +2175,53 @@ 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '86400', # This data should be long-lived for performance, BundleCache handles invalidation - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'course_structure_cache': { 'KEY_PREFIX': 'course_structure', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '7200', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'celery': { 'KEY_PREFIX': 'celery', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '7200', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'mongo_metadata_inheritance': { 'KEY_PREFIX': 'mongo_metadata_inheritance', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': 300, - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'staticfiles': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'staticfiles_general', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'default': { 'VERSION': '1', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'default', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'configuration': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'configuration', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'general': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'general', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, } diff --git a/cms/envs/devstack-experimental.yml b/cms/envs/devstack-experimental.yml index 6a1d0ec14ae4..67356ad4b3f8 100644 --- a/cms/envs/devstack-experimental.yml +++ b/cms/envs/devstack-experimental.yml @@ -53,47 +53,47 @@ BULK_EMAIL_EMAILS_PER_TASK: 500 BULK_EMAIL_LOG_SENT_EMAILS: false CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: celery LOCATION: - edx.devstack.memcached:11211 TIMEOUT: '7200' configuration: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: 78f87108afce LOCATION: - edx.devstack.memcached:11211 course_structure_cache: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: course_structure LOCATION: - edx.devstack.memcached:11211 TIMEOUT: '7200' default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: default LOCATION: - edx.devstack.memcached:11211 VERSION: '1' general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: general LOCATION: - edx.devstack.memcached:11211 mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: mongo_metadata_inheritance LOCATION: - edx.devstack.memcached:11211 TIMEOUT: 300 staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: 78f87108afce_general LOCATION: diff --git a/cms/lib/xblock/tagging/test.py b/cms/lib/xblock/tagging/test.py index 783649946d6b..f00d1a53c89b 100644 --- a/cms/lib/xblock/tagging/test.py +++ b/cms/lib/xblock/tagging/test.py @@ -180,11 +180,11 @@ def test_preview_html(self): self.assertEqual(option_values2, ['Learned a few things', 'Learned everything', 'Learned nothing']) # Now ensure the acid_aside is not in the result - self.assertNotRegexpMatches(problem_html, r"data-block-type=[\"\']acid_aside[\"\']") # lint-amnesty, pylint: disable=deprecated-method + self.assertNotRegex(problem_html, r"data-block-type=[\"\']acid_aside[\"\']") # Ensure about video don't have asides video_html = get_preview_fragment(request, self.video, context).content - self.assertNotRegexpMatches(video_html, " cg.toLowerCase() === gradingType.toLowerCase()) + ).length === 0; + } } var is_proctored_exam = xblockInfo.get('is_proctored_exam'); @@ -269,7 +275,7 @@ if (is_proctored_exam) { ) %> -

+

<% } %> <% } else if ((xblockInfo.get('due_date') && !course.get('self_paced')) || xblockInfo.get('graded')) { %> @@ -299,7 +305,7 @@ if (is_proctored_exam) { ) %> -

+

<% } %> <% } else if (course.get('self_paced') && course.get('is_custom_relative_dates_active') && xblockInfo.get('relative_weeks_due')) { %> @@ -325,7 +331,7 @@ if (is_proctored_exam) { ) %> -

+

<% } %>
@@ -352,6 +358,22 @@ if (is_proctored_exam) { <% } %>
<% } %> + <% if (gradingPolicyMismatch) { %> +
+
+
+

+ + <%- interpolate( + gettext("This subsection is configured as \"%(gradingType)s\", which doesn't exist in the current grading policy."), + { gradingType: gradingType }, + true + ) %> +

+
+
+
+ <% } %> <% } %> <% } %> diff --git a/common/djangoapps/student/auth.py b/common/djangoapps/student/auth.py index 66bd6806a6ef..abb98dcaffdc 100644 --- a/common/djangoapps/student/auth.py +++ b/common/djangoapps/student/auth.py @@ -154,6 +154,18 @@ def has_studio_read_access(user, course_key): return bool(STUDIO_VIEW_CONTENT & get_user_permissions(user, course_key)) +def is_content_creator(user, org): + """ + Check if the user has the role to create content. + + This function checks if the User has role to create content + or if the org is supplied, it checks for Org level course content + creator. + """ + return (user_has_role(user, CourseCreatorRole()) or + user_has_role(user, OrgContentCreatorRole(org=org))) + + def add_users(caller, role, *users): """ The caller requests adding the given users to the role. Checks that the caller diff --git a/common/djangoapps/util/file.py b/common/djangoapps/util/file.py index ba59f86f3635..66fc2a6f5f35 100644 --- a/common/djangoapps/util/file.py +++ b/common/djangoapps/util/file.py @@ -8,7 +8,8 @@ from django.conf import settings from django.core.exceptions import PermissionDenied -from django.core.files.storage import DefaultStorage, get_valid_filename +from django.core.files.storage import DefaultStorage +from django.utils.text import get_valid_filename from django.utils.translation import gettext as _ from django.utils.translation import ngettext from pytz import UTC diff --git a/common/static/data/geoip/GeoLite2-Country.mmdb b/common/static/data/geoip/GeoLite2-Country.mmdb index 79e755e15760..cf331c3a8a77 100644 Binary files a/common/static/data/geoip/GeoLite2-Country.mmdb and b/common/static/data/geoip/GeoLite2-Country.mmdb differ diff --git a/conf/locale/config.yaml b/conf/locale/config.yaml index 4f4d32ed2fb5..23cb9e0d8537 100644 --- a/conf/locale/config.yaml +++ b/conf/locale/config.yaml @@ -98,8 +98,6 @@ dummy_locales: # Directories we don't search for strings. ignore_dirs: - - common/static/xmodule/modules - - common/static/xmodule/descriptors # Directories with no user-facing code. - '*/migrations' - '*/envs' diff --git a/docs/decisions/0017-reimplement-asset-processing.rst b/docs/decisions/0017-reimplement-asset-processing.rst index f77b4add1ba1..1048edf4c075 100644 --- a/docs/decisions/0017-reimplement-asset-processing.rst +++ b/docs/decisions/0017-reimplement-asset-processing.rst @@ -218,7 +218,9 @@ The three top-level edx-platform asset processing actions are *build*, *collect* - ``npm run watch`` - Bash wrappers around invocation(s) of `watchman `_, a popular file-watching library maintained by Meta. Watchman is already installed into edx-platform (and other services) via the pywatchman pip wrapper package. + Bash wrappers around invocations of the `watchdog library `_ for themable/themed assets, and `webpack --watch `_ for Webpack-managed assets. Both of these tools are available via dependencies that are already installed into edx-platform. + + We considered using `watchman `_, a popular file-watching library maintained by Meta, but found that the Python release of the library is poorly maintained (latest release 2017) and the documentation is difficult to follow. `Django uses pywatchman but is planning to migrate off of it `_ and onto `watchfiles `_. We considered watchfiles, but decided against adding another developer dependency to edx-platform. Future developers could consider migrating to watchfiles if it seemed worthwile. Build Configuration diff --git a/lms/djangoapps/courseware/tests/test_discussion_xblock.py b/lms/djangoapps/courseware/tests/test_discussion_xblock.py index 573dbf6fce6f..bbe0e68bdcd1 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_xblock.py +++ b/lms/djangoapps/courseware/tests/test_discussion_xblock.py @@ -221,7 +221,7 @@ def test_has_permission(self): """ permission_canary = object() with mock.patch( - 'lms.djangoapps.discussion.django_comment_client.permissions.has_permission', + 'xmodule.discussion_block.has_permission', return_value=permission_canary, ) as has_perm: actual_permission = self.block.has_permission("test_permission") diff --git a/lms/djangoapps/learner_home/serializers.py b/lms/djangoapps/learner_home/serializers.py index 3cb5dabd6e84..b9243a86d70a 100644 --- a/lms/djangoapps/learner_home/serializers.py +++ b/lms/djangoapps/learner_home/serializers.py @@ -563,7 +563,7 @@ class EnterpriseDashboardSerializer(serializers.Serializer): label = serializers.CharField(source="name") url = serializers.SerializerMethodField() uuid = serializers.UUIDField() - authOrgId = serializers.CharField(source="auth_org_id") + authOrgId = serializers.CharField(source="auth_org_id", allow_null=True) isLearnerPortalEnabled = serializers.BooleanField(source="enable_learner_portal") def get_url(self, instance): diff --git a/lms/djangoapps/learner_home/test_serializers.py b/lms/djangoapps/learner_home/test_serializers.py index 6e53c3a6ab60..f588af58aee4 100644 --- a/lms/djangoapps/learner_home/test_serializers.py +++ b/lms/djangoapps/learner_home/test_serializers.py @@ -1164,6 +1164,12 @@ def test_happy_path(self): }, ) + def test_no_auth_org_id(self): + """ Test for missing auth_org_id """ + input_data = self.generate_test_enterprise_customer() + del input_data['auth_org_id'] + self.assertIsNone(EnterpriseDashboardSerializer(input_data).data['authOrgId']) + class TestSocialMediaSettingsSiteSerializer(TestCase): """Tests for the SocialMediaSiteSettingsSerializer""" diff --git a/lms/envs/bok_choy.yml b/lms/envs/bok_choy.yml index 53675e6e5626..8379bf20e963 100644 --- a/lms/envs/bok_choy.yml +++ b/lms/envs/bok_choy.yml @@ -19,27 +19,27 @@ BLOCK_STRUCTURES_SETTINGS: CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_celery LOCATION: ['localhost:11211'] default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_default LOCATION: ['localhost:11211'] general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_general LOCATION: ['localhost:11211'] mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_mongo_metadata_inheritance LOCATION: ['localhost:11211'] staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_static_files LOCATION: ['localhost:11211'] @@ -252,4 +252,3 @@ XQUEUE_INTERFACE: ZENDESK_API_KEY: '' ZENDESK_USER: '' - diff --git a/lms/envs/bok_choy_docker.yml b/lms/envs/bok_choy_docker.yml index 5431910e05c0..83c830172f21 100644 --- a/lms/envs/bok_choy_docker.yml +++ b/lms/envs/bok_choy_docker.yml @@ -8,27 +8,27 @@ BUGS_EMAIL: bugs@example.com BULK_EMAIL_DEFAULT_FROM_EMAIL: no-reply@example.com CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_celery LOCATION: ['edx.devstack.memcached:11211'] default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_default LOCATION: ['edx.devstack.memcached:11211'] general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: sandbox_general LOCATION: ['edx.devstack.memcached:11211'] mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_mongo_metadata_inheritance LOCATION: ['edx.devstack.memcached:11211'] staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: integration_static_files LOCATION: ['edx.devstack.memcached:11211'] @@ -161,4 +161,3 @@ XQUEUE_INTERFACE: url: '** OVERRIDDEN **' ZENDESK_API_KEY: '' ZENDESK_USER: '' - diff --git a/lms/envs/common.py b/lms/envs/common.py index 07da2ca6e61b..af9c7daa6731 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1113,53 +1113,53 @@ 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '86400', # This data should be long-lived for performance, BundleCache handles invalidation - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'course_structure_cache': { 'KEY_PREFIX': 'course_structure', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '7200', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'celery': { 'KEY_PREFIX': 'celery', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': '7200', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'mongo_metadata_inheritance': { 'KEY_PREFIX': 'mongo_metadata_inheritance', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'TIMEOUT': 300, - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'staticfiles': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'staticfiles_general', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'default': { 'VERSION': '1', 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'default', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'configuration': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'configuration', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, 'general': { 'KEY_FUNCTION': 'common.djangoapps.util.memcache.safe_key', 'LOCATION': ['localhost:11211'], 'KEY_PREFIX': 'general', - 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache', }, } @@ -3198,6 +3198,10 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # Course Goals 'lms.djangoapps.course_goals.apps.CourseGoalsConfig', + # Tagging + 'openedx_tagging.core.tagging.apps.TaggingConfig', + 'openedx.features.content_tagging', + # Features 'openedx.features.calendar_sync', 'openedx.features.course_bookmarks', diff --git a/lms/envs/devstack-experimental.yml b/lms/envs/devstack-experimental.yml index 98b11750df58..328318f53570 100644 --- a/lms/envs/devstack-experimental.yml +++ b/lms/envs/devstack-experimental.yml @@ -72,47 +72,47 @@ BULK_EMAIL_LOG_SENT_EMAILS: false BULK_EMAIL_ROUTING_KEY_SMALL_JOBS: edx.lms.core.default CACHES: celery: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: celery LOCATION: - edx.devstack.memcached:11211 TIMEOUT: '7200' configuration: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: 78f87108afce LOCATION: - edx.devstack.memcached:11211 course_structure_cache: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: course_structure LOCATION: - edx.devstack.memcached:11211 TIMEOUT: '7200' default: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: default LOCATION: - edx.devstack.memcached:11211 VERSION: '1' general: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: general LOCATION: - edx.devstack.memcached:11211 mongo_metadata_inheritance: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: mongo_metadata_inheritance LOCATION: - edx.devstack.memcached:11211 TIMEOUT: 300 staticfiles: - BACKEND: django.core.cache.backends.memcached.MemcachedCache + BACKEND: django.core.cache.backends.memcached.PyMemcacheCache KEY_FUNCTION: common.djangoapps.util.memcache.safe_key KEY_PREFIX: 78f87108afce_general LOCATION: diff --git a/lms/static/js/instructor_dashboard/membership.js b/lms/static/js/instructor_dashboard/membership.js index 20fb73753386..bcac912b0177 100644 --- a/lms/static/js/instructor_dashboard/membership.js +++ b/lms/static/js/instructor_dashboard/membership.js @@ -765,7 +765,7 @@ such that the value can be defined later than this assignment (file load order). } if (allowed.length && emailStudents) { // Translators: A list of users appears after this sentence; - renderList(gettext('Successfully sent enrollment emails to the following users. They will be allowed to enroll once they register:'), (function() { // eslint-disable-line max-len + renderList(gettext('Email cannot be sent to the following users via batch enrollment. They will be allowed to enroll once they register:'), (function() { // eslint-disable-line max-len var k, len2, results; results = []; for (k = 0, len2 = allowed.length; k < len2; k++) { @@ -789,7 +789,7 @@ such that the value can be defined later than this assignment (file load order). } if (autoenrolled.length && emailStudents) { // Translators: A list of users appears after this sentence; - renderList(gettext('Successfully sent enrollment emails to the following users. They will be enrolled once they register:'), (function() { // eslint-disable-line max-len + renderList(gettext('Email cannot be sent to the following users via batch enrollment. They will be enrolled once they register:'), (function() { // eslint-disable-line max-len var k, len2, results; results = []; for (k = 0, len2 = autoenrolled.length; k < len2; k++) { diff --git a/openedx/core/djangoapps/course_live/plugins.py b/openedx/core/djangoapps/course_live/plugins.py index 4ef546a22887..fbb85a1d1b9f 100644 --- a/openedx/core/djangoapps/course_live/plugins.py +++ b/openedx/core/djangoapps/course_live/plugins.py @@ -44,7 +44,7 @@ def is_enabled(cls, course_key: CourseKey) -> bool: return CourseLiveConfiguration.is_enabled(course_key) @classmethod - def set_enabled(cls, course_key: CourseKey, enabled: bool, user: 'User') -> bool: + def set_enabled(cls, course_key: CourseKey, enabled: bool, user: User) -> bool: """ Set live enabled status in CourseLiveConfiguration model. """ diff --git a/openedx/core/djangoapps/course_live/tab.py b/openedx/core/djangoapps/course_live/tab.py index 34bcc8ba1655..3fe7a64587c8 100644 --- a/openedx/core/djangoapps/course_live/tab.py +++ b/openedx/core/djangoapps/course_live/tab.py @@ -1,20 +1,40 @@ """ Configurations to render Course Live Tab """ +from django.contrib.auth.base_user import AbstractBaseUser from django.utils.translation import gettext_lazy from lti_consumer.models import LtiConfiguration +from opaque_keys.edx.keys import CourseKey - -from common.djangoapps.student.roles import CourseStaffRole, CourseInstructorRole -from xmodule.course_block import CourseBlock -from xmodule.tabs import TabFragmentViewMixin +from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff from lms.djangoapps.courseware.tabs import EnrolledTab from openedx.core.djangoapps.course_live.config.waffle import ENABLE_COURSE_LIVE from openedx.core.djangoapps.course_live.models import CourseLiveConfiguration -from openedx.core.djangoapps.course_live.providers import ProviderManager, HasGlobalCredentials +from openedx.core.djangoapps.course_live.providers import HasGlobalCredentials, ProviderManager from openedx.core.lib.cache_utils import request_cached from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url from openedx.features.lti_course_tab.tab import LtiCourseLaunchMixin +from xmodule.course_block import CourseBlock +from xmodule.tabs import TabFragmentViewMixin + + +@request_cached() +def provider_is_zoom(course_key: CourseKey) -> bool: + """ + Check if the provider exists and is Zoom. + """ + course_live_configurations = CourseLiveConfiguration.get(course_key) + + if not course_live_configurations: + return False + return course_live_configurations.provider_type == "zoom" + + +def user_is_staff_or_instructor(user: AbstractBaseUser, course: CourseBlock) -> bool: + """ + Check if the user is a staff or instructor for the course. + """ + return CourseStaffRole(course.id).has_user(user) or CourseInstructorRole(course.id).has_user(user) class CourseLiveTab(LtiCourseLaunchMixin, TabFragmentViewMixin, EnrolledTab): @@ -74,14 +94,15 @@ def is_enabled(cls, course, user=None): def _get_pii_lti_parameters(self, course, request): pii_config = super()._get_pii_lti_parameters(course, request) - provider_type = '' - - course_live_configurations = CourseLiveConfiguration.get(course.id) - if course_live_configurations: - provider_type = course_live_configurations.provider_type - - if provider_type == 'zoom' and (CourseStaffRole(course.id).has_user(request.user) or - CourseInstructorRole(course.id).has_user(request.user)): + if provider_is_zoom(course.id) and user_is_staff_or_instructor(request.user, course): pii_config['person_contact_email_primary'] = request.user.email - return pii_config + + def _get_lti_roles(self, user: AbstractBaseUser, course_key: CourseKey) -> str: + """ + Get LTI roles for the user and course. + If the user is a global staff member, return the student role. + """ + if provider_is_zoom(course_key) and GlobalStaff().has_user(user): + return self.ROLE_MAP.get('student') + return super()._get_lti_roles(user, course_key) diff --git a/openedx/core/lib/safe_lxml/__init__.py b/openedx/core/lib/safe_lxml/__init__.py index b17efb15cc15..d7d5239c5102 100644 --- a/openedx/core/lib/safe_lxml/__init__.py +++ b/openedx/core/lib/safe_lxml/__init__.py @@ -7,8 +7,6 @@ def defuse_xml_libs(): """ Monkey patch and defuse all stdlib xml packages and lxml. """ - from defusedxml import defuse_stdlib - defuse_stdlib() import lxml import lxml.etree diff --git a/openedx/core/lib/safe_lxml/etree.py b/openedx/core/lib/safe_lxml/etree.py index 5555d1310844..9b685ac66162 100644 --- a/openedx/core/lib/safe_lxml/etree.py +++ b/openedx/core/lib/safe_lxml/etree.py @@ -19,9 +19,7 @@ # These private elements are used in some libraries to also defuse xml exploits for their own purposes. # We need to re-expose them so that the libraries still work. from lxml.etree import _Comment, _Element, _ElementTree, _Entity, _ProcessingInstruction - -# This should be imported after lxml.etree so that it overrides the following attributes. -from defusedxml.lxml import XML, fromstring, parse +from .xmlparser import XML, fromstring, parse class XMLParser(_XMLParser): # pylint: disable=function-redefined diff --git a/openedx/core/lib/safe_lxml/tests.py b/openedx/core/lib/safe_lxml/tests.py index 3608d43bfa93..ceb6b9828124 100644 --- a/openedx/core/lib/safe_lxml/tests.py +++ b/openedx/core/lib/safe_lxml/tests.py @@ -1,29 +1,35 @@ """ Test that we have defused XML. - -For these tests, the defusing will happen in one or more of the `conftest.py` -files that runs at pytest startup calls `defuse_xml_libs()`. - -In production, the defusing happens when the LMS or Studio `wsgi.py` files -call `defuse_xml_libs()`. """ -import defusedxml from lxml import etree +from defusedxml.lxml import EntitiesForbidden +from .xmlparser import fromstring import pytest -@pytest.mark.parametrize("attr", ["XML", "fromstring", "parse"]) -def test_etree_is_defused(attr): - func = getattr(etree, attr) - assert "defused" in func.__code__.co_filename +def test_entities_arent_resolved_exception(): + # Make sure we have disabled entity resolution. + xml = ']> &hi;' + parser = etree.XMLParser() + with pytest.raises(EntitiesForbidden): + _ = etree.XML(xml, parser=parser) + + +def test_entities_resolved(): + xml = ']> &hi;' + parser = etree.XMLParser(resolve_entities=True) + tree = fromstring(xml, parser=parser, forbid_entities=False) + pr = etree.tostring(tree) + assert pr == b'Hello' def test_entities_arent_resolved(): # Make sure we have disabled entity resolution. xml = ']> &hi;' parser = etree.XMLParser() - with pytest.raises(defusedxml.EntitiesForbidden): - _ = etree.XML(xml, parser=parser) + tree = fromstring(xml, parser=parser, forbid_entities=False) + pr = etree.tostring(tree) + assert pr == b'&hi;' diff --git a/openedx/core/lib/safe_lxml/xmlparser.py b/openedx/core/lib/safe_lxml/xmlparser.py new file mode 100644 index 000000000000..b271b43a0fbf --- /dev/null +++ b/openedx/core/lib/safe_lxml/xmlparser.py @@ -0,0 +1,138 @@ +# Based on the lxml example from defusedxml +# +"""lxml.etree protection""" + +from __future__ import print_function, absolute_import + +import threading + +from lxml import etree as _etree + +from defusedxml.lxml import DTDForbidden, EntitiesForbidden, NotSupportedError + +LXML3 = _etree.LXML_VERSION[0] >= 3 + +__origin__ = "lxml.etree" + +tostring = _etree.tostring + + +class RestrictedElement(_etree.ElementBase): + """A restricted Element class that filters out instances of some classes + """ + + __slots__ = () + blacklist = (_etree._Entity, _etree._ProcessingInstruction, _etree._Comment) # pylint: disable=protected-access + + def _filter(self, iterator): # pylint: disable=missing-function-docstring + blacklist = self.blacklist + for child in iterator: + if isinstance(child, blacklist): + continue + yield child + + def __iter__(self): + iterator = super(RestrictedElement, self).__iter__() # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iterchildren(self, tag=None, reversed=False): # pylint: disable=redefined-builtin + iterator = super(RestrictedElement, self).iterchildren(tag=tag, reversed=reversed) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iter(self, tag=None, *tags): # pylint: disable=keyword-arg-before-vararg + iterator = super(RestrictedElement, self).iter(tag=tag, *tags) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iterdescendants(self, tag=None, *tags): # pylint: disable=keyword-arg-before-vararg + iterator = super(RestrictedElement, self).iterdescendants(tag=tag, *tags) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def itersiblings(self, tag=None, preceding=False): + iterator = super(RestrictedElement, self).itersiblings(tag=tag, preceding=preceding) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def getchildren(self): + iterator = super(RestrictedElement, self).__iter__() # pylint: disable=super-with-arguments + return list(self._filter(iterator)) + + def getiterator(self, tag=None): + iterator = super(RestrictedElement, self).getiterator(tag) # pylint: disable=super-with-arguments + return self._filter(iterator) + + +class GlobalParserTLS(threading.local): + """Thread local context for custom parser instances + """ + + parser_config = { + "resolve_entities": False, + } + + element_class = RestrictedElement + + def createDefaultParser(self): # pylint: disable=missing-function-docstring + parser = _etree.XMLParser(**self.parser_config) + element_class = self.element_class + if self.element_class is not None: + lookup = _etree.ElementDefaultClassLookup(element=element_class) + parser.set_element_class_lookup(lookup) + return parser + + def setDefaultParser(self, parser): + self._default_parser = parser # pylint: disable=attribute-defined-outside-init + + def getDefaultParser(self): # pylint: disable=missing-function-docstring + parser = getattr(self, "_default_parser", None) + if parser is None: + parser = self.createDefaultParser() + self.setDefaultParser(parser) + return parser + + +_parser_tls = GlobalParserTLS() +getDefaultParser = _parser_tls.getDefaultParser + + +def check_docinfo(elementtree, forbid_dtd=False, forbid_entities=True): + """Check docinfo of an element tree for DTD and entity declarations + The check for entity declarations needs lxml 3 or newer. lxml 2.x does + not support dtd.iterentities(). + """ + docinfo = elementtree.docinfo + if docinfo.doctype: + if forbid_dtd: + raise DTDForbidden(docinfo.doctype, docinfo.system_url, docinfo.public_id) + if forbid_entities and not LXML3: + # lxml < 3 has no iterentities() + raise NotSupportedError("Unable to check for entity declarations " "in lxml 2.x") # pylint: disable=implicit-str-concat + + if forbid_entities: + for dtd in docinfo.internalDTD, docinfo.externalDTD: + if dtd is None: + continue + for entity in dtd.iterentities(): + raise EntitiesForbidden(entity.name, entity.content, None, None, None, None) + + +def parse(source, parser=None, base_url=None, forbid_dtd=False, forbid_entities=True): # pylint: disable=missing-function-docstring + if parser is None: + parser = getDefaultParser() + elementtree = _etree.parse(source, parser, base_url=base_url) + check_docinfo(elementtree, forbid_dtd, forbid_entities) + return elementtree + + +def fromstring(text, parser=None, base_url=None, forbid_dtd=False, forbid_entities=True): # pylint: disable=missing-function-docstring + if parser is None: + parser = getDefaultParser() + rootelement = _etree.fromstring(text, parser, base_url=base_url) + elementtree = rootelement.getroottree() + check_docinfo(elementtree, forbid_dtd, forbid_entities) + return rootelement + + +XML = fromstring + + +def iterparse(*args, **kwargs): + raise NotSupportedError("iterparse not available") diff --git a/openedx/features/content_tagging/__init__.py b/openedx/features/content_tagging/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/admin.py b/openedx/features/content_tagging/admin.py new file mode 100644 index 000000000000..a9fa146ce8bd --- /dev/null +++ b/openedx/features/content_tagging/admin.py @@ -0,0 +1,6 @@ +""" Tagging app admin """ +from django.contrib import admin + +from .models import TaxonomyOrg + +admin.site.register(TaxonomyOrg) diff --git a/openedx/features/content_tagging/api.py b/openedx/features/content_tagging/api.py new file mode 100644 index 000000000000..f1bb43f66ce9 --- /dev/null +++ b/openedx/features/content_tagging/api.py @@ -0,0 +1,157 @@ +""" +Content Tagging APIs +""" +from typing import Iterator, List, Type, Union + +import openedx_tagging.core.tagging.api as oel_tagging +from django.db.models import QuerySet +from opaque_keys.edx.keys import LearningContextKey +from opaque_keys.edx.locator import BlockUsageLocator +from openedx_tagging.core.tagging.models import Taxonomy +from organizations.models import Organization + +from .models import ContentObjectTag, ContentTaxonomy, TaxonomyOrg + + +def create_taxonomy( + name: str, + description: str = None, + enabled=True, + required=False, + allow_multiple=False, + allow_free_text=False, + taxonomy_class: Type = ContentTaxonomy, +) -> Taxonomy: + """ + Creates, saves, and returns a new Taxonomy with the given attributes. + + If `taxonomy_class` not provided, then uses ContentTaxonomy. + """ + return oel_tagging.create_taxonomy( + name=name, + description=description, + enabled=enabled, + required=required, + allow_multiple=allow_multiple, + allow_free_text=allow_free_text, + taxonomy_class=taxonomy_class, + ) + + +def set_taxonomy_orgs( + taxonomy: Taxonomy, + all_orgs=False, + orgs: List[Organization] = None, + relationship: TaxonomyOrg.RelType = TaxonomyOrg.RelType.OWNER, +): + """ + Updates the list of orgs associated with the given taxonomy. + + Currently, we only have an "owner" relationship, but there may be other types added in future. + + When an org has an "owner" relationship with a taxonomy, that taxonomy is available for use by content in that org, + mies + + If `all_orgs`, then the taxonomy is associated with all organizations, and the `orgs` parameter is ignored. + + If not `all_orgs`, the taxonomy is associated with each org in the `orgs` list. If that list is empty, the + taxonomy is not associated with any orgs. + """ + TaxonomyOrg.objects.filter( + taxonomy=taxonomy, + rel_type=relationship, + ).delete() + + # org=None means the relationship is with "all orgs" + if all_orgs: + orgs = [None] + if orgs: + TaxonomyOrg.objects.bulk_create( + [ + TaxonomyOrg( + taxonomy=taxonomy, + org=org, + rel_type=relationship, + ) + for org in orgs + ] + ) + + +def get_taxonomies_for_org( + enabled=True, + org_owner: Organization = None, +) -> QuerySet: + """ + Generates a list of the enabled Taxonomies available for the given org, sorted by name. + + We return a QuerySet here for ease of use with Django Rest Framework and other query-based use cases. + So be sure to use `Taxonomy.cast()` to cast these instances to the appropriate subclass before use. + + If no `org` is provided, then only Taxonomies which are available for _all_ Organizations are returned. + + If you want the disabled Taxonomies, pass enabled=False. + If you want all Taxonomies (both enabled and disabled), pass enabled=None. + """ + taxonomies = oel_tagging.get_taxonomies(enabled=enabled) + return ContentTaxonomy.taxonomies_for_org( + org=org_owner, + queryset=taxonomies, + ) + + +def get_content_tags( + object_id: str, taxonomy: Taxonomy = None, valid_only=True +) -> Iterator[ContentObjectTag]: + """ + Generates a list of content tags for a given object. + + Pass taxonomy to limit the returned object_tags to a specific taxonomy. + + Pass valid_only=False when displaying tags to content authors, so they can see invalid tags too. + Invalid tags will (probably) be hidden from learners. + """ + for object_tag in oel_tagging.get_object_tags( + object_id=object_id, + taxonomy=taxonomy, + valid_only=valid_only, + ): + yield ContentObjectTag.cast(object_tag) + + +def tag_content_object( + taxonomy: Taxonomy, + tags: List, + object_id: Union[BlockUsageLocator, LearningContextKey], +) -> List[ContentObjectTag]: + """ + This is the main API to use when you want to add/update/delete tags from a content object (e.g. an XBlock or + course). + + It works one "Taxonomy" at a time, i.e. one field at a time, so you can set call it with taxonomy=Keywords, + tags=["gravity", "newton"] to replace any "Keywords" [Taxonomy] tags on the given content object with "gravity" and + "newton". Doing so to change the "Keywords" Taxonomy won't affect other Taxonomy's tags (other fields) on the + object, such as "Language: [en]" or "Difficulty: [hard]". + + If it's a free-text taxonomy, then the list should be a list of tag values. + Otherwise, it should be a list of existing Tag IDs. + + Raises ValueError if the proposed tags are invalid for this taxonomy. + Preserves existing (valid) tags, adds new (valid) tags, and removes omitted (or invalid) tags. + """ + content_tags = [] + for object_tag in oel_tagging.tag_object( + taxonomy=taxonomy, + tags=tags, + object_id=str(object_id), + ): + content_tags.append(ContentObjectTag.cast(object_tag)) + return content_tags + + +# Expose the oel_tagging APIs + +get_taxonomy = oel_tagging.get_taxonomy +get_taxonomies = oel_tagging.get_taxonomies +get_tags = oel_tagging.get_tags +resync_object_tags = oel_tagging.resync_object_tags diff --git a/openedx/features/content_tagging/apps.py b/openedx/features/content_tagging/apps.py new file mode 100644 index 000000000000..29f9c5005f43 --- /dev/null +++ b/openedx/features/content_tagging/apps.py @@ -0,0 +1,12 @@ +""" +Define the content tagging Django App. +""" + +from django.apps import AppConfig + + +class ContentTaggingConfig(AppConfig): + """App config for the content tagging feature""" + + default_auto_field = "django.db.models.BigAutoField" + name = "openedx.features.content_tagging" diff --git a/openedx/features/content_tagging/migrations/0001_initial.py b/openedx/features/content_tagging/migrations/0001_initial.py new file mode 100644 index 000000000000..7d74d7ab73a0 --- /dev/null +++ b/openedx/features/content_tagging/migrations/0001_initial.py @@ -0,0 +1,86 @@ +# Generated by Django 3.2.20 on 2023-07-25 06:17 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + initial = True + + dependencies = [ + ("oel_tagging", "0002_auto_20230718_2026"), + ("organizations", "0003_historicalorganizationcourse"), + ] + + operations = [ + migrations.CreateModel( + name="ContentObjectTag", + fields=[], + options={ + "proxy": True, + "indexes": [], + "constraints": [], + }, + bases=("oel_tagging.objecttag",), + ), + migrations.CreateModel( + name="ContentTaxonomy", + fields=[], + options={ + "proxy": True, + "indexes": [], + "constraints": [], + }, + bases=("oel_tagging.taxonomy",), + ), + migrations.CreateModel( + name="TaxonomyOrg", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "rel_type", + models.CharField( + choices=[("OWN", "owner")], default="OWN", max_length=3 + ), + ), + ( + "org", + models.ForeignKey( + default=None, + help_text="Organization that is related to this taxonomy.If None, then this taxonomy is related to all organizations.", + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="organizations.organization", + ), + ), + ( + "taxonomy", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="oel_tagging.taxonomy", + ), + ), + ], + ), + migrations.AddIndex( + model_name="taxonomyorg", + index=models.Index( + fields=["taxonomy", "rel_type"], name="content_tag_taxonom_b04dd1_idx" + ), + ), + migrations.AddIndex( + model_name="taxonomyorg", + index=models.Index( + fields=["taxonomy", "rel_type", "org"], + name="content_tag_taxonom_70d60b_idx", + ), + ), + ] diff --git a/openedx/features/content_tagging/migrations/__init__.py b/openedx/features/content_tagging/migrations/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/models.py b/openedx/features/content_tagging/models.py new file mode 100644 index 000000000000..a3b094a8e9ff --- /dev/null +++ b/openedx/features/content_tagging/models.py @@ -0,0 +1,166 @@ +""" +Content Tagging models +""" +from typing import List, Union + +from django.db import models +from django.db.models import Exists, OuterRef, Q, QuerySet +from django.utils.translation import gettext as _ +from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import LearningContextKey +from opaque_keys.edx.locator import BlockUsageLocator +from openedx_tagging.core.tagging.models import ObjectTag, Taxonomy +from organizations.models import Organization + + +class TaxonomyOrg(models.Model): + """ + Represents the many-to-many relationship between Taxonomies and Organizations. + + We keep this as a separate class from ContentTaxonomy so that class can remain a proxy for Taxonomy, keeping the + data models and usage simple. + """ + + class RelType(models.TextChoices): + OWNER = "OWN", _("owner") + + taxonomy = models.ForeignKey(Taxonomy, on_delete=models.CASCADE) + org = models.ForeignKey( + Organization, + null=True, + default=None, + on_delete=models.CASCADE, + help_text=_( + "Organization that is related to this taxonomy." + "If None, then this taxonomy is related to all organizations." + ), + ) + rel_type = models.CharField( + max_length=3, + choices=RelType.choices, + default=RelType.OWNER, + ) + + class Meta: + indexes = [ + models.Index(fields=["taxonomy", "rel_type"]), + models.Index(fields=["taxonomy", "rel_type", "org"]), + ] + + @classmethod + def get_relationships( + cls, taxonomy: Taxonomy, rel_type: RelType, org_short_name: str = None + ) -> QuerySet: + """ + Returns the relationships of the given rel_type and taxonomy where: + * the relationship is available for all organizations, OR + * (if provided) the relationship is available to the org with the given org_short_name + """ + # A relationship with org=None means all Organizations + org_filter = Q(org=None) + if org_short_name is not None: + org_filter |= Q(org__short_name=org_short_name) + return cls.objects.filter( + taxonomy=taxonomy, + rel_type=rel_type, + ).filter(org_filter) + + @classmethod + def get_organizations( + cls, taxonomy: Taxonomy, rel_type: RelType + ) -> List[Organization]: + """ + Returns the list of Organizations which have the given relationship to the taxonomy. + """ + rels = cls.objects.filter( + taxonomy=taxonomy, + rel_type=rel_type, + ) + # A relationship with org=None means all Organizations + if rels.filter(org=None).exists(): + return list(Organization.objects.all()) + return [rel.org for rel in rels] + + +class ContentObjectTag(ObjectTag): + """ + ObjectTag that requires an LearningContextKey or BlockUsageLocator as the object ID. + """ + + class Meta: + proxy = True + + @property + def object_key(self) -> Union[BlockUsageLocator, LearningContextKey]: + """ + Returns the object ID parsed as a UsageKey or LearningContextKey. + Raises InvalidKeyError object_id cannot be parse into one of those key types. + + Returns None if there's no object_id. + """ + try: + return LearningContextKey.from_string(str(self.object_id)) + except InvalidKeyError: + return BlockUsageLocator.from_string(str(self.object_id)) + + +class ContentTaxonomy(Taxonomy): + """ + Taxonomy which can only tag Content objects (e.g. XBlocks or Courses) via ContentObjectTag. + + Also ensures a valid TaxonomyOrg owner relationship with the content object. + """ + + class Meta: + proxy = True + + @classmethod + def taxonomies_for_org( + cls, + queryset: QuerySet, + org: Organization = None, + ) -> QuerySet: + """ + Filters the given QuerySet to those ContentTaxonomies which are available for the given organization. + + If no `org` is provided, then only ContentTaxonomies available to all organizations are returned. + If `org` is provided, then ContentTaxonomies available to this organizations are also returned. + """ + org_short_name = org.short_name if org else None + return queryset.filter( + Exists( + TaxonomyOrg.get_relationships( + taxonomy=OuterRef("pk"), + rel_type=TaxonomyOrg.RelType.OWNER, + org_short_name=org_short_name, + ) + ) + ) + + def _check_object(self, object_tag: ObjectTag) -> bool: + """ + Returns True if this ObjectTag has a valid object_id. + """ + content_tag = ContentObjectTag.cast(object_tag) + try: + content_tag.object_key + except InvalidKeyError: + return False + return super()._check_object(content_tag) + + def _check_taxonomy(self, object_tag: ObjectTag) -> bool: + """ + Returns True if this taxonomy is owned by the tag's org. + """ + content_tag = ContentObjectTag.cast(object_tag) + try: + object_key = content_tag.object_key + except InvalidKeyError: + return False + if not TaxonomyOrg.get_relationships( + taxonomy=self, + rel_type=TaxonomyOrg.RelType.OWNER, + org_short_name=object_key.org, + ).exists(): + return False + return super()._check_taxonomy(content_tag) diff --git a/openedx/features/content_tagging/rules.py b/openedx/features/content_tagging/rules.py new file mode 100644 index 000000000000..9ab7072bf96e --- /dev/null +++ b/openedx/features/content_tagging/rules.py @@ -0,0 +1,106 @@ +"""Django rules-based permissions for tagging""" + +import openedx_tagging.core.tagging.rules as oel_tagging +import rules +from django.contrib.auth import get_user_model + +from common.djangoapps.student.auth import is_content_creator + +from .models import TaxonomyOrg + +User = get_user_model() + + +def is_taxonomy_admin(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: + """ + Returns True if the given user is a Taxonomy Admin for the given content taxonomy. + + Global Taxonomy Admins include global staff and superusers, plus course creators who can create courses for any org. + Otherwise, a taxonomy must be provided to determine if the user is a org-level course creator for one of the + taxonomy's org owners. + """ + if oel_tagging.is_taxonomy_admin(user): + return True + + if not taxonomy: + return is_content_creator(user, None) + + taxonomy_orgs = TaxonomyOrg.get_organizations( + taxonomy=taxonomy, + rel_type=TaxonomyOrg.RelType.OWNER, + ) + for org in taxonomy_orgs: + if is_content_creator(user, org.short_name): + return True + return False + + +@rules.predicate +def can_view_taxonomy(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: + """ + Anyone can view an enabled taxonomy, + but only taxonomy admins can view a disabled taxonomy. + """ + if taxonomy: + taxonomy = taxonomy.cast() + return (taxonomy and taxonomy.enabled) or is_taxonomy_admin(user, taxonomy) + + +@rules.predicate +def can_change_taxonomy(user: User, taxonomy: oel_tagging.Taxonomy = None) -> bool: + """ + Even taxonomy admins cannot change system taxonomies. + """ + if taxonomy: + taxonomy = taxonomy.cast() + return is_taxonomy_admin(user, taxonomy) and ( + not taxonomy or (taxonomy and not taxonomy.system_defined) + ) + + +@rules.predicate +def can_change_taxonomy_tag(user: User, tag: oel_tagging.Tag = None) -> bool: + """ + Even taxonomy admins cannot add tags to system taxonomies (their tags are system-defined), or free-text taxonomies + (these don't have predefined tags). + """ + taxonomy = tag.taxonomy if tag else None + if taxonomy: + taxonomy = taxonomy.cast() + return is_taxonomy_admin(user, taxonomy) and ( + not tag + or not taxonomy + or (taxonomy and not taxonomy.allow_free_text and not taxonomy.system_defined) + ) + + +@rules.predicate +def can_change_object_tag(user: User, object_tag: oel_tagging.ObjectTag = None) -> bool: + """ + Taxonomy admins can create or modify object tags on enabled taxonomies. + """ + taxonomy = object_tag.taxonomy if object_tag else None + if taxonomy: + taxonomy = taxonomy.cast() + return is_taxonomy_admin(user, taxonomy) and ( + not object_tag or not taxonomy or (taxonomy and taxonomy.cast().enabled) + ) + + +# Taxonomy +rules.set_perm("oel_tagging.add_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.change_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.delete_taxonomy", can_change_taxonomy) +rules.set_perm("oel_tagging.view_taxonomy", can_view_taxonomy) + +# Tag +rules.set_perm("oel_tagging.add_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.change_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.delete_tag", can_change_taxonomy_tag) +rules.set_perm("oel_tagging.view_tag", rules.always_allow) + +# ObjectTag +rules.set_perm("oel_tagging.add_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.change_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.delete_object_tag", can_change_object_tag) +rules.set_perm("oel_tagging.view_object_tag", rules.always_allow) diff --git a/openedx/features/content_tagging/tests/__init__.py b/openedx/features/content_tagging/tests/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/openedx/features/content_tagging/tests/test_api.py b/openedx/features/content_tagging/tests/test_api.py new file mode 100644 index 000000000000..f97958f9f60c --- /dev/null +++ b/openedx/features/content_tagging/tests/test_api.py @@ -0,0 +1,245 @@ +"""Tests for the Tagging models""" +import ddt +from django.test.testcases import TestCase +from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx_tagging.core.tagging.models import ObjectTag, Tag +from organizations.models import Organization + +from .. import api + + +class TestTaxonomyMixin: + """ + Sets up data for testing Content Taxonomies. + """ + + def setUp(self): + super().setUp() + self.org1 = Organization.objects.create(name="OpenedX", short_name="OeX") + self.org2 = Organization.objects.create(name="Axim", short_name="Ax") + # Taxonomies + self.taxonomy_disabled = api.create_taxonomy( + name="Learning Objectives", + enabled=False, + ) + api.set_taxonomy_orgs(self.taxonomy_disabled, all_orgs=True) + self.taxonomy_all_orgs = api.create_taxonomy( + name="Content Types", + enabled=True, + ) + api.set_taxonomy_orgs(self.taxonomy_all_orgs, all_orgs=True) + self.taxonomy_both_orgs = api.create_taxonomy( + name="OpenedX/Axim Content Types", + enabled=True, + ) + api.set_taxonomy_orgs(self.taxonomy_both_orgs, orgs=[self.org1, self.org2]) + self.taxonomy_one_org = api.create_taxonomy( + name="OpenedX Content Types", + enabled=True, + ) + api.set_taxonomy_orgs(self.taxonomy_one_org, orgs=[self.org1]) + self.taxonomy_no_orgs = api.create_taxonomy( + name="No orgs", + enabled=True, + ) + # Tags + self.tag_disabled = Tag.objects.create( + taxonomy=self.taxonomy_disabled, + value="learning", + ) + self.tag_all_orgs = Tag.objects.create( + taxonomy=self.taxonomy_all_orgs, + value="learning", + ) + self.tag_both_orgs = Tag.objects.create( + taxonomy=self.taxonomy_both_orgs, + value="learning", + ) + self.tag_one_org = Tag.objects.create( + taxonomy=self.taxonomy_one_org, + value="learning", + ) + self.tag_no_orgs = Tag.objects.create( + taxonomy=self.taxonomy_no_orgs, + value="learning", + ) + # ObjectTags + self.all_orgs_course_tag = api.tag_content_object( + taxonomy=self.taxonomy_all_orgs, + tags=[self.tag_all_orgs.id], + object_id=CourseKey.from_string("course-v1:OeX+DemoX+Demo_Course"), + )[0] + self.all_orgs_block_tag = api.tag_content_object( + taxonomy=self.taxonomy_all_orgs, + tags=[self.tag_all_orgs.id], + object_id=UsageKey.from_string( + "block-v1:Ax+DemoX+Demo_Course+type@vertical+block@abcde" + ), + )[0] + self.both_orgs_course_tag = api.tag_content_object( + taxonomy=self.taxonomy_both_orgs, + tags=[self.tag_both_orgs.id], + object_id=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), + )[0] + self.both_orgs_block_tag = api.tag_content_object( + taxonomy=self.taxonomy_both_orgs, + tags=[self.tag_both_orgs.id], + object_id=UsageKey.from_string( + "block-v1:OeX+DemoX+Demo_Course+type@video+block@abcde" + ), + )[0] + self.one_org_block_tag = api.tag_content_object( + taxonomy=self.taxonomy_one_org, + tags=[self.tag_one_org.id], + object_id=UsageKey.from_string( + "block-v1:OeX+DemoX+Demo_Course+type@html+block@abcde" + ), + )[0] + self.disabled_course_tag = api.tag_content_object( + taxonomy=self.taxonomy_disabled, + tags=[self.tag_disabled.id], + object_id=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), + )[0] + + # Invalid object tags must be manually created + self.all_orgs_invalid_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_all_orgs, + tag=self.tag_all_orgs, + object_id="course-v1_OpenedX_DemoX_Demo_Course", + ) + self.one_org_invalid_org_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_one_org, + tag=self.tag_one_org, + object_id="block-v1_OeX_DemoX_Demo_Course_type_html_block@abcde", + ) + self.no_orgs_invalid_tag = ObjectTag.objects.create( + taxonomy=self.taxonomy_no_orgs, + tag=self.tag_no_orgs, + object_id=CourseKey.from_string("course-v1:Ax+DemoX+Demo_Course"), + ) + + +@ddt.ddt +class TestAPITaxonomy(TestTaxonomyMixin, TestCase): + """ + Tests the Content Taxonomy APIs. + """ + + def test_get_taxonomies_enabled_subclasses(self): + with self.assertNumQueries(1): + taxonomies = list(taxonomy.cast() for taxonomy in api.get_taxonomies()) + assert taxonomies == [ + self.taxonomy_all_orgs, + self.taxonomy_no_orgs, + self.taxonomy_one_org, + self.taxonomy_both_orgs, + ] + + @ddt.data( + # All orgs + (None, True, ["taxonomy_all_orgs"]), + (None, False, ["taxonomy_disabled"]), + (None, None, ["taxonomy_all_orgs", "taxonomy_disabled"]), + # Org 1 + ("org1", True, ["taxonomy_all_orgs", "taxonomy_one_org", "taxonomy_both_orgs"]), + ("org1", False, ["taxonomy_disabled"]), + ( + "org1", + None, + [ + "taxonomy_all_orgs", + "taxonomy_disabled", + "taxonomy_one_org", + "taxonomy_both_orgs", + ], + ), + # Org 2 + ("org2", True, ["taxonomy_all_orgs", "taxonomy_both_orgs"]), + ("org2", False, ["taxonomy_disabled"]), + ( + "org2", + None, + ["taxonomy_all_orgs", "taxonomy_disabled", "taxonomy_both_orgs"], + ), + ) + @ddt.unpack + def test_get_taxonomies_for_org(self, org_attr, enabled, expected): + org_owner = getattr(self, org_attr) if org_attr else None + taxonomies = list( + taxonomy.cast() + for taxonomy in api.get_taxonomies_for_org( + org_owner=org_owner, enabled=enabled + ) + ) + assert taxonomies == [ + getattr(self, taxonomy_attr) for taxonomy_attr in expected + ] + + @ddt.data( + ("taxonomy_all_orgs", "all_orgs_course_tag"), + ("taxonomy_all_orgs", "all_orgs_block_tag"), + ("taxonomy_both_orgs", "both_orgs_course_tag"), + ("taxonomy_both_orgs", "both_orgs_block_tag"), + ("taxonomy_one_org", "one_org_block_tag"), + ) + @ddt.unpack + def test_get_content_tags_valid_for_org( + self, + taxonomy_attr, + object_tag_attr, + ): + taxonomy_id = getattr(self, taxonomy_attr).id + taxonomy = api.get_taxonomy(taxonomy_id) + object_tag = getattr(self, object_tag_attr) + with self.assertNumQueries(1): + valid_tags = list( + api.get_content_tags( + taxonomy=taxonomy, + object_id=object_tag.object_id, + valid_only=True, + ) + ) + assert len(valid_tags) == 1 + assert valid_tags[0].id == object_tag.id + + @ddt.data( + ("taxonomy_disabled", "disabled_course_tag"), + ("taxonomy_all_orgs", "all_orgs_course_tag"), + ("taxonomy_all_orgs", "all_orgs_block_tag"), + ("taxonomy_all_orgs", "all_orgs_invalid_tag"), + ("taxonomy_both_orgs", "both_orgs_course_tag"), + ("taxonomy_both_orgs", "both_orgs_block_tag"), + ("taxonomy_one_org", "one_org_block_tag"), + ("taxonomy_one_org", "one_org_invalid_org_tag"), + ) + @ddt.unpack + def test_get_content_tags_include_invalid( + self, + taxonomy_attr, + object_tag_attr, + ): + taxonomy_id = getattr(self, taxonomy_attr).id + taxonomy = api.get_taxonomy(taxonomy_id) + object_tag = getattr(self, object_tag_attr) + with self.assertNumQueries(1): + valid_tags = list( + api.get_content_tags( + taxonomy=taxonomy, + object_id=object_tag.object_id, + valid_only=False, + ) + ) + assert len(valid_tags) == 1 + assert valid_tags[0].id == object_tag.id + + @ddt.data( + "all_orgs_invalid_tag", + "one_org_invalid_org_tag", + "no_orgs_invalid_tag", + ) + def test_object_tag_not_valid_check_object(self, tag_attr): + object_tag = getattr(self, tag_attr) + assert not object_tag.is_valid() + + def test_get_tags(self): + assert api.get_tags(self.taxonomy_all_orgs) == [self.tag_all_orgs] diff --git a/openedx/features/content_tagging/tests/test_rules.py b/openedx/features/content_tagging/tests/test_rules.py new file mode 100644 index 000000000000..029657e44f23 --- /dev/null +++ b/openedx/features/content_tagging/tests/test_rules.py @@ -0,0 +1,481 @@ +"""Tests content_tagging rules-based permissions""" + +import ddt +from django.contrib.auth import get_user_model +from django.test.testcases import TestCase, override_settings +from openedx_tagging.core.tagging.models import ObjectTag, Tag +from organizations.models import Organization + +from common.djangoapps.student.auth import add_users, update_org_role +from common.djangoapps.student.roles import CourseCreatorRole, OrgContentCreatorRole + +from .. import api +from .test_api import TestTaxonomyMixin + +User = get_user_model() + + +@ddt.ddt +@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": True}) +class TestRulesTaxonomy(TestTaxonomyMixin, TestCase): + """ + Tests that the expected rules have been applied to the Taxonomy models. + + We set ENABLE_CREATOR_GROUP for these tests, otherwise all users have course creator access for all orgs. + """ + + def setUp(self): + super().setUp() + self.superuser = User.objects.create( + username="superuser", + email="superuser@example.com", + is_superuser=True, + ) + self.staff = User.objects.create( + username="staff", + email="staff@example.com", + is_staff=True, + ) + # Normal user: grant course creator role (for all orgs) + self.user_all_orgs = User.objects.create( + username="user_all_orgs", + email="staff+all@example.com", + ) + add_users(self.staff, CourseCreatorRole(), self.user_all_orgs) + + # Normal user: grant course creator access to both org1 and org2 + self.user_both_orgs = User.objects.create( + username="user_both_orgs", + email="staff+both@example.com", + ) + update_org_role( + self.staff, + OrgContentCreatorRole, + self.user_both_orgs, + [self.org1.short_name, self.org2.short_name], + ) + + # Normal user: grant course creator access to org2 + self.user_org2 = User.objects.create( + username="user_org2", + email="staff+org2@example.com", + ) + update_org_role( + self.staff, OrgContentCreatorRole, self.user_org2, [self.org2.short_name] + ) + + # Normal user: no course creator access + self.learner = User.objects.create( + username="learner", + email="learner@example.com", + ) + + def _expected_users_have_perm( + self, perm, obj, learner_perm=False, learner_obj=False, user_org2=True + ): + """ + Checks that all users have the given permission on the given object. + + If learners_too, then the learner user should have it too. + """ + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm) + assert self.superuser.has_perm(perm, obj) + assert self.staff.has_perm(perm) + assert self.staff.has_perm(perm, obj) + assert self.user_all_orgs.has_perm(perm) + assert self.user_all_orgs.has_perm(perm, obj) + + # Org content creators are bound by a taxonomy's org restrictions + assert self.user_both_orgs.has_perm(perm) == learner_perm + assert self.user_both_orgs.has_perm(perm, obj) + assert self.user_org2.has_perm(perm) == learner_perm + # user_org2 does not have course creator access for org 1 + assert self.user_org2.has_perm(perm, obj) == user_org2 + + # Learners can't do much but view + assert self.learner.has_perm(perm) == learner_perm + assert self.learner.has_perm(perm, obj) == learner_obj + + # Taxonomy + + @ddt.data( + ("oel_tagging.add_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_disabled"), + ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_disabled"), + ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), + ) + @ddt.unpack + def test_change_taxonomy_all_orgs(self, perm, taxonomy_attr): + """Taxonomy administrators with course creator access for the taxonomy org""" + taxonomy = getattr(self, taxonomy_attr) + self._expected_users_have_perm(perm, taxonomy) + + @ddt.data( + ("oel_tagging.add_taxonomy", "taxonomy_one_org"), + ("oel_tagging.change_taxonomy", "taxonomy_one_org"), + ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), + ) + @ddt.unpack + def test_change_taxonomy_org1(self, perm, taxonomy_attr): + taxonomy = getattr(self, taxonomy_attr) + self._expected_users_have_perm(perm, taxonomy, user_org2=False) + + @ddt.data( + "oel_tagging.add_taxonomy", + "oel_tagging.change_taxonomy", + "oel_tagging.delete_taxonomy", + ) + def test_system_taxonomy(self, perm): + """Taxonomy administrators cannot edit system taxonomies""" + system_taxonomy = api.create_taxonomy( + name="System Languages", + ) + system_taxonomy.system_defined = True + assert self.superuser.has_perm(perm, system_taxonomy) + assert not self.staff.has_perm(perm, system_taxonomy) + assert not self.user_all_orgs.has_perm(perm, system_taxonomy) + assert not self.user_both_orgs.has_perm(perm, system_taxonomy) + assert not self.user_org2.has_perm(perm, system_taxonomy) + assert not self.learner.has_perm(perm, system_taxonomy) + + @ddt.data( + (True, "taxonomy_all_orgs"), + (False, "taxonomy_all_orgs"), + (True, "taxonomy_both_orgs"), + (False, "taxonomy_both_orgs"), + ) + @ddt.unpack + def test_view_taxonomy_enabled(self, enabled, taxonomy_attr): + """Anyone can see enabled taxonomies, but learners cannot see disabled taxonomies""" + taxonomy = getattr(self, taxonomy_attr) + taxonomy.enabled = enabled + perm = "oel_tagging.view_taxonomy" + self._expected_users_have_perm(perm, taxonomy, learner_obj=enabled) + + @ddt.data( + (True, "taxonomy_no_orgs"), + (False, "taxonomy_no_orgs"), + ) + @ddt.unpack + def test_view_taxonomy_no_orgs(self, enabled, taxonomy_attr): + """ + Enabled taxonomies with no org can be viewed by anyone. + Disabled taxonomies with no org can only be viewed by staff/superusers. + """ + taxonomy = getattr(self, taxonomy_attr) + taxonomy.enabled = enabled + perm = "oel_tagging.view_taxonomy" + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert self.user_all_orgs.has_perm(perm, taxonomy) == enabled + assert self.user_both_orgs.has_perm(perm, taxonomy) == enabled + assert self.user_org2.has_perm(perm, taxonomy) == enabled + assert self.learner.has_perm(perm, taxonomy) == enabled + + @ddt.data( + ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), + ) + @ddt.unpack + def test_change_taxonomy_no_orgs(self, perm, taxonomy_attr): + """ + Taxonomies with no org can only be changed by staff and superusers. + """ + taxonomy = getattr(self, taxonomy_attr) + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm, taxonomy) + assert not self.user_all_orgs.has_perm(perm, taxonomy) + assert not self.user_both_orgs.has_perm(perm, taxonomy) + assert not self.user_org2.has_perm(perm, taxonomy) + assert not self.learner.has_perm(perm, taxonomy) + + # Tag + + @ddt.data( + ("oel_tagging.add_tag", "tag_all_orgs"), + ("oel_tagging.add_tag", "tag_both_orgs"), + ("oel_tagging.add_tag", "tag_disabled"), + ("oel_tagging.change_tag", "tag_all_orgs"), + ("oel_tagging.change_tag", "tag_both_orgs"), + ("oel_tagging.change_tag", "tag_disabled"), + ("oel_tagging.delete_tag", "tag_all_orgs"), + ("oel_tagging.delete_tag", "tag_both_orgs"), + ("oel_tagging.delete_tag", "tag_disabled"), + ) + @ddt.unpack + def test_change_tag_all_orgs(self, perm, tag_attr): + """Taxonomy administrators can modify tags on non-free-text taxonomies""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, tag) + + @ddt.data( + ("oel_tagging.add_tag", "tag_one_org"), + ("oel_tagging.change_tag", "tag_one_org"), + ("oel_tagging.delete_tag", "tag_one_org"), + ) + @ddt.unpack + def test_change_tag_org1(self, perm, tag_attr): + """Taxonomy administrators can modify tags on non-free-text taxonomies""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, tag, user_org2=False) + + @ddt.data( + "oel_tagging.add_tag", + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) + def test_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify any Tag, even those with no Taxonnmy.""" + tag = Tag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, tag) + assert self.staff.has_perm(perm, tag) + assert self.user_all_orgs.has_perm(perm, tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # so if there's no taxonomy, they can't do anything to it. + assert not self.user_both_orgs.has_perm(perm, tag) + assert not self.user_org2.has_perm(perm, tag) + assert not self.learner.has_perm(perm, tag) + + @ddt.data( + "tag_all_orgs", + "tag_both_orgs", + "tag_one_org", + "tag_disabled", + "tag_no_orgs", + ) + def test_view_tag(self, tag_attr): + """Anyone can view any Tag""" + tag = getattr(self, tag_attr) + self._expected_users_have_perm( + "oel_tagging.view_tag", tag, learner_perm=True, learner_obj=True + ) + + # ObjectTag + + @ddt.data( + ("oel_tagging.add_object_tag", "disabled_course_tag"), + ("oel_tagging.change_object_tag", "disabled_course_tag"), + ("oel_tagging.delete_object_tag", "disabled_course_tag"), + ) + @ddt.unpack + def test_object_tag_disabled_taxonomy(self, perm, tag_attr): + """Taxonomy administrators cannot create/edit an ObjectTag with a disabled Taxonomy""" + object_tag = getattr(self, tag_attr) + assert self.superuser.has_perm(perm, object_tag) + assert not self.staff.has_perm(perm, object_tag) + assert not self.user_all_orgs.has_perm(perm, object_tag) + assert not self.user_both_orgs.has_perm(perm, object_tag) + assert not self.user_org2.has_perm(perm, object_tag) + assert not self.learner.has_perm(perm, object_tag) + + @ddt.data( + ("oel_tagging.add_object_tag", "no_orgs_invalid_tag"), + ("oel_tagging.change_object_tag", "no_orgs_invalid_tag"), + ("oel_tagging.delete_object_tag", "no_orgs_invalid_tag"), + ) + @ddt.unpack + def test_object_tag_no_orgs(self, perm, tag_attr): + """Only staff & superusers can create/edit an ObjectTag with a no-org Taxonomy""" + object_tag = getattr(self, tag_attr) + assert self.superuser.has_perm(perm, object_tag) + assert self.staff.has_perm(perm, object_tag) + assert not self.user_all_orgs.has_perm(perm, object_tag) + assert not self.user_both_orgs.has_perm(perm, object_tag) + assert not self.user_org2.has_perm(perm, object_tag) + assert not self.learner.has_perm(perm, object_tag) + + @ddt.data( + ("oel_tagging.add_object_tag", "all_orgs_course_tag"), + ("oel_tagging.add_object_tag", "all_orgs_block_tag"), + ("oel_tagging.add_object_tag", "both_orgs_course_tag"), + ("oel_tagging.add_object_tag", "both_orgs_block_tag"), + ("oel_tagging.add_object_tag", "all_orgs_invalid_tag"), + ("oel_tagging.change_object_tag", "all_orgs_course_tag"), + ("oel_tagging.change_object_tag", "all_orgs_block_tag"), + ("oel_tagging.change_object_tag", "both_orgs_course_tag"), + ("oel_tagging.change_object_tag", "both_orgs_block_tag"), + ("oel_tagging.change_object_tag", "all_orgs_invalid_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_course_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_block_tag"), + ("oel_tagging.delete_object_tag", "both_orgs_course_tag"), + ("oel_tagging.delete_object_tag", "both_orgs_block_tag"), + ("oel_tagging.delete_object_tag", "all_orgs_invalid_tag"), + ) + @ddt.unpack + def test_change_object_tag_all_orgs(self, perm, tag_attr): + """Taxonomy administrators can create/edit an ObjectTag on taxonomies in their org.""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, object_tag) + + @ddt.data( + ("oel_tagging.add_object_tag", "one_org_block_tag"), + ("oel_tagging.add_object_tag", "one_org_invalid_org_tag"), + ("oel_tagging.change_object_tag", "one_org_block_tag"), + ("oel_tagging.change_object_tag", "one_org_invalid_org_tag"), + ("oel_tagging.delete_object_tag", "one_org_block_tag"), + ("oel_tagging.delete_object_tag", "one_org_invalid_org_tag"), + ) + @ddt.unpack + def test_change_object_tag_org1(self, perm, tag_attr): + """Taxonomy administrators can create/edit an ObjectTag on taxonomies in their org.""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm(perm, object_tag, user_org2=False) + + @ddt.data( + "oel_tagging.add_object_tag", + "oel_tagging.change_object_tag", + "oel_tagging.delete_object_tag", + ) + def test_object_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify an ObjectTag with no Taxonomy""" + object_tag = ObjectTag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, object_tag) + assert self.staff.has_perm(perm, object_tag) + assert self.user_all_orgs.has_perm(perm, object_tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # so if there's no taxonomy, they can't do anything to it. + assert not self.user_both_orgs.has_perm(perm, object_tag) + assert not self.user_org2.has_perm(perm, object_tag) + assert not self.learner.has_perm(perm, object_tag) + + @ddt.data( + "all_orgs_course_tag", + "all_orgs_block_tag", + "both_orgs_course_tag", + "both_orgs_block_tag", + "one_org_block_tag", + "all_orgs_invalid_tag", + "one_org_invalid_org_tag", + "no_orgs_invalid_tag", + "disabled_course_tag", + ) + def test_view_object_tag(self, tag_attr): + """Anyone can view any ObjectTag""" + object_tag = getattr(self, tag_attr) + self._expected_users_have_perm( + "oel_tagging.view_object_tag", + object_tag, + learner_perm=True, + learner_obj=True, + ) + + +@ddt.ddt +@override_settings(FEATURES={"ENABLE_CREATOR_GROUP": False}) +class TestRulesTaxonomyNoCreatorGroup( + TestRulesTaxonomy +): # pylint: disable=test-inherits-tests + """ + Run the above tests with ENABLE_CREATOR_GROUP unset, to demonstrate that all users have course creator access for + all orgs, and therefore everyone is a Taxonomy Administrator. + + However, if there are no Organizations in the database, then nobody has access to the Tagging models. + """ + + def _expected_users_have_perm( + self, perm, obj, learner_perm=False, learner_obj=False, user_org2=True + ): + """ + When ENABLE_CREATOR_GROUP is disabled, all users have all permissions. + """ + super()._expected_users_have_perm( + perm=perm, + obj=obj, + learner_perm=True, + learner_obj=True, + user_org2=True, + ) + + @ddt.data( + "oel_tagging.add_tag", + "oel_tagging.change_tag", + "oel_tagging.delete_tag", + ) + def test_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify any Tag, even those with no Taxonnmy.""" + tag = Tag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, tag) + assert self.staff.has_perm(perm, tag) + assert self.user_all_orgs.has_perm(perm, tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # but since there's no org restrictions enabled, anyone has these permissions. + assert self.user_both_orgs.has_perm(perm, tag) + assert self.user_org2.has_perm(perm, tag) + assert self.learner.has_perm(perm, tag) + + @ddt.data( + "oel_tagging.add_object_tag", + "oel_tagging.change_object_tag", + "oel_tagging.delete_object_tag", + ) + def test_object_tag_no_taxonomy(self, perm): + """Taxonomy administrators can modify an ObjectTag with no Taxonomy""" + object_tag = ObjectTag() + + # Global Taxonomy Admins can do pretty much anything + assert self.superuser.has_perm(perm, object_tag) + assert self.staff.has_perm(perm, object_tag) + assert self.user_all_orgs.has_perm(perm, object_tag) + + # Org content creators are bound by a taxonomy's org restrictions, + # but since there's no org restrictions enabled, anyone has these permissions. + assert self.user_both_orgs.has_perm(perm, object_tag) + assert self.user_org2.has_perm(perm, object_tag) + assert self.learner.has_perm(perm, object_tag) + + # Taxonomy + + @ddt.data( + ("oel_tagging.add_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.add_taxonomy", "taxonomy_disabled"), + ("oel_tagging.add_taxonomy", "taxonomy_one_org"), + ("oel_tagging.add_taxonomy", "taxonomy_no_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.change_taxonomy", "taxonomy_disabled"), + ("oel_tagging.change_taxonomy", "taxonomy_one_org"), + ("oel_tagging.change_taxonomy", "taxonomy_no_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_all_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_both_orgs"), + ("oel_tagging.delete_taxonomy", "taxonomy_disabled"), + ("oel_tagging.delete_taxonomy", "taxonomy_one_org"), + ("oel_tagging.delete_taxonomy", "taxonomy_no_orgs"), + ) + @ddt.unpack + def test_no_orgs_no_perms(self, perm, taxonomy_attr): + """ + Org-level permissions are revoked when there are no orgs. + """ + Organization.objects.all().delete() + taxonomy = getattr(self, taxonomy_attr) + # Superusers & Staff always have access + assert self.superuser.has_perm(perm) + assert self.superuser.has_perm(perm, taxonomy) + assert self.staff.has_perm(perm) + assert self.staff.has_perm(perm, taxonomy) + + # But everyone else's object-level access is removed + assert self.user_all_orgs.has_perm(perm) + assert not self.user_all_orgs.has_perm(perm, taxonomy) + assert self.user_both_orgs.has_perm(perm) + assert not self.user_both_orgs.has_perm(perm, taxonomy) + assert self.user_org2.has_perm(perm) + assert not self.user_org2.has_perm(perm, taxonomy) + assert self.learner.has_perm(perm) + assert not self.learner.has_perm(perm, taxonomy) diff --git a/package.json b/package.json index f83e28f31026..a589b3da57e6 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,16 @@ "version": "0.1.0", "repository": "https://github.com/openedx/edx-platform", "scripts": { - "postinstall": "scripts/copy-node-modules.sh" + "postinstall": "scripts/copy-node-modules.sh", + "build": "echo 'WARNING: `npm run build` in edx-platform is experimental. Use at your own risk.' && npm run webpack && npm run compile-sass", + "build-dev": "echo 'WARNING: `npm run build-dev` in edx-platform is experimental. Use at your own risk.' && npm run webpack-dev && npm run compile-sass-dev", + "webpack": "NODE_ENV=${NODE_ENV:-production} \"$(npm bin)/webpack\" --config=${WEBPACK_CONFIG_PATH:-webpack.prod.config.js}", + "webpack-dev": "NODE_ENV=development \"$(npm bin)/webpack\" --config=webpack.dev.config.js", + "compile-sass": "scripts/compile_sass.py --env=${NODE_ENV:-production}", + "compile-sass-dev": "scripts/compile_sass.py --env=development", + "watch": "echo 'WARNING: `npm run watch` in edx-platform is experimental. Use at your own risk.' && { npm run watch-webpack& npm run watch-sass& } && sleep infinity", + "watch-webpack": "npm run webpack-dev -- --watch", + "watch-sass": "scripts/watch_sass.sh" }, "dependencies": { "@babel/core": "7.19.0", diff --git a/pavelib/assets.py b/pavelib/assets.py index 566092a2666e..8f950fe3f647 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -323,7 +323,6 @@ class SassWatcher(PatternMatchingEventHandler): """ ignore_directories = True patterns = ['*.scss'] - ignore_patterns = ['common/static/xmodule/*'] def register(self, observer, directories): """ @@ -352,47 +351,6 @@ def on_any_event(self, event): traceback.print_exc() -class XModuleSassWatcher(SassWatcher): - """ - Watches for sass file changes - """ - ignore_directories = True - ignore_patterns = [] - - @debounce() - def on_any_event(self, event): - print('\tCHANGED:', event.src_path) - try: - process_xmodule_assets() - except Exception: # pylint: disable=broad-except - traceback.print_exc() - - -class XModuleAssetsWatcher(PatternMatchingEventHandler): - """ - Watches for css and js file changes - """ - ignore_directories = True - patterns = ['*.css', '*.js'] - - def register(self, observer): - """ - Register files with observer - """ - observer.schedule(self, 'xmodule/', recursive=True) - - @debounce() - def on_any_event(self, event): - print('\tCHANGED:', event.src_path) - try: - process_xmodule_assets() - except Exception: # pylint: disable=broad-except - traceback.print_exc() - - # To refresh the hash values of static xmodule content - restart_django_servers() - - @task @no_help @cmdopts([ @@ -615,16 +573,13 @@ def process_npm_assets(): @task -@needs( - 'pavelib.prereqs.install_python_prereqs', -) @no_help def process_xmodule_assets(): """ Process XModule static assets. """ - sh('xmodule_assets common/static/xmodule') - print("\t\tFinished processing xmodule assets.") + print("\t\tProcessing xmodule assets is no longer needed. This task is now a no-op.") + print("\t\tWhen paver is removed from edx-platform, this step will not replaced.") def restart_django_servers(): @@ -822,8 +777,6 @@ def watch_assets(options): observer = Observer(timeout=wait) SassWatcher().register(observer, sass_directories) - XModuleSassWatcher().register(observer, ['xmodule/']) - XModuleAssetsWatcher().register(observer) print("Starting asset watcher...") observer.start() @@ -845,6 +798,7 @@ def watch_assets(options): @task @needs( 'pavelib.prereqs.install_node_prereqs', + 'pavelib.prereqs.install_python_prereqs', ) @consume_args @timed @@ -896,8 +850,6 @@ def update_assets(args): args = parser.parse_args(args) collect_log_args = {} - process_xmodule_assets() - # Build Webpack call_task('pavelib.assets.webpack', options={'settings': args.settings}) diff --git a/pavelib/js_test.py b/pavelib/js_test.py index fed1f0fe102f..3f84a3361dba 100644 --- a/pavelib/js_test.py +++ b/pavelib/js_test.py @@ -26,7 +26,6 @@ @needs( 'pavelib.prereqs.install_node_prereqs', 'pavelib.utils.test.utils.clean_reports_dir', - 'pavelib.assets.process_xmodule_assets', ) @cmdopts([ ("suite=", "s", "Test suite to run"), diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py index dbbab3b00b6b..e9a07b94f0c6 100644 --- a/pavelib/paver_tests/test_servers.py +++ b/pavelib/paver_tests/test_servers.py @@ -214,7 +214,6 @@ def verify_server_task(self, task_name, options): expected_asset_settings = "test_static_optimized" expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS if not is_fast: - expected_messages.append("xmodule_assets common/static/xmodule") expected_messages.append("install npm_assets") expected_messages.extend( [c.format(settings=expected_asset_settings, @@ -259,7 +258,6 @@ def verify_run_all_servers_task(self, options): expected_collect_static = not is_fast and expected_settings != Env.DEVSTACK_SETTINGS expected_messages = [] if not is_fast: - expected_messages.append("xmodule_assets common/static/xmodule") expected_messages.append("install npm_assets") expected_messages.extend( [c.format(settings=expected_asset_settings, diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 0b2852765284..6f8d43fef312 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -28,7 +28,6 @@ django-storages==1.9.1 # This is to allow them to better control its deployment and to do it in a process that works better # for them. - # oauthlib>3.0.1 causes test failures ( also remove the django-oauth-toolkit constraint when this is fixed ) oauthlib==3.0.1 @@ -110,7 +109,6 @@ drf-yasg<1.21.6 # Adding pin to avoid any major upgrade djangorestframework<3.15.0 - # tests failing with greater version. Fix this in separate ticket. pillow<10.0.0 @@ -118,3 +116,9 @@ pillow<10.0.0 # 1.16.0 works with Django 3.2 through 4.1 django-stubs==1.16.0 djangorestframework-stubs==3.14.0 # Pinned to match django-stubs. Remove this when we can remove the above pin. + +# Our legacy Sass code is incompatible with anything except this ancient libsass version. +# Here is a ticket to upgrade, but it's of debatable importance given that we are rapidly moving +# away from legacy LMS/CMS frontends: +# https://github.com/openedx/edx-platform/issues/31616 +libsass==0.10.0 diff --git a/requirements/edx/assets.in b/requirements/edx/assets.in new file mode 100644 index 000000000000..16211c3fadff --- /dev/null +++ b/requirements/edx/assets.in @@ -0,0 +1,11 @@ +# Requirements for running `npm run build`. + +# This file is NOT included into base.txt, because we do not want to +# superfluously install frontend build requirements into the production +# backend requirements. + +-c ../constraints.txt + +click +libsass +nodeenv diff --git a/requirements/edx/assets.txt b/requirements/edx/assets.txt new file mode 100644 index 000000000000..bcac144e0edb --- /dev/null +++ b/requirements/edx/assets.txt @@ -0,0 +1,21 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# make upgrade +# +click==8.1.6 + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/assets.in +libsass==0.10.0 + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/assets.in +nodeenv==1.8.0 + # via -r requirements/edx/assets.in +six==1.16.0 + # via libsass + +# The following packages are considered to be unsafe in a requirements file: +# setuptools diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index a7666c5f8621..2e41e0af6ba1 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -243,6 +243,7 @@ django==3.2.20 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # skill-tagging # super-csv @@ -393,6 +394,7 @@ djangorestframework==3.14.0 # edx-proctoring # edx-submissions # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -659,7 +661,9 @@ lazy==1.5 # ora2 # xblock libsass==0.10.0 - # via -r requirements/edx/paver.txt + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/paver.txt loremipsum==1.0.5 # via ora2 lti-consumer-xblock==9.5.7 @@ -766,6 +770,8 @@ openedx-filters==1.4.0 # -r requirements/edx/kernel.in # lti-consumer-xblock # skill-tagging +openedx-learning==0.1.0 + # via -r requirements/edx/kernel.in openedx-mongodbproxy==0.2.0 # via -r requirements/edx/kernel.in optimizely-sdk==4.1.1 @@ -1004,6 +1010,7 @@ rules==3.3 # -r requirements/edx/kernel.in # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via boto3 sailthru-client==2.2.3 diff --git a/requirements/edx/development.in b/requirements/edx/development.in index 5909f60268ff..00c9e533b126 100644 --- a/requirements/edx/development.in +++ b/requirements/edx/development.in @@ -13,6 +13,7 @@ -r ../pip-tools.txt # pip-tools and its dependencies, for managing requirements files -r testing.txt # Dependencies for running the various test suites -r doc.txt # Dependencies for building the documentation locally. +-r assets.txt # Allow developers to rebuild assets with `npm run build`. click # Used for perf_tests utilities in modulestore django-debug-toolbar # A set of panels that display debug information about the current request/response @@ -21,3 +22,4 @@ djangorestframework-stubs # Typing stubs for DRF mypy # static type checking pywatchman # More efficient checking for runserver reload trigger events vulture # Detects possible dead/unused code, used in scripts/find-dead-code.sh +watchdog # Used by `npm run watch` to auto-recompile when assets are changed diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 4b3e91e0c6bc..cf5a4f68439d 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -214,6 +214,7 @@ click==8.1.6 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/../pip-tools.txt + # -r requirements/edx/assets.txt # -r requirements/edx/development.in # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt @@ -403,6 +404,7 @@ django==3.2.20 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # skill-tagging # super-csv @@ -619,6 +621,7 @@ djangorestframework==3.14.0 # edx-proctoring # edx-submissions # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-stubs==3.14.0 @@ -1122,6 +1125,8 @@ lazy-object-proxy==1.9.0 # astroid libsass==0.10.0 # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/assets.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt loremipsum==1.0.5 @@ -1242,6 +1247,7 @@ nltk==3.8.1 # chem nodeenv==1.8.0 # via + # -r requirements/edx/assets.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt numpy==1.22.4 @@ -1299,6 +1305,10 @@ openedx-filters==1.4.0 # -r requirements/edx/testing.txt # lti-consumer-xblock # skill-tagging +openedx-learning==0.1.0 + # via + # -r requirements/edx/doc.txt + # -r requirements/edx/testing.txt openedx-mongodbproxy==0.2.0 # via # -r requirements/edx/doc.txt @@ -1761,6 +1771,7 @@ rules==3.3 # -r requirements/edx/testing.txt # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via # -r requirements/edx/doc.txt @@ -1802,6 +1813,7 @@ singledispatch==4.0.0 # via -r requirements/edx/testing.txt six==1.16.0 # via + # -r requirements/edx/assets.txt # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # analytics-python diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 4e849be1e3d8..c211a9f2f42f 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -291,6 +291,7 @@ django==3.2.20 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # skill-tagging # super-csv @@ -457,6 +458,7 @@ djangorestframework==3.14.0 # edx-proctoring # edx-submissions # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -785,7 +787,9 @@ lazy==1.5 # ora2 # xblock libsass==0.10.0 - # via -r requirements/edx/base.txt + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/base.txt loremipsum==1.0.5 # via # -r requirements/edx/base.txt @@ -909,6 +913,8 @@ openedx-filters==1.4.0 # -r requirements/edx/base.txt # lti-consumer-xblock # skill-tagging +openedx-learning==0.1.0 + # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1 @@ -1193,6 +1199,7 @@ rules==3.3 # -r requirements/edx/base.txt # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via # -r requirements/edx/base.txt diff --git a/requirements/edx/kernel.in b/requirements/edx/kernel.in index f283f811f0cb..be6f1421877f 100644 --- a/requirements/edx/kernel.in +++ b/requirements/edx/kernel.in @@ -115,6 +115,7 @@ openedx-calc # Library supporting mathematical calculatio openedx-django-require openedx-events>=8.3.0 # Open edX Events from Hooks Extension Framework (OEP-50) openedx-filters # Open edX Filters from Hooks Extension Framework (OEP-50) +openedx-learning<=0.1 openedx-mongodbproxy openedx-django-wiki openedx-blockstore diff --git a/requirements/edx/paver.in b/requirements/edx/paver.in index caaf349592d2..6987ede82275 100644 --- a/requirements/edx/paver.in +++ b/requirements/edx/paver.in @@ -12,7 +12,7 @@ edx-opaque-keys # Create and introspect course and xblock identities lazy # Lazily-evaluated attributes for Python objects -libsass==0.10.0 # Python bindings for the LibSass CSS compiler +libsass # Python bindings for the LibSass CSS compiler markupsafe # XML/HTML/XHTML Markup safe strings mock # Stub out code with mock objects and make assertions about how they have been used path # Easier manipulation of filesystem paths diff --git a/requirements/edx/paver.txt b/requirements/edx/paver.txt index c42666cd1742..ae5a7829252d 100644 --- a/requirements/edx/paver.txt +++ b/requirements/edx/paver.txt @@ -17,7 +17,9 @@ idna==3.4 lazy==1.5 # via -r requirements/edx/paver.in libsass==0.10.0 - # via -r requirements/edx/paver.in + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/paver.in markupsafe==2.1.3 # via -r requirements/edx/paver.in mock==5.1.0 diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 55b6d44003c8..70e38dc092f6 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -323,6 +323,7 @@ django==3.2.20 # openedx-django-wiki # openedx-events # openedx-filters + # openedx-learning # ora2 # skill-tagging # super-csv @@ -489,6 +490,7 @@ djangorestframework==3.14.0 # edx-proctoring # edx-submissions # openedx-blockstore + # openedx-learning # ora2 # super-csv djangorestframework-xml==2.0.0 @@ -852,7 +854,9 @@ lazy==1.5 lazy-object-proxy==1.9.0 # via astroid libsass==0.10.0 - # via -r requirements/edx/base.txt + # via + # -c requirements/edx/../constraints.txt + # -r requirements/edx/base.txt loremipsum==1.0.5 # via # -r requirements/edx/base.txt @@ -978,6 +982,8 @@ openedx-filters==1.4.0 # -r requirements/edx/base.txt # lti-consumer-xblock # skill-tagging +openedx-learning==0.1.0 + # via -r requirements/edx/base.txt openedx-mongodbproxy==0.2.0 # via -r requirements/edx/base.txt optimizely-sdk==4.1.1 @@ -1331,6 +1337,7 @@ rules==3.3 # -r requirements/edx/base.txt # edx-enterprise # edx-proctoring + # openedx-learning s3transfer==0.1.13 # via # -r requirements/edx/base.txt diff --git a/scripts/compile_sass.py b/scripts/compile_sass.py new file mode 100755 index 000000000000..8a21f718fefe --- /dev/null +++ b/scripts/compile_sass.py @@ -0,0 +1,438 @@ +#!/usr/bin/env python +""" +Defines a CLI for compiling Sass (both default and themed) into CSS. + +Should be run from the root of edx-platform using `npm run` wrapper. +Requirements for this scripts are stored in requirements/edx/assets.in. + +Get more details: + + npm run compile-sass -- --help + npm run compile-sass -- --dry + +Setup (Tutor and Devstack will do this for you): + + python -m venv venv + . venv/bin/activate + pip install -r requirements/edx/assets.txt + +Usage: + + npm run compile-sass # prod, no args + npm run compile-sass -- ARGS # prod, with args + npm run compile-sass-dev # dev, no args + npm run compile-sass-dev -- ARGS # dev, with args + +This script is intentionally implemented in a very simplistic way. It prefers repetition +over abstraction, and its dependencies are minimal (just click and libsass-python, ideally). +We do this because: + +* If and when we migrate from libsass-python to something less ancient like node-sass or dart-sass, + we will want to re-write this script in Bash or JavaScript so that it can work without any + backend tooling. By keeping the script dead simple, that will be easier. + +* The features this script supports (legacy frontends & comprehensive theming) are on the way out, + in favor of micro-frontends, branding, and Paragon design tokens. We're not sure how XBlock + view styling will fit into that, but it probably can be much simpler than comprehensive theming. + So, we don't need this script to be modular and extensible. We just need it to be obvious, robust, + and easy to maintain until we can delete it. + +See docs/decisions/0017-reimplement-asset-processing.rst for more details. +""" +from __future__ import annotations + +import glob +import subprocess +from pathlib import Path + +import click + + +# Accept both long- and short-forms of these words, but normalize to long form. +# We accept both because edx-platform asset build scripts historically use the short form, +# but NODE_ENV uses the long form, so to make them integrate more seamlessly we accept both. +NORMALIZED_ENVS = { + "prod": "production", + "dev": "development", + "production": "production", + "development": "development", +} + + +@click.option( + "-T", + "--theme-dir", + "theme_dirs", + metavar="PATH", + multiple=True, + envvar="EDX_PLATFORM_THEME_DIRS", + type=click.Path( + exists=True, file_okay=False, readable=True, writable=True, path_type=Path + ), + help=( + "Consider sub-dirs of PATH as themes. " + "Multiple theme dirs are accepted. " + "If none are provided, we look at colon-separated paths on the EDX_PLATFORM_THEME_DIRS env var." + ), +) +@click.option( + "-t", + "--theme", + "themes", + metavar="NAME", + multiple=True, + type=str, + help=( + "A theme to compile. " + "NAME should be a sub-dir of a PATH provided by --theme-dir. " + "Multiple themes are accepted. " + "If none are provided, all available themes are compiled." + ), +) +@click.option( + "--skip-default", + is_flag=True, + help="Don't compile default Sass.", +) +@click.option( + "--skip-themes", + is_flag=True, + help="Don't compile any themes (overrides --theme* options).", +) +@click.option( + "--skip-lms", + is_flag=True, + help="Don't compile any LMS Sass.", +) +@click.option( + "--skip-cms", + is_flag=True, + help="Don't compile any CMS Sass.", +) +@click.option( + "--env", + type=click.Choice(["dev", "development", "prod", "production"]), + default="prod", + help="Optimize CSS for this environment. Defaults to 'prod'.", +) +@click.option( + "--dry", + is_flag=True, + help="Print what would be compiled, but don't compile it.", +) +@click.option( + "-h", + "--help", + "show_help", + is_flag=True, + help="Print this help.", +) +@click.command() +@click.pass_context +def main( + context: click.Context, + theme_dirs: list[Path], + themes: list[str], + skip_default: bool, + skip_themes: bool, + skip_lms: bool, + skip_cms: bool, + env: str, + dry: bool, + show_help: bool, +) -> None: + """ + Compile Sass for edx-platform and its themes. + + Default Sass is compiled unless explicitly skipped. + Additionally, any number of themes may be specified using --theme-dir and --theme. + + Default CSS is compiled to css/ directories in edx-platform. + Themed CSS is compiled to css/ directories in their source themes. + """ + + def compile_sass_dir( + message: str, + source: Path, + dest: Path, + includes: list[Path], + tolerate_missing: bool = False, + ) -> None: + """ + Compile a directory of Sass into a target CSS directory, and generate any missing RTL CSS. + + Structure of source dir is mirrored in target dir. + """ + use_dev_settings = NORMALIZED_ENVS[env] == "development" + click.secho(f" {message}...", fg="cyan") + click.secho(f" Source: {source}") + click.secho(f" Target: {dest}") + if not source.is_dir(): + if tolerate_missing: + click.secho(f" Skipped because source directory does not exist.", fg="yellow") + return + else: + raise FileNotFoundError(f"missing Sass source dir: {source}") + click.echo(f" Include paths:") + for include in includes: + click.echo(f" {include}") + if not dry: + # Import sass late so that this script can be dry-run without installing + # libsass, which takes a while as it must be compiled from its C source. + import sass + + dest.mkdir(parents=True, exist_ok=True) + sass.compile( + dirname=(str(source), str(dest)), + include_paths=[str(include_path) for include_path in includes], + source_comments=use_dev_settings, + output_style=("nested" if use_dev_settings else "compressed"), + ) + click.secho(f" Compiled.", fg="green") + # For Sass files without explicit RTL versions, generate + # an RTL version of the CSS using the rtlcss library. + for sass_path in glob.glob(str(source) + "/**/*.scss"): + if Path(sass_path).name.startswith("_"): + # Don't generate RTL CSS for partials + continue + if sass_path.endswith("-rtl.scss"): + # Don't generate RTL CSS if the file is itself an RTL version + continue + if Path(sass_path.replace(".scss", "-rtl.scss")).exists(): + # Don't generate RTL CSS if there is an explicit Sass version for RTL + continue + click.echo(" Generating missing right-to-left CSS:") + source_css_file = sass_path.replace(str(source), str(dest)).replace( + ".scss", ".css" + ) + target_css_file = source_css_file.replace(".css", "-rtl.css") + click.echo(f" Source: {source_css_file}") + click.echo(f" Target: {target_css_file}") + if not dry: + subprocess.run(["rtlcss", source_css_file, target_css_file]) + click.secho(" Generated.", fg="green") + + # Information + click.secho(f"USING ENV: {NORMALIZED_ENVS[env]}", fg="blue") + if dry: + click.secho(f"DRY RUN: Will print compile steps, but will not compile anything.", fg="blue") + click.echo() + + # Warnings + click.secho("WARNING: `npm run compile-sass` is experimental. Use at your own risk.", fg="yellow", bold=True) + if show_help: + click.echo(context.get_help()) + return + if skip_lms and skip_cms: + click.secho("WARNING: You are skipping both LMS and CMS... nothing will be compiled!", fg="yellow") + if skip_default and skip_themes: + click.secho("WARNING: You are skipped both default Sass and themed Sass... nothing will be compiled!", fg="yellow") + click.echo() + + # Build a list of theme paths: + if skip_themes: + theme_paths = [] + else: + theme_paths = [ + theme_dir / theme + # For every theme dir, + for theme_dir in theme_dirs + for theme in ( + # for every theme name (if theme names provided), + themes + or + # or for every subdir of theme dirs (if no theme name provided), + (theme_dir_entry.name for theme_dir_entry in theme_dir.iterdir()) + ) + # consider the path a theme if it has a lms/ or cms/ subdirectory. + if (theme_dir / theme / "lms").is_dir() or (theme_dir / theme / "cms").is_dir() + ] + + # We expect this script to be run from the edx-platform root. + repo = Path(".") + if not (repo / "xmodule").is_dir(): + # Sanity check: If the xmodule/ folder is missing, we're definitely not at the root + # of edx-platform, so save the user some headache by exiting early. + raise Exception(f"{__file__} must be run from the root of edx-platform") + + # Every Sass compilation will use have these directories as lookup paths, + # regardless of theme. + common_includes = [ + repo / "common" / "static", + repo / "common" / "static" / "sass", + repo / "node_modules" / "@edx", + repo / "node_modules", + ] + + if not skip_default: + click.secho(f"Compiling default Sass...", fg="cyan", bold=True) + if not skip_lms: + compile_sass_dir( + "Compiling default LMS Sass", + repo / "lms" / "static" / "sass", + repo / "lms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling default certificate Sass", + repo / "lms" / "static" / "certificates" / "sass", + repo / "lms" / "static" / "certificates" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for default LMS", + repo / "xmodule" / "assets", + repo / "lms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + if not skip_cms: + compile_sass_dir( + "Compiling default CMS Sass", + repo / "cms" / "static" / "sass", + repo / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for default CMS", + repo / "xmodule" / "assets", + repo / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + click.secho(f"Done compiling default Sass!", fg="cyan", bold=True) + click.echo() + + for theme in theme_paths: + click.secho(f"Compiling Sass for theme at {theme}...", fg="cyan", bold=True) + if not skip_lms: + compile_sass_dir( + "Compiling default LMS Sass with themed partials", + repo / "lms" / "static" / "sass", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed LMS Sass as overrides to CSS from previous step", + theme / "lms" / "static" / "sass", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed certificate Sass", + theme / "lms" / "static" / "certificates" / "sass", + theme / "lms" / "static" / "certificates" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "lms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for themed LMS", + repo / "xmodule" / "assets", + theme / "lms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + if not skip_cms: + compile_sass_dir( + "Compiling default CMS Sass with themed partials", + repo / "cms" / "static" / "sass", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling themed CMS Sass as overrides to CSS from previous step", + theme / "cms" / "static" / "sass", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + repo / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass", + ], + tolerate_missing=True, + ) + compile_sass_dir( + "Compiling built-in XBlock Sass for themed CMS", + repo / "xmodule" / "assets", + theme / "cms" / "static" / "css", + includes=[ + *common_includes, + theme / "lms" / "static" / "sass" / "partials", + theme / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass" / "partials", + repo / "cms" / "static" / "sass" / "partials", + repo / "lms" / "static" / "sass", + repo / "cms" / "static" / "sass", + ], + ) + click.secho(f"Done compiling Sass for theme at {theme}!", fg="cyan", bold=True) + click.echo() + + # Report what we did. + click.secho("Successfully compiled:", fg="green", bold=True) + if not skip_default: + click.secho(f" - {repo.absolute()} (default Sass)", fg="green") + for theme in theme_paths: + click.secho(f" - {theme}", fg="green") + if skip_lms: + click.secho(f"(skipped LMS)", fg="yellow") + if skip_cms: + click.secho(f"(skipped CMS)", fg="yellow") + + +if __name__ == "__main__": + main(prog_name="npm run compile-sass --") diff --git a/scripts/watch_sass.sh b/scripts/watch_sass.sh new file mode 100755 index 000000000000..dee0d88f6215 --- /dev/null +++ b/scripts/watch_sass.sh @@ -0,0 +1,170 @@ +#!/usr/bin/env bash + +# Wait for changes to Sass, and recompile. +# Invoke from repo root as `npm run watch-sass`. +# This script tries to recompile the minimal set of Sass for any given change. + +# By default, only watches default Sass. +# To watch themes too, provide colon-separated paths in the EDX_PLATFORM_THEME_DIRS environment variable. +# Each path will be treated as a "theme dir", which means that every immediate child directory is watchable as a theme. +# For example: +# +# EDX_PLATFORM_THEME_DIRS=/openedx/themes:./themes npm run watch-sass +# +# would watch default Sass as well as /openedx/themes/indigo, /openedx/themes/mytheme, ./themes/red-theme, etc. + +set -euo pipefail + +COL_SECTION="\e[1;36m" # Section header color (bold cyan) +COL_LOG="\e[36m" # Log color (cyan) +COL_WARNING="\e[1;33m" # Warning (bold yellow) +COL_ERROR="\e[1;31m" # Error (bold red) +COL_CMD="\e[35m" # Command echoing (magenta) +COL_OFF="\e[0m" # Normal color + +section() { + # Print a header in bold cyan to indicate sections of output. + echo -e "${COL_SECTION}$*${COL_OFF}" +} +log() { + # Info line. Indented by one space so that it appears as nested under section headers. + echo -e " ${COL_LOG}$*${COL_OFF}" +} +warning() { + # Bright yellow warning message. + echo -e "${COL_WARNING}WARNING: $*${COL_OFF}" +} +error() { + # Bright red error message. + echo -e "${COL_ERROR}ERROR: $*${COL_OFF}" +} +echo_quoted_cmd() { + # Echo args, each single-quoted, so that the user could copy-paste and run them as a command. + # Indented by two spaces so it appears as nested under log lines. + echo -e " ${COL_CMD}$(printf "'%s' " "$@")${COL_OFF}" +} + +start_sass_watch() { + # Start a watch for .scss files in a particular dir. Run in the background. + # start_sass_watch NAME_FOR_LOGGING WATCH_ROOT_PATH HANDLER_COMMAND + local name="$1" + local path="$2" + local handler="$3" + log "Starting watcher for $name:" + # Note: --drop means that we should ignore any change events that happen during recompilation. + # This is good because it means that if you edit 3 files, we won't fire off three simultaneous compiles. + # It's not perfect, though, because if you change 3 files, only the first one will trigger a recompile, + # so depending on the timing, your latest changes may or may not be picked up. We accept this as a reasonable + # tradeoff. Some watcher libraries are smarter than watchdog, in that they wait until the filesystem "settles" + # before firing off a the recompilation. This sounds nice but did not seem worth the effort for legacy assets. + local watch_cmd=(watchmedo shell-command -v --patterns '*.scss' --recursive --drop --command "$handler" "$path") + echo_quoted_cmd "${watch_cmd[@]}" + "${watch_cmd[@]}" & +} + +clean_up() { + # Kill all background processes we started. + # Since they're all 'watchmedo' instances, we can just use killall. + log "Stopping all watchers:" + local stop_cmd=(killall watchmedo) + echo_quoted_cmd "${stop_cmd[@]}" + "${stop_cmd[@]}" || true + log "Watchers stopped." +} + +warning "'npm run watch-sass' in edx-platform is experimental. Use at your own risk." + +if [[ ! -d common/static/sass ]] ; then + error 'This command must be run from the root of edx-platform!' + exit 1 +fi +if ! type watchmedo 1>/dev/null 2>&1 ; then + error "command not found: watchmedo" + log "The \`watchdog\` Python package is probably not installed. You can install it with:" + log " pip install -r requirements/edx/development.txt" + exit 1 +fi + +trap clean_up EXIT + +# Start by compiling all watched Sass right away, mirroring the behavior of webpack --watch. +section "COMPILING SASS:" +npm run compile-sass +echo +echo + +section "STARTING DEFAULT SASS WATCHERS:" + +# Changes to LMS Sass require a full recompilation, since LMS Sass can be used in CMS and in themes. +start_sass_watch "default LMS Sass" \ + lms/static/sass \ + 'npm run compile-sass-dev' + +# Changes to default cert Sass only require recompilation of default cert Sass, since cert Sass +# cannot be included into LMS, CMS, or themes. +start_sass_watch "default certificate Sass" \ + lms/static/certificates/sass \ + 'npm run compile-sass-dev -- --skip-cms --skip-themes' + +# Changes to CMS Sass require recompilation of default & themed CMS Sass, but not LMS Sass. +start_sass_watch "default CMS Sass" \ + cms/static/sass \ + 'npm run compile-sass-dev -- --skip-lms' + +# Sass changes in common, node_modules, and xmodule all require full recompilations. +start_sass_watch "default common Sass" \ + common/static \ + 'npm run compile-sass-dev' +start_sass_watch "node_modules Sass" \ + node_modules \ + 'npm run compile-sass-dev' +start_sass_watch "builtin XBlock Sass" \ + xmodule/assets \ + 'npm run compile-sass-dev' + +export IFS=";" +for theme_dir in ${EDX_PLATFORM_THEME_DIRS:-} ; do + for theme_path in "$theme_dir"/* ; do + + theme_name="${theme_path#"$theme_dir/"}" + lms_sass="$theme_path/lms/static/sass" + cert_sass="$theme_path/lms/static/certificates/sass" + cms_sass="$theme_path/cms/static/sass" + + if [[ -d "$lms_sass" ]] || [[ -d "$cert_sass" ]] || [[ -d "$cms_sass" ]] ; then + section "STARTING WATCHERS FOR THEME '$theme_name':" + fi + + # Changes to a theme's LMS Sass require that the full theme is recompiled, since LMS + # Sass is used in certs and CMS. + if [[ -d "$lms_sass" ]] ; then + start_sass_watch "$theme_name LMS Sass" \ + "$lms_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default" + fi + + # Changes to a theme's certs Sass only requires that its certs Sass be recompiled. + if [[ -d "$cert_sass" ]] ; then + start_sass_watch "$theme_name certificate Sass" \ + "$cert_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-cms" + fi + + # Changes to a theme's CMS Sass only requires that its CMS Sass be recompiled. + if [[ -d "$cms_sass" ]] ; then + start_sass_watch "$theme_name CMS Sass" \ + "$cms_sass" \ + "npm run compile-sass-dev -- -T $theme_dir -t $theme_name --skip-default --skip-lms" + fi + + done +done + +sleep infinity & +echo +echo "Watching Open edX LMS & CMS Sass for changes." +echo "Use Ctrl+c to exit." +echo +echo +wait $! + diff --git a/webpack-config/file-lists.js b/webpack-config/file-lists.js index 0ea827a10633..ddb9cf4f7806 100644 --- a/webpack-config/file-lists.js +++ b/webpack-config/file-lists.js @@ -10,9 +10,7 @@ module.exports = { path.resolve(__dirname, '../common/static/common/js/components/views/paging_footer.js'), path.resolve(__dirname, '../cms/static/js/views/paging.js'), path.resolve(__dirname, '../common/static/common/js/components/utils/view_utils.js'), - /descriptors\/js/, - /modules\/js/, - /xmodule\/js\/src\//, + /xmodule\/js\/src/, path.resolve(__dirname, '../openedx/features/course_bookmarks/static/course_bookmarks/js/views/bookmark_button.js') ], diff --git a/webpack.builtinblocks.config.js b/webpack.builtinblocks.config.js new file mode 100644 index 000000000000..30f5bdb2a507 --- /dev/null +++ b/webpack.builtinblocks.config.js @@ -0,0 +1,136 @@ +module.exports = { + "entry": { + "AboutBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js", + "./xmodule/js/src/html/imageModal.js", + "./xmodule/js/common_static/js/vendor/draggabilly.js" + ], + "AboutBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/edit.js" + ], + "AnnotatableBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/display.js", + "./xmodule/js/src/annotatable/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js" + ], + "AnnotatableBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/raw/edit/xml.js" + ], + "ConditionalBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/conditional/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js" + ], + "ConditionalBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/sequence/edit.js" + ], + "CourseInfoBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js", + "./xmodule/js/src/html/imageModal.js", + "./xmodule/js/common_static/js/vendor/draggabilly.js" + ], + "CourseInfoBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/edit.js" + ], + "CustomTagBlockDisplay": "./xmodule/js/src/xmodule.js", + "CustomTagBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/raw/edit/xml.js" + ], + "HtmlBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js", + "./xmodule/js/src/html/imageModal.js", + "./xmodule/js/common_static/js/vendor/draggabilly.js" + ], + "HtmlBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/edit.js" + ], + "LTIBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/lti/lti.js" + ], + "LTIBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/raw/edit/metadata-only.js" + ], + "LibraryContentBlockDisplay": "./xmodule/js/src/xmodule.js", + "LibraryContentBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/vertical/edit.js" + ], + "PollBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/poll/poll.js", + "./xmodule/js/src/poll/poll_main.js" + ], + "PollBlockEditor": "./xmodule/js/src/xmodule.js", + "ProblemBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/capa/display.js", + "./xmodule/js/src/collapsible.js", + "./xmodule/js/src/capa/imageinput.js", + "./xmodule/js/src/capa/schematic.js" + ], + "ProblemBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/problem/edit.js" + ], + "SequenceBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/sequence/display.js" + ], + "SequenceBlockEditor": "./xmodule/js/src/xmodule.js", + "SplitTestBlockDisplay": "./xmodule/js/src/xmodule.js", + "SplitTestBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/sequence/edit.js" + ], + "StaticTabBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/display.js", + "./xmodule/js/src/javascript_loader.js", + "./xmodule/js/src/collapsible.js", + "./xmodule/js/src/html/imageModal.js", + "./xmodule/js/common_static/js/vendor/draggabilly.js" + ], + "StaticTabBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/html/edit.js" + ], + "VideoBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/video/10_main.js" + ], + "VideoBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/tabs/tabs-aggregator.js" + ], + "WordCloudBlockDisplay": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/assets/word_cloud/src/js/word_cloud.js" + ], + "WordCloudBlockEditor": [ + "./xmodule/js/src/xmodule.js", + "./xmodule/js/src/raw/edit/metadata-only.js" + ] + } +}; diff --git a/webpack.common.config.js b/webpack.common.config.js index bebd07cbb599..d81a36b28777 100644 --- a/webpack.common.config.js +++ b/webpack.common.config.js @@ -9,13 +9,11 @@ var StringReplace = require('string-replace-webpack-plugin'); var Merge = require('webpack-merge'); var files = require('./webpack-config/file-lists.js'); -var xmoduleJS = require('./common/static/xmodule/webpack.xmodule.config.js'); +var builtinBlocksJS = require('./webpack.builtinblocks.config.js'); var filesWithRequireJSBlocks = [ path.resolve(__dirname, 'common/static/common/js/components/utils/view_utils.js'), - /descriptors\/js/, - /modules\/js/, - /xmodule\/js\/src\// + /xmodule\/js\/src/ ]; var defineHeader = /\(function ?\(((define|require|requirejs|\$)(, )?)+\) ?\{/; @@ -26,6 +24,9 @@ var defineFooter = new RegExp('(' + defineCallFooter.source + ')|(' + defineDirectFooter.source + ')|(' + defineFancyFooter.source + ')', 'm'); +var staticRootLms = process.env.STATIC_ROOT_LMS || "./test_root/staticfiles"; +var staticRootCms = process.env.STATIC_ROOT_CMS || (staticRootLms + "/studio"); + var workerConfig = function() { try { return { @@ -39,7 +40,7 @@ var workerConfig = function() { }, plugins: [ new BundleTracker({ - path: process.env.STATIC_ROOT_LMS, + path: staticRootLms, filename: 'webpack-worker-stats.json' }), new webpack.DefinePlugin({ @@ -131,14 +132,15 @@ module.exports = Merge.smart({ }, plugins: [ + new webpack.ProgressPlugin(), // report progress during compilation new webpack.NoEmitOnErrorsPlugin(), new webpack.NamedModulesPlugin(), new BundleTracker({ - path: process.env.STATIC_ROOT_CMS, + path: staticRootCms, filename: 'webpack-stats.json' }), new BundleTracker({ - path: process.env.STATIC_ROOT_LMS, + path: staticRootLms, filename: 'webpack-stats.json' }), new webpack.ProvidePlugin({ @@ -307,14 +309,124 @@ module.exports = Merge.smart({ test: /xblock\/runtime.v1/, loader: 'exports-loader?window.XBlock!imports-loader?XBlock=xblock/core,this=>window' }, + /******************************************************************************************************* + /* BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS: + * + * The monstrous list of globally-namespace modules below is the result of a JS build refactoring. + * Originally, all of these modules were copied to common/static/xmodule/js/[module|descriptors]/, and + * this file simply contained the lines: + * + * { + * test: /descriptors\/js/, + * loader: 'imports-loader?this=>window' + * }, + * { + * test: /modules\/js/, + * loader: 'imports-loader?this=>window' + * }, + * + * We removed that asset copying because it added complexity to the build, but as a result, in order to + * preserve exact parity with the preexisting global namespace, we had to enumerate all formely-copied + * modules here. It is very likely that many of these modules do not need to be in this list. Future + * refactorings are welcome to try to prune the list down to the minimal set of modules. As far as + * we know, the only modules that absolutely need to be added to the global namespace are those + * which define module types, for example "Problem" (in xmodule/js/src/capa/display.js). + */ + { + test: /xmodule\/assets\/word_cloud\/src\/js\/word_cloud.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/common_static\/js\/vendor\/draggabilly.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/annotatable\/display.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/capa\/display.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/capa\/imageinput.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/capa\/schematic.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/collapsible.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/conditional\/display.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/html\/display.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/html\/edit.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/html\/imageModal.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/javascript_loader.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/lti\/lti.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/poll\/poll.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/poll\/poll_main.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/problem\/edit.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/raw\/edit\/metadata-only.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/raw\/edit\/xml.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/sequence\/display.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/sequence\/edit.js/, + loader: 'imports-loader?this=>window' + }, + { + test: /xmodule\/js\/src\/tabs\/tabs-aggregator.js/, + loader: 'imports-loader?this=>window' + }, { - test: /descriptors\/js/, + test: /xmodule\/js\/src\/vertical\/edit.js/, loader: 'imports-loader?this=>window' }, { - test: /modules\/js/, + test: /xmodule\/js\/src\/video\/10_main.js/, loader: 'imports-loader?this=>window' }, + /* + * END BUILT-IN XBLOCK ASSETS WITH GLOBAL DEFINITIONS + ******************************************************************************************************/ { test: /codemirror/, loader: 'exports-loader?window.CodeMirror' @@ -441,4 +553,4 @@ module.exports = Merge.smart({ } } -}, {web: xmoduleJS}, workerConfig()); +}, {web: builtinBlocksJS}, workerConfig()); diff --git a/webpack.dev.config.js b/webpack.dev.config.js index 3987e82fdf7d..90018a901002 100644 --- a/webpack.dev.config.js +++ b/webpack.dev.config.js @@ -21,7 +21,7 @@ module.exports = _.values(Merge.smart(commonConfig, { }), new webpack.DefinePlugin({ 'process.env.NODE_ENV': JSON.stringify('development'), - 'process.env.JS_ENV_EXTRA_CONFIG': process.env.JS_ENV_EXTRA_CONFIG + 'process.env.JS_ENV_EXTRA_CONFIG': process.env.JS_ENV_EXTRA_CONFIG || "{}" }) ], module: { diff --git a/webpack.prod.config.js b/webpack.prod.config.js index d71031cffc43..cff1f23699fd 100644 --- a/webpack.prod.config.js +++ b/webpack.prod.config.js @@ -18,7 +18,7 @@ var optimizedConfig = Merge.smart(commonConfig, { plugins: [ new webpack.DefinePlugin({ 'process.env.NODE_ENV': JSON.stringify('production'), - 'process.env.JS_ENV_EXTRA_CONFIG': process.env.JS_ENV_EXTRA_CONFIG + 'process.env.JS_ENV_EXTRA_CONFIG': process.env.JS_ENV_EXTRA_CONFIG || "{}" }), new webpack.LoaderOptionsPlugin({ // This may not be needed; legacy option for loaders written for webpack 1 minimize: true diff --git a/xmodule/README.rst b/xmodule/README.rst index 73aeca96cc7c..5096c78c2abe 100644 --- a/xmodule/README.rst +++ b/xmodule/README.rst @@ -10,7 +10,7 @@ The ``xmodule`` folder contains a variety of old-yet-important functionality cor * the implementations of several different built-in content-level XBlocks, such as ``problem`` and ``html``; and * `assets for those built-in XBlocks`_. -.. _assets for those built-in XBlocks: https://github.com/openedx/edx-platform/tree/master/assets +.. _assets for those built-in XBlocks: https://github.com/openedx/edx-platform/tree/master/xmodule/assets#readme Historical Context ****************** diff --git a/xmodule/annotatable_block.py b/xmodule/annotatable_block.py index 3c78a3b7c0e7..95f68d4370b6 100644 --- a/xmodule/annotatable_block.py +++ b/xmodule/annotatable_block.py @@ -4,7 +4,6 @@ import textwrap from lxml import etree -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Scope, String @@ -15,7 +14,6 @@ from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -35,7 +33,6 @@ class AnnotatableBlock( XmlMixin, EditingMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): @@ -73,22 +70,6 @@ class AnnotatableBlock( uses_xmodule_styles_setup = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/html/display.js'), - resource_filename(__name__, 'js/src/annotatable/display.js'), - resource_filename(__name__, 'js/src/javascript_loader.js'), - resource_filename(__name__, 'js/src/collapsible.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/raw/edit/xml.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } studio_js_module_name = "XMLEditingDescriptor" mako_template = "widgets/raw-edit.html" diff --git a/xmodule/assets/README.rst b/xmodule/assets/README.rst index 618ba029246f..a39b389a357a 100644 --- a/xmodule/assets/README.rst +++ b/xmodule/assets/README.rst @@ -41,8 +41,8 @@ the corresponding folders in any enabled themes, as part of the edx-platform bui It is collected into the static root, and then linked to from XBlock fragments by the ``add_sass_to_fragment`` function in `builtin_assets.py`_. -.. _AnnotatableBlockDisplay: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss -.. _AnnotatableBlockEditor: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss +.. _AnnotatableBlockDisplay.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockDisplay.scss +.. _AnnotatableBlockEditor.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/AnnotatableBlockEditor.scss .. _annotatable/_display.scss: https://github.com/openedx/edx-platform/tree/master/xmodule/assets/annotatable/_display.scss .. _simplify things: https://github.com/openedx/edx-platform/issues/32621 @@ -59,8 +59,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and * For many older blocks, their JS is: - * copied to ``common/static/xmodule`` by `static_content.py`_ (aka ``xmodule_assets``), - * then bundled using a generated Webpack config at ``common/static/xmodule/webpack.xmodule.config.js``, + * bundled using a `webpack.builtinblocks.config.js`_, * which is included into `webpack.common.config.js`_, * allowing it to be included into XBlock fragments using ``add_webpack_js_to_fragment`` from `builtin_assets.py`_. @@ -75,12 +74,7 @@ Currently, edx-platform XBlock JS is defined both here in `xmodule/assets`_ and * `VerticalBlock`_ * `LibrarySourcedBlock`_ -As part of an `active build refactoring`_: - -* We update the older builtin XBlocks to reference their JS directly rather than using copies of it. -* We will move ``webpack.xmodule.config.js`` here instead of generating it. -* We will consolidate all edx-platform XBlock JS here in `xmodule/assets`_. -* We will delete the ``xmodule_assets`` script. +As part of an `active build refactoring`_, we will soon consolidate all edx-platform XBlock JS here in `xmodule/assets`_. .. _xmodule/assets: https://github.com/openedx/edx-platform/tree/master/xmodule/assets .. _xmodule/js: https://github.com/openedx/edx-platform/tree/master/xmodule/js @@ -93,4 +87,5 @@ As part of an `active build refactoring`_: .. _builtin_assets.py: https://github.com/openedx/edx-platform/tree/master/xmodule/util/builtin_assets.py .. _static_content.py: https://github.com/openedx/edx-platform/blob/master/xmodule/static_content.py .. _library_source_block/style.css: https://github.com/openedx/edx-platform/blob/master/xmodule/assets/library_source_block/style.css +.. _webpack.builtinblocks.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.builtinblocks.config.js .. _webpack.common.config.js: https://github.com/openedx/edx-platform/blob/master/webpack.common.config.js diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index 072df22dbc04..797e95e1d5bb 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -35,6 +35,7 @@ from xmodule.capa.safe_exec import safe_exec from xmodule.capa.util import contextualize_text, convert_files_to_filenames, get_course_id_from_capa_block from openedx.core.djangolib.markup import HTML, Text +from openedx.core.lib.safe_lxml.xmlparser import XML from xmodule.stringify import stringify_children # extra things displayed after "show answers" is pressed @@ -183,7 +184,7 @@ def __init__(self, problem_text, id, capa_system, capa_block, # pylint: disable if isinstance(problem_text, str): # etree chokes on Unicode XML with an encoding declaration problem_text = problem_text.encode('utf-8') - self.tree = etree.XML(problem_text) + self.tree = XML(problem_text) try: self.make_xml_compatible(self.tree) diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index 43b5bb3efa42..9b69556e505d 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -19,7 +19,6 @@ from django.utils.encoding import smart_str from django.utils.functional import cached_property from lxml import etree -from pkg_resources import resource_filename from pytz import utc from web_fragments.fragment import Fragment from xblock.core import XBlock @@ -39,7 +38,6 @@ from xmodule.util.sandboxing import SandboxService from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, @@ -131,7 +129,6 @@ class ProblemBlock( XmlMixin, EditingMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): @@ -165,24 +162,6 @@ class ProblemBlock( uses_xmodule_styles_setup = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/javascript_loader.js'), - resource_filename(__name__, 'js/src/capa/display.js'), - resource_filename(__name__, 'js/src/collapsible.js'), - resource_filename(__name__, 'js/src/capa/imageinput.js'), - resource_filename(__name__, 'js/src/capa/schematic.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') - } - - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/problem/edit.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - display_name = String( display_name=_("Display Name"), help=_("The display name for this component."), diff --git a/xmodule/conditional_block.py b/xmodule/conditional_block.py index 5f484050d438..03662deb01e9 100644 --- a/xmodule/conditional_block.py +++ b/xmodule/conditional_block.py @@ -9,7 +9,6 @@ from lazy import lazy from lxml import etree from opaque_keys.edx.locator import BlockUsageLocator -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import ReferenceList, Scope, String @@ -23,7 +22,6 @@ from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, @@ -44,7 +42,6 @@ class ConditionalBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, StudioEditableBlock, @@ -146,21 +143,8 @@ class ConditionalBlock( show_in_read_only_mode = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/conditional/display.js'), - resource_filename(__name__, 'js/src/javascript_loader.js'), - resource_filename(__name__, 'js/src/collapsible.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - mako_template = 'widgets/metadata-edit.html' studio_js_module_name = 'SequenceDescriptor' - studio_view_js = { - 'js': [resource_filename(__name__, 'js/src/sequence/edit.js')], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } # Map # key: diff --git a/xmodule/discussion_block.py b/xmodule/discussion_block.py index 2df7818157b2..7433b51c9f94 100644 --- a/xmodule/discussion_block.py +++ b/xmodule/discussion_block.py @@ -15,11 +15,13 @@ from xblockutils.resources import ResourceLoader from xblockutils.studio_editable import StudioEditableXBlockMixin +from lms.djangoapps.discussion.django_comment_client.permissions import has_permission from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, Provider from openedx.core.djangolib.markup import HTML, Text from openedx.core.lib.xblock_utils import get_css_dependencies, get_js_dependencies from xmodule.xml_block import XmlMixin + log = logging.getLogger(__name__) loader = ResourceLoader(__name__) # pylint: disable=invalid-name @@ -154,9 +156,6 @@ def has_permission(self, permission): :param str permission: Permission :rtype: bool """ - # normal import causes the xmodule_assets command to fail due to circular import - hence importing locally - from lms.djangoapps.discussion.django_comment_client.permissions import has_permission - return has_permission(self.django_user, permission, self.course_key) def student_view(self, context=None): diff --git a/xmodule/error_block.py b/xmodule/error_block.py index fcebf8f744ff..a3379007e19f 100644 --- a/xmodule/error_block.py +++ b/xmodule/error_block.py @@ -18,7 +18,6 @@ from xmodule.errortracker import exc_info_to_str from xmodule.modulestore import EdxJSONEncoder from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, @@ -47,7 +46,6 @@ class ErrorFields: class ErrorBlock( ErrorFields, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): # pylint: disable=abstract-method diff --git a/xmodule/html_block.py b/xmodule/html_block.py index 3027c24b2ba8..688e515c80e9 100644 --- a/xmodule/html_block.py +++ b/xmodule/html_block.py @@ -8,8 +8,6 @@ import textwrap from datetime import datetime -from pkg_resources import resource_filename - from django.conf import settings from fs.errors import ResourceNotFound from lxml import etree @@ -27,7 +25,6 @@ from xmodule.util.misc import escape_html_characters from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -47,7 +44,7 @@ @XBlock.needs("user") class HtmlBlockMixin( # lint-amnesty, pylint: disable=abstract-method XmlMixin, EditingMixin, - XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin, + XModuleToXBlockMixin, ResourceTemplates, XModuleMixin, ): """ The HTML XBlock mixin. @@ -144,17 +141,6 @@ def studio_view(self, _context): shim_xmodule_js(fragment, 'HTMLEditingDescriptor') return fragment - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/html/display.js'), - resource_filename(__name__, 'js/src/javascript_loader.js'), - resource_filename(__name__, 'js/src/collapsible.js'), - resource_filename(__name__, 'js/src/html/imageModal.js'), - resource_filename(__name__, 'js/common_static/js/vendor/draggabilly.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - uses_xmodule_styles_setup = True mako_template = "widgets/html-edit.html" @@ -163,13 +149,6 @@ def studio_view(self, _context): template_dir_name = "html" show_in_read_only_mode = True - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/html/edit.js') - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - # VS[compat] TODO (cpennington): Delete this method once all fall 2012 course # are being edited in the cms @classmethod diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 74ab77630626..047d36abca52 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -17,7 +17,6 @@ from lxml import etree from lxml.etree import XMLSyntaxError from opaque_keys.edx.locator import LibraryLocator -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from webob import Response from xblock.completable import XBlockCompletionMode @@ -31,7 +30,6 @@ from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, @@ -76,7 +74,6 @@ class LibraryContentBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, StudioEditableBlock, @@ -95,19 +92,8 @@ class LibraryContentBlock( resources_dir = 'assets/library_content' - preview_view_js = { - 'js': [], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - mako_template = 'widgets/metadata-edit.html' studio_js_module_name = "VerticalDescriptor" - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/vertical/edit.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } show_in_read_only_mode = True diff --git a/xmodule/lti_block.py b/xmodule/lti_block.py index 80afed7a0887..4228aec74393 100644 --- a/xmodule/lti_block.py +++ b/xmodule/lti_block.py @@ -68,7 +68,6 @@ from django.conf import settings from lxml import etree from oauthlib.oauth1.rfc5849 import signature -from pkg_resources import resource_filename from pytz import UTC from webob import Response from web_fragments.fragment import Fragment @@ -88,7 +87,6 @@ from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -284,7 +282,6 @@ class LTIBlock( EditingMixin, MakoTemplateBlockBase, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): # pylint: disable=abstract-method @@ -372,22 +369,9 @@ class LTIBlock( resources_dir = None uses_xmodule_styles_setup = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/lti/lti.js') - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - mako_template = 'widgets/metadata-only-edit.html' studio_js_module_name = 'MetadataOnlyEditingDescriptor' - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/raw/edit/metadata-only.js') - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } def studio_view(self, _context): """ diff --git a/xmodule/modulestore/split_mongo/mongo_connection.py b/xmodule/modulestore/split_mongo/mongo_connection.py index 76821cd4007a..33f09352b958 100644 --- a/xmodule/modulestore/split_mongo/mongo_connection.py +++ b/xmodule/modulestore/split_mongo/mongo_connection.py @@ -257,7 +257,6 @@ def set(self, key, structure, course_context=None): # .. custom_attribute_description: contains the data chunk size in MBs. The size on which # the memcached client failed to store value in course structure cache. monitoring.set_custom_attribute('split_mongo_compressed_size', chunk_size_in_mbs) - monitoring.record_exception() log.info('Data caching (course structure) failed on chunk size: {} MB'.format(chunk_size_in_mbs)) diff --git a/xmodule/poll_block.py b/xmodule/poll_block.py index 6093237fa3c5..c4d9372883ee 100644 --- a/xmodule/poll_block.py +++ b/xmodule/poll_block.py @@ -13,7 +13,6 @@ from collections import OrderedDict from copy import deepcopy -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from lxml import etree @@ -24,7 +23,6 @@ from xmodule.stringify import stringify_children from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -42,7 +40,6 @@ class PollBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): # pylint: disable=abstract-method @@ -84,22 +81,6 @@ class PollBlock( resources_dir = None uses_xmodule_styles_setup = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/javascript_loader.js'), - resource_filename(__name__, 'js/src/poll/poll.js'), - resource_filename(__name__, 'js/src/poll/poll_main.js') - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - - # There is no studio_view() for this XBlock but this is needed to make the - # the static_content command happy. - studio_view_js = { - 'js': [], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') - } - def handle_ajax(self, dispatch, data): # lint-amnesty, pylint: disable=unused-argument """Ajax handler. diff --git a/xmodule/randomize_block.py b/xmodule/randomize_block.py index c5e1f2c4747f..b8a1432ff311 100644 --- a/xmodule/randomize_block.py +++ b/xmodule/randomize_block.py @@ -11,7 +11,6 @@ from xmodule.seq_block import SequenceMixin from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, STUDENT_VIEW, XModuleMixin, @@ -26,7 +25,6 @@ class RandomizeBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): diff --git a/xmodule/seq_block.py b/xmodule/seq_block.py index b0a2789fd273..5561d034da87 100644 --- a/xmodule/seq_block.py +++ b/xmodule/seq_block.py @@ -14,7 +14,6 @@ from lxml import etree from opaque_keys.edx.keys import UsageKey -from pkg_resources import resource_filename from pytz import UTC from web_fragments.fragment import Fragment from xblock.completable import XBlockCompletionMode @@ -25,7 +24,6 @@ from edx_toggles.toggles import WaffleFlag, SettingDictToggle from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, @@ -258,7 +256,6 @@ class SequenceBlock( MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): @@ -271,20 +268,6 @@ class SequenceBlock( show_in_read_only_mode = True uses_xmodule_styles_setup = True - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/sequence/display.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') - } - - # There is no studio_view() for this XBlock but this is needed to make the - # the static_content command happy. - studio_view_js = { - 'js': [], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') - } - def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/xmodule/split_test_block.py b/xmodule/split_test_block.py index 5984ae531efc..8984dc62dbb5 100644 --- a/xmodule/split_test_block.py +++ b/xmodule/split_test_block.py @@ -12,7 +12,6 @@ from django.utils.functional import cached_property from lxml import etree -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from webob import Response from xblock.core import XBlock @@ -26,7 +25,6 @@ from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, STUDENT_VIEW, @@ -132,7 +130,6 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, StudioEditableBlock, @@ -158,17 +155,8 @@ class SplitTestBlock( # lint-amnesty, pylint: disable=abstract-method show_in_read_only_mode = True - preview_view_js = { - 'js': [], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - mako_template = "widgets/metadata-only-edit.html" studio_js_module_name = 'SequenceDescriptor' - studio_view_js = { - 'js': [resource_filename(__name__, 'js/src/sequence/edit.js')], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } @cached_property def child_block(self): diff --git a/xmodule/static_content.py b/xmodule/static_content.py index 03014476737a..e6c1aed23530 100755 --- a/xmodule/static_content.py +++ b/xmodule/static_content.py @@ -1,254 +1,25 @@ # /usr/bin/env python """ -This module has utility functions for gathering up the javascript -that is defined by XModules and XModuleDescriptors -""" +This module used to hold a CLI utility for gathering up the JS and Sass used by several built-in XBlocks. +It now remains as a stub, just for backwards compatibility. -import errno -import hashlib -import json +It will soon be removed as part of https://github.com/openedx/edx-platform/issues/31798. +""" import logging -import os import sys -import textwrap -from collections import defaultdict -from pkg_resources import resource_filename - -import django -from path import Path as path - -from xmodule.annotatable_block import AnnotatableBlock -from xmodule.capa_block import ProblemBlock -from xmodule.conditional_block import ConditionalBlock -from xmodule.html_block import AboutBlock, CourseInfoBlock, HtmlBlock, StaticTabBlock -from xmodule.library_content_block import LibraryContentBlock -from xmodule.lti_block import LTIBlock -from xmodule.poll_block import PollBlock -from xmodule.seq_block import SequenceBlock -from xmodule.split_test_block import SplitTestBlock -from xmodule.template_block import CustomTagBlock -from xmodule.word_cloud_block import WordCloudBlock -from xmodule.x_module import HTMLSnippet - -LOG = logging.getLogger(__name__) - - -class VideoBlock(HTMLSnippet): # lint-amnesty, pylint: disable=abstract-method - """ - Static assets for VideoBlock. - Kept here because importing VideoBlock code requires Django to be setup. - """ - - preview_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/video/10_main.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js') - } - - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/tabs/tabs-aggregator.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - - -# List of XBlocks which use this static content setup. -# Should only be used for XModules being converted to XBlocks. -XBLOCK_CLASSES = [ - AboutBlock, - AnnotatableBlock, - ConditionalBlock, - CourseInfoBlock, - CustomTagBlock, - HtmlBlock, - LibraryContentBlock, - LTIBlock, - PollBlock, - ProblemBlock, - SequenceBlock, - SplitTestBlock, - StaticTabBlock, - VideoBlock, - WordCloudBlock, -] - - -def write_module_js(output_root): - """Write all registered XModule js and coffee files to output root.""" - return _write_js(output_root, XBLOCK_CLASSES, 'get_preview_view_js') - - -def write_descriptor_js(output_root): - """Write all registered XModuleDescriptor js and coffee files to output root.""" - return _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') - - -def _ensure_dir(directory): - """Ensure that `directory` exists.""" - try: - os.makedirs(directory) - except OSError as exc: - if exc.errno == errno.EEXIST: - pass - else: - raise - - -def _write_js(output_root, classes, js_attribute): - """ - Write the javascript fragments from all XModules in `classes` - into `output_root` as individual files, hashed by the contents to remove - duplicates - - Returns a dictionary mapping class names to the files that they depend on. - """ - file_contents = {} - file_owners = defaultdict(list) - - fragment_owners = defaultdict(list) - for class_ in classes: - module_js = getattr(class_, js_attribute)() - with open(module_js.get('xmodule_js'), 'rb') as xmodule_js_file: - xmodule_js_fragment = xmodule_js_file.read() - # It will enforce 000 prefix for xmodule.js. - fragment_owners[(0, 'js', xmodule_js_fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) - for filetype in ('coffee', 'js'): - for idx, fragment_path in enumerate(module_js.get(filetype, [])): - with open(fragment_path, 'rb') as fragment_file: - fragment = fragment_file.read() - fragment_owners[(idx + 1, filetype, fragment)].append(getattr(class_, js_attribute + '_bundle_name')()) - - for (idx, filetype, fragment), owners in sorted(fragment_owners.items()): - filename = "{idx:0=3d}-{hash}.{type}".format( - idx=idx, - hash=hashlib.md5(fragment).hexdigest(), - type=filetype) - file_contents[filename] = fragment - for owner in owners: - file_owners[owner].append(output_root / filename) - - _write_files(output_root, file_contents, {'.coffee': '.js'}) - - return file_owners - - -def _write_files(output_root, contents, generated_suffix_map=None): - """ - Write file contents to output root. - - Any files not listed in contents that exists in output_root will be deleted, - unless it matches one of the patterns in `generated_suffix_map`. - - output_root (path): The root directory to write the file contents in - contents (dict): A map from filenames to file contents to be written to the output_root - generated_suffix_map (dict): Optional. Maps file suffix to generated file suffix. - For any file in contents, if the suffix matches a key in `generated_suffix_map`, - then the same filename with the suffix replaced by the value from `generated_suffix_map` - will be ignored - """ - _ensure_dir(output_root) - to_delete = {file.basename() for file in output_root.files()} - set(contents.keys()) - - if generated_suffix_map: - for output_file in contents.keys(): - for suffix, generated_suffix in generated_suffix_map.items(): - if output_file.endswith(suffix): - to_delete.discard(output_file.replace(suffix, generated_suffix)) - - for extra_file in to_delete: - (output_root / extra_file).remove_p() - - for filename, file_content in contents.items(): - output_file = output_root / filename - - not_file = not output_file.isfile() - - # Sometimes content is already unicode and sometimes it's not - # so we add this conditional here to make sure that below we're - # always working with streams of bytes. - if not isinstance(file_content, bytes): - file_content = file_content.encode('utf-8') - - # not_file is included to short-circuit this check, because - # read_md5 depends on the file already existing - write_file = not_file or output_file.read_md5() != hashlib.md5(file_content).digest() - if write_file: - LOG.debug("Writing %s", output_file) - output_file.write_bytes(file_content) - else: - LOG.debug("%s unchanged, skipping", output_file) - - -def write_webpack(output_file, module_files, descriptor_files): - """ - Write all xmodule and xmodule descriptor javascript into module-specific bundles. - - The output format should be suitable for smart-merging into an existing webpack configuration. - """ - _ensure_dir(output_file.dirname()) - - config = { - 'entry': {} - } - for (owner, files) in list(module_files.items()) + list(descriptor_files.items()): - unique_files = sorted({f'./{file}' for file in files}) - if len(unique_files) == 1: - unique_files = unique_files[0] - config['entry'][owner] = unique_files - # config['entry']['modules/js/all'] = sorted(set('./{}'.format(file) for file in sum(module_files.values(), []))) - # config['entry']['descriptors/js/all'] = sorted(set('./{}'.format(file) for file in sum(descriptor_files.values(), []))) # lint-amnesty, pylint: disable=line-too-long - - with output_file.open('w') as outfile: - outfile.write( - textwrap.dedent("""\ - module.exports = {config_json}; - """).format( - config_json=json.dumps( - config, - indent=4, - sort_keys=True, - ) - ) - ) def main(): """ - Generate - Usage: static_content.py + Warn that this script is now a stub, and return success (zero). """ - from django.conf import settings - # Install only the apps whose models are imported when this runs - installed_apps = ( - 'django.contrib.auth', - 'django.contrib.contenttypes', - 'config_models', - 'openedx.core.djangoapps.video_config', - 'openedx.core.djangoapps.video_pipeline', + logging.warning( + "xmodule/static_content.py, aka xmodule_assets, is now a no-op. " + "Please remove calls to it from your build pipeline. It will soon be deleted.", ) - try: - import edxval # lint-amnesty, pylint: disable=unused-import - installed_apps += ('edxval',) - except ImportError: - pass - if not settings.configured: - settings.configure( - INSTALLED_APPS=installed_apps, - ) - django.setup() - - try: - root = path(sys.argv[1]) - except IndexError: - sys.exit(main.__doc__) - - descriptor_files = write_descriptor_js(root / 'descriptors/js') - module_files = write_module_js(root / 'modules/js') - write_webpack(root / 'webpack.xmodule.config.js', module_files, descriptor_files) + return 0 -if __name__ == '__main__': +if __name__ == "__main__": sys.exit(main()) diff --git a/xmodule/template_block.py b/xmodule/template_block.py index a27b92cb2c7e..fd5373386061 100644 --- a/xmodule/template_block.py +++ b/xmodule/template_block.py @@ -6,13 +6,11 @@ from xblock.core import XBlock from lxml import etree -from pkg_resources import resource_filename from web_fragments.fragment import Fragment from xmodule.editing_block import EditingMixin from xmodule.raw_block import RawMixin from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -28,7 +26,6 @@ class CustomTagTemplateBlock( # pylint: disable=abstract-method XmlMixin, EditingMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): @@ -65,15 +62,6 @@ class CustomTagBlock(CustomTagTemplateBlock): # pylint: disable=abstract-method resources_dir = None template_dir_name = 'customtag' - preview_view_js = { - 'js': [], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - studio_view_js = { - 'js': [resource_filename(__name__, 'js/src/raw/edit/xml.js')], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - def studio_view(self, _context): """ Return the studio view. diff --git a/xmodule/tests/test_content.py b/xmodule/tests/test_content.py index dabd6d752739..0b566153ad7d 100644 --- a/xmodule/tests/test_content.py +++ b/xmodule/tests/test_content.py @@ -1,17 +1,14 @@ """Tests for contents""" -import os import unittest from unittest.mock import Mock, patch import ddt from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import AssetLocator, CourseLocator -from path import Path as path from xmodule.contentstore.content import ContentStore, StaticContent, StaticContentStream -from xmodule.static_content import XBLOCK_CLASSES, _write_js SAMPLE_STRING = """ This is a sample string with more than 1024 bytes, the default STREAM_DATA_CHUNK_SIZE @@ -205,13 +202,3 @@ def test_static_content_stream_stream_data_in_range(self): total_length += len(chunck) assert total_length == ((last_byte - first_byte) + 1) - - def test_static_content_write_js(self): - """ - Test that only one filename starts with 000. - """ - output_root = path('common/static/xmodule/descriptors/js') - file_owners = _write_js(output_root, XBLOCK_CLASSES, 'get_studio_view_js') - js_file_paths = {file_path for file_path in sum(list(file_owners.values()), []) if os.path.basename(file_path).startswith('000-')} # lint-amnesty, pylint: disable=line-too-long - assert len(js_file_paths) == 1 - assert 'XModule.Descriptor = (function() {' in open(js_file_paths.pop()).read() diff --git a/xmodule/video_block/video_block.py b/xmodule/video_block/video_block.py index 97c41c2f6412..5bf2b3c28cf1 100644 --- a/xmodule/video_block/video_block.py +++ b/xmodule/video_block/video_block.py @@ -52,7 +52,7 @@ from xmodule.video_block import manage_video_subtitles_save from xmodule.x_module import ( PUBLIC_VIEW, STUDENT_VIEW, - HTMLSnippet, ResourceTemplates, shim_xmodule_js, + ResourceTemplates, shim_xmodule_js, XModuleMixin, XModuleToXBlockMixin, ) from xmodule.xml_block import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname @@ -121,7 +121,7 @@ @XBlock.needs('mako', 'user') class VideoBlock( VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers, - EmptyDataRawMixin, XmlMixin, EditingMixin, XModuleToXBlockMixin, HTMLSnippet, + EmptyDataRawMixin, XmlMixin, EditingMixin, XModuleToXBlockMixin, ResourceTemplates, XModuleMixin, LicenseMixin): """ XML source example: @@ -282,6 +282,9 @@ def public_view(self, context): return fragment def get_html(self, view=STUDENT_VIEW, context=None): # lint-amnesty, pylint: disable=arguments-differ, too-many-statements + """ + Return html for a given view of this block. + """ context = context or {} track_status = (self.download_track and self.track) transcript_download_format = self.transcript_download_format if not track_status else None diff --git a/xmodule/word_cloud_block.py b/xmodule/word_cloud_block.py index 8be08242dbae..6a31da5ef811 100644 --- a/xmodule/word_cloud_block.py +++ b/xmodule/word_cloud_block.py @@ -10,8 +10,6 @@ import json import logging -from pkg_resources import resource_filename - from web_fragments.fragment import Fragment from xblock.core import XBlock from xblock.fields import Boolean, Dict, Integer, List, Scope, String @@ -20,7 +18,6 @@ from xmodule.util.builtin_assets import add_webpack_js_to_fragment, add_sass_to_fragment from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - HTMLSnippet, ResourceTemplates, shim_xmodule_js, XModuleMixin, @@ -49,7 +46,6 @@ class WordCloudBlock( # pylint: disable=abstract-method XmlMixin, EditingMixin, XModuleToXBlockMixin, - HTMLSnippet, ResourceTemplates, XModuleMixin, ): @@ -112,19 +108,6 @@ class WordCloudBlock( # pylint: disable=abstract-method resources_dir = 'assets/word_cloud' template_dir_name = 'word_cloud' - preview_view_js = { - 'js': [ - resource_filename(__name__, 'assets/word_cloud/src/js/word_cloud.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } - - studio_view_js = { - 'js': [ - resource_filename(__name__, 'js/src/raw/edit/metadata-only.js'), - ], - 'xmodule_js': resource_filename(__name__, 'js/src/xmodule.js'), - } studio_js_module_name = "MetadataOnlyEditingDescriptor" mako_template = "widgets/metadata-only-edit.html" diff --git a/xmodule/x_module.py b/xmodule/x_module.py index 1c44b2cc0b8d..fd974c6928f8 100644 --- a/xmodule/x_module.py +++ b/xmodule/x_module.py @@ -206,40 +206,6 @@ def create_definition(self, block_type, slug=None): raise NotImplementedError("Specific Modulestores must provide implementations of create_definition") -class HTMLSnippet: - """ - A base class defining an interface for an object that is able to present an - html snippet, along with associated javascript and css - """ - - preview_view_js = {} - studio_view_js = {} - - @classmethod - def get_preview_view_js(cls): - return cls.preview_view_js - - @classmethod - def get_preview_view_js_bundle_name(cls): - return cls.__name__ + 'Display' - - @classmethod - def get_studio_view_js(cls): - return cls.studio_view_js - - @classmethod - def get_studio_view_js_bundle_name(cls): - return cls.__name__ + 'Editor' - - def get_html(self): - """ - Return the html used to display this snippet - """ - raise NotImplementedError( - "get_html() must be provided by specific modules - not present in {}" - .format(self.__class__)) - - def shim_xmodule_js(fragment, js_module_name): """ Set up the XBlock -> XModule shim on the supplied :class:`web_fragments.fragment.Fragment`