refactor: deprecate hostname attribute from ModuleSystem (#30308)

The hostname constructor argument of the XModule ModuleSystem is deprecated
in favour of directly accessing the LMS_BASE value from django.conf.settings.
This commit is contained in:
Arunmozhi
2022-06-09 03:49:58 +10:00
committed by GitHub
parent 8886f29e52
commit aedcd7910d
6 changed files with 36 additions and 13 deletions

View File

@@ -65,6 +65,7 @@ from urllib import parse
import bleach
import oauthlib.oauth1
from django.conf import settings
from lxml import etree
from oauthlib.oauth1.rfc5849 import signature
from pkg_resources import resource_string
@@ -584,7 +585,7 @@ class LTIBlock(
i4x-2-3-lti-31de800015cf4afb973356dbe81496df this part of resource_link_id:
makes resource_link_id to be unique among courses inside same system.
"""
return str(parse.quote(f"{self.system.hostname}-{self.location.html_id()}")) # lint-amnesty, pylint: disable=line-too-long
return str(parse.quote(f"{settings.LMS_BASE}-{self.location.html_id()}"))
def get_lis_result_sourcedid(self):
"""

View File

@@ -153,7 +153,6 @@ def get_test_system(
static_url='/static',
track_function=Mock(name='get_test_system.track_function'),
get_module=get_module,
hostname="edx.org",
services={
'user': user_service,
'mako': mako_service,

View File

@@ -3,12 +3,14 @@
import datetime
import textwrap
import unittest
from copy import copy
from unittest.mock import Mock, PropertyMock, patch
from urllib import parse
import pytest
from django.conf import settings
from django.test import TestCase, override_settings
from lxml import etree
from opaque_keys.edx.locator import BlockUsageLocator
from pytz import UTC
@@ -16,6 +18,7 @@ from webob.request import Request
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
from common.djangoapps.xblock_django.constants import ATTR_KEY_ANONYMOUS_USER_ID
from xmodule.fields import Timedelta
from xmodule.lti_2_util import LTIError
@@ -25,7 +28,8 @@ from xmodule.tests.helpers import StubUserService
from . import get_test_system
class LTIBlockTest(unittest.TestCase):
@override_settings(LMS_BASE="edx.org")
class LTIBlockTest(TestCase):
"""Logic tests for LTI module."""
def setUp(self):
@@ -69,8 +73,9 @@ class LTIBlockTest(unittest.TestCase):
current_user = self.system.service(self.xmodule, 'user').get_current_user()
self.user_id = current_user.opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID)
self.lti_id = self.xmodule.lti_id
self.unquoted_resource_link_id = '{}-i4x-2-3-lti-31de800015cf4afb973356dbe81496df'.format(
self.xmodule.runtime.hostname
settings.LMS_BASE
)
sourced_id = ':'.join(parse.quote(i) for i in (self.lti_id, self.unquoted_resource_link_id, self.user_id)) # lint-amnesty, pylint: disable=line-too-long

View File

@@ -1621,6 +1621,19 @@ class ModuleSystemShim:
DeprecationWarning, stacklevel=3
)
@property
def hostname(self):
"""
Hostname of the site as set in the Django settings `LMS_BASE`
Deprecated in favour of direct import of `django.conf.settings`
"""
warnings.warn(
'runtime.hostname is deprecated. Please use `LMS_BASE` from `django.conf.settings`.',
DeprecationWarning, stacklevel=3,
)
from django.conf import settings
return settings.LMS_BASE
class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim, Runtime):
"""
@@ -1636,11 +1649,18 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
"""
def __init__(
self, static_url, track_function, get_module,
descriptor_runtime, hostname="", publish=None,
course_id=None, error_descriptor_class=None,
field_data=None, rebind_noauth_module_to_user=None,
**kwargs):
self,
static_url,
track_function,
get_module,
descriptor_runtime,
publish=None,
course_id=None,
error_descriptor_class=None,
field_data=None,
rebind_noauth_module_to_user=None,
**kwargs,
):
"""
Create a closure around the system environment.
@@ -1678,7 +1698,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
self.STATIC_URL = static_url
self.track_function = track_function
self.get_module = get_module
self.HOSTNAME = self.hostname = hostname
self.course_id = course_id
if publish:

View File

@@ -729,7 +729,6 @@ def get_module_system_for_user(
static_url=settings.STATIC_URL,
get_module=inner_get_module,
user=user,
hostname=settings.SITE_NAME,
publish=publish,
course_id=course_id,
# TODO: When we merge the descriptor and module systems, we can stop reaching into the mixologist (cpennington)

View File

@@ -43,7 +43,7 @@ class TestLTI(BaseTestXmodule):
context_id = str(self.item_descriptor.course_id)
user_service = self.item_descriptor.xmodule_runtime.service(self.item_descriptor, 'user')
user_id = str(user_service.get_current_user().opt_attrs.get(ATTR_KEY_ANONYMOUS_USER_ID))
hostname = self.item_descriptor.xmodule_runtime.hostname
hostname = settings.LMS_BASE
resource_link_id = str(urllib.parse.quote(f'{hostname}-{self.item_descriptor.location.html_id()}'))
sourcedId = "{context}:{resource_link}:{user_id}".format(