From 1a9172c71b85f9c3d62dc9a78e5d2cb9c15f1102 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Tue, 14 Jul 2026 17:46:14 +0500 Subject: [PATCH 1/8] feat: add ol_openedx_feedback per-block feedback trigger --- src/ol_openedx_feedback/README.rst | 27 ++++++ .../ol_openedx_feedback/__init__.py | 0 .../ol_openedx_feedback/apps.py | 17 ++++ .../ol_openedx_feedback/block.py | 90 +++++++++++++++++++ .../ol_openedx_feedback/compat.py | 20 +++++ .../ol_openedx_feedback/constants.py | 4 + .../static/css/feedback.css | 63 +++++++++++++ .../static/html/student_view.html | 8 ++ .../ol_openedx_feedback/static/js/feedback.js | 72 +++++++++++++++ .../ol_openedx_feedback/utils.py | 9 ++ src/ol_openedx_feedback/pyproject.toml | 40 +++++++++ src/ol_openedx_feedback/setup.cfg | 39 ++++++++ src/ol_openedx_feedback/tests/__init__.py | 0 src/ol_openedx_feedback/tests/conftest.py | 27 ++++++ src/ol_openedx_feedback/tests/test_aside.py | 90 +++++++++++++++++++ src/ol_openedx_feedback/tests/test_utils.py | 26 ++++++ src/ol_openedx_feedback/tests/utils.py | 82 +++++++++++++++++ 17 files changed, 614 insertions(+) create mode 100644 src/ol_openedx_feedback/README.rst create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/__init__.py create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/apps.py create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/block.py create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/compat.py create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/constants.py create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/static/html/student_view.html create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/utils.py create mode 100644 src/ol_openedx_feedback/pyproject.toml create mode 100644 src/ol_openedx_feedback/setup.cfg create mode 100644 src/ol_openedx_feedback/tests/__init__.py create mode 100644 src/ol_openedx_feedback/tests/conftest.py create mode 100644 src/ol_openedx_feedback/tests/test_aside.py create mode 100644 src/ol_openedx_feedback/tests/test_utils.py create mode 100644 src/ol_openedx_feedback/tests/utils.py diff --git a/src/ol_openedx_feedback/README.rst b/src/ol_openedx_feedback/README.rst new file mode 100644 index 000000000..ec5b94a80 --- /dev/null +++ b/src/ol_openedx_feedback/README.rst @@ -0,0 +1,27 @@ +ol-openedx-feedback +################### + +An Open edX plugin that adds a per-block "Send feedback" trigger to applicable +leaf blocks in the LMS via an ``XBlockAside``. The trigger is shown only to +authenticated learners (never in Studio author/preview mode and never to +anonymous users). + +When a learner clicks the trigger the aside posts an ``ol-feedback::drawer-open`` +message to its parent window (the Learning MFE) using ``window.parent.postMessage``. +The message payload carries the block context needed to identify the content +being rated: + +- ``courseId`` — the course key +- ``blockUsageKey`` — the block's usage key +- ``blockType`` — the XBlock category (e.g. ``problem``, ``video``) +- ``blockDisplayName`` — the block's display name + +The Learning MFE receives the message and opens the feedback drawer, which +submits the learner's feedback directly to the **mit-learn** service. This +plugin does **not** persist anything in edx-platform and exposes no REST API. + +Enablement +========== + +Feedback is gated by the ``ol_openedx_feedback.feedback_enabled`` course waffle +flag (default off). Enable it for the desired courses (or globally) to roll out. diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/__init__.py b/src/ol_openedx_feedback/ol_openedx_feedback/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/apps.py b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py new file mode 100644 index 000000000..e3f93f4f9 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py @@ -0,0 +1,17 @@ +"""ol_openedx_feedback Django application initialization.""" + +from django.apps import AppConfig + + +class OLOpenedxFeedbackConfig(AppConfig): + """Configuration for the ol_openedx_feedback Django application. + + Aside-only plugin: the feedback trigger is registered via the + ``xblock_asides.v1`` entry point. There are no URLs, models, or settings — + feedback is persisted in mit-learn, and the MFE owns the submit URL. + """ + + name = "ol_openedx_feedback" + verbose_name = "Open edX Block Feedback" + + plugin_app: dict[str, dict] = {} diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/block.py b/src/ol_openedx_feedback/ol_openedx_feedback/block.py new file mode 100644 index 000000000..7015a4957 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/block.py @@ -0,0 +1,90 @@ +"""XBlockAside that renders the per-block feedback trigger.""" + +import logging + +import pkg_resources +from django.conf import settings +from django.template import Context, Template +from django.utils.translation import gettext +from web_fragments.fragment import Fragment +from xblock.core import XBlockAside + +try: + from xmodule.modulestore.xml import ( + XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 + ) +except ImportError: + from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime + +from xmodule.x_module import STUDENT_VIEW + +from ol_openedx_feedback.compat import get_feedback_enabled_flag +from ol_openedx_feedback.utils import is_aside_applicable_to_block + +log = logging.getLogger(__name__) + + +def _resource(path): + """Return the decoded contents of a packaged resource.""" + return pkg_resources.resource_string(__name__, path).decode("utf-8") + + +def _render(path, context): + """Render a packaged Django template with the given context.""" + return Template(_resource(path)).render(Context(context)) + + +class FeedbackAside(XBlockAside): + """Adds a small 'Send feedback' trigger to applicable blocks in the LMS.""" + + @XBlockAside.aside_for(STUDENT_VIEW) + def student_view_aside(self, block, context=None): # noqa: ARG002 + """Render the feedback trigger for authenticated learners only.""" + fragment = Fragment("") + + # Never render in Studio author/preview or for anonymous/preview users. + if getattr(self.runtime, "is_author_mode", False): + return fragment + if not getattr(self.runtime, "user_id", None): + return fragment + + block_usage_key = block.usage_key + block_id = block_usage_key.block_id + block_type = getattr(block, "category", None) + + fragment.add_content( + _render( + "static/html/student_view.html", + { + "block_id": block_id, + "label": gettext("Send feedback"), + }, + ) + ) + fragment.add_css(_resource("static/css/feedback.css")) + fragment.add_javascript(_resource("static/js/feedback.js")) + fragment.initialize_js( + "FeedbackAsideInit", + json_args={ + "block_id": block_id, + "learning_mfe_base_url": getattr( + settings, "LEARNING_MICROFRONTEND_URL", "" + ), + "drawer_payload": { + "courseId": str(block_usage_key.course_key), + "blockUsageKey": str(block_usage_key), + "blockType": block_type, + "blockDisplayName": block.display_name or "", + }, + }, + ) + return fragment + + @classmethod + def should_apply_to_block(cls, block): + """Apply to all leaf blocks; gated by the course waffle flag in LMS/CMS.""" + if isinstance(block.runtime, XMLImportingModuleStoreRuntime): + return is_aside_applicable_to_block(block) + return get_feedback_enabled_flag().is_enabled( + block.scope_ids.usage_id.context_key + ) and is_aside_applicable_to_block(block) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/compat.py b/src/ol_openedx_feedback/ol_openedx_feedback/compat.py new file mode 100644 index 000000000..214b42fe7 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/compat.py @@ -0,0 +1,20 @@ +"""Compatibility layer isolating core-platform imports.""" + +WAFFLE_FLAG_NAMESPACE = "ol_openedx_feedback" + +# .. toggle_name: ol_openedx_feedback.feedback_enabled +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Enables the per-block feedback trigger for a course. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2026-06-09 +OL_OPENEDX_FEEDBACK_ENABLED = "feedback_enabled" + + +def get_feedback_enabled_flag(): + """Return the CourseWaffleFlag controlling feedback rollout.""" + from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag # noqa: PLC0415 + + return CourseWaffleFlag( + f"{WAFFLE_FLAG_NAMESPACE}.{OL_OPENEDX_FEEDBACK_ENABLED}", __name__ + ) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/constants.py b/src/ol_openedx_feedback/ol_openedx_feedback/constants.py new file mode 100644 index 000000000..466de2ded --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/constants.py @@ -0,0 +1,4 @@ +"""Constants for ol_openedx_feedback.""" + +# Structural/container blocks never get a feedback trigger; everything else does. +EXCLUDED_BLOCK_TYPES = {"course", "chapter", "sequential", "vertical"} diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css new file mode 100644 index 000000000..08b5e7fcd --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css @@ -0,0 +1,63 @@ +.ol-feedback-container { + display: flex; + justify-content: flex-end; + margin: 8px 0 4px; +} + +.ol-feedback-container--relocated { + margin: 0; +} + +.ol-feedback-anchor { + --ol-fb-brand: #8a1e2d; + position: relative; + display: inline-flex; + align-self: center; +} + +.ol-feedback-anchor--docked { + /* Docked next to AskTIM, the megaphone overlaps the capa "saved" notification + line. Mask it with an opaque background (as the AskTIM button does) so the + line breaks cleanly around the icon instead of crossing it. The spacing to + the button lives in padding (not margin) so the background covers it too — + otherwise the line shows through the gap between the icon and the button. */ + margin-right: 0; + padding: 0 16px 0 6px; + background: #fff; + z-index: 1; +} + +.ol-feedback-trigger { + display: inline-flex; + align-items: center; + justify-content: center; + width: 30px; + height: 30px; + padding: 0; + border: 1px solid transparent; + border-radius: 8px; + background: transparent; + color: #e0a106; + cursor: pointer; + transition: background 0.15s ease, color 0.15s ease; +} + +.ol-feedback-trigger:hover { + background: #fbf3dd; + color: #c98a00; +} + +.ol-feedback-trigger:focus-visible { + outline: 2px solid var(--ol-fb-brand); + outline-offset: 2px; +} + +.ol-feedback-trigger:focus:not(:focus-visible) { + outline: none; +} + +.ol-feedback-trigger .ol-fb-ic { + width: 18px; + height: 18px; + display: block; +} diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/html/student_view.html b/src/ol_openedx_feedback/ol_openedx_feedback/static/html/student_view.html new file mode 100644 index 000000000..6bba61c95 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/html/student_view.html @@ -0,0 +1,8 @@ +
+ +
diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js b/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js new file mode 100644 index 000000000..72ad0c211 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js @@ -0,0 +1,72 @@ +(function ($) { + function initFeedback(initArgs) { + var blockId = initArgs.block_id; + var mfeBaseUrl = initArgs.learning_mfe_base_url; + var payload = initArgs.drawer_payload || {}; + + var $trigger = $("#ol-feedback-trigger-" + blockId); + if (!$trigger.length) { + return; + } + var $anchor = $trigger.closest(".ol-feedback-anchor"); + + // Namespace by block id and clear any prior binding so a re-init rebinds + // idempotently instead of stacking duplicate click handlers. + $trigger + .off("click.olFeedback-" + blockId) + .on("click.olFeedback-" + blockId, function (event) { + event.stopPropagation(); + // Post only to the known MFE origin; never "*" (would leak block context). + if (!mfeBaseUrl) { + return; + } + window.parent.postMessage( + { type: "ol-feedback::drawer-open", payload: payload }, + mfeBaseUrl + ); + }); + + // Placement: left of the AskTIM trigger when present, else right-aligned. + var $chatBtn = $("#chat-button-" + blockId); + if ($chatBtn.length) { + $anchor.closest(".ol-feedback-container").addClass("ol-feedback-container--relocated"); + $anchor.addClass("ol-feedback-anchor--docked"); + $chatBtn.before($anchor); + + // AskTIM lifts its button with an out-of-flow offset that varies by block + // type, so align the megaphone's center to the button's rendered position. + var alignFrame = null; + var alignToChatButton = function () { + $anchor.css("transform", ""); + var btnRect = $chatBtn[0].getBoundingClientRect(); + var anchorRect = $anchor[0].getBoundingClientRect(); + var delta = + (btnRect.top + btnRect.height / 2) - + (anchorRect.top + anchorRect.height / 2); + if (Math.abs(delta) > 0.5) { + $anchor.css("transform", "translateY(" + delta + "px)"); + } + }; + var scheduleAlign = function () { + if (alignFrame) { + window.cancelAnimationFrame(alignFrame); + } + alignFrame = window.requestAnimationFrame(alignToChatButton); + }; + scheduleAlign(); + // Namespace by block id so re-init rebinds idempotently instead of + // stacking duplicate handlers; rAF collapses resize bursts to one pass. + $(window) + .off("resize.olFeedback-" + blockId) + .on("resize.olFeedback-" + blockId, scheduleAlign); + } + } + + function FeedbackAsideView(runtime, element, blockElement, initArgs) { + initFeedback(initArgs); + } + + window.FeedbackAsideInit = function (runtime, element, blockElement, initArgs) { + return new FeedbackAsideView(runtime, element, blockElement, initArgs); + }; +})($); diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/utils.py b/src/ol_openedx_feedback/ol_openedx_feedback/utils.py new file mode 100644 index 000000000..1a67c0c35 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/utils.py @@ -0,0 +1,9 @@ +"""Utility helpers for ol_openedx_feedback.""" + +from ol_openedx_feedback.constants import EXCLUDED_BLOCK_TYPES + + +def is_aside_applicable_to_block(block): + """Feedback applies to every block type except structural containers.""" + block_type = getattr(block, "category", None) + return bool(block_type) and block_type not in EXCLUDED_BLOCK_TYPES diff --git a/src/ol_openedx_feedback/pyproject.toml b/src/ol_openedx_feedback/pyproject.toml new file mode 100644 index 000000000..903e9f284 --- /dev/null +++ b/src/ol_openedx_feedback/pyproject.toml @@ -0,0 +1,40 @@ +[project] +name = "ol-openedx-feedback" +version = "0.1.0" +description = "An Open edX plugin to collect per-block learner feedback" +authors = [{name = "MIT Office of Digital Learning"}] +license = "BSD-3-Clause" +readme = "README.rst" +requires-python = ">=3.11" +keywords = ["Python", "edx"] +dependencies = [ + "Django>=4.0", + "XBlock", +] + +[project.entry-points."xblock_asides.v1"] +ol_openedx_feedback = "ol_openedx_feedback.block:FeedbackAside" + +[project.entry-points."lms.djangoapp"] +ol_openedx_feedback = "ol_openedx_feedback.apps:OLOpenedxFeedbackConfig" + +[project.entry-points."cms.djangoapp"] +ol_openedx_feedback = "ol_openedx_feedback.apps:OLOpenedxFeedbackConfig" + +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.targets.wheel] +packages = ["ol_openedx_feedback"] +include = [ + "ol_openedx_feedback/**/*.py", + "ol_openedx_feedback/static/**/*", +] + +[tool.hatch.build.targets.sdist] +include = [ + "ol_openedx_feedback/**/*", + "README.rst", + "pyproject.toml", +] diff --git a/src/ol_openedx_feedback/setup.cfg b/src/ol_openedx_feedback/setup.cfg new file mode 100644 index 000000000..16432cf3c --- /dev/null +++ b/src/ol_openedx_feedback/setup.cfg @@ -0,0 +1,39 @@ +[isort] +include_trailing_comma = True +indent = ' ' +line_length = 120 +multi_line_output = 3 + +[wheel] +universal = 1 + +[tool:pytest] +pep8maxlinelength = 119 +DJANGO_SETTINGS_MODULE = lms.envs.test +addopts = --nomigrations --reuse-db --durations=20 +# Enable default handling for all warnings, including those that are ignored by default; +# but hide rate-limit warnings (because we deliberately don't throttle test user logins) +# and field_data deprecation warnings (because fixing them requires a major low-priority refactoring) +filterwarnings = + default + ignore::xblock.exceptions.FieldDataDeprecationWarning + ignore::pytest.PytestConfigWarning + ignore:No request passed to the backend, unable to rate-limit:UserWarning + ignore:Flags not at the start of the expression:DeprecationWarning + ignore:Using or importing the ABCs from 'collections' instead of from 'collections.abc':DeprecationWarning + ignore:invalid escape sequence:DeprecationWarning + ignore:`formatargspec` is deprecated since Python 3.5:DeprecationWarning + ignore:the imp module is deprecated in favour of importlib:DeprecationWarning + ignore:"is" with a literal:SyntaxWarning + ignore:defusedxml.lxml is no longer supported:DeprecationWarning + ignore: `np.int` is a deprecated alias for the builtin `int`.:DeprecationWarning + ignore: `np.float` is a deprecated alias for the builtin `float`.:DeprecationWarning + ignore: `np.complex` is a deprecated alias for the builtin `complex`.:DeprecationWarning + ignore: 'etree' is deprecated. Use 'xml.etree.ElementTree' instead.:DeprecationWarning + ignore: defusedxml.cElementTree is deprecated, import from defusedxml.ElementTree instead.:DeprecationWarning + + +junit_family = xunit2 +norecursedirs = .* *.egg build conf dist node_modules test_root cms/envs lms/envs +python_classes = +python_files = tests.py test_*.py tests_*.py *_tests.py __init__.py diff --git a/src/ol_openedx_feedback/tests/__init__.py b/src/ol_openedx_feedback/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/ol_openedx_feedback/tests/conftest.py b/src/ol_openedx_feedback/tests/conftest.py new file mode 100644 index 000000000..d16025484 --- /dev/null +++ b/src/ol_openedx_feedback/tests/conftest.py @@ -0,0 +1,27 @@ +"""Pytest config for ol_openedx_feedback.""" + +import logging + + +def pytest_addoption(parser): + """Pytest hook that adds command line options.""" + parser.addoption( + "--disable-logging", + action="store_true", + default=False, + help="Disable all logging during test run", + ) + parser.addoption( + "--error-log-only", + action="store_true", + default=False, + help="Disable all logging output below 'error' level during test run", + ) + + +def pytest_configure(config): + """Pytest hook that runs after command line options have been parsed.""" + if config.getoption("--disable-logging"): + logging.disable(logging.CRITICAL) + elif config.getoption("--error-log-only"): + logging.disable(logging.WARNING) diff --git a/src/ol_openedx_feedback/tests/test_aside.py b/src/ol_openedx_feedback/tests/test_aside.py new file mode 100644 index 000000000..5e9aaf1cf --- /dev/null +++ b/src/ol_openedx_feedback/tests/test_aside.py @@ -0,0 +1,90 @@ +"""Tests for the FeedbackAside trigger rendering and gating.""" + +from unittest.mock import Mock, patch + +from ddt import data, ddt, unpack +from ol_openedx_feedback.block import FeedbackAside +from openedx.core.djangolib.testing.utils import skip_unless_lms + +try: + from xmodule.modulestore.xml import ( + XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 + ) +except ImportError: + from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime + +from tests.utils import OLFeedbackTestCase + + +@ddt +class FeedbackAsideTests(OLFeedbackTestCase): + """Tests for FeedbackAside rendering and gating.""" + + @data( + *[ + [5, False, True], + [None, False, False], + [5, True, False], + ] + ) + @unpack + @skip_unless_lms + def test_student_view_aside(self, user_id, is_author_mode, should_render): + """ + The trigger renders only for authenticated learners and never in + Studio author/preview mode. + """ + self.runtime.user_id = user_id + self.runtime.is_author_mode = is_author_mode + self.video_aside_instance.runtime = self.runtime + + fragment = self.video_aside_instance.student_view_aside(self.video_block) + + assert bool(fragment.content) is should_render + if should_render: + assert ( + f"ol-feedback-trigger-{self.video_block.usage_key.block_id}" + in fragment.content + ) + assert fragment.js_init_fn == "FeedbackAsideInit" + else: + assert fragment.content == "" + assert fragment.js_init_fn is None + + @data( + *[ + ["video", True, False, True], + ["video", False, False, False], + ["video", True, True, True], + ["video", False, True, True], + ["problem", True, False, True], + ["vertical", True, False, False], + ["vertical", False, False, False], + ["vertical", True, True, False], + ] + ) + @unpack + def test_should_apply_to_block( + self, block_category, waffle_flag_enabled, is_import_runtime, should_apply + ): + """ + `should_apply_to_block` is True only for leaf blocks when the course + waffle flag is enabled. During course import the block lacks course + context, so the flag is skipped and only the block type is checked. + """ + block = { + "video": self.video_block, + "problem": self.problem_block, + "vertical": self.vertical, + }[block_category] + + with patch( + "ol_openedx_feedback.block.get_feedback_enabled_flag" + ) as mock_get_feedback_enabled_flag: + mock_get_feedback_enabled_flag.return_value = Mock( + is_enabled=Mock(return_value=waffle_flag_enabled) + ) + if is_import_runtime: + block.runtime = Mock(spec=XMLImportingModuleStoreRuntime) + + assert FeedbackAside.should_apply_to_block(block) is should_apply diff --git a/src/ol_openedx_feedback/tests/test_utils.py b/src/ol_openedx_feedback/tests/test_utils.py new file mode 100644 index 000000000..0bf7ad188 --- /dev/null +++ b/src/ol_openedx_feedback/tests/test_utils.py @@ -0,0 +1,26 @@ +"""Tests for block applicability helper.""" + +from types import SimpleNamespace + +from ol_openedx_feedback.utils import is_aside_applicable_to_block + + +def _block(category): + return SimpleNamespace(category=category) + + +def test_leaf_blocks_are_applicable(): + assert is_aside_applicable_to_block(_block("video")) is True + assert is_aside_applicable_to_block(_block("problem")) is True + assert is_aside_applicable_to_block(_block("html")) is True + + +def test_container_blocks_are_excluded(): + assert is_aside_applicable_to_block(_block("vertical")) is False + assert is_aside_applicable_to_block(_block("sequential")) is False + assert is_aside_applicable_to_block(_block("chapter")) is False + assert is_aside_applicable_to_block(_block("course")) is False + + +def test_missing_category_is_not_applicable(): + assert is_aside_applicable_to_block(_block(None)) is False diff --git a/src/ol_openedx_feedback/tests/utils.py b/src/ol_openedx_feedback/tests/utils.py new file mode 100644 index 000000000..533e07440 --- /dev/null +++ b/src/ol_openedx_feedback/tests/utils.py @@ -0,0 +1,82 @@ +from datetime import datetime + +from pytz import UTC +from xblock.runtime import DictKeyValueStore, KvsFieldData +from xblock.test.tools import TestRuntime +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory + + +class OLFeedbackTestCase(ModuleStoreTestCase): + """Base test case that builds a course tree with leaf and container blocks.""" + + def setUp(self): + super().setUp() + + key_store = DictKeyValueStore() + field_data = KvsFieldData(key_store) + self.runtime = TestRuntime(services={"field-data": field_data}) + + course = CourseFactory.create(default_store=ModuleStoreEnum.Type.split) + self.course = BlockFactory.create( + parent_location=course.location, + category="course", + display_name="Test course", + ) + self.chapter = BlockFactory.create( + parent_location=self.course.location, + category="chapter", + display_name="Week 1", + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.sequential = BlockFactory.create( + parent_location=self.chapter.location, + category="sequential", + display_name="Lesson 1", + publish_item=True, + start=datetime(2015, 3, 1, tzinfo=UTC), + ) + self.vertical = BlockFactory.create( + parent_location=self.sequential.location, + category="vertical", + display_name="Subsection 1", + publish_item=True, + start=datetime(2015, 4, 1, tzinfo=UTC), + ) + self.problem_block = BlockFactory.create( + category="problem", + parent_location=self.vertical.location, + display_name="A Problem Block", + weight=1, + user_id=self.user.id, + publish_item=False, + ) + self.video_block = BlockFactory.create( + parent_location=self.vertical.location, + category="video", + display_name="My Video", + user_id=self.user.id, + ) + self.html_block = BlockFactory.create( + category="html", + parent_location=self.vertical.location, + display_name="An HTML Block", + user_id=self.user.id, + ) + + self.aside_name = "ol_openedx_feedback" + self.video_aside_instance = self.create_aside("video") + self.problem_aside_instance = self.create_aside("problem") + + def create_aside(self, block_type): + """ + Create an aside instance. + """ + def_id = self.runtime.id_generator.create_definition(block_type) + usage_id = self.runtime.id_generator.create_usage(def_id) + _, aside_id = self.runtime.id_generator.create_aside( + def_id, usage_id, self.aside_name + ) + return self.runtime.get_aside(aside_id) From 0259e35e9738175028afce73246604e2eab54149 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Thu, 16 Jul 2026 15:17:59 +0500 Subject: [PATCH 2/8] refactor: address review feedback on ol_openedx_feedback --- src/ol_openedx_feedback/README.rst | 35 ++++++++++++++ .../ol_openedx_feedback/apps.py | 20 ++++++-- .../ol_openedx_feedback/block.py | 18 ++------ .../ol_openedx_feedback/constants.py | 7 ++- .../ol_openedx_feedback/settings/common.py | 18 ++++++++ .../ol_openedx_feedback/utils.py | 23 ++++++++-- src/ol_openedx_feedback/tests/test_aside.py | 27 +++-------- src/ol_openedx_feedback/tests/test_utils.py | 46 +++++++++++++------ 8 files changed, 137 insertions(+), 57 deletions(-) create mode 100644 src/ol_openedx_feedback/ol_openedx_feedback/settings/common.py diff --git a/src/ol_openedx_feedback/README.rst b/src/ol_openedx_feedback/README.rst index ec5b94a80..32e0d1c9c 100644 --- a/src/ol_openedx_feedback/README.rst +++ b/src/ol_openedx_feedback/README.rst @@ -20,8 +20,43 @@ The Learning MFE receives the message and opens the feedback drawer, which submits the learner's feedback directly to the **mit-learn** service. This plugin does **not** persist anything in edx-platform and exposes no REST API. +Installation +============ + +Install the package into the LMS/CMS Python environment: + +.. code-block:: bash + + pip install ol-openedx-feedback + +The plugin registers itself automatically through its entry points — the +``xblock_asides.v1`` aside plus the ``lms.djangoapp`` / ``cms.djangoapp`` app +configs — so no changes to ``INSTALLED_APPS`` are required. Restart the LMS/CMS +after installing. + Enablement ========== Feedback is gated by the ``ol_openedx_feedback.feedback_enabled`` course waffle flag (default off). Enable it for the desired courses (or globally) to roll out. + +Configuration +============= + +By default the trigger renders on every leaf block and is suppressed only on +structural containers (``course`` / ``chapter`` / ``sequential`` / ``vertical``). +To additionally exclude one or more block types (for example ``html``), override +the excluded set through ``ENV_TOKENS`` (e.g. in ``lms.yml`` / ``cms.yml``): + +.. code-block:: yaml + + OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES: + - course + - chapter + - sequential + - vertical + - html + +The plugin reads this value via its ``settings.common`` ``plugin_settings`` hook +and exposes it as the ``OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES`` Django +setting, defaulting to the structural set above when unset. diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/apps.py b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py index e3f93f4f9..01a7343ca 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/apps.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py @@ -1,17 +1,29 @@ """ol_openedx_feedback Django application initialization.""" from django.apps import AppConfig +from edx_django_utils.plugins import PluginSettings +from openedx.core.djangoapps.plugins.constants import ProjectType, SettingsType class OLOpenedxFeedbackConfig(AppConfig): """Configuration for the ol_openedx_feedback Django application. - Aside-only plugin: the feedback trigger is registered via the - ``xblock_asides.v1`` entry point. There are no URLs, models, or settings — - feedback is persisted in mit-learn, and the MFE owns the submit URL. + Trigger-only plugin: the feedback trigger is registered via the + ``xblock_asides.v1`` entry point (no URLs or models — feedback is persisted + in mit-learn and the MFE owns the submit URL). The only Django settings are + the plugin defaults populated by ``settings.common``. """ name = "ol_openedx_feedback" verbose_name = "Open edX Block Feedback" - plugin_app: dict[str, dict] = {} + plugin_app = { + PluginSettings.CONFIG: { + ProjectType.LMS: { + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: "settings.common"}, + }, + ProjectType.CMS: { + SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: "settings.common"}, + }, + }, + } diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/block.py b/src/ol_openedx_feedback/ol_openedx_feedback/block.py index 7015a4957..18c169cbb 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/block.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/block.py @@ -8,14 +8,6 @@ from django.utils.translation import gettext from web_fragments.fragment import Fragment from xblock.core import XBlockAside - -try: - from xmodule.modulestore.xml import ( - XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 - ) -except ImportError: - from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime - from xmodule.x_module import STUDENT_VIEW from ol_openedx_feedback.compat import get_feedback_enabled_flag @@ -43,9 +35,9 @@ def student_view_aside(self, block, context=None): # noqa: ARG002 fragment = Fragment("") # Never render in Studio author/preview or for anonymous/preview users. - if getattr(self.runtime, "is_author_mode", False): - return fragment - if not getattr(self.runtime, "user_id", None): + if getattr(self.runtime, "is_author_mode", False) or not getattr( + self.runtime, "user_id", None + ): return fragment block_usage_key = block.usage_key @@ -82,9 +74,7 @@ def student_view_aside(self, block, context=None): # noqa: ARG002 @classmethod def should_apply_to_block(cls, block): - """Apply to all leaf blocks; gated by the course waffle flag in LMS/CMS.""" - if isinstance(block.runtime, XMLImportingModuleStoreRuntime): - return is_aside_applicable_to_block(block) + """Apply to leaf blocks when the course waffle flag is enabled.""" return get_feedback_enabled_flag().is_enabled( block.scope_ids.usage_id.context_key ) and is_aside_applicable_to_block(block) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/constants.py b/src/ol_openedx_feedback/ol_openedx_feedback/constants.py index 466de2ded..a4258ce66 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/constants.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/constants.py @@ -1,4 +1,7 @@ """Constants for ol_openedx_feedback.""" -# Structural/container blocks never get a feedback trigger; everything else does. -EXCLUDED_BLOCK_TYPES = {"course", "chapter", "sequential", "vertical"} +# Default structural/container blocks that never get a feedback trigger. +# Deployments can override this via the +# ``OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES`` Django setting (e.g. to also +# exclude a content type like ``html``). +DEFAULT_EXCLUDED_BLOCK_TYPES = {"course", "chapter", "sequential", "vertical"} diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/settings/common.py b/src/ol_openedx_feedback/ol_openedx_feedback/settings/common.py new file mode 100644 index 000000000..93ca697a5 --- /dev/null +++ b/src/ol_openedx_feedback/ol_openedx_feedback/settings/common.py @@ -0,0 +1,18 @@ +# noqa: INP001 + +"""Settings to provide to edX""" + +from ol_openedx_feedback.constants import DEFAULT_EXCLUDED_BLOCK_TYPES + + +def plugin_settings(settings): + """ + Populate common settings + """ + env_tokens = getattr(settings, "ENV_TOKENS", {}) + settings.OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES = set( + env_tokens.get( + "OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES", + DEFAULT_EXCLUDED_BLOCK_TYPES, + ) + ) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/utils.py b/src/ol_openedx_feedback/ol_openedx_feedback/utils.py index 1a67c0c35..c27aa3194 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/utils.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/utils.py @@ -1,9 +1,26 @@ """Utility helpers for ol_openedx_feedback.""" -from ol_openedx_feedback.constants import EXCLUDED_BLOCK_TYPES +from django.conf import settings + +from ol_openedx_feedback.constants import DEFAULT_EXCLUDED_BLOCK_TYPES + + +def get_excluded_block_types(): + """Return the block types that never get a feedback trigger. + + Defaults to structural containers; overridable per deployment via the + ``OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES`` setting. + """ + return set( + getattr( + settings, + "OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES", + DEFAULT_EXCLUDED_BLOCK_TYPES, + ) + ) def is_aside_applicable_to_block(block): - """Feedback applies to every block type except structural containers.""" + """Feedback applies to every block type except excluded containers.""" block_type = getattr(block, "category", None) - return bool(block_type) and block_type not in EXCLUDED_BLOCK_TYPES + return bool(block_type) and block_type not in get_excluded_block_types() diff --git a/src/ol_openedx_feedback/tests/test_aside.py b/src/ol_openedx_feedback/tests/test_aside.py index 5e9aaf1cf..6262d288e 100644 --- a/src/ol_openedx_feedback/tests/test_aside.py +++ b/src/ol_openedx_feedback/tests/test_aside.py @@ -6,13 +6,6 @@ from ol_openedx_feedback.block import FeedbackAside from openedx.core.djangolib.testing.utils import skip_unless_lms -try: - from xmodule.modulestore.xml import ( - XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 - ) -except ImportError: - from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime - from tests.utils import OLFeedbackTestCase @@ -53,24 +46,20 @@ def test_student_view_aside(self, user_id, is_author_mode, should_render): @data( *[ - ["video", True, False, True], - ["video", False, False, False], - ["video", True, True, True], - ["video", False, True, True], - ["problem", True, False, True], - ["vertical", True, False, False], - ["vertical", False, False, False], - ["vertical", True, True, False], + ["video", True, True], + ["video", False, False], + ["problem", True, True], + ["vertical", True, False], + ["vertical", False, False], ] ) @unpack def test_should_apply_to_block( - self, block_category, waffle_flag_enabled, is_import_runtime, should_apply + self, block_category, waffle_flag_enabled, should_apply ): """ `should_apply_to_block` is True only for leaf blocks when the course - waffle flag is enabled. During course import the block lacks course - context, so the flag is skipped and only the block type is checked. + waffle flag is enabled. """ block = { "video": self.video_block, @@ -84,7 +73,5 @@ def test_should_apply_to_block( mock_get_feedback_enabled_flag.return_value = Mock( is_enabled=Mock(return_value=waffle_flag_enabled) ) - if is_import_runtime: - block.runtime = Mock(spec=XMLImportingModuleStoreRuntime) assert FeedbackAside.should_apply_to_block(block) is should_apply diff --git a/src/ol_openedx_feedback/tests/test_utils.py b/src/ol_openedx_feedback/tests/test_utils.py index 0bf7ad188..e57ef3564 100644 --- a/src/ol_openedx_feedback/tests/test_utils.py +++ b/src/ol_openedx_feedback/tests/test_utils.py @@ -2,6 +2,8 @@ from types import SimpleNamespace +import pytest +from django.test import override_settings from ol_openedx_feedback.utils import is_aside_applicable_to_block @@ -9,18 +11,34 @@ def _block(category): return SimpleNamespace(category=category) -def test_leaf_blocks_are_applicable(): +@pytest.mark.parametrize( + ("category", "expected"), + [ + ("video", True), + ("problem", True), + ("html", True), + ("vertical", False), + ("sequential", False), + ("chapter", False), + ("course", False), + (None, False), + ], +) +def test_is_aside_applicable_to_block(category, expected): + """Applies to leaf blocks; excludes structural containers and missing types.""" + assert is_aside_applicable_to_block(_block(category)) is expected + + +@override_settings( + OL_OPENEDX_FEEDBACK_EXCLUDED_BLOCK_TYPES={ + "course", + "chapter", + "sequential", + "vertical", + "html", + } +) +def test_excluded_block_types_setting_override(): + """A deployment can exclude an extra block type via the setting.""" + assert is_aside_applicable_to_block(_block("html")) is False assert is_aside_applicable_to_block(_block("video")) is True - assert is_aside_applicable_to_block(_block("problem")) is True - assert is_aside_applicable_to_block(_block("html")) is True - - -def test_container_blocks_are_excluded(): - assert is_aside_applicable_to_block(_block("vertical")) is False - assert is_aside_applicable_to_block(_block("sequential")) is False - assert is_aside_applicable_to_block(_block("chapter")) is False - assert is_aside_applicable_to_block(_block("course")) is False - - -def test_missing_category_is_not_applicable(): - assert is_aside_applicable_to_block(_block(None)) is False From 2d8e713a971b9a55b692d799b5de6028a451a637 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Thu, 16 Jul 2026 15:17:59 +0500 Subject: [PATCH 3/8] style: always render feedback trigger as a button + mask problem line - Style the trigger as a button (border + background) whether standalone or docked next to AskTIM, with a full-cover grey + red border on click - Mask the problem-block "saved/submit" notification line across the gap to the AskTIM button (JS flags problem blocks via payload.blockType) --- .../static/css/feedback.css | 47 +++++++++++-------- .../ol_openedx_feedback/static/js/feedback.js | 5 ++ 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css index 08b5e7fcd..5fd4507ab 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css @@ -16,30 +16,36 @@ } .ol-feedback-anchor--docked { - /* Docked next to AskTIM, the megaphone overlaps the capa "saved" notification - line. Mask it with an opaque background (as the AskTIM button does) so the - line breaks cleanly around the icon instead of crossing it. The spacing to - the button lives in padding (not margin) so the background covers it too — - otherwise the line shows through the gap between the icon and the button. */ - margin-right: 0; - padding: 0 16px 0 6px; - background: #fff; + /* Docked next to AskTIM: only positioning here (a gap before the AskTIM + button). The button appearance lives on .ol-feedback-trigger so the + megaphone always looks like a button, docked or standalone. */ + margin-right: 10px; z-index: 1; } +/* Problem blocks draw a horizontal "saved/submit" notification line behind the + button row. Project an opaque white block across the gap to the AskTIM button + so the line is masked there too (invisible on the white problem-block + background). Not applied to video/html blocks, which have no such line. */ +.ol-feedback-anchor--line-masked { + box-shadow: 12px 0 0 0 #fff; +} + .ol-feedback-trigger { + /* Always styled as a button (border + background), whether standalone or + docked next to the AskTIM button. */ display: inline-flex; align-items: center; justify-content: center; - width: 30px; - height: 30px; + width: 32px; + height: 32px; padding: 0; - border: 1px solid transparent; - border-radius: 8px; - background: transparent; + border: 1px solid #b8c2cc; + border-radius: 6px; + background: #fff; color: #e0a106; cursor: pointer; - transition: background 0.15s ease, color 0.15s ease; + transition: background 0.15s ease, color 0.15s ease, border-color 0.15s ease; } .ol-feedback-trigger:hover { @@ -47,12 +53,13 @@ color: #c98a00; } -.ol-feedback-trigger:focus-visible { - outline: 2px solid var(--ol-fb-brand); - outline-offset: 2px; -} - -.ol-feedback-trigger:focus:not(:focus-visible) { +/* On click/focus the background fills the whole button and the border turns + red. */ +.ol-feedback-trigger:active, +.ol-feedback-trigger:focus { + background: #e6e6e6; + border-color: var(--ol-fb-brand); + color: #c98a00; outline: none; } diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js b/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js index 72ad0c211..d8c089835 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/js/feedback.js @@ -31,6 +31,11 @@ if ($chatBtn.length) { $anchor.closest(".ol-feedback-container").addClass("ol-feedback-container--relocated"); $anchor.addClass("ol-feedback-anchor--docked"); + // Problem blocks render a horizontal "saved/submit" notification line that + // runs behind the button row; flag them so CSS can mask it across the gap. + if (payload.blockType === "problem") { + $anchor.addClass("ol-feedback-anchor--line-masked"); + } $chatBtn.before($anchor); // AskTIM lifts its button with an out-of-flow offset that varies by block From 3b57e9c09024db0c944dbb6c736cb1b24da25964 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Thu, 16 Jul 2026 17:46:18 +0500 Subject: [PATCH 4/8] fix: guard should_apply_to_block against course-import runtime Reinstate the XMLImportingModuleStoreRuntime guard so should_apply_to_block skips the waffle-flag / context_key lookup during course import (when the block has no resolvable course context) and gates on block type only. Restore the import-path test cases. Addresses the Sentry review finding. --- .../ol_openedx_feedback/block.py | 13 +++++++++ src/ol_openedx_feedback/tests/test_aside.py | 27 ++++++++++++++----- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/block.py b/src/ol_openedx_feedback/ol_openedx_feedback/block.py index 18c169cbb..91a17d38b 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/block.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/block.py @@ -8,6 +8,14 @@ from django.utils.translation import gettext from web_fragments.fragment import Fragment from xblock.core import XBlockAside + +try: + from xmodule.modulestore.xml import ( + XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 + ) +except ImportError: + from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime + from xmodule.x_module import STUDENT_VIEW from ol_openedx_feedback.compat import get_feedback_enabled_flag @@ -75,6 +83,11 @@ def student_view_aside(self, block, context=None): # noqa: ARG002 @classmethod def should_apply_to_block(cls, block): """Apply to leaf blocks when the course waffle flag is enabled.""" + # During course import the block lacks a resolvable course context, so + # skip the waffle-flag check (which reads the course context key) and + # gate on block type only. + if isinstance(block.runtime, XMLImportingModuleStoreRuntime): + return is_aside_applicable_to_block(block) return get_feedback_enabled_flag().is_enabled( block.scope_ids.usage_id.context_key ) and is_aside_applicable_to_block(block) diff --git a/src/ol_openedx_feedback/tests/test_aside.py b/src/ol_openedx_feedback/tests/test_aside.py index 6262d288e..5e9aaf1cf 100644 --- a/src/ol_openedx_feedback/tests/test_aside.py +++ b/src/ol_openedx_feedback/tests/test_aside.py @@ -6,6 +6,13 @@ from ol_openedx_feedback.block import FeedbackAside from openedx.core.djangolib.testing.utils import skip_unless_lms +try: + from xmodule.modulestore.xml import ( + XMLImportingModuleStoreRuntime as XMLImportingModuleStoreRuntime, # noqa: PLC0414 + ) +except ImportError: + from xmodule.modulestore.xml import ImportSystem as XMLImportingModuleStoreRuntime + from tests.utils import OLFeedbackTestCase @@ -46,20 +53,24 @@ def test_student_view_aside(self, user_id, is_author_mode, should_render): @data( *[ - ["video", True, True], - ["video", False, False], - ["problem", True, True], - ["vertical", True, False], - ["vertical", False, False], + ["video", True, False, True], + ["video", False, False, False], + ["video", True, True, True], + ["video", False, True, True], + ["problem", True, False, True], + ["vertical", True, False, False], + ["vertical", False, False, False], + ["vertical", True, True, False], ] ) @unpack def test_should_apply_to_block( - self, block_category, waffle_flag_enabled, should_apply + self, block_category, waffle_flag_enabled, is_import_runtime, should_apply ): """ `should_apply_to_block` is True only for leaf blocks when the course - waffle flag is enabled. + waffle flag is enabled. During course import the block lacks course + context, so the flag is skipped and only the block type is checked. """ block = { "video": self.video_block, @@ -73,5 +84,7 @@ def test_should_apply_to_block( mock_get_feedback_enabled_flag.return_value = Mock( is_enabled=Mock(return_value=waffle_flag_enabled) ) + if is_import_runtime: + block.runtime = Mock(spec=XMLImportingModuleStoreRuntime) assert FeedbackAside.should_apply_to_block(block) is should_apply From 995441f1fe3bd678f048049c1b33022f027458b4 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Thu, 16 Jul 2026 18:01:39 +0500 Subject: [PATCH 5/8] chore: make ol_openedx_feedback LMS-only The feedback trigger renders only in the learner (student) view, so the plugin does not need to load in Studio/CMS. Drop the cms.djangoapp entry point and the ProjectType.CMS settings registration; the aside still evaluates during course import via the xblock_asides.v1 entry point (guarded), and the excluded-block- types setting falls back to its default there. --- src/ol_openedx_feedback/README.rst | 11 ++++++----- src/ol_openedx_feedback/ol_openedx_feedback/apps.py | 8 +++----- src/ol_openedx_feedback/pyproject.toml | 3 --- 3 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/ol_openedx_feedback/README.rst b/src/ol_openedx_feedback/README.rst index 32e0d1c9c..b90acebaf 100644 --- a/src/ol_openedx_feedback/README.rst +++ b/src/ol_openedx_feedback/README.rst @@ -23,16 +23,17 @@ plugin does **not** persist anything in edx-platform and exposes no REST API. Installation ============ -Install the package into the LMS/CMS Python environment: +Install the package into the LMS Python environment: .. code-block:: bash pip install ol-openedx-feedback The plugin registers itself automatically through its entry points — the -``xblock_asides.v1`` aside plus the ``lms.djangoapp`` / ``cms.djangoapp`` app -configs — so no changes to ``INSTALLED_APPS`` are required. Restart the LMS/CMS -after installing. +``xblock_asides.v1`` aside plus the ``lms.djangoapp`` app config — so no changes +to ``INSTALLED_APPS`` are required. Restart the LMS after installing. (The +trigger is learner-facing only, so the plugin is LMS-only and is not installed +in Studio/CMS.) Enablement ========== @@ -46,7 +47,7 @@ Configuration By default the trigger renders on every leaf block and is suppressed only on structural containers (``course`` / ``chapter`` / ``sequential`` / ``vertical``). To additionally exclude one or more block types (for example ``html``), override -the excluded set through ``ENV_TOKENS`` (e.g. in ``lms.yml`` / ``cms.yml``): +the excluded set through ``ENV_TOKENS`` (e.g. in ``lms.yml``): .. code-block:: yaml diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/apps.py b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py index 01a7343ca..a3ee96fbf 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/apps.py +++ b/src/ol_openedx_feedback/ol_openedx_feedback/apps.py @@ -10,8 +10,9 @@ class OLOpenedxFeedbackConfig(AppConfig): Trigger-only plugin: the feedback trigger is registered via the ``xblock_asides.v1`` entry point (no URLs or models — feedback is persisted - in mit-learn and the MFE owns the submit URL). The only Django settings are - the plugin defaults populated by ``settings.common``. + in mit-learn and the MFE owns the submit URL). LMS-only, since the trigger + renders in the learner (student) view; the only Django settings are the + plugin defaults populated by ``settings.common``. """ name = "ol_openedx_feedback" @@ -22,8 +23,5 @@ class OLOpenedxFeedbackConfig(AppConfig): ProjectType.LMS: { SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: "settings.common"}, }, - ProjectType.CMS: { - SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: "settings.common"}, - }, }, } diff --git a/src/ol_openedx_feedback/pyproject.toml b/src/ol_openedx_feedback/pyproject.toml index 903e9f284..3ffd73047 100644 --- a/src/ol_openedx_feedback/pyproject.toml +++ b/src/ol_openedx_feedback/pyproject.toml @@ -18,9 +18,6 @@ ol_openedx_feedback = "ol_openedx_feedback.block:FeedbackAside" [project.entry-points."lms.djangoapp"] ol_openedx_feedback = "ol_openedx_feedback.apps:OLOpenedxFeedbackConfig" -[project.entry-points."cms.djangoapp"] -ol_openedx_feedback = "ol_openedx_feedback.apps:OLOpenedxFeedbackConfig" - [build-system] requires = ["hatchling"] build-backend = "hatchling.build" From 4584903a2eee1e8214afa36f91c7a79f0bca4414 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Tue, 21 Jul 2026 17:09:00 +0500 Subject: [PATCH 6/8] fix: keep video Staff Debug button visible from the feedback plugin Handle the AskTIM chat container's whitesmoke box-shadow that covers the platform "Staff Debug Info" link on video blocks within feedback.css, so the fix holds independent of the installed ol_openedx_chat version. Give the staff-info row its own stacking context and matching background, scoped to video blocks only. Co-Authored-By: Claude Opus 4.7 --- .../ol_openedx_feedback/static/css/feedback.css | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css index 5fd4507ab..1f2657262 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css @@ -68,3 +68,19 @@ height: 18px; display: block; } + +/* On video blocks the AskTIM chat container (.video-block-chat-button-container) + paints an opaque whitesmoke background plus an upward box-shadow inside its own + stacking context, which rides up over the platform's preceding "Staff Debug + Info" link (.wrap-instructor-info) and hides it for staff. Fix it here in the + feedback plugin so it holds regardless of the installed chat-plugin version. + The staff-info row is the adjacent sibling of the block's wrapper + (data-block-type="video"), so scope to video only: give the row its own + stacking context above the paint, and match the chat container's whitesmoke + background so it blends into the section instead of showing a white patch. + Problem/HTML rows are untouched. */ +[data-block-type="video"] + .wrap-instructor-info { + position: relative; + z-index: 2; + background: whitesmoke; +} From 1a086bfd000904a1bfedd2150784f2046984ea58 Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Tue, 21 Jul 2026 17:40:59 +0500 Subject: [PATCH 7/8] fix: stop problem line-mask from overlapping AskTIM button border The docked line-mask box-shadow projected 12px of white but the gap to the AskTIM button is only 10px, so it painted over the button's left border. Match the offset to the gap so the mask fills it without overlap. Co-Authored-By: Claude Opus 4.7 --- .../ol_openedx_feedback/static/css/feedback.css | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css index 1f2657262..488857b39 100644 --- a/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css +++ b/src/ol_openedx_feedback/ol_openedx_feedback/static/css/feedback.css @@ -26,9 +26,11 @@ /* Problem blocks draw a horizontal "saved/submit" notification line behind the button row. Project an opaque white block across the gap to the AskTIM button so the line is masked there too (invisible on the white problem-block - background). Not applied to video/html blocks, which have no such line. */ + background). Not applied to video/html blocks, which have no such line. + Offset must match the docked gap (margin-right above) so the mask fills the + gap but stops at the AskTIM button's edge instead of overlapping its border. */ .ol-feedback-anchor--line-masked { - box-shadow: 12px 0 0 0 #fff; + box-shadow: 10px 0 0 0 #fff; } .ol-feedback-trigger { From ecb3ad892c2618c125f88fab05cec27db690e16f Mon Sep 17 00:00:00 2001 From: zamanafzal Date: Tue, 21 Jul 2026 17:58:16 +0500 Subject: [PATCH 8/8] docs: add step to enable XBlock asides in LMS admin Asides only render when XBlockAsidesConfig is enabled (off by default), so document enabling it and checking the disabled_blocks list as part of installation. Co-Authored-By: Claude Opus 4.7 --- src/ol_openedx_feedback/README.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/ol_openedx_feedback/README.rst b/src/ol_openedx_feedback/README.rst index b90acebaf..d52e20a4c 100644 --- a/src/ol_openedx_feedback/README.rst +++ b/src/ol_openedx_feedback/README.rst @@ -35,6 +35,16 @@ to ``INSTALLED_APPS`` are required. Restart the LMS after installing. (The trigger is learner-facing only, so the plugin is LMS-only and is not installed in Studio/CMS.) +Enable XBlock asides in the LMS admin +------------------------------------- + +XBlock asides must be turned on for any aside (including this one) to render. +In the LMS Django admin, open **XBlock Asides Config** +(``/admin/lms_xblock/xblockasidesconfig/``), add a new entry, and check +**Enabled**. Make sure the block types you want the feedback trigger on are +**not** listed in **Disabled blocks** (the space-separated field defaults to +``about course_info static_tab``). + Enablement ==========