Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: overwrite previous tags on import [FC-0036] #33752

Merged
merged 4 commits into from
Nov 27, 2023
Merged

fix: overwrite previous tags on import [FC-0036] #33752

merged 4 commits into from
Nov 27, 2023

Conversation

rpenido
Copy link
Contributor

@rpenido rpenido commented Nov 20, 2023 •
edited
Loading

Description

Updates the openedx-learning library to change the import endpoint to overwrite previous tags from the target taxonomy.

Supporting Information

Testing instructions

  • Please ensure that the tests cover the expected behavior of the view

Before merge

Private-ref: FAL-3536

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 20, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @rpenido! 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.

Please let us know once your PR is ready for our review and all tests are green.

@open-craft-grove
Copy link

Sandbox deploy request received. Deployment will start soon.

@open-craft-grove
Copy link

Sandbox deployment started.

@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Nov 20, 2023
@open-craft-grove
Copy link

Sandbox deployment successful.

Sandbox LMS is available at pr-33752-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33752-139931.staging.do.opencraft.hosting

pomegranited
pomegranited approved these changes Nov 21, 2023
View reviewed changes
Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

  • I tested this using my devstack's REST API
  • I read through the code
  • I checked for accessibility issues N/A
  • Includes documentation N/A
  • User-facing strings are extracted for translation N/A

@pomegranited
Copy link
Contributor

@rpenido Could you put FC-0036 in these PR titles? Thanks :)

@rpenido rpenido changed the title fix: overwrite previous tags on import fix: overwrite previous tags on import [FC-0036] Nov 21, 2023
pomegranited
pomegranited reviewed Nov 21, 2023
View reviewed changes
requirements/edx/base.txt Outdated
@@ -803,7 +803,7 @@ openedx-filters==1.6.0
# via
# -r requirements/edx/kernel.in
# lti-consumer-xblock
openedx-learning==0.3.4
openedx-learning @ git+https://github.com/open-craft/openedx-learning.git@rpenido/fal-3536-overwrite-tags-on-import
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openedx-learning==0.3.5 is ready now: https://pypi.org/project/openedx-learning/0.3.5/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 0d9b82e!

@rpenido rpenido force-pushed the rpenido/fal-3536-overwrite-tags-on-import branch from c9a3f73 to b9be898 Compare November 21, 2023 02:32
@rpenido
Copy link
Contributor Author

rpenido commented Nov 21, 2023

@rpenido Could you put FC-0036 in these PR titles? Thanks :)

This is for all PRs (in all repositories) related to Step 3?

@open-craft-grove
Copy link

Sandbox update request received. Deployment will start soon.

@open-craft-grove
Copy link

Sandbox deployment started.

@open-craft-grove
Copy link

Sandbox deployment successful.

Sandbox LMS is available at pr-33752-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33752-139931.staging.do.opencraft.hosting

@open-craft-grove
Copy link

Sandbox update request received. Deployment will start soon.

@open-craft-grove
Copy link

Sandbox deployment started.

@open-craft-grove
Copy link

Sandbox deployment successful.

Sandbox LMS is available at pr-33752-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33752-139931.staging.do.opencraft.hosting

@pomegranited
Copy link
Contributor

@rpenido Could you put FC-0036 in these PR titles? Thanks :)
This is for all PRs (in all repositories) related to Step 3?

Yep!

@open-craft-grove
Copy link

Sandbox update request received. Deployment will start soon.

@open-craft-grove
Copy link

Sandbox deployment started.

@open-craft-grove
Copy link

Sandbox deployment successful.

Sandbox LMS is available at pr-33752-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33752-139931.staging.do.opencraft.hosting

@rpenido rpenido mentioned this pull request Nov 22, 2023
Closed
bradenmacdonald
bradenmacdonald approved these changes Nov 22, 2023
View reviewed changes
Copy link
Contributor

@bradenmacdonald bradenmacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I won't be able to merge this until Monday due to the Thanksgiving holiday.

@rpenido rpenido mentioned this pull request Nov 24, 2023
Merged
@bradenmacdonald bradenmacdonald merged commit 6d4463d into openedx:master Nov 27, 2023
63 checks passed
@bradenmacdonald bradenmacdonald deleted the rpenido/fal-3536-overwrite-tags-on-import branch November 27, 2023 19:43
@openedx-webhooks
Copy link

@rpenido 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@open-craft-grove
Copy link

Sandbox destroy request received.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pomegranited pomegranited pomegranited approved these changes

@bradenmacdonald bradenmacdonald bradenmacdonald approved these changes

Assignees
No one assigned
Labels
FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rpenido @openedx-webhooks @open-craft-grove @pomegranited @edx-pipeline-bot @bradenmacdonald @mphilbrick211