feat: support component static files and update create_next_version f…#401
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @dwong2708! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are 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. DetailsWhere 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 @ormsbee , would you mind taking a look at the create_next_version changes? I’d like to confirm if you’re good with this approach or if you think it should be adjusted. If it looks fine, I’ll go ahead and add the tests. |
|
@dwong2708: This PR is still marked as being in Draft status. Is it ready for review? |
Ready to review now @ormsbee . Thank you. |
mphilbrick211
added
the
mao-onboarding
| created_by: int | None, | ||
| container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] | ||
| entities_action: ChildrenEntitiesAction = ChildrenEntitiesAction.REPLACE, | ||
| force_version_num: int | None = None, |
There was a problem hiding this comment.
Please add an entry for this in the Args: part of the docstring, and explain why someone would use it. Please do the same for all other functions where this has been added as an optional arg.
There was a problem hiding this comment.
Changes applied. Thank you.
| if not num_version: | ||
| return resolved_files |
There was a problem hiding this comment.
Please add a comment that explains the reasoning for this.
There was a problem hiding this comment.
it is no longer needed since it was already validated in the serializers. I adjusted it. Thanks
| content_to_replace = self._resolve_static_files(valid_draft, entity_key, component_static_files) | ||
| components_api.create_next_component_version( | ||
| self.components_map_by_key[entity_key].publishable_entity.id, | ||
| content_to_replace=content_to_replace, |
There was a problem hiding this comment.
Will this do the right thing if I have a file in a published version and that file is not there in the draft version? The content_to_replace param is basically a diff, so if we'd need to do something like:
content_to_replace_for_published = {
'static/profile.webp': Content(),
'static/background.webp': Content(),
}
# after we create the published version data, make the replacement
# dict remove all the static assets
content_to_replace_for_draft = {
key: None for key in content_to_replace_for_published
}
# now selectively re-add the stuff that appears in the draft version
content_to_replace_for_draft['static/new_image.web'] = Content()Actually, now that I think on it, maybe we could make an optional "ignore_previous_content" param to create_next_component_version? So if ignore_previous_content=True, we could skip this whole section:
There was a problem hiding this comment.
Wonderful. ignore_previous_content makes the implementation easier to handle this case. Thank you
| ) | ||
|
|
||
| def test_create_next_version_forcing_num_version(self): | ||
| """Test creating a next version with a forced version number.""" |
There was a problem hiding this comment.
Please expand this test to include adding content to it and having different content between the two versions that are created.
…ion and update docstrings
4 participants
Resolves: #384 and #400
Changes
create_next_version_*APIs to accept a new parameterforce_version_num, which keeps the provided version number instead of auto-incrementing.Input:
test2.zip
Output:
Component versions
HTML component
HTML component - v4 and static files