Skip to content

[BB-6206] Introduce a Filter to hook into courseware XBlock rendering - #35

Closed
tecoholic wants to merge 2 commits into
openedx:mainfrom
open-craft:tecoholic/BB-6206-edit-links-poc
Closed

[BB-6206] Introduce a Filter to hook into courseware XBlock rendering#35
tecoholic wants to merge 2 commits into
openedx:mainfrom
open-craft:tecoholic/BB-6206-edit-links-poc

Conversation

@tecoholic

@tecoholic tecoholic commented Jun 24, 2022

Copy link
Copy Markdown
Contributor

Description:

The PR introduces a new filter named XBlockRenderStarted that allows apps to modify the output of the XBlock using pipelines. The corresponding change in the platform is introduced in openedx/openedx-platform#30650

JIRA:

OpenCraft JIRA - https://tasks.opencraft.com/browse/BB-6206

Dependencies:

Merge deadline: List merge deadline (if any)

Installation instructions:
Nothing specific

Testing instructions:

The example application using this Hook is implemented in openedx-edit-link and its PR contains the testing instructions.

Reviewers:

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPI after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns:
None

@mariajgrimaldi

Copy link
Copy Markdown
Member

Thanks for your contribution! I'll be reviewing this as soon as possible :)

@mariajgrimaldi mariajgrimaldi self-assigned this Jun 24, 2022
@mariajgrimaldi

Copy link
Copy Markdown
Member

Hello! @tecoholic can you take a look into the failing tests?

@mariajgrimaldi
mariajgrimaldi self-requested a review July 1, 2022 14:31
@tecoholic

Copy link
Copy Markdown
Contributor Author

@mariajgrimaldi Sure thing :) I will get them fixed and let you know.

message: error message for the exception.
response: custom response which will be returned by the render_xblock view.
"""
super().__init__( message, response=response)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know these kinds of warnings are cumbersome, but to mitigate them you can follow these tests examples

view
"""
data = super().run_pipeline(block=block, context=context, template_name=template_name)
return data.get("context"), data.get("template_name")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And here you can follow this one

@tecoholic

Copy link
Copy Markdown
Contributor Author

I am replacing this PR with #38 based on the feedback on the PoC.

@mariajgrimaldi Thank you for your time and patience. I will ping you on the other PR once all the details are filled in. This time I have the proper docs and tests :)

@tecoholic tecoholic closed this Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants