-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make copied tags editable again after breaking the upstream link to library content [FC-0076] #36228
Make copied tags editable again after breaking the upstream link to library content [FC-0076] #36228
Conversation
Thanks for the pull request, @pomegranited! 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. 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pomegranited Works as expected 💯 Just left a suggestion about using the event to handle the tags, let me know if you disagree.
7e57960
to
4cba25c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pomegranited Thanks! Looks good.
👍
- I tested this: (Followed test instructions)
- I read through the code
- I checked for accessibility issues
- Includes documentation
# If we delete the upstream library block... | ||
library_api.delete_library_block(self.lib_block_key) | ||
|
||
# ...the copied tags remain, but should no longer be marked as "copied" | ||
object_tags = tagging_api.get_object_tags(new_block_key) | ||
assert len(object_tags) == len(self.lib_block_tags) | ||
for object_tag in object_tags: | ||
assert object_tag.value in self.lib_block_tags | ||
assert not object_tag.is_copied | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we depend on openedx-events for this to work, it would be safe to explicitly enable the required events to make sure that the test passes (had an issue before with a test case that failed only in CI but passed locally, most probably due to some events being disabled in some previous test in CI).
You can inherit from OpenEdxEventsTestMixin
(Note that it should be added before ModuleStoreTestCase
) and enable below events.
class ClipboardPasteFromV2LibraryTestCase(OpenEdxEventsTestMixin, ModuleStoreTestCase):
"""
Test Clipboard Paste functionality with a "new" (as of Sumac) library
"""
ENABLED_OPENEDX_EVENTS = [
LIBRARY_BLOCK_DELETED.event_type,
XBLOCK_UPDATED.event_type,
]
3f9a78c
to
240c5b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍 I will merge it tomorrow morning
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
…ibrary content [FC-0076] (openedx#36228) When deleting an upstream library block, ensure that any tags that may have been copied to downstream blocks are made editable again. This is achieved by un-setting the `is_copied` flag on the downstream tags.
Description
When deleting an upstream library block, ensure that any tags that may have been copied to downstream blocks are made editable again. This is achieved by un-setting the
is_copied
flag on the downstream tags.This change affects Content Authors using linked library content.
Supporting information
Part of: openedx/modular-learning#244
Based on: #36111 -- compare changes
Depends on: openedx/openedx-learning#276
Private-ref: FAL-4008
Testing instructions
Note: There's a bug in the Authoring MFE's Course Unit page that prevents the tag drawer from opening. So ensure you have disabled the
contentstore.new_studio_mfe.use_new_unit_page
waffle flag to use the legacy Studio unit interface during testing.Should see the tags copied from the library block on the new course block, and they should not be deleteable.
These tags should be deletable.
All the tags should remain, and all are deletable now.
Deadline
None