Support Zip files for Course Import #33552

rodfersou · 2023-10-20T14:43:36Z

Description

Support zip files for Course import.

Please see more details at this issue: openedx/platform-roadmap#285

Useful information to include:

Which edX user roles will this change impact?
=> "Course Author".
Include screenshots for changes to the UI (ideally, both "before" and "after" screenshots, if applicable).
=> Before

=> After

- Provide links to the description of corresponding configuration changes. Remember to correctly annotate these changes.

=> None

Supporting information

Link to other information about the change, such as Jira issues, GitHub issues, or Discourse discussions.
Be sure to check they are publicly readable, or if not, repeat the information here.
https://tasks.opencraft.com/browse/BB-8025
openedx/platform-roadmap#285

Testing instructions

Select the course
1. Check out this PR
2. Open the CMS (Studio)
3. Create a course or use the example one
4. Select the course
5. In the top menu, select Tools / Export
6. Click in Export Course Content
7. Save the .tar.gz file in a new folder at your computer
Modify the course
1. Find this file in your computer
2. Extract the course
3. Modify the course content - Optional
4. Compact the course again in .zip file
5. Rename the compressed file (can have spaces in the name) - Optional
Upload changes
1. Open the CMS (Studio)
2. Select the course
3. In the top menu, select Tools / Import
4. Click in Choose new file
5. Select the created .zip file
6. Click in Replace my course with the selected file
7. Wait the import steps to finish
8. Check if changes are persisted - Optional

Deadline

=> 2023-10-29

Other information

Include anything else that will help reviewers and consumers understand the change.

Does this change depend on other changes elsewhere?
=> No
Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.
=> No
If your database migration can't be rolled back easily.
=> No

openedx-webhooks · 2023-10-20T14:43:42Z

Thanks for the pull request, @rodfersou! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 · 2023-10-20T17:43:15Z

Hi @rodfersou! Thank you for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

rodfersou · 2023-10-20T19:48:52Z

Hi @rodfersou! Thank you for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

@mphilbrick211 done ✅

xitij2000

I've just gone through the code, haven't tested it yet, will do next.

xitij2000 · 2023-10-23T09:44:44Z

cms/djangoapps/contentstore/utils.py

@@ -87,6 +87,8 @@
 from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService


+# https://stackoverflow.com/a/18351977


I don't think this comment is relevant here, it's when it's being used that it matters.
Additionally I don't think we need to comment about standard library features.

xitij2000 · 2023-10-23T09:45:58Z

cms/djangoapps/contentstore/utils.py

@@ -87,6 +87,8 @@
 from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService


+# https://stackoverflow.com/a/18351977
+IMPORT_EXTENSIONS = ('.tar.gz', '.zip')


I think this name is a bit ambiguous. Perhaps IMPORTABLE_ARCHIVE_EXTENSIONS?

I think extensions can also sometimes mean plugins etc so just want this to be clear. After all it is entirely reasonable to make the import architecture pluggable and support a plugin to expand file formats.

how about:

Suggested change

IMPORT_EXTENSIONS = ('.tar.gz', '.zip')

IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip')

xitij2000 · 2023-10-23T09:48:15Z

cms/djangoapps/contentstore/views/import_export.py

-        if not filename.endswith('.tar.gz'):
-            error_message = _('We only support uploading a .tar.gz file.')
+        if not filename.endswith(IMPORT_EXTENSIONS):
+            error_message = _('We support uploading a .tar.gz or .zip file.')


Perhaps reword this to error message to also use the variable? i.e.
'We support uploading files in one of the following formats: {extensions}'

xitij2000 · 2023-10-23T12:08:02Z

cms/templates/import.html

        %if library:
            <p>${Text(_("Be sure you want to import a library before continuing. The contents of the imported library will replace the contents of the existing library. {em_start}You cannot undo a library import{em_end}. Before you proceed, we recommend that you export the current library, so that you have a backup copy of it.")).format(em_start=HTML('<strong>'), em_end=HTML("</strong>"))}</p>
