Add openedx-filters hook to VerticalBlock before rendering of child blocks#30773
Conversation
|
Thanks for the pull request, @tecoholic! 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. |
openedx-webhooks
added
the
open-source-contribution
tecoholic
force-pushed
the
tecoholic/add-filter-hook-to-vertical-block-rendering
branch
2 times, most recently
from
176c5f1 to
986d1dd
Compare
tecoholic
force-pushed
the
tecoholic/add-filter-hook-to-vertical-block-rendering
branch
2 times, most recently
from
f263364 to
948c311
Compare
|
@bradenmacdonald Can you kindly take a look at this when you have some time? |
|
@tecoholic Thank you for your contribution. Please let me know once this is ready for our review. |
|
@natabene This is ready for review. The Javascript test failing seems unrelated to the change. I will try to get a rerun. |
|
@tecoholic Sure, I can review this, probably next week. |
|
@bradenmacdonald That's great. Thank you. |
|
Thank you for reviewing here @bradenmacdonald ! FYI the OC task is BB-6457. |
|
@tecoholic The code looks good and seems consistent with some of the other hooks. Is there a sandbox where it can be tested? I also left some comments on open-craft/openedx-edit-links#1 - I think the comment about wrapping get_html instead of using a private XBlock method is important; the others are optional. |
|
@bradenmacdonald Thank you for reviewing this. Wrapping the |
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
Looks great to me, thanks! Approved. You will however need to rebase it and update the "openedx-filter" dependency before this can be merged.
👍
- I tested this: on the provided sandbox at https://app.pr30773.sandbox.opencraft.hosting/
- I read through the code
- I checked for accessibility issues: n/a, backend only
- Includes documentation - that's being discussed in the filters PR
This commit adds a openedx-filters hook to the VerticalBlock XBlock before rendering of it's children. This allows Open edX plugins to customize the presentation of specific blocks based on the context.
tecoholic
force-pushed
the
tecoholic/add-filter-hook-to-vertical-block-rendering
branch
from
341fb42 to
6867d55
Compare
|
@bradenmacdonald Thank your for the review and the approval. @pomegranited I have update the PR:
Kindly give it a look and merge it if everything looks to be in place. |
|
Hi @tecoholic , thanks for the ping :) I'm not a core committer on edx-platform so I can't merge this for you. |
I can take a look. |
|
@tecoholic 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Agrendalath
deleted the
tecoholic/add-filter-hook-to-vertical-block-rendering
branch
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
7 participants
Description
Supersedes #30650
This PR introduces a new openedx-filters event
org.openedx.learning.vertical_block_child.render.started.v1hook in the platform atxmodule/vertical_block.py. This is implemented as a part of the effort to implement "Edit Link" in the course content for Open Source courses stored in OLX format.Users impacted by the change:
Supporting information
No direct openedx related ticket is available. Related tickets that add context to this change are:
Testing instructions
The change can be tested as a part of testing the openedx-edit-links plugin
Deadline
"None"
Other information
Once accepted, this should be merged only after openedx/openedx-filters#38 is merged and the requirements are updated.
Settings