-
Notifications
You must be signed in to change notification settings - Fork 17
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: add filter for hooking XBlock Render #171
feat: add filter for hooking XBlock Render #171
Conversation
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.
The filter definition seems reasonable to me. Given the location you are planning to use the filter, it looks to me more similar to a RenderXBlockCompleted
.
I left this comment at the commit you shared. openedx/edx-platform@28e1601#commitcomment-142615584
Scratch that, after thinking more about it, have moved filter location per your suggestion. |
@felipemontoya / @ormsbee , looking for re-review when you have time :) |
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.
The filter looks good, but in order to give users of this filter more range of action I suggest we allow them to render something controllable and not only a 500 error.
@felipemontoya , ready for re-review |
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.
Brilliant. Thanks a lot @nsprenkle. I think the filter is ready.
@mariajgrimaldi correct me please if I'm wrong. To merge we need to do a minor bump and add the changelog, right?
such as with https://github.com/openedx/openedx-filters/pull/158/files
@nsprenkle @felipemontoya: yes, that's all we need! Thank you both. |
Will add, thank you! |
ffadc2d
to
a0492a1
Compare
- chore: bump version to 1.9.0
a0492a1
to
2b7e510
Compare
Description: Add a filter just before rendering of an XBlock scope. See discussion forum.
Justification: Some of our current work requires an ability to read and modify data in the XBlock render pipeline. While there already exist filters in the Vertical rendering pipeline, these are limited compared to the XBlock rendering pipeline in 2 important ways.
JIRA: AU-2039 and 2037
Merge deadline: List merge deadline (if any)
Installation instructions: n/a
Testing instructions:
openedx-filters
.http://{lms}/xblock/block-v1:{block-id}?view=student_view
Reviewers:
Merge checklist:
Post merge:
finished.