Skip to content

Commit

Permalink
feat: add filter to hook render of XBlock
Browse files Browse the repository at this point in the history
  • Loading branch information
nsprenkle committed May 24, 2024
1 parent 362899e commit 28e1601
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions lms/djangoapps/courseware/views/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@
from django.views.generic import View
from edx_django_utils.monitoring import set_custom_attribute, set_custom_attributes_for_course_key
from ipware.ip import get_client_ip
from lms.djangoapps.static_template_view.views import render_500
from markupsafe import escape
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey, UsageKey
from openedx_filters.learning.filters import CourseAboutRenderStarted
from openedx_filters.learning.filters import CourseAboutRenderStarted, RenderXBlockStarted
from requests.exceptions import ConnectionError, Timeout # pylint: disable=redefined-builtin
from pytz import UTC
from rest_framework import status
Expand Down Expand Up @@ -1669,7 +1670,18 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_sta

**optimization_flags,
}
return render_to_response('courseware/courseware-chromeless.html', context)

try:
# .. filter_implemented_name: RenderXBlockStarted
# .. filter_type: org.openedx.learning.xblock.render.started.v1
context, student_view_context = RenderXBlockStarted.run_filter(
context=context, student_view_context=student_view_context
)
except RenderXBlockStarted.PreventXBlockBlockRender as exc:
log.info("Skipping rendering block %s. Reason: %s", usage_key_string, exc.message)
return render_500()

This comment has been minimized.

Copy link
@felipemontoya

felipemontoya May 31, 2024

Member

I think you should pipe something in the exception to the rendering. As a consumer of this filter you might choose not to use it, but it is customary to allow it.

This comment has been minimized.

Copy link
@nsprenkle

nsprenkle Jun 3, 2024

Author Contributor

@felipemontoya , do you have a suggestion of how to do that here? The implementation is different enough and we don't have a clear failure template so I'm not sure of a good way to implement that.

This comment has been minimized.

Copy link
@felipemontoya

felipemontoya Jun 4, 2024

Member

I think the easiest would be to add a RenderCustomResponse exception such as https://github.com/openedx/openedx-filters/blob/16de16c27a4956211cb21f27a3828e265de1be74/openedx_filters/learning/filters.py#L271.

Then at the platform you can pass the response from the exception to the view.
Such as


return render_to_response('courseware/courseware-chromeless.html', context, request=request)


def get_optimization_flags_for_content(block, fragment):
Expand Down

2 comments on commit 28e1601

@felipemontoya
Copy link
Member

Choose a reason for hiding this comment

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

Looking at

child, child_block_context = VerticalBlockChildRenderStarted.run_filter(
vs
_, fragment, context, view = VerticalBlockRenderCompleted.run_filter(
, to maintain consistency you would want to call this RenderXBlockCompleted or move the block before the fragment is rendered as to be able to affect the student_view_context before is passed to fragment renderization.

@nsprenkle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at

child, child_block_context = VerticalBlockChildRenderStarted.run_filter(

vs

_, fragment, context, view = VerticalBlockRenderCompleted.run_filter(

, to maintain consistency you would want to call this RenderXBlockCompleted or move the block before the fragment is rendered as to be able to affect the student_view_context before is passed to fragment renderization.

Per your suggestion, refactored to move to before rendering of fragment.

Please sign in to comment.