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 abb8b6f..d384d00 100644 --- a/image_explorer/image_explorer.py +++ b/image_explorer/image_explorer.py @@ -29,11 +29,8 @@ import logging import textwrap from io import StringIO -from urllib.parse import urljoin -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 +42,7 @@ log = logging.getLogger(__name__) +@XBlock.wants('replace_urls') @XBlock.needs('i18n') class ImageExplorerBlock(XBlock): """ @@ -210,7 +208,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 +325,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 +333,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 +347,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.js b/image_explorer/public/js/image_explorer.js index c4e2679..0ab68df 100644 --- a/image_explorer/public/js/image_explorer.js +++ b/image_explorer/public/js/image_explorer.js @@ -4,9 +4,7 @@ function applyHotspotButtonStyle(element, data) { 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. 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..05a6e29 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -14,10 +14,6 @@ botocore==1.41.2 # 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 +32,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 +41,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 +57,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..b1a74da 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -49,11 +49,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 +91,6 @@ lazy==1.6 lxml==6.0.2 # via # -r requirements/base.txt - # parsel # xblock # xblock-sdk mako==1.3.10 @@ -123,10 +117,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 +194,6 @@ six==1.17.0 # edx-lint # fs # fs-s3fs - # parsel # python-dateutil sqlparse==0.5.3 # via @@ -223,10 +212,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..c881a6d 100644 --- a/tests/test_image_explorer.py +++ b/tests/test_image_explorer.py @@ -1,9 +1,9 @@ import unittest +from unittest.mock import Mock +import xml 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 +24,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 +67,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): @@ -98,10 +103,8 @@ def test_static_urls_conversion(self): 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 = etree.HTML(description).find('.//img').get('src') + self.assertEqual(relative_url, self.processed_absolute_url) def test_student_view_multi_device_support(self): """