Skip to content

E
edx-platform
Commit 2428b3ca by brianhw
Options

Merge pull request #1524 from edx/rc-peer-linking 

Ensure that peer grading modules whose linked problem is removed change ...
parents 2673049c 9f9f5e4c
Showing with 136 additions and 67 deletions
common/lib/xmodule/xmodule/peer_grading_module.py
......@@ -104,30 +104,28 @@ class PeerGradingModule(PeerGradingFields, XModule):
def __init__(self, *args, **kwargs):
super(PeerGradingModule, self).__init__(*args, **kwargs)
#We need to set the location here so the child modules can use it
# Copy this to a new variable so that we can edit it if needed.
# We need to edit it if the linked module cannot be found, so
# we can revert to panel model.
self.use_for_single_location_local = self.use_for_single_location
# We need to set the location here so the child modules can use it.
self.runtime.set('location', self.location)
if (self.runtime.open_ended_grading_interface):
self.peer_gs = PeerGradingService(self.system.open_ended_grading_interface, self.system)
else:
self.peer_gs = MockPeerGradingService()
if self.use_for_single_location:
try:
linked_descriptors = self.descriptor.get_required_module_descriptors()
if len(linked_descriptors) == 0:
error_msg = "Peer grading module {0} is trying to use single problem mode without "
"a location specified.".format(self.location)
log.error(error_msg)
raise InvalidLinkLocation(error_msg)
if self.use_for_single_location_local:
linked_descriptors = self.descriptor.get_required_module_descriptors()
if len(linked_descriptors) == 0:
error_msg = "Peer grading module {0} is trying to use single problem mode without "
"a location specified.".format(self.location)
log.error(error_msg)
# Change module over to panel mode from single problem mode.
self.use_for_single_location_local = False
else:
self.linked_problem = self.system.get_module(linked_descriptors[0])
except ItemNotFoundError:
log.error("Linked location {0} for peer grading module {1} does not exist".format(
self.link_to_location, self.location))
raise
except NoPathToItem:
log.error("Linked location {0} for peer grading module {1} cannot be linked to.".format(
self.link_to_location, self.location))
raise
try:
self.timeinfo = TimeInfo(self.due, self.graceperiod)
......@@ -175,7 +173,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
"""
if self.closed():
return self.peer_grading_closed()
if not self.use_for_single_location:
if not self.use_for_single_location_local:
return self.peer_grading()
else:
return self.peer_grading_problem({'location': self.link_to_location})['html']
......@@ -236,7 +234,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
'score': score,
'total': max_score,
}
if not self.use_for_single_location or not self.graded:
if not self.use_for_single_location_local or not self.graded:
return score_dict
try:
......@@ -270,7 +268,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
randomization, and 5/7 on another
'''
max_grade = None
if self.use_for_single_location and self.graded:
if self.use_for_single_location_local and self.graded:
max_grade = self.weight
return max_grade
......@@ -492,7 +490,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
Show the Peer grading closed template
'''
html = self.system.render_template('peer_grading/peer_grading_closed.html', {
'use_for_single_location': self.use_for_single_location
'use_for_single_location': self.use_for_single_location_local
})
return html
......@@ -578,7 +576,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
'error_text': error_text,
# Checked above
'staff_access': False,
'use_single_location': self.use_for_single_location,
'use_single_location': self.use_for_single_location_local,
})
return html
......@@ -588,7 +586,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
Show individual problem interface
'''
if data is None or data.get('location') is None:
if not self.use_for_single_location:
if not self.use_for_single_location_local:
# This is an error case, because it must be set to use a single location to be called without get parameters
# This is a dev_facing_error
log.error(
......@@ -610,7 +608,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
# Checked above
'staff_access': False,
'track_changes': getattr(module, 'track_changes', False),
'use_single_location': self.use_for_single_location,
'use_single_location': self.use_for_single_location_local,
})
return {'html': html, 'success': True}
......@@ -656,10 +654,24 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor):
return non_editable_fields
def get_required_module_descriptors(self):
"""Returns a list of XModuleDescritpor instances upon which this module depends, but are
not children of this module"""
"""
Returns a list of XModuleDescriptor instances upon which this module depends, but are
not children of this module.
"""
# If use_for_single_location is True, this is linked to an open ended problem.
if self.use_for_single_location:
return [self.system.load_item(self.link_to_location)]
# Try to load the linked module.
# If we can't load it, return empty list to avoid exceptions on progress page.
try:
linked_module = self.system.load_item(self.link_to_location)
return [linked_module]
except (NoPathToItem, ItemNotFoundError):
error_message = ("Cannot find the combined open ended module "
"at location {0} being linked to from peer "
"grading module {1}").format(self.link_to_location, self.location)
log.error(error_message)
return []
else:
return []
......
common/lib/xmodule/xmodule/tests/test_peer_grading.py
import unittest
from xmodule.modulestore import Location
from .import get_test_system
from test_util_open_ended import MockQueryDict, DummyModulestore
from xmodule.open_ended_grading_classes.peer_grading_service import MockPeerGradingService
import json
import logging
from mock import Mock
from xmodule.peer_grading_module import PeerGradingModule, InvalidLinkLocation
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
import json
import logging
from xmodule.modulestore import Location
from xmodule.tests import get_test_system, get_test_descriptor_system
from xmodule.tests.test_util_open_ended import MockQueryDict, DummyModulestore
from xmodule.open_ended_grading_classes.peer_grading_service import MockPeerGradingService
from xmodule.peer_grading_module import PeerGradingModule, PeerGradingDescriptor
from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
log = logging.getLogger(__name__)
......@@ -37,7 +38,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
'feedback': "",
'rubric_scores[]': [0, 1],
'submission_flagged': False,
'answer_unknown' : False,
'answer_unknown': False,
})
def setUp(self):
......@@ -57,22 +58,22 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
closed = self.peer_grading.closed()
self.assertEqual(closed, False)
self.assertFalse(closed)
def test_get_html(self):
"""
Test to see if the module can be rendered
@return:
"""
html = self.peer_grading.get_html()
_html = self.peer_grading.get_html()
def test_get_data(self):
"""
Try getting data from the external grading service
@return:
"""
success, data = self.peer_grading.query_data_for_location(self.problem_location.url())
self.assertEqual(success, True)
success, _data = self.peer_grading.query_data_for_location(self.problem_location.url())
self.assertTrue(success)
def test_get_score(self):
"""
......@@ -80,7 +81,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
score = self.peer_grading.get_score()
self.assertEquals(score['score'], None)
self.assertIsNone(score['score'])
def test_get_max_score(self):
"""
......@@ -95,7 +96,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
Test to see if we can get the next mock submission
@return:
"""
success, next_submission = self.peer_grading.get_next_submission({'location': 'blah'})
success, _next_submission = self.peer_grading.get_next_submission({'location': 'blah'})
self.assertEqual(success, True)
def test_save_grade(self):
......@@ -111,9 +112,8 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
Check to see if the student has calibrated yet
@return:
"""
calibrated_dict = {'location': "blah"}
response = self.peer_grading.is_student_calibrated(self.calibrated_dict)
self.assertEqual(response['success'], True)
self.assertTrue(response['success'])
def test_show_calibration_essay(self):
"""
......@@ -121,7 +121,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
response = self.peer_grading.show_calibration_essay(self.calibrated_dict)
self.assertEqual(response['success'], True)
self.assertTrue(response['success'])
def test_save_calibration_essay(self):
"""
......@@ -129,7 +129,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
response = self.peer_grading.save_calibration_essay(self.save_dict)
self.assertEqual(response['success'], True)
self.assertTrue(response['success'])
def test_peer_grading_problem(self):
"""
......@@ -137,7 +137,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
response = self.peer_grading.peer_grading_problem(self.coe_dict)
self.assertEqual(response['success'], True)
self.assertTrue(response['success'])
def test___find_corresponding_module_for_location_exceptions(self):
"""
......@@ -146,7 +146,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
@return:
"""
with self.assertRaises(ItemNotFoundError):
self.peer_grading._find_corresponding_module_for_location(Location('i4x','a','b','c','d'))
self.peer_grading._find_corresponding_module_for_location(Location('i4x', 'a', 'b', 'c', 'd'))
def test_get_instance_state(self):
"""
......@@ -155,6 +155,7 @@ class PeerGradingModuleTest(unittest.TestCase, DummyModulestore):
"""
self.peer_grading.get_instance_state()
class MockPeerGradingServiceProblemList(MockPeerGradingService):
def get_problem_list(self, course_id, grader_id):
return {'success': True,
......@@ -162,13 +163,16 @@ class MockPeerGradingServiceProblemList(MockPeerGradingService):
{"num_graded": 3, "num_pending": 681, "num_required": 3, "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion", "problem_name": "Peer-Graded Essay"},
]}
class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
"""
Test peer grading xmodule at the unit level. More detailed tests are difficult, as the module relies on an
external grading service.
"""
problem_location = Location(["i4x", "edX", "open_ended", "peergrading",
"PeerGradingScored"])
problem_location = Location(
["i4x", "edX", "open_ended", "peergrading", "PeerGradingScored"]
)
def setUp(self):
"""
Create a peer grading module from a test system
......@@ -180,7 +184,7 @@ class PeerGradingModuleScoredTest(unittest.TestCase, DummyModulestore):
def test_metadata_load(self):
peer_grading = self.get_module_from_location(self.problem_location, COURSE)
self.assertEqual(peer_grading.closed(), False)
self.assertFalse(peer_grading.closed())
def test_problem_list(self):
"""
......@@ -217,6 +221,37 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
self.test_system.open_ended_grading_interface = None
self.setup_modulestore(COURSE)
@property
def field_data(self):
"""
Setup the proper field data for a peer grading module.
"""
return DictFieldData({
'data': '<peergrading/>',
'location': self.problem_location,
'use_for_single_location': True,
'link_to_location': self.coe_location.url(),
'graded': True,
})
@property
def scope_ids(self):
"""
Return the proper scope ids for the peer grading module.
"""
return ScopeIds(None, None, self.problem_location, self.problem_location)
def _create_peer_grading_descriptor_with_linked_problem(self):
# Initialize the peer grading module.
system = get_test_descriptor_system()
return system.construct_xblock_from_class(
PeerGradingDescriptor,
field_data=self.field_data,
scope_ids=self.scope_ids
)
def _create_peer_grading_with_linked_problem(self, location, valid_linked_descriptor=True):
"""
Create a peer grading problem with a linked location.
......@@ -235,34 +270,56 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
else:
pg_descriptor.get_required_module_descriptors = lambda: []
# Setup the proper field data for the peer grading module.
field_data = DictFieldData({
'data': '<peergrading/>',
'location': self.problem_location,
'use_for_single_location': True,
'link_to_location': self.coe_location.url(),
'graded': True,
})
# Initialize the peer grading module.
peer_grading = PeerGradingModule(
pg_descriptor,
self.test_system,
field_data,
ScopeIds(None, None, self.problem_location, self.problem_location)
self.field_data,
self.scope_ids,
)
return peer_grading
def _get_descriptor_with_invalid_link(self, exception_to_raise):
"""
Ensure that a peer grading descriptor with an invalid link will return an empty list.
"""
# Create a descriptor, and make loading an item throw an error.
descriptor = self._create_peer_grading_descriptor_with_linked_problem()
descriptor.system.load_item = Mock(side_effect=exception_to_raise)
# Ensure that modules is a list of length 0.
modules = descriptor.get_required_module_descriptors()
self.assertIsInstance(modules, list)
self.assertEqual(len(modules), 0)
def test_descriptor_with_nopath(self):
"""
Test to see if a descriptor with a NoPathToItem error when trying to get
its linked module behaves properly.
"""
self._get_descriptor_with_invalid_link(NoPathToItem)
def test_descriptor_with_item_not_found(self):
"""
Test to see if a descriptor with an ItemNotFound error when trying to get
its linked module behaves properly.
"""
self._get_descriptor_with_invalid_link(ItemNotFoundError)
def test_invalid_link(self):
"""
Ensure that a peer grading problem with no linked locations raises an error.
Ensure that a peer grading problem with no linked locations stays in panel mode.
"""
# Setup the peer grading module with no linked locations.
with self.assertRaises(InvalidLinkLocation):
self._create_peer_grading_with_linked_problem(self.coe_location, valid_linked_descriptor=False)
peer_grading = self._create_peer_grading_with_linked_problem(self.coe_location, valid_linked_descriptor=False)
self.assertFalse(peer_grading.use_for_single_location_local)
self.assertTrue(peer_grading.use_for_single_location)
def test_linked_problem(self):
"""
......@@ -284,7 +341,7 @@ class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
peer_grading = self._create_peer_grading_with_linked_problem(self.coe_location)
# If we specify a location, it will render the problem for that location.
data = peer_grading.handle_ajax('problem', {'location' : self.coe_location})
data = peer_grading.handle_ajax('problem', {'location': self.coe_location})
self.assertTrue(json.loads(data)['success'])
# If we don't specify a location, it should use the linked location.
......@@ -344,5 +401,5 @@ class PeerGradingModuleTrackChangesTest(unittest.TestCase, DummyModulestore):
"""
self.peer_grading._find_corresponding_module_for_location = self.mock_track_changes_problem
response = self.peer_grading.peer_grading_problem({'location': 'mocked'})
self.assertEqual(response['success'], True)
self.assertTrue(response['success'])
self.assertIn("'track_changes': True", response['html'])
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment