Skip to content

Commit

Permalink
Merge branch 'master' into bilalqamar95/edx-ui-toolkit-upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
BilalQamar95 authored Mar 11, 2024
2 parents 448b15d + 1e140a6 commit 3ee240d
Show file tree
Hide file tree
Showing 61 changed files with 1,597 additions and 297 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/migrations-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,17 @@ jobs:
# within the global constraint i.e. Django==4.2.8 in current case
# because we have global constraint of Django<4.2
django-version: ["pinned"]
mongo-version: ["4"]
mongo-version: ["4", "7"]
mysql-version: ["8"]
services:
mongo:
image: mongo:${{ matrix.mongo-version }}
ports:
- 27017:27017
# Note: Calling mongo here only works with mongo 4, in newer versions of mongo
# we'll have to use `mongosh`
# we'll have to use `mongosh`, hence the 'which mongosh mongo'.
options: >-
--health-cmd "mongo --quiet --eval 'db.runCommand(\"ping\")'"
--health-cmd "$(which mongosh mongo) --quiet --eval 'db.runCommand(\"ping\")'"
--health-interval 10s
--health-timeout 5s
--health-retries 3
Expand Down
14 changes: 14 additions & 0 deletions .github/workflows/static-assets-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,20 @@ jobs:
python-version: [ 3.8 ]
node-version: [ 16 ]
npm-version: [ 8.5.x ]
mongo-version: ["4.4", "7.0"]

services:
mongo:
image: mongo:${{ matrix.mongo-version }}
ports:
- 27017:27017
# Note: Calling mongo here only works with mongo 4, in newer versions of mongo
# we'll have to use `mongosh`, hence the 'which mongosh mongo'.
options: >-
--health-cmd "$(which mongosh mongo) --quiet --eval 'db.runCommand(\"ping\")'"
--health-interval 10s
--health-timeout 5s
--health-retries 3
steps:
- name: Checkout repo
Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/unit-tests-gh-hosted.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ jobs:
- "common-with-cms"
- "xmodule-with-lms"
- "xmodule-with-cms"
name: gh-hosted-python-${{ matrix.python-version }},django-${{ matrix.django-version }},${{ matrix.shard_name }}
mongo-version:
- "4.4"
- "7.0"
name: gh-hosted-python-${{ matrix.python-version }},django-${{ matrix.django-version }},mongo-${{ matrix.mongo-version }}${{ matrix.shard_name }}
steps:
- uses: actions/checkout@v2

Expand All @@ -46,7 +49,7 @@ jobs:
- name: Start MongoDB
uses: supercharge/[email protected]
with:
mongodb-version: 4.4
mongodb-version: ${{ matrix.mongo-version}}

- name: Setup Python
uses: actions/setup-python@v4
Expand Down
15 changes: 14 additions & 1 deletion .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:

jobs:
run-tests:
name: python-${{ matrix.python-version }},django-${{ matrix.django-version }},${{ matrix.shard_name }}
name: python-${{ matrix.python-version }},django-${{ matrix.django-version }},mongo-${{ matrix.mongo-version }},${{ matrix.shard_name }}
if: (github.repository == 'edx/edx-platform-private') || (github.repository == 'openedx/edx-platform' && (startsWith(github.base_ref, 'open-release') == false))
runs-on: [ edx-platform-runner ]
strategy:
Expand Down Expand Up @@ -36,13 +36,26 @@ jobs:
- "common-with-cms"
- "xmodule-with-lms"
- "xmodule-with-cms"
mongo-version:
- "4.4"
- "7.0"
# We expect Django 4.0 to fail, so don't stop when it fails.
continue-on-error: ${{ matrix.django-version == '4.0' }}

steps:
- name: sync directory owner
run: sudo chown runner:runner -R .*

- name: install mongo version
run: |
if [[ "${{ matrix.mongo-version }}" != "4.4" ]]; then
sudo apt-get purge -y "mongodb-org*"
sudo apt-get remove -y mongodb-org
wget -qO - https://www.mongodb.org/static/pgp/server-${{ matrix.mongo-version }}.asc | sudo apt-key add -
echo "deb https://repo.mongodb.org/apt/ubuntu focal/mongodb-org/${{ matrix.mongo-version }} multiverse" | sudo tee /etc/apt/sources.list.d/mongodb-org-${{ matrix.mongo-version }}.list
sudo apt-get update && sudo apt-get install -y mongodb-org="${{ matrix.mongo-version }}.*"
fi
- name: checkout repo
uses: actions/checkout@v3

