feat: adds VerticalBlockRenderCompleted filter hook

[tecoholic]

Description

This introduces the VerticalBlockRenderCompleted filter that will be called after a VerticalBlock's rendering is completed. This filter is passed the instance of the VerticalBlock, web fragment that has been created after the rendering, the context dictionary and the view name.

The applications implementing the filter can modify the content that will be presented to the learner based on its own parameters. Some example scenarios:

1. Add extra HTML to ask for feedback about the unit's content
2. Embed an external service using IFrame
3. Replace the VerticalBlock completely for specific users based on some criteria...etc.,

Which edX user roles will this change impact?

• Learner

Configuration

The filter can be configured as given below

OPEN_EDX_FILTERS_CONFIG = {
    "org.openedx.learning.vertical_block.render.completed.v1": {
        "fail_silently": False,
        "pipeline": [
            "my-package.pipeline.ActionStep"
        ]
    }
}

Supporting information

This relies on the VerticalBlockRenderCompleted introduced in this PR - openedx/openedx-filters#52 - and should be merged after the openedx-filters package is released.

Testing instructions

The testing instructions assuming you have working devstack setup.

1. Pull the PR branch
2. Install the dependencies

   make lms-shell
   # To install openedx-events branch with the VerticalBlockRenderCompleted filter implemented
   pip install -r requirements/edx/development.txt
   # A simple openedx plugin for testing and development
   pip install git+https://github.com/open-craft/openedx-filter-demos.git 

3. Edit/Add the file lms/envs/private.py and include the following contents

   OPEN_EDX_FILTERS_CONFIG = {
       "org.openedx.learning.vertical_block.render.completed.v1": {
           "fail_silently": False,
           "pipeline": [
               "openedx_filter_demos.pipeline.AddDebugInfoBlock",
               "openedx_filter_demos.pipeline.AddAJAXButtonBlock" # OPTIONAL - to test JSON Handler of XBlock
           ]
       }
   }

4. OPTIONAL - Add the following dummy handler to class VerticalBlock to test the AJAX Calls.

   diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py
   index a6d79a543e..a0542fd73a 100644
   --- a/xmodule/vertical_block.py
   +++ b/xmodule/vertical_block.py
   @@ -72,6 +72,15 @@ class VerticalBlock(
   
        show_in_read_only_mode = True
   
   +    @XBlock.json_handler
   +    def dummy_json(self, data, suffix=""):
   +        """
   +        Dummy JSON Handler to test the veritical block render filter
   +        """
   +        print("[DUMMY JSON Handler]")
   +        print(f"Data passed to the endpoint: {data}")
   +        return {"status": "success"}
   +

5. Restart the LMS make lms-restart

Now when any unit is opened in the LMS, the bottom should have 2 red boxeslike the one below (only 1 if the AJAX Button Block was skipped). This is injected by the pipeline steps of the the openedx_filter_demos via the VerticalBlockRenderCompleted filter.

Corresponding logs like the example below can also be see in the LMS logs make lms-logs.

pipeline.py:43 - =========== AddDebugInfoBlock Pipeline Step Start ==========
pipeline.py:44 - Fragment: <web_fragments.fragment.Fragment object at 0x7ff3f01d22b0>
pipeline.py:45 - Context: {'show_title': False, 'show_bookmark_button': False, 'recheck_access': '1', 'view': 'student_view', 'hide_access_error_blocks': True, 'is_mobile_app': False}
pipeline.py:46 - View: student_view
pipeline.py:47 - =========== AddDebugInfoBlock Pipeline Step Stop ==========

OPTIONAL - If you have added the AJAX Button block in config and added the dummy_json handler, then clicking the button should populate the Response block with {"status": "success"}.

Deadline

"None" if there's no rush, or provide a specific date or event (and reason) if there is one.

Other information

Similar prior work: #30773

[openedx-webhooks]

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:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

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.

[navinkarkera]

@tecoholic Looks good! 👍

• I tested this: Tested the filter as described as well in combination of mixin implemented in https://github.com/open-craft/xblock-skill-tagging/pull/1
• I read through the code
• I checked for accessibility issues
• Includes documentation
• I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

[mphilbrick211]

Hi @tecoholic - just checking to see if you are planning to pursue this pull request? Looks like some tests still need to be run.

CC: @navinkarkera

[navinkarkera]

Adding this for reference of an usage of this filter: https://github.com/open-craft/xblock-skill-tagging/pull/3

[tecoholic]

@mphilbrick211 Hi, yes. I am still very much pursuing this PR :) I think the CI data has become outdated, and as is the PR w.r.t changes on master. Let me rebase it and get the CI run again so you can review it.

[mphilbrick211]

Hi @mariajgrimaldi and @ormsbee - Ed suggested it might be good for you both to review this. Thanks!

[ormsbee]

I don't usually review filters, but this seems reasonable to me. The one thing I will say is that in the longer term, we'll probably want render hooks that are independent of the XBlock API–e.g. UnitRender, ComponentRender, etc. But the infrastructure for that probably won't be around until Palm or the Q-release, and it looks like there are immediate use cases around accessing VerticalBlock fields today, so I think it's fine to go ahead with this filter hook.

[ormsbee]

@mariajgrimaldi: Do you have time to do this review? I'm always skittish of doing the reviews for filters, since I'm not the target audience and I almost never write these kinds of integrations.

[mariajgrimaldi]

Hi @ormsbee! Sure. I'll do it as soon as I can.

[mariajgrimaldi]

I'll be reviewing this PR once the changes in the dependent PR -in openedx-filters- are addressed 😄

[tecoholic]

@mariajgrimaldi Based on your feedback on the openedx-filters PR openedx/openedx-filters#52, I have updated the hooks in the vertical block to use try...except blocks and have added unittests for them as well. Can you kindly review this?

cc: @navinkarkera

[tecoholic]

These links will be updated once openedx/openedx-filters#52 is merged.

[mariajgrimaldi]

this looks great! Can you include the PR where we included VerticalBlockChildRenderStarted, I'd like to test that as well with the new changes. It'd be wonderful If you have test cases for that as well! Thansk

[tecoholic]

@mariajgrimaldi Definitely. You should find the full details about the filter and test instructions in #30773. I have also updated the requirements files in this PR to point to openedx-filters version 1.1.0.

[mariajgrimaldi]

This looks great! Thank you again :)

[openedx-webhooks]

@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.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.