fix: refactor MakoService to allow specifying template more explicitly (Take 2) (openedx#33077)

* fix: refactor MakoService to allow specifying namespace per template (openedx#33061)

* fix: instr. dashboard broken by bulk email reusing HtmlBlock studio_view

* fix: lint issue from unused import

Author: bradenmacdonald

cms/djangoapps/contentstore/views/tests/test_preview.py

@@ -186,7 +186,7 @@ def student_view(self, context):
         Renders the output that a student will see.
         """
         fragment = Fragment()
-        fragment.add_content(self.runtime.service(self, 'mako').render_template('edxmako.html', context))
+        fragment.add_content(self.runtime.service(self, 'mako').render_lms_template('edxmako.html', context))
         return fragment
 
 

cms/lib/xblock/authoring_mixin.py

@@ -37,7 +37,7 @@ def visibility_view(self, _context=None):
         """
         fragment = Fragment()
         from cms.djangoapps.contentstore.utils import reverse_course_url
-        fragment.add_content(self.runtime.service(self, 'mako').render_template('visibility_editor.html', {
+        fragment.add_content(self.runtime.service(self, 'mako').render_cms_template('visibility_editor.html', {
             'xblock': self,
             'manage_groups_url': reverse_course_url('group_configurations_list_handler', self.location.course_key),
         }))

lms/templates/library-block-author-preview-header.html cms/templates/library-block-author-preview-header.html

@@ -1,10 +1,23 @@
 """
 Supports rendering an XBlock to HTML using mako templates.
 """
-
+from django.template import engines
+from django.template.utils import InvalidTemplateEngineError
 from xblock.reference.plugins import Service
 
 from common.djangoapps.edxmako.shortcuts import render_to_string
+from common.djangoapps.edxmako import Engines
+
+try:
+    engines[Engines.PREVIEW]
+except InvalidTemplateEngineError:
+    # We are running in the CMS:
+    lms_mako_namespace = "main"
+    cms_mako_namespace = None
+else:
+    # We are running in the LMS:
+    lms_mako_namespace = "lms.main"
+    cms_mako_namespace = "main"
 
 
 class MakoService(Service):
@@ -21,10 +34,41 @@ def __init__(
         **kwargs
     ):
         super().__init__(**kwargs)
+        # Set the "default" namespace prefix, in case it's not specified when render_template() is called.
         self.namespace_prefix = namespace_prefix
 
     def render_template(self, template_file, dictionary, namespace='main'):
         """
-        Takes (template_file, dictionary) and returns rendered HTML.
+        DEPRECATED. Takes (template_file, dictionary) and returns rendered HTML.
+
+        Use render_lms_template or render_cms_template instead. Or better yet,
+        don't use mako templates at all. React or django templates are much
+        safer.
         """
         return render_to_string(template_file, dictionary, namespace=self.namespace_prefix + namespace)
+
+    def render_lms_template(self, template_file, dictionary):
+        """
+        Render a template which is found in one of the LMS edx-platform template
+        dirs. (lms.envs.common.MAKO_TEMPLATE_DIRS_BASE)
+
+        Templates which are in these dirs will only work with this function:
+            edx-platform/lms/templates/
+            edx-platform/xmodule/capa/templates/
+            openedx/features/course_experience/templates
+        """
+        return render_to_string(template_file, dictionary, namespace=lms_mako_namespace)
+
+    def render_cms_template(self, template_file, dictionary):
+        """
+        Render a template which is found in one of the CMS edx-platform template
+        dirs. (cms.envs.common.MAKO_TEMPLATE_DIRS_BASE)
+
+        Templates which are in these dirs will only work with this function:
+            edx-platform/cms/templates/
+            common/static/
+            openedx/features/course_experience/templates
+        """
+        if cms_mako_namespace is None:
+            raise RuntimeError("Cannot access CMS templates from the LMS")
+        return render_to_string(template_file, dictionary, namespace=cms_mako_namespace)

common/djangoapps/edxmako/services.py

@@ -148,7 +148,7 @@ def test_video_constructor(self):
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
-               get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+               get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
 
 class TestVideoNonYouTube(TestVideo):  # pylint: disable=test-inherits-tests
@@ -233,7 +233,7 @@ def test_video_constructor(self):
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         expected_result = get_context_dict_from_string(
-            mako_service.render_template('video.html', expected_context)
+            mako_service.render_lms_template('video.html', expected_context)
         )
         assert get_context_dict_from_string(context) == expected_result
         assert expected_result['download_video_link'] == 'example.mp4'
@@ -506,7 +506,7 @@ def test_get_html_track(self):
 
             mako_service = self.block.runtime.service(self.block, 'mako')
             assert get_context_dict_from_string(context) ==\
-                   get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+                   get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     def test_get_html_source(self):
         # lint-amnesty, pylint: disable=invalid-name, redefined-outer-name
@@ -620,7 +620,7 @@ def test_get_html_source(self):
 
             mako_service = self.block.runtime.service(self.block, 'mako')
             assert get_context_dict_from_string(context) ==\
-                   get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+                   get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     def test_get_html_with_non_existent_edx_video_id(self):
         """
@@ -766,7 +766,7 @@ def test_get_html_with_mocked_edx_video_id(self):
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
-               get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+               get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     def test_get_html_with_existing_edx_video_id(self):
         """
@@ -794,7 +794,7 @@ def test_get_html_with_existing_edx_video_id(self):
         context, expected_context = self.helper_get_html_with_edx_video_id(data)
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
-               get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+               get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     def test_get_html_with_existing_unstripped_edx_video_id(self):
         """
@@ -825,7 +825,7 @@ def test_get_html_with_existing_unstripped_edx_video_id(self):
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
-               get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+               get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     def encode_and_create_video(self, edx_video_id):
         """
@@ -1050,7 +1050,7 @@ def side_effect(*args, **kwargs):  # lint-amnesty, pylint: disable=unused-argume
 
             mako_service = self.block.runtime.service(self.block, 'mako')
             assert get_context_dict_from_string(context) ==\
-                   get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+                   get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     # pylint: disable=invalid-name
     def test_get_html_cdn_source_external_video(self):
@@ -1156,7 +1156,7 @@ def test_get_html_cdn_source_external_video(self):
 
             mako_service = self.block.runtime.service(self.block, 'mako')
             assert get_context_dict_from_string(context) ==\
-                   get_context_dict_from_string(mako_service.render_template('video.html', expected_context))
+                   get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
 
     @ddt.data(
         (True, ['youtube', 'desktop_webm', 'desktop_mp4', 'hls']),
@@ -2409,7 +2409,7 @@ def test_bumper_metadata(self, get_url_for_profiles, get_bumper_settings, is_bum
         }
 
         mako_service = self.block.runtime.service(self.block, 'mako')
-        expected_content = mako_service.render_template('video.html', expected_context)
+        expected_content = mako_service.render_lms_template('video.html', expected_context)
         assert get_context_dict_from_string(content) == get_context_dict_from_string(expected_content)
 
 
@@ -2506,7 +2506,7 @@ def assert_content_matches_expectations(self, autoadvanceenabled_must_be, autoad
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         with override_settings(FEATURES=self.FEATURES):
-            expected_content = mako_service.render_template('video.html', expected_context)
+            expected_content = mako_service.render_lms_template('video.html', expected_context)
 
         assert get_context_dict_from_string(content) == get_context_dict_from_string(expected_content)
 

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -1,14 +1,11 @@
 """
 Instructor Dashboard Views
 """
-
-
 import datetime
 import logging
-import uuid
 from functools import reduce
-from unittest.mock import patch
 
+import markupsafe
 import pytz
 from django.conf import settings
 from django.contrib.auth.decorators import login_required
@@ -25,11 +22,9 @@
 from edx_django_utils.plugins import get_plugins_view_context
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
-from xblock.field_data import DictFieldData
-from xblock.fields import ScopeIds
 
 from common.djangoapps.course_modes.models import CourseMode, CourseModesArchive
-from common.djangoapps.edxmako.shortcuts import render_to_response
+from common.djangoapps.edxmako.shortcuts import render_to_response, render_to_string
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.roles import (
     CourseFinanceAdminRole,
@@ -62,9 +57,6 @@
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 from openedx.core.djangolib.markup import HTML, Text
 from openedx.core.lib.courses import get_course_by_id
-from openedx.core.lib.url_utils import quote_slashes
-from openedx.core.lib.xblock_utils import wrap_xblock
-from xmodule.html_block import HtmlBlock  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.tabs import CourseTab  # lint-amnesty, pylint: disable=wrong-import-order
 
@@ -662,28 +654,29 @@ def _section_send_email(course, access):
     """ Provide data for the corresponding bulk email section """
     course_key = course.id
 
-    # Monkey-patch applicable_aside_types to return no asides for the duration of this render
-    with patch.object(course.runtime, 'applicable_aside_types', null_applicable_aside_types):
-        # This HtmlBlock is only being used to generate a nice text editor.
-        html_block = HtmlBlock(
-            course.runtime,
-            DictFieldData({'data': ''}),
-            ScopeIds(None, None, None, course_key.make_usage_key('html', 'fake'))
-        )
-        fragment = course.runtime.render(html_block, 'studio_view')
-    fragment = wrap_xblock(
-        'LmsRuntime', html_block, 'studio_view', fragment, None,
-        extra_data={"course-id": str(course_key)},
-        usage_id_serializer=lambda usage_id: quote_slashes(str(usage_id)),
-        # Generate a new request_token here at random, because this module isn't connected to any other
-        # xblock rendering.
-        request_token=uuid.uuid1().hex
-    )
+    # Render an HTML editor, using the same template as the HTML XBlock's visual
+    # editor. This basically pretends to be an HTML XBlock so that the XBlock
+    # initialization JS will manage the editor for us. This is a hack, and
+    # should be replace by a proper HTML Editor React component.
+    fake_block_data = {
+        "init": "XBlockToXModuleShim",
+        "usage-id": course_key.make_usage_key('html', 'fake'),
+        "runtime-version": "1",
+        "runtime-class": "LmsRuntime",
+    }
+    email_editor = render_to_string("xblock_wrapper.html", {
+        # This minimal version of the wrapper context is extracted from wrap_xblock():
+        "classes": ["xblock", "xblock-studio_view", "xmodule_edit", "xmodule_HtmlBlock"],
+        "data_attributes": ' '.join(f'data-{markupsafe.escape(key)}="{markupsafe.escape(value)}"'
+                                    for key, value in fake_block_data.items()),
+        "js_init_parameters": {"xmodule-type": "HTMLEditingDescriptor"},
+        "content": render_to_string("widgets/html-edit.html", {"editor": "visual", "data": ""}),
+    })
+
     cohorts = []
     if is_course_cohorted(course_key):
         cohorts = get_course_cohorts(course)
     course_modes = CourseMode.modes_for_course(course_key, include_expired=True, only_selectable=False)
-    email_editor = fragment.content
     section_data = {
         'section_key': 'send_email',
         'section_display_name': _('Email'),

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -172,7 +172,7 @@ def get_html(self):
             'content_html': self._render_content()
         }
 
-        return self.runtime.service(self, 'mako').render_template('annotatable.html', context)
+        return self.runtime.service(self, 'mako').render_lms_template('annotatable.html', context)
 
     def student_view(self, context):  # lint-amnesty, pylint: disable=unused-argument
         """
@@ -191,7 +191,7 @@ def studio_view(self, _context):
         Return the studio view.
         """
         fragment = Fragment(
-            self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context())
+            self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context())
         )
         add_sass_to_fragment(fragment, 'AnnotatableBlockEditor.scss')
         add_webpack_js_to_fragment(fragment, 'AnnotatableBlockEditor')

xmodule/annotatable_block.py

@@ -356,7 +356,7 @@ def studio_view(self, _context):
         Return the studio view.
         """
         fragment = Fragment(
-            self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context())
+            self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context())
         )
         add_sass_to_fragment(fragment, 'ProblemBlockEditor.scss')
         add_webpack_js_to_fragment(fragment, 'ProblemBlockEditor')
@@ -898,7 +898,7 @@ def get_html(self):
         """
         curr_score, total_possible = self.get_display_progress()
 
-        return self.runtime.service(self, 'mako').render_template('problem_ajax.html', {
+        return self.runtime.service(self, 'mako').render_lms_template('problem_ajax.html', {
             'element_id': self.location.html_id(),
             'id': str(self.location),
             'ajax_url': self.ajax_url,
@@ -1247,7 +1247,7 @@ def get_problem_html(self, encapsulate=True, submit_notification=False):
             'submit_disabled_cta': submit_disabled_ctas[0] if submit_disabled_ctas else None,
         }
 
-        html = self.runtime.service(self, 'mako').render_template('problem.html', context)
+        html = self.runtime.service(self, 'mako').render_lms_template('problem.html', context)
 
         if encapsulate:
             html = HTML('<div id="problem_{id}" class="problem" data-url="{ajax_url}">{html}</div>').format(
@@ -1548,7 +1548,7 @@ def get_answer(self, _data):
 
         return {
             'answers': new_answers,
-            'correct_status_html': self.runtime.service(self, 'mako').render_template(
+            'correct_status_html': self.runtime.service(self, 'mako').render_lms_template(
                 'status_span.html',
                 {'status': Status('correct', self.runtime.service(self, "i18n").gettext)}
             )

xmodule/capa_block.py

@@ -223,7 +223,7 @@ def student_view(self, _context):
 
     def get_html(self):
         required_html_ids = [block.location.html_id() for block in self.get_required_blocks]
-        return self.runtime.service(self, 'mako').render_template('conditional_ajax.html', {
+        return self.runtime.service(self, 'mako').render_lms_template('conditional_ajax.html', {
             'element_id': self.location.html_id(),
             'ajax_url': self.ajax_url,
             'depends': ';'.join(required_html_ids)
@@ -249,7 +249,7 @@ def studio_view(self, _context):
         Return the studio view.
         """
         fragment = Fragment(
-            self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context())
+            self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context())
         )
         add_webpack_js_to_fragment(fragment, 'ConditionalBlockEditor')
         shim_xmodule_js(fragment, self.studio_js_module_name)
@@ -262,7 +262,7 @@ def handle_ajax(self, _dispatch, _data):
         if not self.is_condition_satisfied():
             context = {'module': self,
                        'message': self.conditional_message}
-            html = self.runtime.service(self, 'mako').render_template('conditional_block.html', context)
+            html = self.runtime.service(self, 'mako').render_lms_template('conditional_block.html', context)
             return json.dumps({'fragments': [{'content': html}], 'message': bool(self.conditional_message)})
 
         fragments = [child.render(STUDENT_VIEW).to_dict() for child in self.get_children()]

xmodule/conditional_block.py

@@ -204,7 +204,7 @@ def student_view(self, context=None):
                 'login_msg': login_msg,
             }
             fragment.add_content(
-                self.runtime.service(self, 'mako').render_template('discussion/_discussion_inline.html', context)
+                self.runtime.service(self, 'mako').render_lms_template('discussion/_discussion_inline.html', context)
             )
 
         fragment.initialize_js('DiscussionInlineBlock')
@@ -216,7 +216,8 @@ def author_view(self, context=None):  # pylint: disable=unused-argument
         Renders author view for Studio.
         """
         fragment = Fragment()
-        fragment.add_content(self.runtime.service(self, 'mako').render_template(
+        # For historic reasons, this template is in the LMS templates folder:
+        fragment.add_content(self.runtime.service(self, 'mako').render_lms_template(
             'discussion/_discussion_inline_studio.html',
             {
                 'discussion_id': self.discussion_id,

xmodule/discussion_block.py

@@ -60,7 +60,7 @@ def student_view(self, _context):
         """
         Return a fragment that contains the html for the student view.
         """
-        fragment = Fragment(self.runtime.service(self, 'mako').render_template('module-error.html', {
+        fragment = Fragment(self.runtime.service(self, 'mako').render_lms_template('module-error.html', {
             'staff_access': True,
             'data': self.contents,
             'error': self.error_msg,

xmodule/error_block.py

@@ -134,7 +134,7 @@ def studio_view(self, _context):
         Return the studio view.
         """
         fragment = Fragment(
-            self.runtime.service(self, 'mako').render_template(self.mako_template, self.get_context())
+            self.runtime.service(self, 'mako').render_cms_template(self.mako_template, self.get_context())
         )
         add_sass_to_fragment(fragment, 'HtmlBlockEditor.scss')
         add_webpack_js_to_fragment(fragment, 'HtmlBlockEditor')
@@ -463,7 +463,7 @@ def get_html(self):
                 'visible_updates': course_updates[:3],
                 'hidden_updates': course_updates[3:],
             }
-            return self.runtime.service(self, 'mako').render_template(
+            return self.runtime.service(self, 'mako').render_lms_template(
                 f"{self.TEMPLATE_DIR}/course_updates.html",
                 context,
             )