Expand Down
8 changes: 5 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ endif
push_translations: ## push source strings to Transifex for translation
i18n_tool transifex push

pull_plugin_translations: ## Pull translations from Transifex for edx_django_utils.plugins for both lms and cms
pull_plugin_translations: ## Pull translations for edx_django_utils.plugins for both lms and cms
rm -rf conf/plugins-locale/plugins # Clean up existing atlas translations
mkdir -p conf/plugins-locale/plugins
python manage.py lms pull_plugin_translations --verbose $(ATLAS_OPTIONS)
Expand All @@ -70,7 +70,7 @@ pull_xblock_translations: ## pull xblock translations via atlas
python manage.py cms compile_xblock_translations

pull_translations: ## pull translations from Transifex
git clean -fdX conf/locale
git clean -fdX conf/locale conf/plugins-locale/studio-frontend
ifeq ($(OPENEDX_ATLAS_PULL),)
i18n_tool transifex pull
i18n_tool extract
Expand All @@ -83,7 +83,9 @@ else
make pull_xblock_translations
make pull_plugin_translations
find conf/locale -mindepth 1 -maxdepth 1 -type d -exec rm -r {} \;
atlas pull $(ATLAS_OPTIONS) translations/edx-platform/conf/locale:conf/locale
atlas pull $(ATLAS_OPTIONS) \
translations/edx-platform/conf/locale:conf/locale \
translations/studio-frontend/src/i18n/messages:conf/plugins-locale/studio-frontend
i18n_tool generate
endif
python manage.py lms compilejsi18n
Expand Down
2 changes: 1 addition & 1 deletion catalog-info.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ metadata:
title: "Documentation"
icon: "Web"
spec:
owner: group:arch-bom
owner: group:2u-arch-bom
type: 'service'
lifecycle: 'production'
11 changes: 6 additions & 5 deletions cms/djangoapps/contentstore/api/views/course_import.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from cms.djangoapps.contentstore.storage import course_import_export_storage
from cms.djangoapps.contentstore.tasks import CourseImportTask, import_olx
from cms.djangoapps.contentstore.utils import IMPORTABLE_FILE_TYPES
from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes

