diff --git a/cms/djangoapps/contentstore/api/views/course_import.py b/cms/djangoapps/contentstore/api/views/course_import.py
index a673d3dd5a40..dd7828c2d94f 100644
--- a/cms/djangoapps/contentstore/api/views/course_import.py
+++ b/cms/djangoapps/contentstore/api/views/course_import.py
@@ -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
@@ -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**
@@ -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**
@@ -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**
@@ -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',
diff --git a/cms/djangoapps/contentstore/errors.py b/cms/djangoapps/contentstore/errors.py
index 7b24c3f535b5..77eab369fd22 100644
--- a/cms/djangoapps/contentstore/errors.py
+++ b/cms/djangoapps/contentstore/errors.py
@@ -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.')
diff --git a/cms/djangoapps/contentstore/management/commands/import_content_library.py b/cms/djangoapps/contentstore/management/commands/import_content_library.py
index 782db4988061..b5fc2ccc2945 100644
--- a/cms/djangoapps/contentstore/management/commands/import_content_library.py
+++ b/cms/djangoapps/contentstore/management/commands/import_content_library.py
@@ -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
@@ -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_archives 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
@@ -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()
# Paths to the library.xml file
abs_xml_path = os.path.join(course_dir, 'library')
diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py
index 434aba419af6..c2119887f19c 100644
--- a/cms/djangoapps/contentstore/tasks.py
+++ b/cms/djangoapps/contentstore/tasks.py
@@ -47,7 +47,12 @@
SearchIndexingError
)
from cms.djangoapps.contentstore.storage import course_import_export_storage
-from cms.djangoapps.contentstore.utils import initialize_permissions, reverse_usage_url, translation_language
+from cms.djangoapps.contentstore.utils import (
+ initialize_permissions,
+ reverse_usage_url,
+ translation_language,
+ IMPORTABLE_FILE_TYPES,
+)
from cms.djangoapps.models.settings.course_metadata import CourseMetadata
from common.djangoapps.course_action_state.models import CourseRerunState
@@ -60,7 +65,7 @@
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 openedx.core.lib.extract_archives import safe_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
@@ -448,7 +453,7 @@ def generate_name(cls, arguments_dict):
# 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'
@@ -485,7 +490,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}'
@@ -614,17 +619,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)
diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py
index 48fc2c962cf1..1f5df7d4ff33 100644
--- a/cms/djangoapps/contentstore/utils.py
+++ b/cms/djangoapps/contentstore/utils.py
@@ -87,6 +87,7 @@
from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService
+IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip')
log = logging.getLogger(__name__)
diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py
index bb1616c04eb2..e7fc07ce3182 100644
--- a/cms/djangoapps/contentstore/views/import_export.py
+++ b/cms/djangoapps/contentstore/views/import_export.py
@@ -42,8 +42,13 @@
from ..storage import course_import_export_storage
from ..tasks import CourseExportTask, CourseImportTask, export_olx, import_olx
from ..toggles import use_new_export_page, use_new_import_page
-from ..utils import reverse_course_url, reverse_library_url, get_export_url, get_import_url
-
+from ..utils import (
+ reverse_course_url,
+ reverse_library_url,
+ get_export_url,
+ get_import_url,
+ IMPORTABLE_FILE_TYPES,
+)
__all__ = [
'import_handler', 'import_status_handler',
'export_handler', 'export_output_handler', 'export_status_handler',
@@ -70,7 +75,7 @@ def import_handler(request, course_key_string):
html: return html page for import page
json: not supported
POST or PUT
- json: import a course via the .tar.gz file specified in request.FILES
+ json: import a course via the .tar.gz or .zip file specified in request.FILES
"""
courselike_key = CourseKey.from_string(course_key_string)
library = isinstance(courselike_key, LibraryLocator)
@@ -122,7 +127,7 @@ def _write_chunk(request, courselike_key): # lint-amnesty, pylint: disable=too-
"""
Write the OLX file data chunk from the given request to the local filesystem.
"""
- # Upload .tar.gz to local filesystem for one-server installations not using S3 or Swift
+ # Upload .tar.gz or .zip to local filesystem for one-server installations not using S3 or Swift
data_root = path(settings.GITHUB_REPO_ROOT)
subdir = base64.urlsafe_b64encode(repr(courselike_key).encode('utf-8')).decode('utf-8')
course_dir = data_root / subdir
@@ -140,8 +145,8 @@ def error_response(message, status, stage):
# Use sessions to keep info about import progress
_save_request_status(request, courselike_string, 0)
- if not filename.endswith('.tar.gz'):
- error_message = _('We only support uploading a .tar.gz file.')
+ if not filename.endswith(IMPORTABLE_FILE_TYPES):
+ error_message = _('We support uploading files in one of the following formats: {IMPORTABLE_FILE_TYPES}')
_save_request_status(request, courselike_string, -1)
monitor_import_failure(courselike_key, current_step, message=error_message)
return error_response(error_message, 415, 0)
diff --git a/cms/djangoapps/contentstore/views/tests/test_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py
index d01f6800c105..fc2a7cb53a39 100644
--- a/cms/djangoapps/contentstore/views/tests/test_import_export.py
+++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py
@@ -6,6 +6,7 @@
import json
import logging
import os
+import random
import re
import shutil
import tarfile
@@ -13,6 +14,7 @@
from io import BytesIO
from unittest.mock import Mock, patch
from uuid import uuid4
+from zipfile import ZipFile
import ddt
import lxml
@@ -37,7 +39,7 @@
from common.djangoapps.student import auth
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole
from common.djangoapps.util import milestones_helpers
-from openedx.core.lib.extract_tar import safetar_extractall
+from openedx.core.lib.extract_archives import safe_extractall
from xmodule.contentstore.django import contentstore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore import LIBRARY_ROOT, ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
@@ -183,6 +185,13 @@ def touch(name):
with tarfile.open(self.good_tar, "w:gz") as gtar:
gtar.add(good_dir)
+ self.good_zip = os.path.join(self.content_dir, "good.zip")
+ with ZipFile(self.good_zip, "w") as gzip:
+ for folder_name, subfolders, filenames in os.walk(good_dir):
+ for filename in filenames:
+ file_path = os.path.join(folder_name, filename)
+ gzip.write(file_path, file_path[len(good_dir) + 1:])
+
# Bad course (no 'course.xml' file):
bad_dir = tempfile.mkdtemp(dir=self.content_dir)
touch(os.path.join(bad_dir, "bad.xml"))
@@ -190,6 +199,13 @@ def touch(name):
with tarfile.open(self.bad_tar, "w:gz") as btar:
btar.add(bad_dir)
+ self.bad_zip = os.path.join(self.content_dir, "bad.zip")
+ with ZipFile(self.bad_zip, "w") as bzip:
+ for folder_name, subfolders, filenames in os.walk(bad_dir):
+ for filename in filenames:
+ file_path = os.path.join(folder_name, filename)
+ bzip.write(file_path, file_path[len(good_dir) + 1:])
+
self.unsafe_common_dir = path(tempfile.mkdtemp(dir=self.content_dir))
self.log_prefix = f"Course import {self.course.id}:"
@@ -203,6 +219,14 @@ def setUpClass(cls):
cls.VerifyingError = -2
cls.UpdatingError = -3
+ @property
+ def good_file(self):
+ return random.choice([self.good_tar, self.good_zip])
+
+ @property
+ def bad_file(self):
+ return random.choice([self.bad_tar, self.bad_zip])
+
def assertImportStatusResponse(self, response, status=None, expected_message=None):
"""
Fail if the import response does not match with the provided status and message.
@@ -211,53 +235,56 @@ def assertImportStatusResponse(self, response, status=None, expected_message=Non
if expected_message:
self.assertEqual(response['Message'], expected_message)
- def get_import_status(self, course_id, tarfile_path):
+ def get_import_status(self, course_id, file_path):
"""Helper method to get course import status."""
resp = self.client.get(
reverse_course_url(
'import_status_handler',
course_id,
- kwargs={'filename': os.path.split(tarfile_path)[1]}
+ kwargs={'filename': os.path.split(file_path)[1]}
)
)
return json.loads(resp.content)
- def import_tarfile_in_course(self, tarfile_path):
- """Helper method to import provided tarfile in the course."""
- with open(tarfile_path, 'rb') as gtar:
- args = {"name": tarfile_path, "course-data": [gtar]}
+ def import_file_in_course(self, file_path):
+ """Helper method to import provided file in the course."""
+ with open(file_path, 'rb') as file_data:
+ args = {"name": file_path, "course-data": [file_data]}
return self.client.post(self.url, args)
@patch(TASK_LOGGER)
def test_no_coursexml(self, mocked_log):
"""
- Check that the response for a tar.gz import without a course.xml is
+ Check that the response for a file import without a course.xml is
correct.
"""
+ bad_file = self.bad_file
error_msg = import_error.FILE_MISSING.format('course.xml')
expected_error_mesg = f'{self.log_prefix} {error_msg}'
- response = self.import_tarfile_in_course(self.bad_tar)
+ response = self.import_file_in_course(bad_file)
self.assertEqual(response.status_code, 200)
mocked_log.error.assert_called_once_with(expected_error_mesg)
# Check that `import_status` returns the appropriate stage (i.e., the
# stage at which import failed).
- resp_status = self.get_import_status(self.course.id, self.bad_tar)
+ resp_status = self.get_import_status(self.course.id, bad_file)
self.assertImportStatusResponse(resp_status, self.VerifyingError, error_msg)
def test_with_coursexml(self):
"""
- Check that the response for a tar.gz import with a course.xml is
+ Check that the response for a file import with a course.xml is
correct.
"""
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(self.good_file)
self.assertEqual(response.status_code, 200)
def test_import_in_existing_course(self):
"""
Check that course is imported successfully in existing course and users have their access roles
"""
+ good_file = self.good_file
+
# Create a non_staff user and add it to course staff only
__, nonstaff_user = self.create_non_staff_authed_user_client()
auth.add_users(self.user, CourseStaffRole(self.course.id), nonstaff_user)
@@ -267,7 +294,7 @@ def test_import_in_existing_course(self):
display_name_before_import = course.display_name
# Check that global staff user can import course
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
course = self.store.get_course(self.course.id)
@@ -283,7 +310,7 @@ def test_import_in_existing_course(self):
# Now course staff user can also successfully import course
self.client.login(username=nonstaff_user.username, password='foo')
- resp = self.import_tarfile_in_course(self.good_tar)
+ resp = self.import_file_in_course(good_file)
self.assertEqual(resp.status_code, 200)
# Now check that non_staff user has his same role
@@ -375,7 +402,7 @@ def test_unsafe_tar(self):
def try_tar(tarpath):
""" Attempt to tar an unacceptable file """
- resp = self.import_tarfile_in_course(tarpath)
+ resp = self.import_file_in_course(tarpath)
self.assertEqual(resp.status_code, 200)
resp = self.get_import_status(self.course.id, tarpath)
@@ -452,8 +479,7 @@ def test_library_import(self):
extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR)
try:
- with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar:
- safetar_extractall(tar, extract_dir)
+ safe_extractall(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz', extract_dir)
library_items = import_library_from_xml(
self.store,
self.user.id,
@@ -497,8 +523,7 @@ def test_library_import_branch_settings(self, branch_setting):
extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR)
try:
- with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar:
- safetar_extractall(tar, extract_dir)
+ safe_extractall(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz', extract_dir)
import_library_from_xml(
self.store,
self.user.id,
@@ -530,8 +555,7 @@ def test_library_import_branch_settings_again(self, branch_setting):
extract_dir_relative = path.relpath(extract_dir, settings.DATA_DIR)
try:
- with tarfile.open(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz') as tar:
- safetar_extractall(tar, extract_dir)
+ safe_extractall(path(TEST_DATA_DIR) / 'imports' / 'library.HhJfPD.tar.gz', extract_dir)
import_library_from_xml(
source_store,
self.user.id,
@@ -551,13 +575,14 @@ def test_import_failed_with_no_user_permission(self, mocked_log):
"""
Tests course import failure when user have no permission
"""
+ good_file = self.good_file
expected_error_mesg = f'{self.log_prefix} User permission denied: {self.user.username}'
with patch('cms.djangoapps.contentstore.tasks.has_course_author_access', Mock(return_value=False)):
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.error.assert_called_once_with(expected_error_mesg)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.UnpackingError, import_error.COURSE_PERMISSION_DENIED)
@patch(TASK_LOGGER)
@@ -567,42 +592,45 @@ def test_import_failed_with_unknown_user(self, mocked_log):
"""
expected_error_mesg = f'{self.log_prefix} Unknown User: {self.user.id}'
+ good_file = self.good_file
with patch('django.contrib.auth.models.User.objects.get', side_effect=User.DoesNotExist):
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.error.assert_called_once_with(expected_error_mesg)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.UnpackingError, import_error.USER_PERMISSION_DENIED)
@patch(TASK_LOGGER)
- def test_import_failed_with_unsafe_tarfile(self, mocked_log):
+ def test_import_failed_with_unsafe_file(self, mocked_log):
"""
- Tests course import failure with unsafe tar file.
+ Tests course import failure with unsafe file.
"""
- expected_error_mesg = f'{self.log_prefix} Unsafe tar file'
- with patch('cms.djangoapps.contentstore.tasks.safetar_extractall', side_effect=SuspiciousOperation):
- response = self.import_tarfile_in_course(self.good_tar)
+ good_file = self.good_file
+ expected_error_mesg = f'{self.log_prefix} Unsafe archive file'
+ with patch('cms.djangoapps.contentstore.tasks.safe_extractall', side_effect=SuspiciousOperation):
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.error.assert_called_once_with(expected_error_mesg)
- status_response = self.get_import_status(self.course.id, self.good_tar)
- self.assertImportStatusResponse(status_response, self.UnpackingError, import_error.UNSAFE_TAR_FILE)
+ status_response = self.get_import_status(self.course.id, good_file)
+ self.assertImportStatusResponse(status_response, self.UnpackingError, import_error.UNSAFE_ARCHIVE_FILE)
@patch(TASK_LOGGER)
def test_import_failed_with_unknown_unpacking_error(self, mocked_log):
"""
Tests that course import failure for unknown error while unpacking
"""
+ good_file = self.good_file
expected_error_mesg = f'{self.log_prefix} Unknown error while unpacking'
with patch.object(course_import_export_storage, 'open', side_effect=Exception):
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.exception.assert_called_once_with(expected_error_mesg, exc_info=True)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.UnpackingError, import_error.UNKNOWN_ERROR_IN_UNPACKING)
@patch(TASK_LOGGER)
@@ -613,6 +641,7 @@ def test_import_failed_with_olx_validations(self, mocked_report, mocked_summary,
"""
Tests that course import failure for unknown error while unpacking
"""
+ good_file = self.good_file
errors = [Mock(description='DuplicateURLNameError', level_val=3)]
mocked_summary.return_value = [f'ERROR {error.description} found in content' for error in errors]
mocked_report.return_value = [f'Errors: {len(errors)}']
@@ -621,12 +650,12 @@ def test_import_failed_with_olx_validations(self, mocked_report, mocked_summary,
]
expected_error_mesg = f'{self.log_prefix} CourseOlx validation failed.'
with patch.dict(settings.FEATURES, ENABLE_COURSE_OLX_VALIDATION=True):
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.error.assert_called_once_with(expected_error_mesg)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.VerifyingError, import_error.OLX_VALIDATION_FAILED)
@patch(TASK_LOGGER)
@@ -645,13 +674,14 @@ def test_import_failure_is_descriptive_for_known_failures(self, exc, expected_me
"""
Test that when course import fails with a known failure, user get a descriptive error message.
"""
+ good_file = self.good_file
mocked_import.side_effect = exc
expected_exception_messages = f"{self.log_prefix} Error while importing course: {str(exc)}"
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.exception.assert_called_once_with(expected_exception_messages)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.UpdatingError, expected_mesg)
@patch(TASK_LOGGER)
@@ -665,13 +695,14 @@ def test_import_failure_for_unknown_failures(self, exception, mocked_import, moc
"""
Test that import status and logged exception when course import fails with an unknown failure.
"""
+ good_file = self.good_file
mocked_import.side_effect = exception
expected_exc_mesg = f"{self.log_prefix} Error while importing course: {str(exception)}"
- response = self.import_tarfile_in_course(self.good_tar)
+ response = self.import_file_in_course(good_file)
self.assertEqual(response.status_code, 200)
mocked_log.exception.assert_called_once_with(expected_exc_mesg)
- status_response = self.get_import_status(self.course.id, self.good_tar)
+ status_response = self.get_import_status(self.course.id, good_file)
self.assertImportStatusResponse(status_response, self.UpdatingError, import_error.UNKNOWN_ERROR_IN_IMPORT)
def test_import_status_response_is_not_cached(self):
@@ -680,7 +711,7 @@ def test_import_status_response_is_not_cached(self):
reverse_course_url(
'import_status_handler',
self.course.id,
- kwargs={'filename': os.path.split(self.good_tar)[1]}
+ kwargs={'filename': os.path.split(self.good_file)[1]}
)
)
self.assertEqual(resp.headers['Cache-Control'], 'no-cache, no-store, must-revalidate')
diff --git a/cms/static/js/features/import/factories/import.js b/cms/static/js/features/import/factories/import.js
index 5ec2ff09415f..d9fe7b06fab4 100644
--- a/cms/static/js/features/import/factories/import.js
+++ b/cms/static/js/features/import/factories/import.js
@@ -9,6 +9,8 @@ define([
], function(domReady, Import, $, gettext) {
'use strict';
+ const IMPORTABLE_FILE_TYPES = /\.tar\.gz$|\.zip$/;
+
return {
Import: function(feedbackUrl, library) {
var dbError,
@@ -35,7 +37,7 @@ define([
var filepath = $(this).val(),
msg;
- if (filepath.substr(filepath.length - 6, 6) === 'tar.gz') {
+ if (IMPORTABLE_FILE_TYPES.test(filepath)) {
$('.error-block').hide();
$('.file-name').text($(this).val().replace('C:\\fakepath\\', ''));
$('.file-name-block').show();
@@ -44,7 +46,7 @@ define([
$('.progress').show();
} else {
msg = gettext('File format not supported. Please upload a file with a {ext} extension.')
- .replace('{ext}', 'tar.gz
');
+ .replace('{ext}', 'tar.gz or zip
');
$('.error-block').text(msg).show();
}
@@ -84,7 +86,7 @@ define([
file = data.files[0];
- if (file.name.match(/tar\.gz$/)) {
+ if (IMPORTABLE_FILE_TYPES.test(file.name)) {
$submitBtn.click(function(event) {
event.preventDefault();
diff --git a/cms/templates/import.html b/cms/templates/import.html
index 5791fb2f1cd9..eb127eaccb2b 100644
--- a/cms/templates/import.html
+++ b/cms/templates/import.html
@@ -45,13 +45,14 @@