Merge pull request #2425 from edx/usman/lms1199-make-histograms-optional
Added feature setting to disable histograms in Staff Debug Info panel.
This commit is contained in:
@@ -126,8 +126,12 @@ def replace_static_urls(data_dir, block, view, frag, context, course_id=None, st
|
||||
|
||||
|
||||
def grade_histogram(module_id):
|
||||
''' Print out a histogram of grades on a given problem.
|
||||
Part of staff member debug info.
|
||||
'''
|
||||
Print out a histogram of grades on a given problem in staff member debug info.
|
||||
|
||||
Warning: If a student has just looked at an xmodule and not attempted
|
||||
it, their grade is None. Since there will always be at least one such student
|
||||
this function almost always returns [].
|
||||
'''
|
||||
from django.db import connection
|
||||
cursor = connection.cursor()
|
||||
@@ -147,7 +151,7 @@ def grade_histogram(module_id):
|
||||
return grades
|
||||
|
||||
|
||||
def add_histogram(user, block, view, frag, context): # pylint: disable=unused-argument
|
||||
def add_staff_debug_info(user, block, view, frag, context): # pylint: disable=unused-argument
|
||||
"""
|
||||
Updates the supplied module with a new get_html function that wraps
|
||||
the output of the old get_html function with additional information
|
||||
@@ -161,7 +165,7 @@ def add_histogram(user, block, view, frag, context): # pylint: disable=unused-a
|
||||
return frag
|
||||
|
||||
block_id = block.id
|
||||
if block.has_score:
|
||||
if block.has_score and settings.FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'):
|
||||
histogram = grade_histogram(block_id)
|
||||
render_histogram = len(histogram) > 0
|
||||
else:
|
||||
|
||||
@@ -38,7 +38,7 @@ from xmodule.modulestore import Location
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
from xmodule.util.duedate import get_extended_due_date
|
||||
from xmodule_modifiers import replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_histogram, wrap_xblock
|
||||
from xmodule_modifiers import replace_course_urls, replace_jump_to_id_urls, replace_static_urls, add_staff_debug_info, wrap_xblock
|
||||
from xmodule.lti_module import LTIModule
|
||||
from xmodule.x_module import XModuleDescriptor
|
||||
|
||||
@@ -368,9 +368,9 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
|
||||
reverse('jump_to_id', kwargs={'course_id': course_id, 'module_id': ''}),
|
||||
))
|
||||
|
||||
if settings.FEATURES.get('DISPLAY_HISTOGRAMS_TO_STAFF'):
|
||||
if settings.FEATURES.get('DISPLAY_DEBUG_INFO_TO_STAFF'):
|
||||
if has_access(user, descriptor, 'staff', course_id):
|
||||
block_wrappers.append(partial(add_histogram, user))
|
||||
block_wrappers.append(partial(add_staff_debug_info, user))
|
||||
|
||||
# These modules store data using the anonymous_student_id as a key.
|
||||
# To prevent loss of data, we will continue to provide old modules with
|
||||
|
||||
@@ -12,6 +12,7 @@ from django.test import TestCase
|
||||
from django.test.client import RequestFactory
|
||||
from django.test.utils import override_settings
|
||||
|
||||
from capa.tests.response_xml_factory import OptionResponseXMLFactory
|
||||
from xblock.field_data import FieldData
|
||||
from xblock.runtime import Runtime
|
||||
from xblock.fields import ScopeIds
|
||||
@@ -21,14 +22,14 @@ from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory
|
||||
from xmodule.x_module import XModuleDescriptor
|
||||
import courseware.module_render as render
|
||||
|
||||
from courseware import module_render as render
|
||||
from courseware.courses import get_course_with_access, course_image_url, get_course_info_section
|
||||
from courseware.model_data import FieldDataCache
|
||||
from courseware.tests.factories import StudentModuleFactory, UserFactory
|
||||
from courseware.tests.tests import LoginEnrollmentTestCase
|
||||
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
|
||||
from courseware.model_data import FieldDataCache
|
||||
|
||||
from courseware.courses import get_course_with_access, course_image_url, get_course_info_section
|
||||
|
||||
from .factories import UserFactory
|
||||
from lms.lib.xblock.runtime import quote_slashes
|
||||
|
||||
|
||||
@@ -506,8 +507,42 @@ class TestHtmlModifiers(ModuleStoreTestCase):
|
||||
result_fragment.content
|
||||
)
|
||||
|
||||
@patch('courseware.module_render.has_access', Mock(return_value=True))
|
||||
def test_histogram(self):
|
||||
|
||||
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
|
||||
@patch.dict('django.conf.settings.FEATURES', {'DISPLAY_DEBUG_INFO_TO_STAFF': True, 'DISPLAY_HISTOGRAMS_TO_STAFF': True})
|
||||
@patch('courseware.module_render.has_access', Mock(return_value=True))
|
||||
class TestStaffDebugInfo(ModuleStoreTestCase):
|
||||
"""Tests to verify that Staff Debug Info panel and histograms are displayed to staff."""
|
||||
|
||||
def setUp(self):
|
||||
self.user = UserFactory.create()
|
||||
self.request = RequestFactory().get('/')
|
||||
self.request.user = self.user
|
||||
self.request.session = {}
|
||||
self.course = CourseFactory.create()
|
||||
|
||||
problem_xml = OptionResponseXMLFactory().build_xml(
|
||||
question_text='The correct answer is Correct',
|
||||
num_inputs=2,
|
||||
weight=2,
|
||||
options=['Correct', 'Incorrect'],
|
||||
correct_option='Correct'
|
||||
)
|
||||
self.descriptor = ItemFactory.create(
|
||||
category='problem',
|
||||
data=problem_xml,
|
||||
display_name='Option Response Problem'
|
||||
)
|
||||
|
||||
self.location = self.descriptor.location
|
||||
self.field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
|
||||
self.course.id,
|
||||
self.user,
|
||||
self.descriptor
|
||||
)
|
||||
|
||||
@patch.dict('django.conf.settings.FEATURES', {'DISPLAY_DEBUG_INFO_TO_STAFF': False})
|
||||
def test_staff_debug_info_disabled(self):
|
||||
module = render.get_module(
|
||||
self.user,
|
||||
self.request,
|
||||
@@ -516,11 +551,78 @@ class TestHtmlModifiers(ModuleStoreTestCase):
|
||||
self.course.id,
|
||||
)
|
||||
result_fragment = module.render('student_view')
|
||||
self.assertNotIn('Staff Debug', result_fragment.content)
|
||||
|
||||
self.assertIn(
|
||||
'Staff Debug',
|
||||
result_fragment.content
|
||||
def test_staff_debug_info_enabled(self):
|
||||
module = render.get_module(
|
||||
self.user,
|
||||
self.request,
|
||||
self.location,
|
||||
self.field_data_cache,
|
||||
self.course.id,
|
||||
)
|
||||
result_fragment = module.render('student_view')
|
||||
self.assertIn('Staff Debug', result_fragment.content)
|
||||
|
||||
@patch.dict('django.conf.settings.FEATURES', {'DISPLAY_HISTOGRAMS_TO_STAFF': False})
|
||||
def test_histogram_disabled(self):
|
||||
module = render.get_module(
|
||||
self.user,
|
||||
self.request,
|
||||
self.location,
|
||||
self.field_data_cache,
|
||||
self.course.id,
|
||||
)
|
||||
result_fragment = module.render('student_view')
|
||||
self.assertNotIn('histrogram', result_fragment.content)
|
||||
|
||||
def test_histogram_enabled_for_unscored_xmodules(self):
|
||||
"""Histograms should not display for xmodules which are not scored."""
|
||||
|
||||
html_descriptor = ItemFactory.create(
|
||||
category='html',
|
||||
data='Here are some course details.'
|
||||
)
|
||||
field_data_cache = FieldDataCache.cache_for_descriptor_descendents(
|
||||
self.course.id,
|
||||
self.user,
|
||||
self.descriptor
|
||||
)
|
||||
with patch('xmodule_modifiers.grade_histogram') as mock_grade_histogram:
|
||||
mock_grade_histogram.return_value = []
|
||||
module = render.get_module(
|
||||
self.user,
|
||||
self.request,
|
||||
html_descriptor.location,
|
||||
field_data_cache,
|
||||
self.course.id,
|
||||
)
|
||||
module.render('student_view')
|
||||
self.assertFalse(mock_grade_histogram.called)
|
||||
|
||||
def test_histogram_enabled_for_scored_xmodules(self):
|
||||
"""Histograms should display for xmodules which are scored."""
|
||||
|
||||
StudentModuleFactory.create(
|
||||
course_id=self.course.id,
|
||||
module_state_key=self.location,
|
||||
student=UserFactory(),
|
||||
grade=1,
|
||||
max_grade=1,
|
||||
state="{}",
|
||||
)
|
||||
with patch('xmodule_modifiers.grade_histogram') as mock_grade_histogram:
|
||||
mock_grade_histogram.return_value = []
|
||||
module = render.get_module(
|
||||
self.user,
|
||||
self.request,
|
||||
self.location,
|
||||
self.field_data_cache,
|
||||
self.course.id,
|
||||
)
|
||||
module.render('student_view')
|
||||
self.assertTrue(mock_grade_histogram.called)
|
||||
|
||||
|
||||
PER_COURSE_ANONYMIZED_DESCRIPTORS = (LTIDescriptor, )
|
||||
|
||||
|
||||
@@ -55,7 +55,10 @@ DISCUSSION_SETTINGS = {
|
||||
FEATURES = {
|
||||
'SAMPLE': False,
|
||||
'USE_DJANGO_PIPELINE': True,
|
||||
'DISPLAY_HISTOGRAMS_TO_STAFF': True,
|
||||
|
||||
'DISPLAY_DEBUG_INFO_TO_STAFF': True,
|
||||
'DISPLAY_HISTOGRAMS_TO_STAFF': False, # For large courses this slows down courseware access for staff.
|
||||
|
||||
'REROUTE_ACTIVATION_EMAIL': False, # nonempty string = address for all activation emails
|
||||
'DEBUG_LEVEL': 0, # 0 = lowest level, least verbose, 255 = max level, most verbose
|
||||
|
||||
|
||||
Reference in New Issue
Block a user