Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 8 additions & 27 deletions image_explorer/image_explorer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -45,6 +44,7 @@
log = logging.getLogger(__name__)


@XBlock.wants('replace_urls')
@XBlock.needs('i18n')
class ImageExplorerBlock(XBlock):
"""
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
"""
Expand All @@ -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

Expand All @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions image_explorer/public/js/image_explorer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions image_explorer/public/js/image_explorer_edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion requirements/base.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
-c constraints.txt

xblock[django]
parsel
15 changes: 1 addition & 14 deletions requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
16 changes: 0 additions & 16 deletions requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 0 additions & 15 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -96,7 +91,6 @@ lazy==1.6
lxml==6.0.2
# via
# -r requirements/base.txt
# parsel
# xblock
# xblock-sdk
mako==1.3.10
Expand All @@ -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
Expand Down Expand Up @@ -204,7 +194,6 @@ six==1.17.0
# edx-lint
# fs
# fs-s3fs
# parsel
# python-dateutil
sqlparse==0.5.3
# via
Expand All @@ -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
Expand Down
19 changes: 11 additions & 8 deletions tests/test_image_explorer.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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 = '<p>Test Descrption</p><img src="/static/test.jpg" />'
self.image_explorer_xml = """
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
"""
Expand Down