from .utils import course_author_access_required
Expand All @@ -44,8 +45,8 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):
"""
**Use Case**
* Start an asynchronous task to import a course from a .tar.gz file into
the specified course ID, overwriting the existing course
* Start an asynchronous task to import a course from a .tar.gz or .zip
file into the specified course ID, overwriting the existing course
* Get a status on an asynchronous task import
**Example Requests**
Expand All @@ -59,7 +60,7 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):
* course_id: (required) A string representation of a Course ID,
e.g., course-v1:edX+DemoX+Demo_Course
* course_data: (required) The course .tar.gz file to import
* course_data: (required) The course .tar.gz or .zip file to import
**POST Response Values**
Expand All @@ -83,7 +84,7 @@ class CourseImportView(CourseImportExportViewMixin, GenericAPIView):
A GET request must include the following parameters.
* task_id: (required) The UUID of the task to check, e.g. "4b357bb3-2a1e-441d-9f6c-2210cf76606f"
* filename: (required) The filename of the uploaded course .tar.gz
* filename: (required) The filename of the uploaded course .tar.gz or .zip
**GET Response Values**
Expand Down Expand Up @@ -124,7 +125,7 @@ def post(self, request, course_key):
)

filename = request.FILES['course_data'].name
if not filename.endswith('.tar.gz'):
if not filename.endswith(IMPORTABLE_FILE_TYPES):
raise self.api_error(
status_code=status.HTTP_400_BAD_REQUEST,
developer_message='Parameter in the wrong format',
Expand Down
2 changes: 1 addition & 1 deletion cms/djangoapps/contentstore/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
UNKNOWN_ERROR_IN_IMPORT = _('Unknown error while importing course.')
UNKNOWN_ERROR_IN_UNPACKING = _('An Unknown error occurred during the unpacking step.')
UNKNOWN_USER_ID = _('Unknown User ID: {0}')
UNSAFE_TAR_FILE = _('Unsafe tar file. Aborting import.')
UNSAFE_ARCHIVE_FILE = _('Unsafe archive file. Aborting import.')
USER_PERMISSION_DENIED = _('User permission denied.')
47 changes: 40 additions & 7 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from mimetypes import guess_type

from attrs import frozen, Factory
from django.conf import settings
from django.utils.translation import gettext as _
from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
from opaque_keys.edx.locator import DefinitionLocator, LocalId
Expand All @@ -17,11 +18,11 @@
from xmodule.contentstore.content import StaticContent
from xmodule.contentstore.django import contentstore
from xmodule.exceptions import NotFoundError
from xmodule.library_content_block import LibraryContentBlock
from xmodule.modulestore.django import modulestore
from xmodule.xml_block import XmlMixin

from cms.djangoapps.models.settings.course_grading import CourseGradingModel
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
import openedx.core.djangoapps.content_staging.api as content_staging_api

from .utils import reverse_course_url, reverse_library_url, reverse_usage_url
Expand Down Expand Up @@ -125,6 +126,34 @@ def xblock_studio_url(xblock, parent_xblock=None, find_parent=False):
return reverse_usage_url('container_handler', xblock.location)


def xblock_lms_url(xblock) -> str:
"""
Returns the LMS URL for the specified xblock.
Args:
xblock: The xblock to get the LMS URL for.
Returns:
str: The LMS URL for the specified xblock.
"""
lms_root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)
return f"{lms_root_url}/courses/{xblock.location.course_key}/jump_to/{xblock.location}"


def xblock_embed_lms_url(xblock) -> str:
"""
Returns the LMS URL for the specified xblock in embed mode.
Args:
xblock: The xblock to get the LMS URL for.
Returns:
str: The LMS URL for the specified xblock in embed mode.
"""
lms_root_url = configuration_helpers.get_value('LMS_ROOT_URL', settings.LMS_ROOT_URL)
return f"{lms_root_url}/xblock/{xblock.location}"


def xblock_type_display_name(xblock, default_display_name=None):
"""
Returns the display name for the specified type of xblock. Note that an instance can be passed in
Expand Down Expand Up @@ -337,14 +366,18 @@ def _import_xml_node_to_parent(
new_xblock = store.update_item(temp_xblock, user_id, allow_not_found=True)
parent_xblock.children.append(new_xblock.location)
store.update_item(parent_xblock, user_id)
if isinstance(new_xblock, LibraryContentBlock):
# Special case handling for library content. If we need this for other blocks in the future, it can be made into
# an API, and we'd call new_block.studio_post_paste() instead of this code.
# In this case, we want to pull the children from the library and let library_tools assign their IDs.
new_xblock.sync_from_library(upgrade_to_latest=False)
else:

children_handled = False
if hasattr(new_xblock, 'studio_post_paste'):
# Allow an XBlock to do anything fancy it may need to when pasted from the clipboard.
# These blocks may handle their own children or parenting if needed. Let them return booleans to
# let us know if we need to handle these or not.
children_handed = new_xblock.studio_post_paste(store, node)

if not children_handled:
for child_node in child_nodes:
_import_xml_node_to_parent(child_node, new_xblock, store, user_id=user_id)

return new_xblock


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import base64
import os
import tarfile

from django.conf import settings
from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user
Expand All @@ -16,7 +15,7 @@
from path import Path

from cms.djangoapps.contentstore.utils import add_instructor
from openedx.core.lib.extract_tar import safetar_extractall
from openedx.core.lib.extract_archive import safe_extractall
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
Expand Down Expand Up @@ -47,13 +46,10 @@ def handle(self, *args, **options):
course_dir = data_root / subdir

# Extract library archive
tar_file = tarfile.open(archive_path) # lint-amnesty, pylint: disable=consider-using-with
try:
safetar_extractall(tar_file, course_dir)
safe_extractall(archive_path, course_dir)
except SuspiciousOperation as exc:
raise CommandError(f'\n=== Course import {archive_path}: Unsafe tar file - {exc.args[0]}\n') # lint-amnesty, pylint: disable=raise-missing-from
finally:
tar_file.close()
raise CommandError(f'\n=== Course import {archive_path}: Unsafe tar file - {exc.args[0]}\n') from exc

# Paths to the library.xml file
abs_xml_path = os.path.join(course_dir, 'library')
Expand Down
45 changes: 21 additions & 24 deletions cms/djangoapps/contentstore/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,13 @@
SearchIndexingError
)
from cms.djangoapps.contentstore.storage import course_import_export_storage
from cms.djangoapps.contentstore.utils import delete_course # lint-amnesty, pylint: disable=wrong-import-order
from cms.djangoapps.contentstore.utils import initialize_permissions, reverse_usage_url, translation_language
from cms.djangoapps.contentstore.utils import (
IMPORTABLE_FILE_TYPES,
initialize_permissions,
reverse_usage_url,
translation_language,
delete_course
)
from cms.djangoapps.models.settings.course_metadata import CourseMetadata
from common.djangoapps.course_action_state.models import CourseRerunState
from common.djangoapps.student.auth import has_course_author_access
Expand All @@ -62,20 +67,15 @@
from openedx.core.djangoapps.discussions.tasks import update_unit_discussion_state_from_discussion_blocks
from openedx.core.djangoapps.embargo.models import CountryAccessRule, RestrictedCourse
from openedx.core.lib.blockstore_api import get_collection
from openedx.core.lib.extract_tar import safetar_extractall
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.course_block import CourseFields # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.exceptions import SerializationError # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import DuplicateCourseError, InvalidProctoringProvider
from xmodule.modulestore.xml_exporter import export_library_to_xml # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.xml_exporter import export_course_to_xml
from xmodule.modulestore.xml_importer import import_library_from_xml # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.xml_importer import CourseImportException, import_course_from_xml

