From 6cf2cd11f26ef4143f9b2433d4621dd45ce161c0 Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 17:55:52 -0500 Subject: [PATCH 1/9] fix: fixed the lms link imports to avoid a runtime error --- feedback/extensions/filters.py | 2 +- feedback/utils.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/feedback/extensions/filters.py b/feedback/extensions/filters.py index 2813907..88a3c44 100644 --- a/feedback/extensions/filters.py +++ b/feedback/extensions/filters.py @@ -9,7 +9,7 @@ from web_fragments.fragment import Fragment try: - from cms.djangoapps.contentstore.utils import get_lms_link_for_item + from feedback.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 diff --git a/feedback/utils.py b/feedback/utils.py index 6b01fe2..92af142 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -1,6 +1,39 @@ """Utilities for feedback app""" +# feedbackblock/feedback/utils.py +from urllib.parse import urlencode, urlparse, urlunparse +from opaque_keys.edx.keys import UsageKey +from django.conf import settings +from openedx.core.djangoapps.site_configuration.models import SiteConfiguration 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) + + lms_base = SiteConfiguration.get_value_for_org( + location.org, + "LMS_ROOT_URL", + settings.LMS_ROOT_URL + ) + query_string = '' + + if lms_base is None: + return None + + if preview: + query_string = urlencode({'preview': '1'}) + + url_parts = list(urlparse(lms_base)) + url_parts[2] = '/courses/{course_key}/jump_to/{location}'.format( + course_key=str(location.course_key), + location=str(location), + ) + url_parts[4] = query_string + + return urlunparse(url_parts) From e4d785903220cc609b5a5342602137c80143e31e Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 18:35:46 -0500 Subject: [PATCH 2/9] fix: fixed import error to get lms link function --- feedback/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/feedback/utils.py b/feedback/utils.py index 92af142..624935d 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -6,10 +6,12 @@ from django.conf import settings from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + 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. From bdc932df80b55c2ba5c21f0a097600f228ae88b1 Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 18:49:21 -0500 Subject: [PATCH 3/9] chore: fixed unit test import --- feedback/utils.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/feedback/utils.py b/feedback/utils.py index 624935d..c2ddb63 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -4,7 +4,6 @@ from urllib.parse import urlencode, urlparse, urlunparse from opaque_keys.edx.keys import UsageKey from django.conf import settings -from openedx.core.djangoapps.site_configuration.models import SiteConfiguration def _(text): @@ -18,24 +17,26 @@ def get_lms_link_for_item(location, preview=False): """ assert isinstance(location, UsageKey) + try: + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + except ImportError: + return None # or raise a clearer error, or fallback + lms_base = SiteConfiguration.get_value_for_org( location.org, "LMS_ROOT_URL", settings.LMS_ROOT_URL ) - query_string = '' 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] = '/courses/{course_key}/jump_to/{location}'.format( - course_key=str(location.course_key), - location=str(location), - ) + url_parts[2] = f'/courses/{location.course_key}/jump_to/{location}' url_parts[4] = query_string return urlunparse(url_parts) From 46e7e3da89370b93405be1f0c56a7c059c7265eb Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 18:52:18 -0500 Subject: [PATCH 4/9] chore: disable pylint warning --- feedback/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/feedback/utils.py b/feedback/utils.py index c2ddb63..0dd0b14 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -18,7 +18,7 @@ def get_lms_link_for_item(location, preview=False): assert isinstance(location, UsageKey) try: - from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration # pylint: disable=import-outside-toplevel except ImportError: return None # or raise a clearer error, or fallback From 08c58d5b5789aebd0efbb75a4096264bf1b7a94f Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 18:56:49 -0500 Subject: [PATCH 5/9] chore: fixed quality test --- feedback/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/feedback/utils.py b/feedback/utils.py index 0dd0b14..92c152f 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -18,7 +18,9 @@ def get_lms_link_for_item(location, preview=False): assert isinstance(location, UsageKey) try: - from openedx.core.djangoapps.site_configuration.models import SiteConfiguration # pylint: disable=import-outside-toplevel + # pylint: disable=import-outside-toplevel + from openedx.core.djangoapps.site_configuration.models import SiteConfiguration + except ImportError: return None # or raise a clearer error, or fallback From a03dac8b1efc92385d545fc6b9cc2f5f4a199814 Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 19:54:31 -0500 Subject: [PATCH 6/9] chore: added unit test for new function --- feedbacktests/test_utils.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 feedbacktests/test_utils.py diff --git a/feedbacktests/test_utils.py b/feedbacktests/test_utils.py new file mode 100644 index 0000000..e1e5269 --- /dev/null +++ b/feedbacktests/test_utils.py @@ -0,0 +1,27 @@ +from feedback.utils import get_lms_link_for_item +from opaque_keys.edx.keys import UsageKey +from unittest.mock import Mock + + +def test_get_lms_link_default(monkeypatch): + location = Mock(spec=UsageKey) + location.org = "edX" + location.course_key = "course-v1:edX+DemoX+2024" + location.__str__ = lambda self=location: "dummy" + + class MockSettings: + LMS_ROOT_URL = "https://example.com" + monkeypatch.setattr("feedback.utils.settings", MockSettings) + + class MockSiteConfiguration: + @staticmethod + def get_value_for_org(org, key, default): + return default # simulate missing config + monkeypatch.setitem( + __import__("sys").modules, + "openedx.core.djangoapps.site_configuration.models", + __import__("types").SimpleNamespace( + SiteConfiguration=MockSiteConfiguration)) + + result = get_lms_link_for_item(location) + assert result == "https://example.com/courses/course-v1:edX+DemoX+2024/jump_to/dummy" From 2913fb00253bd1ba8525c2264c1c1ec80a4a64fb Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 20:33:03 -0500 Subject: [PATCH 7/9] chore: added more tests for the new function --- feedback/settings/test.py | 2 ++ feedback/utils.py | 2 +- feedbacktests/test_utils.py | 58 ++++++++++++++++++++++++++++++++----- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/feedback/settings/test.py b/feedback/settings/test.py index 1277001..9f8c731 100644 --- a/feedback/settings/test.py +++ b/feedback/settings/test.py @@ -22,3 +22,5 @@ } SECRET_KEY = 'fake-key' + +LMS_ROOT_URL = "https://example.com" diff --git a/feedback/utils.py b/feedback/utils.py index 92c152f..4168f29 100644 --- a/feedback/utils.py +++ b/feedback/utils.py @@ -22,7 +22,7 @@ def get_lms_link_for_item(location, preview=False): from openedx.core.djangoapps.site_configuration.models import SiteConfiguration except ImportError: - return None # or raise a clearer error, or fallback + return None lms_base = SiteConfiguration.get_value_for_org( location.org, diff --git a/feedbacktests/test_utils.py b/feedbacktests/test_utils.py index e1e5269..63138bb 100644 --- a/feedbacktests/test_utils.py +++ b/feedbacktests/test_utils.py @@ -1,6 +1,8 @@ from feedback.utils import get_lms_link_for_item from opaque_keys.edx.keys import UsageKey from unittest.mock import Mock +import sys +import types def test_get_lms_link_default(monkeypatch): @@ -9,19 +11,59 @@ def test_get_lms_link_default(monkeypatch): location.course_key = "course-v1:edX+DemoX+2024" location.__str__ = lambda self=location: "dummy" - class MockSettings: - LMS_ROOT_URL = "https://example.com" - monkeypatch.setattr("feedback.utils.settings", MockSettings) - class MockSiteConfiguration: @staticmethod def get_value_for_org(org, key, default): - return default # simulate missing config + return default # simulate missing org-specific config + monkeypatch.setitem( - __import__("sys").modules, + sys.modules, "openedx.core.djangoapps.site_configuration.models", - __import__("types").SimpleNamespace( - SiteConfiguration=MockSiteConfiguration)) + types.SimpleNamespace(SiteConfiguration=MockSiteConfiguration) + ) result = get_lms_link_for_item(location) assert result == "https://example.com/courses/course-v1:edX+DemoX+2024/jump_to/dummy" + + +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" + From 85a68b402007f6969b84efd27527ba100648b44b Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 20:59:21 -0500 Subject: [PATCH 8/9] chore: added none import test case --- feedbacktests/test_utils.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/feedbacktests/test_utils.py b/feedbacktests/test_utils.py index 63138bb..05fc503 100644 --- a/feedbacktests/test_utils.py +++ b/feedbacktests/test_utils.py @@ -5,26 +5,19 @@ import types -def test_get_lms_link_default(monkeypatch): +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" - class MockSiteConfiguration: - @staticmethod - def get_value_for_org(org, key, default): - return default # simulate missing org-specific config + 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", - types.SimpleNamespace(SiteConfiguration=MockSiteConfiguration) - ) + monkeypatch.setattr("feedback.utils.settings", types.SimpleNamespace(LMS_ROOT_URL="https://example.com")) result = get_lms_link_for_item(location) - assert result == "https://example.com/courses/course-v1:edX+DemoX+2024/jump_to/dummy" - + assert result is None def test_get_lms_link_with_null_lms_base(monkeypatch): location = Mock(spec=UsageKey) From 794d7529a14bfa255609a568af1a50977bd4fd78 Mon Sep 17 00:00:00 2001 From: Andres Espinel Date: Mon, 9 Jun 2025 21:05:30 -0500 Subject: [PATCH 9/9] fix: fixed the lms link imports to avoid a runtime error