diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
index 0e5c9cdda1..ecac9a8a00 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
@@ -94,9 +94,9 @@ class MockPeerGradingService(object):
'success': True,
'submission_id': 1,
'submission_key': "",
- 'student_response': 'fake student response',
- 'prompt': 'fake submission prompt',
- 'rubric': 'fake rubric',
+ 'student_response': 'Sample student response.',
+ 'prompt': 'Sample submission prompt.',
+ 'rubric': 'Placeholder text for the full rubric.',
'max_score': 4
}
@@ -110,9 +110,9 @@ class MockPeerGradingService(object):
return {'success': True,
'submission_id': 1,
'submission_key': '',
- 'student_response': 'fake student response',
- 'prompt': 'fake submission prompt',
- 'rubric': 'fake rubric',
+ 'student_response': 'Sample student response.',
+ 'prompt': 'Sample submission prompt.',
+ 'rubric': 'Placeholder text for the full rubric.',
'max_score': 4}
def save_calibration_essay(self, **kwargs):
diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py
index 3a4bbdb9f6..b890270f5b 100644
--- a/common/lib/xmodule/xmodule/peer_grading_module.py
+++ b/common/lib/xmodule/xmodule/peer_grading_module.py
@@ -8,7 +8,7 @@ from pkg_resources import resource_string
from .capa_module import ComplexEncoder
from .x_module import XModule
from xmodule.raw_module import RawDescriptor
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from .timeinfo import TimeInfo
from xblock.fields import Dict, String, Scope, Boolean, Float
from xmodule.fields import Date, Timedelta
@@ -72,6 +72,12 @@ class PeerGradingFields(object):
scope=Scope.content
)
+class InvalidLinkLocation(Exception):
+ """
+ Exception for the case in which a peer grading module tries to link to an invalid location.
+ """
+ pass
+
class PeerGradingModule(PeerGradingFields, XModule):
"""
@@ -103,11 +109,21 @@ class PeerGradingModule(PeerGradingFields, XModule):
if self.use_for_single_location:
try:
- self.linked_problem = self.system.get_module(self.link_to_location)
+ 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)
+ 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
due_date = self.linked_problem.due
if due_date:
self.due = due_date
@@ -514,14 +530,18 @@ class PeerGradingModule(PeerGradingFields, XModule):
def _find_corresponding_module_for_location(location):
- '''
- find the peer grading module that links to the given location
- '''
+ """
+ Find the peer grading module that exists at the given location.
+ """
try:
- return modulestore().get_instance(self.system.course_id, location)
- except Exception:
- # the linked problem doesn't exist
- log.error("Problem {0} does not exist in this course".format(location))
+ return self.descriptor.system.load_item(location)
+ except ItemNotFoundError:
+ # The linked problem doesn't exist.
+ log.error("Problem {0} does not exist in this course.".format(location))
+ raise
+ except NoPathToItem:
+ # The linked problem does not have a path to it (ie is in a draft or other strange state).
+ log.error("Cannot find a path to problem {0} in this course.".format(location))
raise
good_problem_list = []
@@ -529,7 +549,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
problem_location = problem['location']
try:
descriptor = _find_corresponding_module_for_location(problem_location)
- except Exception:
+ except (NoPathToItem, ItemNotFoundError):
continue
if descriptor:
problem['due'] = descriptor.due
diff --git a/common/lib/xmodule/xmodule/tests/test_peer_grading.py b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
index 240fef4e87..0197e36cf5 100644
--- a/common/lib/xmodule/xmodule/tests/test_peer_grading.py
+++ b/common/lib/xmodule/xmodule/tests/test_peer_grading.py
@@ -2,6 +2,12 @@ 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
+from mock import Mock
+from xmodule.peer_grading_module import PeerGradingModule
+from xblock.field_data import DictFieldData
+from xblock.fields import ScopeIds
import logging
@@ -136,6 +142,13 @@ 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,
+ 'problem_list': [
+ {"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
@@ -155,3 +168,70 @@ 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)
+
+ def test_problem_list(self):
+ """
+ Test to see if a peer grading problem list can be correctly initialized.
+ """
+
+ # Initialize peer grading module.
+ peer_grading = self.get_module_from_location(self.problem_location, COURSE)
+
+ # Ensure that it cannot find any peer grading.
+ html = peer_grading.peer_grading()
+ self.assertNotIn("Peer-Graded", html)
+
+ # Swap for our mock class, which will find peer grading.
+ peer_grading.peer_gs = MockPeerGradingServiceProblemList()
+ html = peer_grading.peer_grading()
+ self.assertIn("Peer-Graded", html)
+
+class PeerGradingModuleLinkedTest(unittest.TestCase, DummyModulestore):
+ """
+ Test peer grading that is linked to an open ended module.
+ """
+ problem_location = Location(["i4x", "edX", "open_ended", "peergrading",
+ "PeerGradingLinked"])
+ coe_location = Location(["i4x", "edX", "open_ended", "combinedopenended",
+ "SampleQuestion"])
+
+ def setUp(self):
+ """
+ Create a peer grading module from a test system.
+ """
+ self.test_system = get_test_system()
+ self.test_system.open_ended_grading_interface = None
+ self.setup_modulestore(COURSE)
+
+ def test_linked_problem(self):
+ """
+ Check to see if a peer grading module with a linked problem loads properly.
+ """
+
+ # Mock the linked problem descriptor.
+ linked_descriptor = Mock()
+ linked_descriptor.location = self.coe_location
+
+ # Mock the peer grading descriptor.
+ pg_descriptor = Mock()
+ pg_descriptor.location = self.problem_location
+ pg_descriptor.get_required_module_descriptors = lambda: [linked_descriptor, ]
+
+ # Setup the proper field data for the peer grading module.
+ field_data = DictFieldData({
+ 'data': '',
+ 'location': self.problem_location,
+ 'use_for_single_location': True,
+ 'link_to_location': self.coe_location,
+ })
+
+ # Initialize the peer grading module.
+ peer_grading = PeerGradingModule(
+ pg_descriptor,
+ self.test_system,
+ field_data,
+ ScopeIds(None, None, self.problem_location, self.problem_location)
+ )
+
+ # Ensure that it is properly setup.
+ self.assertTrue(peer_grading.use_for_single_location)
diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml
index 609d12f94c..232de855cc 100644
--- a/common/test/data/open_ended/course/2012_Fall.xml
+++ b/common/test/data/open_ended/course/2012_Fall.xml
@@ -4,5 +4,6 @@
+
diff --git a/common/test/data/open_ended/peergrading/PeerGradingLinked.xml b/common/test/data/open_ended/peergrading/PeerGradingLinked.xml
new file mode 100644
index 0000000000..b2380b1e1b
--- /dev/null
+++ b/common/test/data/open_ended/peergrading/PeerGradingLinked.xml
@@ -0,0 +1 @@
+
\ No newline at end of file
diff --git a/common/test/data/open_ended_nopath/README.md b/common/test/data/open_ended_nopath/README.md
new file mode 100644
index 0000000000..1e57b483bc
--- /dev/null
+++ b/common/test/data/open_ended_nopath/README.md
@@ -0,0 +1 @@
+This is a very very simple course, useful for debugging open ended grading code. This is specifically for testing if a peer grading module with no path to it in the course will be handled properly.
diff --git a/common/test/data/open_ended_nopath/course.xml b/common/test/data/open_ended_nopath/course.xml
new file mode 100644
index 0000000000..d60df1a471
--- /dev/null
+++ b/common/test/data/open_ended_nopath/course.xml
@@ -0,0 +1 @@
+
diff --git a/common/test/data/open_ended_nopath/course/2012_Fall.xml b/common/test/data/open_ended_nopath/course/2012_Fall.xml
new file mode 100644
index 0000000000..2535b4f462
--- /dev/null
+++ b/common/test/data/open_ended_nopath/course/2012_Fall.xml
@@ -0,0 +1,4 @@
+
+
+
+
diff --git a/common/test/data/open_ended_nopath/peergrading/PeerGradingNoPath.xml b/common/test/data/open_ended_nopath/peergrading/PeerGradingNoPath.xml
new file mode 100644
index 0000000000..7e3afddf3a
--- /dev/null
+++ b/common/test/data/open_ended_nopath/peergrading/PeerGradingNoPath.xml
@@ -0,0 +1 @@
+
\ No newline at end of file
diff --git a/common/test/data/open_ended_nopath/policies/2012_Fall.json b/common/test/data/open_ended_nopath/policies/2012_Fall.json
new file mode 100644
index 0000000000..321cdf21df
--- /dev/null
+++ b/common/test/data/open_ended_nopath/policies/2012_Fall.json
@@ -0,0 +1,11 @@
+{
+ "course/2012_Fall": {
+ "graceperiod": "2 days 5 hours 59 minutes 59 seconds",
+ "start": "2015-07-17T12:00",
+ "display_name": "Self Assessment Test",
+ "graded": "true"
+ },
+ "chapter/Overview": {
+ "display_name": "Overview"
+ }
+}
diff --git a/lms/djangoapps/courseware/tests/modulestore_config.py b/lms/djangoapps/courseware/tests/modulestore_config.py
index 74fd3da57f..681f3fd4d1 100644
--- a/lms/djangoapps/courseware/tests/modulestore_config.py
+++ b/lms/djangoapps/courseware/tests/modulestore_config.py
@@ -22,5 +22,6 @@ MAPPINGS = {
'edX/test_about_blob_end_date/2012_Fall': 'xml',
'edX/graded/2012_Fall': 'xml',
'edX/open_ended/2012_Fall': 'xml',
+ 'edX/open_ended_nopath/2012_Fall': 'xml',
}
TEST_DATA_MIXED_MODULESTORE = mixed_store_config(TEST_DATA_DIR, MAPPINGS)
diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py
index 27e6fd6059..7ea81f4f3f 100644
--- a/lms/djangoapps/open_ended_grading/tests.py
+++ b/lms/djangoapps/open_ended_grading/tests.py
@@ -320,3 +320,22 @@ class TestPanel(ModuleStoreTestCase, LoginEnrollmentTestCase):
request = Mock(user=self.user)
response = views.student_problem_list(request, self.course.id)
self.assertRegexpMatches(response.content, "Here are a list of open ended problems for this course.")
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class TestPeerGradingFound(ModuleStoreTestCase):
+ """
+ Test to see if peer grading modules can be found properly.
+ """
+
+ def setUp(self):
+ self.course_name = 'edX/open_ended_nopath/2012_Fall'
+ self.course = modulestore().get_course(self.course_name)
+
+ def test_peer_grading_nopath(self):
+ """
+ The open_ended_nopath course contains a peer grading module with no path to it.
+ Ensure that the exception is caught.
+ """
+
+ found, url = views.find_peer_grading_module(self.course)
+ self.assertEqual(found, False)
\ No newline at end of file
diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py
index 4c299db38d..f5a2471226 100644
--- a/lms/djangoapps/open_ended_grading/views.py
+++ b/lms/djangoapps/open_ended_grading/views.py
@@ -20,7 +20,7 @@ import open_ended_notifications
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import search
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
from django.http import HttpResponse, Http404, HttpResponseRedirect
from mitxmako.shortcuts import render_to_string
@@ -97,25 +97,32 @@ def find_peer_grading_module(course):
@param course: A course object.
@return: boolean found_module, string problem_url
"""
- #Reverse the base course url
+
+ # Reverse the base course url.
base_course_url = reverse('courses')
found_module = False
problem_url = ""
- #Get the course id and split it
+ # Get the course id and split it.
course_id_parts = course.id.split("/")
log.info("COURSE ID PARTS")
log.info(course_id_parts)
- #Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs.
+ # Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs.
items = modulestore().get_items(['i4x', course_id_parts[0], course_id_parts[1], 'peergrading', None],
course_id=course.id)
#See if any of the modules are centralized modules (ie display info from multiple problems)
items = [i for i in items if not getattr(i, "use_for_single_location", True)]
- #Get the first one
- if len(items) > 0:
- item_location = items[0].location
- #Generate a url for the first module and redirect the user to it
- problem_url_parts = search.path_to_location(modulestore(), course.id, item_location)
+ # Loop through all potential peer grading modules, and find the first one that has a path to it.
+ for item in items:
+ item_location = item.location
+ # Generate a url for the first module and redirect the user to it.
+ try:
+ problem_url_parts = search.path_to_location(modulestore(), course.id, item_location)
+ except NoPathToItem:
+ # In the case of nopathtoitem, the peer grading module that was found is in an invalid state, and
+ # can no longer be accessed. Log an informational message, but this will not impact normal behavior.
+ log.info("Invalid peer grading module location {0} in course {1}. This module may need to be removed.".format(item_location, course.id))
+ continue
problem_url = generate_problem_url(problem_url_parts, base_course_url)
found_module = True