from openedx.core.lib.extract_archive import safe_extractall
from xmodule.contentstore.django import contentstore
from xmodule.course_block import CourseFields
from xmodule.exceptions import SerializationError
from xmodule.modulestore import COURSE_ROOT, LIBRARY_ROOT, ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import DuplicateCourseError, InvalidProctoringProvider, ItemNotFoundError
from xmodule.modulestore.xml_exporter import export_course_to_xml, export_library_to_xml
from xmodule.modulestore.xml_importer import CourseImportException, import_course_from_xml, import_library_from_xml
from .outlines import update_outline_from_modulestore
from .outlines_regenerate import CourseOutlineRegenerate
from .toggles import bypass_olx_failure_enabled
Expand Down Expand Up @@ -480,7 +480,7 @@ def sync_discussion_settings(course_key, user):
# lint-amnesty, pylint: disable=too-many-statements
def import_olx(self, user_id, course_key_string, archive_path, archive_name, language):
"""
Import a course or library from a provided OLX .tar.gz archive.
Import a course or library from a provided OLX .tar.gz or .zip archive.
"""
set_code_owner_attribute_from_module(__name__)
current_step = 'Unpacking'
Expand Down Expand Up @@ -517,7 +517,7 @@ def user_has_access(user):

def file_is_supported():
"""Check if it is a supported file."""
file_is_valid = archive_name.endswith('.tar.gz')
file_is_valid = archive_name.endswith(IMPORTABLE_FILE_TYPES)

if not file_is_valid:
message = f'Unsupported file {archive_name}'
Expand Down Expand Up @@ -647,17 +647,14 @@ def read_chunk():

# try-finally block for proper clean up after receiving file.
try:
tar_file = tarfile.open(temp_filepath) # lint-amnesty, pylint: disable=consider-using-with
try:
safetar_extractall(tar_file, (course_dir + '/'))
safe_extractall(temp_filepath, course_dir)
except SuspiciousOperation as exc:
with translation_language(language):
self.status.fail(UserErrors.UNSAFE_TAR_FILE)
LOGGER.error(f'{log_prefix}: Unsafe tar file')
self.status.fail(UserErrors.UNSAFE_ARCHIVE_FILE)
LOGGER.error(f'{log_prefix}: Unsafe archive file')
monitor_import_failure(courselike_key, current_step, exception=exc)
return
finally:
tar_file.close()

current_step = 'Verifying'
self.status.set_state(current_step)
Expand Down
Loading

0 comments on commit 3ee240d

Please sign in to comment.