refactor: removes xqueue_callback_url_prefix argument
from the lms get_module_for_descriptor and related methods. This prefix was previously inferred from the request's base URL and passed through several methods to reach the XQueueService creation. But the request base URL always matches the LMS_ROOT_URL (or the preview URL), so this change simply uses that setting instead.
This commit is contained in:
committed by
Piotr Surowiec
parent
31827bc3b1
commit
2ef69d513f
@@ -364,21 +364,6 @@ def display_access_messages(user, block, view, frag, context): # pylint: disabl
|
||||
return msg_fragment
|
||||
|
||||
|
||||
def get_xqueue_callback_url_prefix(request):
|
||||
"""
|
||||
Calculates default prefix based on request, but allows override via settings
|
||||
|
||||
This is separated from get_module_for_descriptor so that it can be called
|
||||
by the LMS before submitting background tasks to run. The xqueue callbacks
|
||||
should go back to the LMS, not to the worker.
|
||||
"""
|
||||
prefix = '{proto}://{host}'.format(
|
||||
proto=request.META.get('HTTP_X_FORWARDED_PROTO', 'https' if request.is_secure() else 'http'),
|
||||
host=request.get_host()
|
||||
)
|
||||
return settings.XQUEUE_INTERFACE.get('callback_url', prefix)
|
||||
|
||||
|
||||
# pylint: disable=too-many-statements
|
||||
def get_module_for_descriptor(user, request, descriptor, field_data_cache, course_key,
|
||||
position=None, wrap_xmodule_display=True, grade_bucket_type=None,
|
||||
@@ -392,7 +377,6 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours
|
||||
See get_module() docstring for further details.
|
||||
"""
|
||||
track_function = make_track_function(request)
|
||||
xqueue_callback_url_prefix = get_xqueue_callback_url_prefix(request)
|
||||
|
||||
user_location = getattr(request, 'session', {}).get('country_code')
|
||||
|
||||
@@ -407,7 +391,6 @@ def get_module_for_descriptor(user, request, descriptor, field_data_cache, cours
|
||||
student_data=student_data,
|
||||
course_id=course_key,
|
||||
track_function=track_function,
|
||||
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
|
||||
position=position,
|
||||
wrap_xmodule_display=wrap_xmodule_display,
|
||||
grade_bucket_type=grade_bucket_type,
|
||||
@@ -427,7 +410,6 @@ def get_module_system_for_user(
|
||||
descriptor,
|
||||
course_id,
|
||||
track_function,
|
||||
xqueue_callback_url_prefix,
|
||||
request_token,
|
||||
position=None,
|
||||
wrap_xmodule_display=True,
|
||||
@@ -471,6 +453,7 @@ def get_module_system_for_user(
|
||||
dispatch=dispatch
|
||||
),
|
||||
)
|
||||
xqueue_callback_url_prefix = settings.XQUEUE_INTERFACE.get('callback_url', settings.LMS_ROOT_URL)
|
||||
return xqueue_callback_url_prefix + relative_xqueue_callback_url
|
||||
|
||||
# Default queuename is course-specific and is derived from the course that
|
||||
@@ -501,7 +484,6 @@ def get_module_system_for_user(
|
||||
student_data=student_data,
|
||||
course_id=course_id,
|
||||
track_function=track_function,
|
||||
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
|
||||
position=position,
|
||||
wrap_xmodule_display=wrap_xmodule_display,
|
||||
grade_bucket_type=grade_bucket_type,
|
||||
@@ -660,7 +642,6 @@ def get_module_system_for_user(
|
||||
descriptor=module,
|
||||
course_id=course_id,
|
||||
track_function=track_function,
|
||||
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
|
||||
position=position,
|
||||
wrap_xmodule_display=wrap_xmodule_display,
|
||||
grade_bucket_type=grade_bucket_type,
|
||||
@@ -845,7 +826,7 @@ def get_module_system_for_user(
|
||||
# TODO: Find all the places that this method is called and figure out how to
|
||||
# get a loaded course passed into it
|
||||
def get_module_for_descriptor_internal(user, descriptor, student_data, course_id,
|
||||
track_function, xqueue_callback_url_prefix, request_token,
|
||||
track_function, request_token,
|
||||
position=None, wrap_xmodule_display=True, grade_bucket_type=None,
|
||||
static_asset_path='', user_location=None, disable_staff_debug_info=False,
|
||||
course=None, will_recheck_access=False):
|
||||
@@ -864,7 +845,6 @@ def get_module_for_descriptor_internal(user, descriptor, student_data, course_id
|
||||
descriptor=descriptor,
|
||||
course_id=course_id,
|
||||
track_function=track_function,
|
||||
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
|
||||
position=position,
|
||||
wrap_xmodule_display=wrap_xmodule_display,
|
||||
grade_bucket_type=grade_bucket_type,
|
||||
|
||||
@@ -300,7 +300,6 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
|
||||
student_data=mock.Mock(name='student_data'),
|
||||
course_id=self.course.id,
|
||||
track_function=mock.Mock(name='track_function'),
|
||||
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
|
||||
request_token='request_token',
|
||||
)
|
||||
|
||||
@@ -321,7 +320,6 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
|
||||
student_data=mock.Mock(name='student_data'),
|
||||
course_id=self.course.id,
|
||||
track_function=mock.Mock(name='track_function'),
|
||||
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
|
||||
request_token='request_token',
|
||||
)
|
||||
|
||||
@@ -375,7 +373,6 @@ class TestXBlockInCourse(SharedModuleStoreTestCase):
|
||||
student_data=mock.Mock(name='student_data'),
|
||||
course_id=self.course.id,
|
||||
track_function=mock.Mock(name='track_function'),
|
||||
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
|
||||
request_token='request_token',
|
||||
)
|
||||
|
||||
@@ -447,7 +444,6 @@ class TestXBlockQueryLoad(SharedModuleStoreTestCase):
|
||||
student_data=mock.Mock(name='student_data'),
|
||||
course_id=course.id,
|
||||
track_function=mock.Mock(name='track_function'),
|
||||
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
|
||||
request_token='request_token',
|
||||
)
|
||||
with self.assertNumQueries(num_queries):
|
||||
|
||||
@@ -1931,7 +1931,6 @@ class TestAnonymousStudentId(SharedModuleStoreTestCase, LoginEnrollmentTestCase)
|
||||
student_data=Mock(spec=FieldData, name='student_data'),
|
||||
course_id=course_id,
|
||||
track_function=Mock(name='track_function'), # Track Function
|
||||
xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'), # XQueue Callback Url Prefix
|
||||
request_token='request_token',
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2291,7 +2290,6 @@ class LMSXBlockServiceBindingTest(SharedModuleStoreTestCase):
|
||||
self.user = UserFactory()
|
||||
self.student_data = Mock()
|
||||
self.track_function = Mock()
|
||||
self.xqueue_callback_url_prefix = Mock()
|
||||
self.request_token = Mock()
|
||||
|
||||
@XBlock.register_temp_plugin(PureXBlock, identifier='pure')
|
||||
@@ -2307,7 +2305,6 @@ class LMSXBlockServiceBindingTest(SharedModuleStoreTestCase):
|
||||
descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course
|
||||
)
|
||||
@@ -2326,7 +2323,6 @@ class LMSXBlockServiceBindingTest(SharedModuleStoreTestCase):
|
||||
descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course
|
||||
)
|
||||
@@ -2589,7 +2585,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.user = UserFactory(id=232)
|
||||
self.student_data = Mock()
|
||||
self.track_function = Mock()
|
||||
self.xqueue_callback_url_prefix = 'https://lms.url'
|
||||
self.request_token = Mock()
|
||||
|
||||
@ddt.data(
|
||||
@@ -2608,7 +2603,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2622,7 +2616,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2650,7 +2643,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2667,7 +2659,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.problem_descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2680,7 +2671,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2697,7 +2687,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2734,7 +2723,6 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
@@ -2748,14 +2736,46 @@ class LmsModuleSystemShimTest(SharedModuleStoreTestCase):
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.xqueue_callback_url_prefix,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
xqueue = runtime.xqueue
|
||||
assert isinstance(xqueue['interface'], XQueueInterface)
|
||||
assert xqueue['interface'].url == 'http://sandbox-xqueue.edx.org'
|
||||
assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest'
|
||||
assert xqueue['waittime'] == 5
|
||||
callback_url = f'https://lms.url/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/{self.descriptor.location}'
|
||||
callback_url = ('http://localhost:8000/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/'
|
||||
+ str(self.descriptor.location))
|
||||
assert xqueue['construct_callback']() == f'{callback_url}/score_update'
|
||||
assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch'
|
||||
|
||||
@override_settings(
|
||||
XQUEUE_INTERFACE={
|
||||
'callback_url': 'http://alt.url',
|
||||
'url': 'http://xqueue.url',
|
||||
'django_auth': {
|
||||
'username': 'user',
|
||||
'password': 'password',
|
||||
},
|
||||
'basic_auth': ('basic', 'auth'),
|
||||
},
|
||||
XQUEUE_WAITTIME_BETWEEN_REQUESTS=15,
|
||||
)
|
||||
def test_xqueue_settings(self):
|
||||
runtime, _ = render.get_module_system_for_user(
|
||||
self.user,
|
||||
self.student_data,
|
||||
self.descriptor,
|
||||
self.course.id,
|
||||
self.track_function,
|
||||
self.request_token,
|
||||
course=self.course,
|
||||
)
|
||||
xqueue = runtime.xqueue
|
||||
assert isinstance(xqueue['interface'], XQueueInterface)
|
||||
assert xqueue['interface'].url == 'http://xqueue.url'
|
||||
assert xqueue['default_queuename'] == 'edX-LmsModuleShimTest'
|
||||
assert xqueue['waittime'] == 15
|
||||
callback_url = f'http://alt.url/courses/edX/LmsModuleShimTest/2021_Fall/xqueue/232/{self.descriptor.location}'
|
||||
assert xqueue['construct_callback']() == f'{callback_url}/score_update'
|
||||
assert xqueue['construct_callback']('mock_dispatch') == f'{callback_url}/mock_dispatch'
|
||||
|
||||
@@ -17,7 +17,6 @@ from opaque_keys.edx.keys import UsageKey
|
||||
|
||||
from common.djangoapps.util.db import outer_atomic
|
||||
from lms.djangoapps.courseware.courses import get_problems_in_section
|
||||
from lms.djangoapps.courseware.module_render import get_xqueue_callback_url_prefix
|
||||
from lms.djangoapps.instructor_task.models import PROGRESS, InstructorTask
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
@@ -131,8 +130,7 @@ def _get_xmodule_instance_args(request, task_id):
|
||||
Calculate parameters needed for instantiating xmodule instances.
|
||||
|
||||
The `request_info` will be passed to a tracking log function, to provide information
|
||||
about the source of the task request. The `xqueue_callback_url_prefix` is used to
|
||||
permit old-style xqueue callbacks directly to the appropriate module in the LMS.
|
||||
about the source of the task request.
|
||||
The `task_id` is also passed to the tracking log function.
|
||||
"""
|
||||
request_info = {'username': request.user.username,
|
||||
@@ -142,8 +140,7 @@ def _get_xmodule_instance_args(request, task_id):
|
||||
'host': request.META['SERVER_NAME'],
|
||||
}
|
||||
|
||||
xmodule_instance_args = {'xqueue_callback_url_prefix': get_xqueue_callback_url_prefix(request),
|
||||
'request_info': request_info,
|
||||
xmodule_instance_args = {'request_info': request_info,
|
||||
'task_id': task_id,
|
||||
}
|
||||
return xmodule_instance_args
|
||||
|
||||
@@ -357,16 +357,12 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xmodule
|
||||
'''
|
||||
return lambda event_type, event: task_track(request_info, task_info, event_type, event, page='x_module_task')
|
||||
|
||||
xqueue_callback_url_prefix = xmodule_instance_args.get('xqueue_callback_url_prefix', '') \
|
||||
if xmodule_instance_args is not None else ''
|
||||
|
||||
return get_module_for_descriptor_internal(
|
||||
user=student,
|
||||
descriptor=module_descriptor,
|
||||
student_data=student_data,
|
||||
course_id=course_id,
|
||||
track_function=make_track_function(),
|
||||
xqueue_callback_url_prefix=xqueue_callback_url_prefix,
|
||||
grade_bucket_type=grade_bucket_type,
|
||||
# This module isn't being used for front-end rendering
|
||||
request_token=None,
|
||||
|
||||
@@ -83,7 +83,6 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
|
||||
Calculate dummy values for parameters needed for instantiating xmodule instances.
|
||||
"""
|
||||
return {
|
||||
'xqueue_callback_url_prefix': 'dummy_value',
|
||||
'request_info': {
|
||||
'username': 'dummy_username',
|
||||
'user_id': 'dummy_id',
|
||||
|
||||
Reference in New Issue
Block a user