diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index c4f5f4ee61..ea7bb60b8e 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -351,7 +351,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_create_static_tab_and_rename(self): module_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) item = ItemFactory.create(parent_location=course_location, category='static_tab', display_name="My Tab") @@ -733,7 +733,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # we want to assert equality between the objects, but we know the locations # differ, so just make them equal for testing purposes - source_item.scope_ids = source_item.scope_ids._replace(def_id=new_loc, usage_id=new_loc) + source_item.location = new_loc if hasattr(source_item, 'data') and hasattr(lookup_item, 'data'): self.assertEqual(source_item.data, lookup_item.data) @@ -914,8 +914,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): depth=1 ) # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. - draft_loc = mongo.draft.as_draft(vertical.location.replace(name='no_references')) - vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc) + vertical.location = mongo.draft.as_draft(vertical.location.replace(name='no_references')) draft_store.save_xmodule(vertical) orphan_vertical = draft_store.get_item(vertical.location) @@ -933,8 +932,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): root_dir = path(mkdtemp_clean()) # now create a new/different private (draft only) vertical - draft_loc = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) - vertical.scope_ids = vertical.scope_ids._replace(def_id=draft_loc, usage_id=draft_loc) + vertical.location = mongo.draft.as_draft(Location(['i4x', 'edX', 'toy', 'vertical', 'a_private_vertical', None])) draft_store.save_xmodule(vertical) private_vertical = draft_store.get_item(vertical.location) vertical = None # blank out b/c i destructively manipulated its location 2 lines above @@ -983,7 +981,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(on_disk['course/2012_Fall'], own_metadata(course)) # remove old course - delete_course(module_store, content_store, location) + delete_course(module_store, content_store, location, commit=True) # reimport import_from_xml(module_store, root_dir, ['test_export'], draft_store=draft_store) diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index 610f7b1b8a..f79a8b0e58 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py @@ -42,7 +42,7 @@ def wrap_draft(item): non-draft location in either case """ setattr(item, 'is_draft', item.location.revision == DRAFT) - item.scope_ids = item.scope_ids._replace(usage_id=item.location.replace(revision=None)) + item.location = item.location.replace(revision=None) return item diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index a025eb7c82..a69ea32761 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -110,27 +110,19 @@ def _clone_modules(modulestore, modules, source_location, dest_location): original_loc = Location(module.location) if original_loc.category != 'course': - new_location = module.location._replace( + module.location = module.location._replace( tag=dest_location.tag, org=dest_location.org, course=dest_location.course ) - module.scope_ids = module.scope_ids._replace( - def_id=new_location, - usage_id=new_location - ) else: # on the course module we also have to update the module name - new_location = module.location._replace( + module.location = module.location._replace( tag=dest_location.tag, org=dest_location.org, course=dest_location.course, name=dest_location.name ) - module.scope_ids = module.scope_ids._replace( - def_id=new_location, - usage_id=new_location - ) print "Cloning module {0} to {1}....".format(original_loc, module.location) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index a9a7601e33..9ee6fe66a3 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -375,30 +375,22 @@ def remap_namespace(module, target_location_namespace): # This looks a bit wonky as we need to also change the 'name' of the imported course to be what # the caller passed in if module.location.category != 'course': - new_location = module.location._replace( + module.location = module.location._replace( tag=target_location_namespace.tag, org=target_location_namespace.org, course=target_location_namespace.course ) - module.scope_ids = module.scope_ids._replace( - def_id=new_location, - usage_id=new_location - ) else: original_location = module.location # # module is a course module # - new_location = module.location._replace( + module.location = module.location._replace( tag=target_location_namespace.tag, org=target_location_namespace.org, course=target_location_namespace.course, name=target_location_namespace.name ) - module.scope_ids = module.scope_ids._replace( - def_id=new_location, - usage_id=new_location - ) # # There is more re-namespacing work we have to do when importing course modules # 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/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 02feebea1b..d912dfaceb 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -146,6 +146,13 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): else: return BlockUsageLocator(self.scope_ids.usage_id) + @location.setter + def location(self, value): + self.scope_ids = self.scope_ids._replace( + def_id=value, + usage_id=value, + ) + @property def url_name(self): if self.descriptor: @@ -457,6 +464,13 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): else: return BlockUsageLocator(self.scope_ids.usage_id) + @location.setter + def location(self, value): + self.scope_ids = self.scope_ids._replace( + def_id=value, + usage_id=value, + ) + @property def url_name(self): if isinstance(self.location, Location): diff --git a/common/templates/course_modes/choose.html b/common/templates/course_modes/choose.html index 2b943ed4bc..88f45f024d 100644 --- a/common/templates/course_modes/choose.html +++ b/common/templates/course_modes/choose.html @@ -93,7 +93,7 @@ $(document).ready(function() { % if "honor" in modes:
${_("What if I can't afford it or don't have the necessary equipment?")}
-

${_("If you can't afford the minimum fee, don't have a webcam, credit card, debit card or acceptable ID, you can audit the course for free. You may also elect to pursue an Honor Code certificate, but you will need to tell us why you would like the fee waived below. Then click the 'Select Certificate' button to complete your registration.")}

+

${_("If you can't afford the minimum fee or don't meet the requirements, you can audit the course for free. You may also elect to pursue an Honor Code certificate, but you will need to tell us why you would like the fee waived below. Then click the 'Select Certificate' button to complete your registration.")}