From a5fd2025dcf850dcaaa00b96cf7ed94188b57cb3 Mon Sep 17 00:00:00 2001 From: salman2013 Date: Tue, 14 Apr 2026 12:37:21 +0500 Subject: [PATCH 1/3] fix: fix the instructor tab issue for feedback xblock --- src/feedback/extensions/filters.py | 4 +- src/feedback/feedbacktests/test_utils.py | 64 ++++++++++++++++++++++++ src/feedback/settings/test.py | 2 + src/feedback/utils.py | 34 +++++++++++++ 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 src/feedback/feedbacktests/test_utils.py diff --git a/src/feedback/extensions/filters.py b/src/feedback/extensions/filters.py index b015e87..42e24d2 100644 --- a/src/feedback/extensions/filters.py +++ b/src/feedback/extensions/filters.py @@ -11,10 +11,12 @@ from web_fragments.fragment import Fragment try: - from cms.djangoapps.contentstore.utils import get_lms_link_for_item + # from cms.djangoapps.contentstore.utils import get_lms_link_for_item from lms.djangoapps.courseware.block_render import get_block_by_usage_id, load_single_xblock from openedx.core.djangoapps.enrollments.data import get_user_enrollments from xmodule.modulestore.django import modulestore + + from feedback.utils import get_lms_link_for_item except ImportError: load_single_xblock = None get_block_by_usage_id = None diff --git a/src/feedback/feedbacktests/test_utils.py b/src/feedback/feedbacktests/test_utils.py new file mode 100644 index 0000000..78aae70 --- /dev/null +++ b/src/feedback/feedbacktests/test_utils.py @@ -0,0 +1,64 @@ +import sys +import types +from unittest.mock import Mock + +from opaque_keys.edx.keys import UsageKey + +from feedback.utils import get_lms_link_for_item + + +def test_get_lms_link_importerror(monkeypatch): + location = Mock(spec=UsageKey) + location.org = "edX" + location.course_key = "course-v1:edX+DemoX+2024" + location.__str__ = lambda self=location: "dummy" + + if "openedx.core.djangoapps.site_configuration.models" in sys.modules: + del sys.modules["openedx.core.djangoapps.site_configuration.models"] + + monkeypatch.setattr("feedback.utils.settings", types.SimpleNamespace(LMS_ROOT_URL="https://example.com")) + + result = get_lms_link_for_item(location) + assert result is None + + +def test_get_lms_link_with_null_lms_base(monkeypatch): + location = Mock(spec=UsageKey) + location.org = "edX" + location.course_key = "dummy" + location.__str__ = lambda self=location: "dummy" + + class MockSiteConfiguration: + @staticmethod + def get_value_for_org(org, key, default): + return None # simulate LMS base not set + + monkeypatch.setitem( + sys.modules, + "openedx.core.djangoapps.site_configuration.models", + types.SimpleNamespace(SiteConfiguration=MockSiteConfiguration), + ) + + result = get_lms_link_for_item(location) + assert result is None + + +def test_get_lms_link_with_preview(monkeypatch): + location = Mock(spec=UsageKey) + location.org = "edX" + location.course_key = "course-v1:edX+DemoX+2024" + location.__str__ = lambda self=location: "dummy" + + class MockSiteConfiguration: + @staticmethod + def get_value_for_org(org, key, default): + return "https://fallback.com" + + monkeypatch.setitem( + sys.modules, + "openedx.core.djangoapps.site_configuration.models", + types.SimpleNamespace(SiteConfiguration=MockSiteConfiguration), + ) + + result = get_lms_link_for_item(location, preview=True) + assert result == "https://fallback.com/courses/course-v1:edX+DemoX+2024/jump_to/dummy?preview=1" diff --git a/src/feedback/settings/test.py b/src/feedback/settings/test.py index 843bee6..92b622c 100644 --- a/src/feedback/settings/test.py +++ b/src/feedback/settings/test.py @@ -23,3 +23,5 @@ } SECRET_KEY = "fake-key" + +LMS_ROOT_URL = "https://example.com" diff --git a/src/feedback/utils.py b/src/feedback/utils.py index 6b01fe2..20d78ad 100644 --- a/src/feedback/utils.py +++ b/src/feedback/utils.py @@ -1,6 +1,40 @@ """Utilities for feedback app""" +from urllib.parse import urlencode, urlparse, urlunparse + +from django.conf import settings +from opaque_keys.edx.keys import UsageKey + def _(text): """Dummy `gettext` replacement to make string extraction tools scrape strings marked for translation""" return text + + +def get_lms_link_for_item(location, preview=False): + """ + Returns an LMS link to the course with a jump_to to the provided location. + """ + assert isinstance(location, UsageKey) + + try: + # pylint: disable=import-outside-toplevel + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + + except ImportError: + return None + + lms_base = SiteConfiguration.get_value_for_org(location.org, "LMS_ROOT_URL", settings.LMS_ROOT_URL) + + if lms_base is None: + return None + + query_string = "" + if preview: + query_string = urlencode({"preview": "1"}) + + url_parts = list(urlparse(lms_base)) + url_parts[2] = f"/courses/{location.course_key}/jump_to/{location}" + url_parts[4] = query_string + + return urlunparse(url_parts) From b37f186fe8718c39e1f97b2d65a5783cbc8252a1 Mon Sep 17 00:00:00 2001 From: salman2013 Date: Tue, 14 Apr 2026 12:40:58 +0500 Subject: [PATCH 2/3] fix: remove commented code --- src/feedback/extensions/filters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/feedback/extensions/filters.py b/src/feedback/extensions/filters.py index 42e24d2..3c07eb4 100644 --- a/src/feedback/extensions/filters.py +++ b/src/feedback/extensions/filters.py @@ -11,7 +11,6 @@ from web_fragments.fragment import Fragment try: - # from cms.djangoapps.contentstore.utils import get_lms_link_for_item from lms.djangoapps.courseware.block_render import get_block_by_usage_id, load_single_xblock from openedx.core.djangoapps.enrollments.data import get_user_enrollments from xmodule.modulestore.django import modulestore From aeeb453133bbc8a1c74e5b0e555a055a87232f6f Mon Sep 17 00:00:00 2001 From: salman2013 Date: Wed, 15 Apr 2026 13:00:13 +0500 Subject: [PATCH 3/3] fix: Add warning to site configurations --- src/feedback/feedbacktests/test_utils.py | 7 +++++-- src/feedback/utils.py | 12 ++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/feedback/feedbacktests/test_utils.py b/src/feedback/feedbacktests/test_utils.py index 78aae70..eb51f25 100644 --- a/src/feedback/feedbacktests/test_utils.py +++ b/src/feedback/feedbacktests/test_utils.py @@ -13,8 +13,11 @@ def test_get_lms_link_importerror(monkeypatch): location.course_key = "course-v1:edX+DemoX+2024" location.__str__ = lambda self=location: "dummy" - if "openedx.core.djangoapps.site_configuration.models" in sys.modules: - del sys.modules["openedx.core.djangoapps.site_configuration.models"] + monkeypatch.setitem( + sys.modules, + "openedx.core.djangoapps.site_configuration.models", + None, + ) monkeypatch.setattr("feedback.utils.settings", types.SimpleNamespace(LMS_ROOT_URL="https://example.com")) diff --git a/src/feedback/utils.py b/src/feedback/utils.py index 20d78ad..7ac46b1 100644 --- a/src/feedback/utils.py +++ b/src/feedback/utils.py @@ -1,5 +1,6 @@ """Utilities for feedback app""" +import sys from urllib.parse import urlencode, urlparse, urlunparse from django.conf import settings @@ -17,12 +18,19 @@ def get_lms_link_for_item(location, preview=False): """ assert isinstance(location, UsageKey) + # Hack: Import SiteConfiguration from openedx-platform. + # Please note that XBlocks should not import core openedx-platform code. Do not + # replicate this pattern elsewhere. If you need information from the platform, it's better + # to catch an event, hook into a filter, or define and use an XBlock runtime service. try: # pylint: disable=import-outside-toplevel from openedx.core.djangoapps.site_configuration.models import SiteConfiguration - except ImportError: - return None + if "unittest" in sys.modules.keys(): + # Fail silently when testing. We can't install openedx-platform for tests. + return None + # Otherwise, fail loudly, so that we will notice if the openedx-platform import path changes. + raise lms_base = SiteConfiguration.get_value_for_org(location.org, "LMS_ROOT_URL", settings.LMS_ROOT_URL)