From e2a14ec340c63c328cae9504c02394c58b1c894a Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Fri, 21 Nov 2025 10:51:15 +0500 Subject: [PATCH 1/6] refactor: Remove parsel dependency and update URL handling in ImageExplorerBlock - Replaced the `_change_relative_url_to_absolute` method with `_replace_relative_static_urls` to utilize a service for URL replacement. - Updated JavaScript to notify runtime on save completion. - Removed parsel from requirements as it is no longer needed. - Upgraded various dependencies in requirements files. --- image_explorer/image_explorer.py | 35 +++++-------------- .../public/js/image_explorer_edit.js | 1 + requirements/base.in | 1 - requirements/base.txt | 20 +++-------- requirements/constraints.txt | 16 --------- requirements/test.txt | 22 +++--------- tests/test_image_explorer.py | 22 ++++++------ 7 files changed, 28 insertions(+), 89 deletions(-) diff --git a/image_explorer/image_explorer.py b/image_explorer/image_explorer.py index abb8b6f..878baec 100644 --- a/image_explorer/image_explorer.py +++ b/image_explorer/image_explorer.py @@ -33,7 +33,6 @@ from django.conf import settings from lxml import etree, html -from parsel import Selector from xblock.completable import XBlockCompletionMode from xblock.core import XBlock from xblock.fragment import Fragment @@ -45,6 +44,7 @@ log = logging.getLogger(__name__) +@XBlock.wants('replace_urls') @XBlock.needs('i18n') class ImageExplorerBlock(XBlock): """ @@ -210,7 +210,7 @@ def student_view_data(self, context=None): description = self._get_description(xmltree, absolute_urls=True) background = self._get_background(xmltree) - background['src'] = self._replace_static_from_url(background['src']) + background['src'] = self._replace_relative_static_urls(background['src']) hotspots = self._get_hotspots(xmltree, absolute_urls=True) return { 'description': description, @@ -327,25 +327,6 @@ def _get_background(xmltree): 'height': background.get('height') }) - def _replace_static_from_url(self, url): - if not url: - return url - try: - from static_replace import replace_static_urls - except ImportError: - return url - - url = '"{}"'.format(url) - lms_relative_url = replace_static_urls(url, course_id=self.course_id) # pylint: disable=no-member - lms_relative_url = lms_relative_url.strip('"') - return self._make_url_absolute(lms_relative_url) - - @staticmethod - def _make_url_absolute(url): - lms_base = settings.ENV_TOKENS.get('LMS_BASE') - scheme = 'https' if settings.HTTPS == 'on' else 'http' - lms_base = '{}://{}'.format(scheme, lms_base) - return urljoin(lms_base, url) def _inner_content(self, tag, absolute_urls=False): """ @@ -354,7 +335,7 @@ def _inner_content(self, tag, absolute_urls=False): if tag is not None: tag_content = ''.join([ html.tostring(e, encoding=str) for e in tag ]) if absolute_urls: - return self._change_relative_url_to_absolute(tag_content) + return self._replace_relative_static_urls(tag_content) return tag_content return None @@ -368,11 +349,11 @@ def _get_description(self, xmltree, absolute_urls=False): return description return None - def _change_relative_url_to_absolute(self, text): - if text: - relative_urls = Selector(text=text).css('::attr(href),::attr(src)').extract() - for url in relative_urls: - text = text.replace(url, self._replace_static_from_url(url)) + def _replace_relative_static_urls(self, text): + if replace_urls_service := self.runtime.service(self, 'replace_urls'): + text = replace_urls_service.replace_urls(text) + else: + log.error('Unable to perform URL substitution on the following content: %s', text) return text def _get_hotspots(self, xmltree, absolute_urls=False): diff --git a/image_explorer/public/js/image_explorer_edit.js b/image_explorer/public/js/image_explorer_edit.js index c9776fb..2d4fb04 100644 --- a/image_explorer/public/js/image_explorer_edit.js +++ b/image_explorer/public/js/image_explorer_edit.js @@ -13,6 +13,7 @@ function ImageExplorerEditBlock(runtime, element) { $('.xblock-editor-error-message', element).css('display', 'none'); $.post(handlerUrl, JSON.stringify(data)).done(function(response) { if (response.result === 'success') { + runtime.notify('save', {state: 'end'}) window.location.reload(false); } else { $('.xblock-editor-error-message', element).html('Error: '+response.message); diff --git a/requirements/base.in b/requirements/base.in index 99aae93..098b2fa 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -2,4 +2,3 @@ -c constraints.txt xblock[django] -parsel diff --git a/requirements/base.txt b/requirements/base.txt index 386b876..6aca6cc 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,18 +6,15 @@ # appdirs==1.4.4 # via fs +asgiref==3.11.0 asgiref==3.11.0 # via django -boto3==1.41.2 +boto3==1.41.0 # via fs-s3fs -botocore==1.41.2 +botocore==1.41.0 # via # boto3 # s3transfer -cssselect==1.2.0 - # via - # -c requirements/constraints.txt - # parsel django==5.2.8 # via # -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt @@ -36,9 +33,7 @@ jmespath==1.0.1 lazy==1.6 # via xblock lxml==6.0.2 - # via - # parsel - # xblock + # via xblock mako==1.3.10 # via xblock markupsafe==3.0.3 @@ -47,10 +42,6 @@ markupsafe==3.0.3 # xblock openedx-django-pyfs==3.8.0 # via xblock -parsel==1.6.0 - # via - # -c requirements/constraints.txt - # -r requirements/base.in python-dateutil==2.9.0.post0 # via # botocore @@ -67,14 +58,11 @@ six==1.17.0 # via # fs # fs-s3fs - # parsel # python-dateutil sqlparse==0.5.3 # via django urllib3==2.5.0 # via botocore -w3lib==2.3.1 - # via parsel web-fragments==3.1.0 # via xblock webob==1.8.9 diff --git a/requirements/constraints.txt b/requirements/constraints.txt index ddf179d..a737d6f 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -10,19 +10,3 @@ # Common constraints for edx repos -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt -# -# Parsel needs to know the lxml version https://github.com/scrapy/parsel/blob/master/parsel/selector.py#L35. -# Since this information was not being passed and etree flavor of openedx doesn't open version for outside -# we had to pin parsel which doesn't have this code branch. -# -# And this version of parsel has cssselect which doesn't expose -# '_unicode_safe_getattr' hence we had to pin cssselect to the required version. -# -# This issue has been explained in https://github.com/openedx/xblock-image-explorer/pull/195#issuecomment-2844971682 -# -# This can be removed once we resolve -# https://github.com/openedx/xblock-image-explorer/issues/197#issue-3048741312, -# one of the ways to do is to remove the dependency on Parsel as explained in -# https://github.com/openedx/xblock-image-explorer/pull/195#pullrequestreview-2808751843 -parsel==v1.6 -cssselect==v1.2 diff --git a/requirements/test.txt b/requirements/test.txt index 583b8b6..ac78481 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -10,6 +10,7 @@ appdirs==1.4.4 # fs arrow==1.4.0 # via cookiecutter +asgiref==3.11.0 asgiref==3.11.0 # via # -r requirements/base.txt @@ -20,21 +21,23 @@ astroid==3.3.11 # pylint-celery binaryornot==0.4.4 # via cookiecutter -boto3==1.41.2 +boto3==1.41.0 # via # -r requirements/base.txt # fs-s3fs -botocore==1.41.2 +botocore==1.41.0 # via # -r requirements/base.txt # boto3 # s3transfer +certifi==2025.11.12 certifi==2025.11.12 # via requests chardet==5.2.0 # via binaryornot charset-normalizer==3.4.4 # via requests +click==8.3.1 click==8.3.1 # via # click-log @@ -49,11 +52,6 @@ cookiecutter==2.6.0 # via xblock-sdk coverage[toml]==7.12.0 # via pytest-cov -cssselect==1.2.0 - # via - # -c requirements/constraints.txt - # -r requirements/base.txt - # parsel dill==0.4.0 # via pylint # via @@ -96,7 +94,6 @@ lazy==1.6 lxml==6.0.2 # via # -r requirements/base.txt - # parsel # xblock # xblock-sdk mako==1.3.10 @@ -123,10 +120,6 @@ openedx-django-pyfs==3.8.0 # xblock packaging==25.0 # via pytest -parsel==1.6.0 - # via - # -c requirements/constraints.txt - # -r requirements/base.txt platformdirs==4.5.0 # via pylint pluggy==1.6.0 @@ -204,7 +197,6 @@ six==1.17.0 # edx-lint # fs # fs-s3fs - # parsel # python-dateutil sqlparse==0.5.3 # via @@ -223,10 +215,6 @@ urllib3==2.5.0 # -r requirements/base.txt # botocore # requests -w3lib==2.3.1 - # via - # -r requirements/base.txt - # parsel web-fragments==3.1.0 # via # -r requirements/base.txt diff --git a/tests/test_image_explorer.py b/tests/test_image_explorer.py index 1d64556..c150aa0 100644 --- a/tests/test_image_explorer.py +++ b/tests/test_image_explorer.py @@ -1,9 +1,8 @@ import unittest +from unittest.mock import Mock from mock import patch +import re from lxml import etree - -from parsel import Selector - from django.test import override_settings from xblock.field_data import DictFieldData @@ -24,7 +23,7 @@ def setUp(self): self.runtime = MockRuntime() patch_static_replace_module() - self.processed_absolute_url = 'https://lms/a/dynamic/url' + self.processed_absolute_url = '/course/test-course/assets/test.jpg' self.image_url = '/static/test.jpg' self.image_explorer_description = '

Test Descrption

' self.image_explorer_xml = """ @@ -67,6 +66,11 @@ def setUp(self): None ) self.image_explorer_block.course_id = 'abc/xyz/123' + self.runtime._services['replace_urls'] = Mock( + replace_urls=lambda html, static_replace_only=False: re.sub( + r'/static/([^"\s]*)', r'/course/test-course/assets/\1', html + ) + ) @override_settings(ENV_TOKENS={'LMS_BASE': 'lms'}, HTTPS='on') def test_student_view_data(self): @@ -94,14 +98,8 @@ def test_static_urls_conversion(self): Test static urls are processed to absolute if `absolute_urls` is set """ - xmltree = etree.fromstring(self.image_explorer_xml) - description = self.image_explorer_block._inner_content( - xmltree.find('description'), absolute_urls=True - ) - - relative_urls = Selector(text=description).css('::attr(href),::attr(src)').extract() - for url in relative_urls: - self.assertEqual(url, self.processed_absolute_url) + relative_url = self.image_explorer_block._replace_relative_static_urls(self.image_url) + self.assertEqual(relative_url, self.processed_absolute_url) def test_student_view_multi_device_support(self): """ From ed994b2a115b17c9020cdee69449572e5bc94df7 Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Mon, 24 Nov 2025 13:49:01 +0500 Subject: [PATCH 2/6] fix: render buttons in LMS These buttons need to be loaded in both lms and studio explanation: https://github.com/openedx/xblock-image-explorer/pull/83#issuecomment-741960809 --- image_explorer/public/js/image_explorer.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/image_explorer/public/js/image_explorer.js b/image_explorer/public/js/image_explorer.js index c4e2679..cbd99b9 100644 --- a/image_explorer/public/js/image_explorer.js +++ b/image_explorer/public/js/image_explorer.js @@ -1,12 +1,11 @@ function applyHotspotButtonStyle(element, data) { + console.log("data", data['hotspot_image']); $(".image-explorer-wrapper button.image-explorer-hotspot", element).css("background", "url("+data['hotspot_image']+") no-repeat scroll 0 0 rgba(0, 0, 0, 0)"); } function ImageExplorerBlock(runtime, element, data) { - if (data['authoring_view'] === 'true') { - applyHotspotButtonStyle(element, data); - } + applyHotspotButtonStyle(element, data); var hotspot_opened_at = null; var active_feedback = null; // Holds a reference to YouTube API Player objects. From cfb3770d4b6addee785c333c31bae25ae9d808b7 Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Mon, 24 Nov 2025 16:11:05 +0500 Subject: [PATCH 3/6] fix: remove console log --- image_explorer/public/js/image_explorer.js | 1 - 1 file changed, 1 deletion(-) diff --git a/image_explorer/public/js/image_explorer.js b/image_explorer/public/js/image_explorer.js index cbd99b9..0ab68df 100644 --- a/image_explorer/public/js/image_explorer.js +++ b/image_explorer/public/js/image_explorer.js @@ -1,5 +1,4 @@ function applyHotspotButtonStyle(element, data) { - console.log("data", data['hotspot_image']); $(".image-explorer-wrapper button.image-explorer-hotspot", element).css("background", "url("+data['hotspot_image']+") no-repeat scroll 0 0 rgba(0, 0, 0, 0)"); } From 28d17e1594f67e64f41788fc0e828f4a1c9f1666 Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Mon, 24 Nov 2025 16:17:36 +0500 Subject: [PATCH 4/6] refactor: improve test case --- tests/test_image_explorer.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_image_explorer.py b/tests/test_image_explorer.py index c150aa0..c881a6d 100644 --- a/tests/test_image_explorer.py +++ b/tests/test_image_explorer.py @@ -1,5 +1,6 @@ import unittest from unittest.mock import Mock +import xml from mock import patch import re from lxml import etree @@ -98,7 +99,11 @@ def test_static_urls_conversion(self): Test static urls are processed to absolute if `absolute_urls` is set """ - relative_url = self.image_explorer_block._replace_relative_static_urls(self.image_url) + xmltree = etree.fromstring(self.image_explorer_xml) + description = self.image_explorer_block._inner_content( + xmltree.find('description'), absolute_urls=True + ) + relative_url = etree.HTML(description).find('.//img').get('src') self.assertEqual(relative_url, self.processed_absolute_url) def test_student_view_multi_device_support(self): From e17aaa4603300cfb984bba3ee0908a4cc8706f81 Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Mon, 24 Nov 2025 17:05:16 +0500 Subject: [PATCH 5/6] chore: remove unintentional changes from deps --- requirements/base.txt | 5 ++--- requirements/test.txt | 7 ++----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/requirements/base.txt b/requirements/base.txt index 6aca6cc..05a6e29 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -6,12 +6,11 @@ # appdirs==1.4.4 # via fs -asgiref==3.11.0 asgiref==3.11.0 # via django -boto3==1.41.0 +boto3==1.41.2 # via fs-s3fs -botocore==1.41.0 +botocore==1.41.2 # via # boto3 # s3transfer diff --git a/requirements/test.txt b/requirements/test.txt index ac78481..b1a74da 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -10,7 +10,6 @@ appdirs==1.4.4 # fs arrow==1.4.0 # via cookiecutter -asgiref==3.11.0 asgiref==3.11.0 # via # -r requirements/base.txt @@ -21,23 +20,21 @@ astroid==3.3.11 # pylint-celery binaryornot==0.4.4 # via cookiecutter -boto3==1.41.0 +boto3==1.41.2 # via # -r requirements/base.txt # fs-s3fs -botocore==1.41.0 +botocore==1.41.2 # via # -r requirements/base.txt # boto3 # s3transfer -certifi==2025.11.12 certifi==2025.11.12 # via requests chardet==5.2.0 # via binaryornot charset-normalizer==3.4.4 # via requests -click==8.3.1 click==8.3.1 # via # click-log From 891afeec503d5da7fd4f766c640b13c8f8fa1141 Mon Sep 17 00:00:00 2001 From: "waleed.mujahid" Date: Fri, 16 Jan 2026 10:51:35 +0500 Subject: [PATCH 6/6] chore: bump version to 3.1.0 and clean up imports --- image_explorer/__init__.py | 2 +- image_explorer/image_explorer.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/image_explorer/__init__.py b/image_explorer/__init__.py index ddabe04..464bf53 100644 --- a/image_explorer/__init__.py +++ b/image_explorer/__init__.py @@ -22,4 +22,4 @@ """ from .image_explorer import ImageExplorerBlock -__version__ = '3.0.2' +__version__ = '3.1.0' diff --git a/image_explorer/image_explorer.py b/image_explorer/image_explorer.py index 878baec..d384d00 100644 --- a/image_explorer/image_explorer.py +++ b/image_explorer/image_explorer.py @@ -29,9 +29,7 @@ import logging import textwrap from io import StringIO -from urllib.parse import urljoin -from django.conf import settings from lxml import etree, html from xblock.completable import XBlockCompletionMode from xblock.core import XBlock