-            <p>${_("The library that you import must be in a .tar.gz file (that is, a .tar file compressed with GNU Zip). This .tar.gz file must contain a library.xml file. It may also contain other files.")}</p>
+            <p>${_("The library that you import should be in a .tar.gz file (that is, a .tar file compressed with GNU Zip). This .tar.gz file must contain a library.xml file. It may also contain other files.")}</p>


This should also mention zip.

I probably missed this part of the work; How can I test the export / import of a library?

xitij2000 · 2023-10-23T12:09:03Z

openedx/core/lib/extract_zip.py

+        raise SuspiciousOperation("Attempted to import course outside of data dir")
+
+    for finfo in members:
+        if _is_bad_path(finfo.filename, base):  # lint-amnesty, pylint: disable=no-else-raise


Remove this and other lint supressions.

openedx-webhooks · 2023-10-23T13:44:18Z

@rodfersou Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks · 2023-10-23T13:44:18Z

@rodfersou Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

rodfersou · 2023-10-23T13:46:16Z

@xitij2000 I'm sorry, push the wrong button on github and close the PR when sync master.. let me open a new PR

rodfersou · 2023-10-23T13:47:40Z

superseded to #33561

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 20, 2023

rodfersou force-pushed the feat/poc-zip-import branch from e5a9a61 to e9c5ec5 Compare October 20, 2023 15:01

mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 20, 2023

xitij2000 reviewed Oct 23, 2023

View reviewed changes

rodfersou requested a review from xitij2000 October 23, 2023 13:11

rodfersou closed this Oct 23, 2023

rodfersou force-pushed the feat/poc-zip-import branch from e9c5ec5 to 7770e51 Compare October 23, 2023 13:44

e0d removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Zip files for Course Import #33552

Support Zip files for Course Import #33552

rodfersou commented Oct 20, 2023 •

edited

Loading

openedx-webhooks commented Oct 20, 2023 •

edited

Loading

mphilbrick211 commented Oct 20, 2023

rodfersou commented Oct 20, 2023 •

edited

Loading

xitij2000 left a comment

xitij2000 Oct 23, 2023

xitij2000 Oct 23, 2023

rodfersou Oct 23, 2023

xitij2000 Oct 23, 2023

xitij2000 Oct 23, 2023

rodfersou Oct 23, 2023

xitij2000 Oct 23, 2023

openedx-webhooks commented Oct 23, 2023

openedx-webhooks commented Oct 23, 2023

rodfersou commented Oct 23, 2023

rodfersou commented Oct 23, 2023

		@@ -87,6 +87,8 @@
		from xmodule.services import SettingsService, ConfigurationService, TeamsConfigurationService


		# https://stackoverflow.com/a/18351977

	IMPORT_EXTENSIONS = ('.tar.gz', '.zip')
	IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip')

Support Zip files for Course Import #33552

Support Zip files for Course Import #33552

Conversation

rodfersou commented Oct 20, 2023 • edited Loading

Description

Supporting information

Testing instructions

Deadline

Other information

openedx-webhooks commented Oct 20, 2023 • edited Loading

mphilbrick211 commented Oct 20, 2023

rodfersou commented Oct 20, 2023 • edited Loading

xitij2000 left a comment

Choose a reason for hiding this comment

xitij2000 Oct 23, 2023

Choose a reason for hiding this comment

xitij2000 Oct 23, 2023

Choose a reason for hiding this comment

rodfersou Oct 23, 2023

Choose a reason for hiding this comment

xitij2000 Oct 23, 2023

Choose a reason for hiding this comment

xitij2000 Oct 23, 2023

Choose a reason for hiding this comment

rodfersou Oct 23, 2023

Choose a reason for hiding this comment

xitij2000 Oct 23, 2023

Choose a reason for hiding this comment

openedx-webhooks commented Oct 23, 2023

openedx-webhooks commented Oct 23, 2023

rodfersou commented Oct 23, 2023

rodfersou commented Oct 23, 2023

rodfersou commented Oct 20, 2023 •

edited

Loading

openedx-webhooks commented Oct 20, 2023 •

edited

Loading

rodfersou commented Oct 20, 2023 •

edited

Loading