Support Zip files for Course Import

[rodfersou]

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]

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.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

[xitij2000]

The code is working, however, I think some changes are needed here.

1. Please undo any removals of lint-amnesty comments etc. I was only asking you not to have those messages in new code or code that you've touched.
2. I think you mentioned earlier about refactoring the archive handling code. If you can do that in the remaining time it would be great.

[e0d]

@rodfersou there are some check failures, can you have a look?

[rodfersou]

@e0d any guidance on how to run lint tool locally?

[xitij2000]

@e0d any guidance on how to run lint tool locally?

I think just running pylint in the container should work?

If you are using tutor then: tutor dev exec lms bash
For the standard docker devstack you can run make dev.shell.lms.

Then you can run pylint in the container.

[xitij2000]

👍 This is working fine, but I think the tests could deduplicated a bit. Other than that it's good to go.

• I tested this: tested on tutor devstack
• I read through the code
• [na] I checked for accessibility issues
• Includes documentation

[xitij2000]

I've tested this again, still seems to be working fine. Just have one small suggestion.

It seems some of your commit messages aren't in the correct format. Could you fix them? Please squish this down to one commit.

[rodfersou]

It seems some of your commit messages aren't in the correct format. Could you fix them? Please squish this down to one commit.

@xitij2000 sure! I'll leave separate just the last changes, but will rebase after your review

[rodfersou]

The idea here is to let CI test each time in different way, like what we do using factory_boy and faker libraries

[Agrendalath]

@rodfersou, what's the point of this? These are four different cases, so we should run them all every time instead of randomly deciding which one will run. If one of these cases fails, we won't even know which one it was.

We should specify all cases with @ddt.data.

[xitij2000]

👍

• I tested this: teted on tutor devstack
• I read through the code
• [na] I checked for accessibility issues
• [na] Includes documentation

[xitij2000]

@rodfersou The tests seem to be failing, they probably just need to be rerun. Could you push an empty commit to trigger them?

[Agrendalath]

@rodfersou, members and archive will be referenced before the assignment if we pass a file with a different extension. A lib function should not rely on external validation of importable file extensions. If a use case is unsupported, it should raise a suitable exception that can be handled "above" (in the code that invokes it). Getting a NameError is confusing.

[mphilbrick211]

Hi @rodfersou - just checking in on this!

[Agrendalath]

@mphilbrick211, I pinged people on our internal ticket to reassign this.

[Agrendalath]

@mphilbrick211, we should be able to move this feature forward in a few weeks. However, we will need to recreate the PR from a different fork, so I'm closing this.

[openedx-webhooks]

@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.