refactor: remove debug property from ModuleSystem (#30450)

This also:
1. Removes this property from XBlock runtime shims.
2. Updates the minimum required version of the LTI Consumer XBlock.
This commit is contained in:
Demid
2022-06-08 19:59:45 +03:00
committed by GitHub
parent c54d8a81bf
commit 8886f29e52
9 changed files with 44 additions and 29 deletions

View File

@@ -212,7 +212,6 @@ def _preview_module_system(request, descriptor, field_data):
# TODO (cpennington): Do we want to track how instructors are using the preview problems?
track_function=lambda event_type, event: None,
get_module=partial(_load_preview_module, request),
debug=True,
mixins=settings.XBLOCK_MIXINS,
course_id=course_id,

View File

@@ -490,6 +490,15 @@ class ProblemBlock(
return self.display_name
@property
def debug(self):
"""
If CAPA block fails to render, we want course authors to be able to see
the error in Studio. At the same time, in production, we don't want
to show errors to students.
"""
return getattr(self.runtime, 'is_author_mode', False) or settings.DEBUG
@classmethod
def filter_templates(cls, template, course):
"""
@@ -825,7 +834,7 @@ class ProblemBlock(
cache=cache_service,
can_execute_unsafe_code=sandbox_service.can_execute_unsafe_code,
get_python_lib_zip=sandbox_service.get_python_lib_zip,
DEBUG=self.runtime.DEBUG,
DEBUG=self.debug,
i18n=self.runtime.service(self, "i18n"),
render_template=self.runtime.service(self, 'mako').render_template,
resources_fs=self.runtime.resources_fs,
@@ -1060,8 +1069,7 @@ class ProblemBlock(
str(err)
)
# TODO (vshnayder): another switch on DEBUG.
if self.runtime.DEBUG:
if self.debug:
msg = HTML(
'[courseware.capa.capa_module] '
'Failed to generate HTML for problem {url}'
@@ -1777,7 +1785,7 @@ class ProblemBlock(
self.set_last_submission_time()
except (StudentInputError, ResponseError, LoncapaProblemError) as inst:
if self.runtime.DEBUG:
if self.debug:
log.warning(
"StudentInputError in capa_module:problem_check",
exc_info=True
@@ -1811,7 +1819,7 @@ class ProblemBlock(
self.set_state_from_lcp()
self.set_score(self.score_from_lcp(self.lcp))
if self.runtime.DEBUG:
if self.debug:
msg = f"Error checking problem: {str(err)}"
msg += f'\nTraceback:\n{traceback.format_exc()}'
return {'success': msg}

View File

@@ -12,6 +12,7 @@ import re
from unittest import mock
from urllib import parse
from django.conf import settings
from oauthlib.oauth1 import Client
from webob import Response
from xblock.core import XBlock
@@ -63,7 +64,7 @@ class LTI20BlockMixin:
Returns:
webob.response: response to this request. See above for details.
"""
if self.system.debug:
if settings.DEBUG:
self._log_correct_authorization_header(request)
if not self.accept_grades_past_due and self.is_past_due():

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,
debug=True,
hostname="edx.org",
services={
'user': user_service,

View File

@@ -17,6 +17,7 @@ import ddt
import requests
import webob
from codejail.safe_exec import SafeExecException
from django.test import override_settings
from django.utils.encoding import smart_str
from edx_user_state_client.interface import XBlockUserState
from lxml import etree
@@ -961,6 +962,7 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# but that this was considered the second attempt for grading purposes
assert module.lcp.context['attempt'] == 2
@override_settings(DEBUG=True)
def test_submit_problem_other_errors(self):
"""
Test that errors other than the expected kinds give an appropriate message.
@@ -970,9 +972,6 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# Create the module
module = CapaFactory.create(attempts=1, user_is_staff=False)
# Ensure that DEBUG is on
module.system.DEBUG = True
# Simulate answering a problem that raises the exception
with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade:
error_msg = "Superterrible error happened: ☠"
@@ -1711,9 +1710,6 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# is asked to render itself as HTML
module.lcp.get_html = Mock(side_effect=Exception("Test"))
# Turn off DEBUG
module.system.DEBUG = False
# Try to render the module with DEBUG turned off
html = module.get_problem_html()
@@ -1727,6 +1723,31 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
# Expect that the module has created a new dummy problem with the error
assert original_problem != module.lcp
def test_get_problem_html_error_preview(self):
"""
Test the html response when an error occurs with DEBUG off in Studio.
"""
render_template = Mock(return_value="<div>Test Template HTML</div>")
module = CapaFactory.create(render_template=render_template)
# Simulate throwing an exception when the capa problem
# is asked to render itself as HTML
error_msg = "Superterrible error happened: ☠"
module.lcp.get_html = Mock(side_effect=Exception(error_msg))
module.system.is_author_mode = True
# Try to render the module with the author mode turned on
html = module.get_problem_html()
assert html is not None
# Check the rendering context
render_args, _ = render_template.call_args
context = render_args[1]
assert error_msg in context['problem']['html']
@override_settings(DEBUG=True)
def test_get_problem_html_error_w_debug(self):
"""
Test the html response when an error occurs with DEBUG on
@@ -1739,9 +1760,6 @@ class ProblemBlockTest(unittest.TestCase): # lint-amnesty, pylint: disable=miss
error_msg = "Superterrible error happened: ☠"
module.lcp.get_html = Mock(side_effect=Exception(error_msg))
# Make sure DEBUG is on
module.system.DEBUG = True
# Try to render the module with DEBUG turned on
html = module.get_problem_html()

View File

@@ -1637,7 +1637,7 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
def __init__(
self, static_url, track_function, get_module,
descriptor_runtime, debug=False, hostname="", publish=None,
descriptor_runtime, hostname="", publish=None,
course_id=None, error_descriptor_class=None,
field_data=None, rebind_noauth_module_to_user=None,
**kwargs):
@@ -1678,7 +1678,6 @@ class ModuleSystem(MetricsMixin, ConfigurableFragmentWrapper, ModuleSystemShim,
self.STATIC_URL = static_url
self.track_function = track_function
self.get_module = get_module
self.DEBUG = self.debug = debug
self.HOSTNAME = self.hostname = hostname
self.course_id = course_id

View File

@@ -729,7 +729,6 @@ def get_module_system_for_user(
static_url=settings.STATIC_URL,
get_module=inner_get_module,
user=user,
debug=settings.DEBUG,
hostname=settings.SITE_NAME,
publish=publish,
course_id=course_id,

View File

@@ -97,14 +97,6 @@ class RuntimeShim:
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
return False # Change this if/when we need to support unsafe courses in the new runtime.
@property
def DEBUG(self):
"""
Should DEBUG mode (?) be used? This flag is only read by capa.
"""
# TODO: Refactor capa to access this directly, don't bother the runtime. Then remove it from here.
return False
def get_python_lib_zip(self):
"""
A function returning a bytestring or None. The bytestring is the

View File

@@ -109,7 +109,7 @@ ipaddress # Ip network support for Embargo feature
jsonfield # Django model field for validated JSON; used in several apps
laboratory # Library for testing that code refactors/infrastructure changes produce identical results
lxml # XML parser
lti-consumer-xblock>=4.1.0
lti-consumer-xblock>=4.1.1
mailsnake # Needed for mailchimp (mailing djangoapp)
mako # Primary template language used for server-side page rendering
Markdown # Convert text markup to HTML; used in capa problems, forums, and course wikis