-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: migrates SupersetXBlock #7
Conversation
* use StudioEditableXBlockMixin * removes sensitive Superset fields from XBlock -- use SUPERSET_CONFIG * SUPERSET_CONFIG: distinguished between service_url and internal_service_url * lets XBlock default to the instructor dashboard UUID, if configured * extracts translated text * denies access to Superset for non course staff * removes unused code and files * adds tests
Thanks for the pull request, @pomegranited! 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. |
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.
Thanks for this contribution! Looking great, just a couple of comments and a problem with a dependency
and fix formatting
Couldn't find an acceptable indentation level for this comment, so just removed it.
9572ef1
to
a82d845
Compare
Thank you for your review @Ian2012 ! If you're happy with my changes, feel free to merge & tag, and I'll update openedx/tutor-contrib-aspects#642 (comment) |
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.
Nice, I'm excited to get this in! :)
Because the plugin's template is a mako template, also had to configure i18n_tools to extract translations from mako templates.
b8a34ac
to
4e40f21
Compare
Hi @Ian2012 , I'm not sure why you're not seeing the XBlock in your course? Could you check that the installed version of platform-plugin-aspects matches this branch? FYI I just pushed another commit to this branch. With this change, you should see a "Go to Superset" link on the Instructor Dashboard plugin. |
@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
@Ian2012 Thank you! |
Description
Migrates the SupersetXBlock code from https://github.com/eduNEXT/platform-plugin-superset
and
internal_service_url
Also adds a "Go to Superset" link to the Instructor Dashboard, to allow users to navigate directly to Superset.
Testing instructions
See openedx/tutor-contrib-aspects#642 for manual testing instructions.
Screenshots
XBlock edit screen:
XBlock staff/instructor view:
XBlock learner view:
Instructor Dashboard "Go to Superset" link:
Merge checklist:
Check off if complete or not applicable: