-
Notifications
You must be signed in to change notification settings - Fork 84
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
fix: CCX LTI configuration compatibility #391
fix: CCX LTI configuration compatibility #391
Conversation
Co-authored-by: Squirrel18 <[email protected]> Co-authored-by: alexjmpb <[email protected]>
Thanks for the pull request, @kuipumu! 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:
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. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #391 +/- ##
==========================================
+ Coverage 97.89% 97.91% +0.02%
==========================================
Files 77 77
Lines 6399 6475 +76
==========================================
+ Hits 6264 6340 +76
Misses 135 135
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Hi @kuipumu! Is this ready for review/merge? Thanks! |
@mphilbrick211 Yes, this is ready for review/merge |
Hi @chennighan2u and @openedx/masters-devs-cosmonauts! Could someone please review and merge this for us? Thanks! |
Hi @kuipumu! Some branch conflicts have popped up while waiting for review - would you mind taking a look? |
Hi @mphilbrick211, I added a commit to resolve the branch conflicts |
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.
This looks good to me 👍
@kuipumu before we merge can you add a changelog entry and update init.py with the new version?
@zacharis278 Great!, I just added a commit to bump the version an update the changelog |
Hi @kuipumu! Looks like some branch conflicts have popped up. Would you mind taking a look? Thanks! |
Hi @mphilbrick211, I pushed a commit to resolve the conflicts |
@kuipumu 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
This PR adds compatibility fix for CCXs and external reusable configurations to the LTI consumer XBlock. This also includes changes to previously added fixes for CCXs using the LTI consumer XBlock. This changes allow the LTI consumer XBlock to sync values from the main CCX LTI configuration model to all of it's children CCX LTI configurations and vice versa.
Type of Change
Testing