Support Zip files for Course Import#33552
Conversation
|
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:
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. |
e5a9a61 to
e9c5ec5
Compare
|
Hi @rodfersou! Thank you for this contribution! Please let me know if you have any questions regarding submitting a CLA form. |
@mphilbrick211 done ✅ |
xitij2000
left a comment
There was a problem hiding this comment.
I've just gone through the code, haven't tested it yet, will do next.
cms/djangoapps/contentstore/utils.py
Outdated
There was a problem hiding this comment.
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.
cms/djangoapps/contentstore/utils.py
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
how about:
| IMPORT_EXTENSIONS = ('.tar.gz', '.zip') | |
| IMPORTABLE_FILE_TYPES = ('.tar.gz', '.zip') |
There was a problem hiding this comment.
Perhaps reword this to error message to also use the variable? i.e.
'We support uploading files in one of the following formats: {extensions}'
cms/templates/import.html
Outdated
There was a problem hiding this comment.
This should also mention zip.
There was a problem hiding this comment.
I probably missed this part of the work; How can I test the export / import of a library?
openedx/core/lib/extract_zip.py
Outdated
There was a problem hiding this comment.
Remove this and other lint supressions.
e9c5ec5 to
7770e51
Compare
|
@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. |
1 similar comment
|
@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. |
|
@xitij2000 I'm sorry, push the wrong button on github and close the PR when sync master.. let me open a new PR |
|
superseded to #33561 |
Description
Support
zipfiles for Course import.Please see more details at this issue: openedx/platform-roadmap#285
Useful information to include:
=> "Course Author".
=> Before
=> 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
Tools / ExportExport Course Content.tar.gzfile in a new folder at your computer.zipfileTools / ImportChoose new file.zipfileReplace my course with the selected fileDeadline
=> 2023-10-29
Other information
Include anything else that will help reviewers and consumers understand the change.
=> No
=> No
=> No