diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py
index 3bd101afcf96ba606727579d6d4dc60f7971f95b..6da50a7544c1306d7813cee05c58fa4327ce2cd0 100644
--- a/common/lib/xmodule/xmodule/peer_grading_module.py
+++ b/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 []
diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
index 53333ea17d92aacef03e1c4b66b760ecc85ba11b..20000f6f602af7607b13fa6ec3ad87361d58a620 100644
--- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py
+++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
@@ -1,16 +1,17 @@
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'])