feat: Source specifying for relative links is allowed #232
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @myhailo-chernyshov-rg! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently unmaintained. To get help with finding a technical reviewer, tag the community contributions project manager for this PR in a comment and let them know that your changes are ready for review:
Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
Hi @myhailo-chernyshov-rg! Thanks for this contribution! It looks like you're contributing on behalf of Raccoon Gang - please have your manager reach out to [email protected] to have you added to Raccoon Gang's existing entity agreement with us. Thank you! |
mphilbrick211
added
the
needs test run
| @@ -250,6 +252,18 @@ def process_external_tools_link(item, html): | |||
| html = html.replace(item, external_tool_url) | |||
| return html | |||
|
|
|||
| def process_extra_static_files(item, html): | |||
There was a problem hiding this comment.
Please add a docstring here explaining what this function is for.
| self.cartridge = cartridge | ||
| self.doc = None | ||
| self.link_file = link_file | ||
| self.passport_file = passport_file | ||
| self.relative_links_source = relative_links_source |
There was a problem hiding this comment.
Please add some validation for the format of relative_links_source.
There was a problem hiding this comment.
Validation is added on the argument parsing step.
| if self.relative_links_source is None: | ||
| return html | ||
|
|
||
| for static_file in self.cartridge.extra_static_files: |
There was a problem hiding this comment.
How large can extra_static_files get? This looks like it could get pretty expensive to loop through every entity in this list for each item.
There was a problem hiding this comment.
During the resource content collection, when the external file is found, it's added to this list. So, the list size isn't limited and depends on CC archive content. But we need to process each such link to make it absolute, so we do need to iterate over the list. I don't think it's too expensive because other exporter tasks require much more actions to perform. Do you have any ideas how we can handle all the links without iterating over them?
mphilbrick211
removed
the
needs test run
Description
It is possible that the
.imscccourse dump contains files with relative links to resources for some reason not included into it (e.g. some static files of LMS that are not processed by its exporting tool). It can become a reason of broken images, not loaded styles etc.To handle such situations, it is allowed to specify the relative links source during converting.Usage instructions
Specify relative links source using
-sflag:Deadline
"None"