From 0887383ca78d852cd90a023dc1e619235430ad03 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 21 May 2013 14:18:00 -0400 Subject: [PATCH 1/9] Don't show peer grading button unless there is a peer grading element in the course --- lms/djangoapps/open_ended_grading/views.py | 53 ++++++++++++++-------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index cb617d609d..1b7ae18029 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -89,6 +89,30 @@ def staff_grading(request, course_id): # Checked above 'staff_access': True, }) +def find_peer_grading_module(course): + #Reverse the base course url + base_course_url = reverse('courses') + found_module = False + problem_url = "" + + #Get the course id and split it + course_id_parts = course.id.split("/") + false_dict = [False, "False", "false", "FALSE"] + #TODO: This will not work with multiple runs of a course. Make it work. The last key in the Location passed + #to get_items is called revision. Is this the same as run? + #Get the peer grading modules currently in the course + items = modulestore().get_items(['i4x', None, course_id_parts[1], 'peergrading', None]) + #See if any of the modules are centralized modules (ie display info from multiple problems) + items = [i for i in items if getattr(i,"use_for_single_location", True) in false_dict] + #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) + problem_url = generate_problem_url(problem_url_parts, base_course_url) + found_module = True + + return found_module, problem_url @cache_control(no_cache=True, no_store=True, must_revalidate=True) def peer_grading(request, course_id): @@ -98,32 +122,16 @@ def peer_grading(request, course_id): #Get the current course course = get_course_with_access(request.user, course_id, 'load') - course_id_parts = course.id.split("/") - false_dict = [False, "False", "false", "FALSE"] - #Reverse the base course url - base_course_url = reverse('courses') - try: - #TODO: This will not work with multiple runs of a course. Make it work. The last key in the Location passed - #to get_items is called revision. Is this the same as run? - #Get the peer grading modules currently in the course - items = modulestore().get_items(['i4x', None, course_id_parts[1], 'peergrading', None]) - #See if any of the modules are centralized modules (ie display info from multiple problems) - items = [i for i in items if getattr(i,"use_for_single_location", True) in false_dict] - #Get the first one - 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) - problem_url = generate_problem_url(problem_url_parts, base_course_url) - - return HttpResponseRedirect(problem_url) - except: + found_module, problem_url = find_peer_grading_module(course) + if not found_module: #This is a student_facing_error error_message = "Error with initializing peer grading. Centralized module does not exist. Please contact course staff." #This is a dev_facing_error log.exception(error_message + "Current course is: {0}".format(course_id)) return HttpResponse(error_message) + return HttpResponseRedirect(problem_url) def generate_problem_url(problem_url_parts, base_course_url): """ @@ -300,7 +308,12 @@ def combined_notifications(request, course_id): 'description': description, 'alert_message': alert_message } - notification_list.append(notification_item) + if human_name == "Peer Grading": + found_module, problem_url = find_peer_grading_module(course) + if found_module: + notification_list.append(notification_item) + else: + notification_list.append(notification_item) ajax_url = _reverse_with_slash('open_ended_notifications', course_id) combined_dict = { From 956669de21553fc6938ad9feb0f80cf448f7c1c4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 21 May 2013 14:21:46 -0400 Subject: [PATCH 2/9] If location cannot be found, move on --- lms/djangoapps/open_ended_grading/views.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 1b7ae18029..baa4df2488 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -175,7 +175,12 @@ def student_problem_list(request, course_id): problem_list = problem_list_dict['problem_list'] for i in xrange(0, len(problem_list)): - problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location']) + try: + problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location']) + except: + error_message = "Could not find module for course {0} at location {1}".format(course.id, problem_list[i]['location']) + log.error(error_message) + continue problem_url = generate_problem_url(problem_url_parts, base_course_url) problem_list[i].update({'actual_url': problem_url}) eta_available = problem_list[i]['eta_available'] From c91fa2f45acfc83da74b3ca1f7d9e730ee16c4d4 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 21 May 2013 14:23:24 -0400 Subject: [PATCH 3/9] Add in some comments --- lms/djangoapps/open_ended_grading/views.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index baa4df2488..30bf3ef4bd 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -165,6 +165,7 @@ def student_problem_list(request, course_id): base_course_url = reverse('courses') try: + #Get list of all open ended problems that the grading server knows about problem_list_json = controller_qs.get_grading_status_list(course_id, unique_id_for_user(request.user)) problem_list_dict = json.loads(problem_list_json) success = problem_list_dict['success'] @@ -176,8 +177,11 @@ def student_problem_list(request, course_id): for i in xrange(0, len(problem_list)): try: + #Try to load each problem in the courseware to get links to them problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location']) except: + #If the problem cannot be found at the location received from the grading controller server, it has been deleted by the course author. + #Continue with the rest of the location to construct the list error_message = "Could not find module for course {0} at location {1}".format(course.id, problem_list[i]['location']) log.error(error_message) continue From c5f9d94cc45497d49d25bc29f2b8cc8b98b4cd63 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Tue, 21 May 2013 14:46:22 -0400 Subject: [PATCH 4/9] Add in comments, fix behavior --- lms/djangoapps/open_ended_grading/views.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 30bf3ef4bd..3ed144cc2b 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -90,6 +90,11 @@ def staff_grading(request, course_id): 'staff_access': True, }) def find_peer_grading_module(course): + """ + Given a course, finds the first peer grading module in it. + @param course: A course object. + @return: boolean found_module, string problem_url + """ #Reverse the base course url base_course_url = reverse('courses') found_module = False @@ -117,7 +122,7 @@ def find_peer_grading_module(course): @cache_control(no_cache=True, no_store=True, must_revalidate=True) def peer_grading(request, course_id): ''' - Show a peer grading interface + Show a peer grading interface to the student. The interface is linked to from the button. ''' #Get the current course @@ -153,7 +158,8 @@ def generate_problem_url(problem_url_parts, base_course_url): @cache_control(no_cache=True, no_store=True, must_revalidate=True) def student_problem_list(request, course_id): ''' - Show a student problem list + Show a student problem list to a student. Fetch the list from the grading controller server, get some metadata, + and then show it to the student. ''' course = get_course_with_access(request.user, course_id, 'load') student_id = unique_id_for_user(request.user) @@ -175,6 +181,8 @@ def student_problem_list(request, course_id): else: problem_list = problem_list_dict['problem_list'] + #A list of problems to remove (problems that can't be found in the course) + list_to_remove = [] for i in xrange(0, len(problem_list)): try: #Try to load each problem in the courseware to get links to them @@ -184,6 +192,8 @@ def student_problem_list(request, course_id): #Continue with the rest of the location to construct the list error_message = "Could not find module for course {0} at location {1}".format(course.id, problem_list[i]['location']) log.error(error_message) + #Mark the problem for removal from the list + list_to_remove.append(i) continue problem_url = generate_problem_url(problem_url_parts, base_course_url) problem_list[i].update({'actual_url': problem_url}) @@ -214,6 +224,8 @@ def student_problem_list(request, course_id): log.error("Problem with results from external grading service for open ended.") success = False + #Remove problems that cannot be found in the courseware from the list + problem_list = [problem_list[i] for i in xrange(0,len(problem_list)) if i not in list_to_remove] ajax_url = _reverse_with_slash('open_ended_problems', course_id) return render_to_response('open_ended_problems/open_ended_problems.html', { From 21f7e222f79a50c7c8e498fcaecadf1c7780b348 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 22 May 2013 10:51:11 -0400 Subject: [PATCH 5/9] Fix exception clause --- lms/djangoapps/open_ended_grading/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 3ed144cc2b..f989d92902 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -21,6 +21,7 @@ import open_ended_notifications from xmodule.modulestore.django import modulestore from xmodule.modulestore import search +from xmodule.modulestore.exceptions import ItemNotFoundError from django.http import HttpResponse, Http404, HttpResponseRedirect from mitxmako.shortcuts import render_to_string @@ -187,7 +188,7 @@ def student_problem_list(request, course_id): try: #Try to load each problem in the courseware to get links to them problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location']) - except: + except ItemNotFoundError: #If the problem cannot be found at the location received from the grading controller server, it has been deleted by the course author. #Continue with the rest of the location to construct the list error_message = "Could not find module for course {0} at location {1}".format(course.id, problem_list[i]['location']) From a44eacd474ad938b5754641f3e7ca7fa7b63d6d7 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 22 May 2013 10:54:52 -0400 Subject: [PATCH 6/9] Make fields stringy, remove false dict --- .../xmodule/combined_open_ended_module.py | 16 ++++++++-------- lms/djangoapps/open_ended_grading/views.py | 3 +-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index f4074283fe..a2ce4d0a65 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -8,7 +8,7 @@ from .x_module import XModule from xblock.core import Integer, Scope, String, Boolean, List from xmodule.open_ended_grading_classes.combined_open_ended_modulev1 import CombinedOpenEndedV1Module, CombinedOpenEndedV1Descriptor from collections import namedtuple -from .fields import Date, StringyFloat +from .fields import Date, StringyFloat, StringyInteger, StringyBoolean log = logging.getLogger("mitx.courseware") @@ -49,19 +49,19 @@ class VersionInteger(Integer): class CombinedOpenEndedFields(object): display_name = String(help="Display name for this module", default="Open Ended Grading", scope=Scope.settings) - current_task_number = Integer(help="Current task that the student is on.", default=0, scope=Scope.user_state) + current_task_number = StringyInteger(help="Current task that the student is on.", default=0, scope=Scope.user_state) task_states = List(help="List of state dictionaries of each task within this module.", scope=Scope.user_state) state = String(help="Which step within the current task that the student is on.", default="initial", scope=Scope.user_state) - student_attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, + student_attempts = StringyInteger(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state) - ready_to_reset = Boolean(help="If the problem is ready to be reset or not.", default=False, + ready_to_reset = StringyBoolean(help="If the problem is ready to be reset or not.", default=False, scope=Scope.user_state) - attempts = Integer(help="Maximum number of attempts that a student is allowed.", default=1, scope=Scope.settings) - is_graded = Boolean(help="Whether or not the problem is graded.", default=False, scope=Scope.settings) - accept_file_upload = Boolean(help="Whether or not the problem accepts file uploads.", default=False, + attempts = StringyInteger(help="Maximum number of attempts that a student is allowed.", default=1, scope=Scope.settings) + is_graded = StringyBoolean(help="Whether or not the problem is graded.", default=False, scope=Scope.settings) + accept_file_upload = StringyBoolean(help="Whether or not the problem accepts file uploads.", default=False, scope=Scope.settings) - skip_spelling_checks = Boolean(help="Whether or not to skip initial spelling checks.", default=True, + skip_spelling_checks = StringyBoolean(help="Whether or not to skip initial spelling checks.", default=True, scope=Scope.settings) due = Date(help="Date that this problem is due by", default=None, scope=Scope.settings) graceperiod = String(help="Amount of time after the due date that submissions will be accepted", default=None, diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index f989d92902..e6da717067 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -103,13 +103,12 @@ def find_peer_grading_module(course): #Get the course id and split it course_id_parts = course.id.split("/") - false_dict = [False, "False", "false", "FALSE"] #TODO: This will not work with multiple runs of a course. Make it work. The last key in the Location passed #to get_items is called revision. Is this the same as run? #Get the peer grading modules currently in the course items = modulestore().get_items(['i4x', None, course_id_parts[1], 'peergrading', None]) #See if any of the modules are centralized modules (ie display info from multiple problems) - items = [i for i in items if getattr(i,"use_for_single_location", True) in false_dict] + 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 From 3bd04290f58a592139850cb72862cedafa6f60a7 Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 22 May 2013 11:58:03 -0400 Subject: [PATCH 7/9] Add tests, address review comments --- common/lib/xmodule/xmodule/modulestore/xml.py | 1 - .../controller_query_service.py | 46 +++++- lms/djangoapps/open_ended_grading/tests.py | 151 ++++++++++-------- lms/djangoapps/open_ended_grading/views.py | 39 +++-- 4 files changed, 156 insertions(+), 81 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 114a3281c6..7128c04a88 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -290,7 +290,6 @@ class XMLModuleStore(ModuleStoreBase): if course_dirs is None: course_dirs = sorted([d for d in os.listdir(self.data_dir) if os.path.exists(self.data_dir / d / "course.xml")]) - for course_dir in course_dirs: self.try_load_course(course_dir) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index 08f2a95387..b807e05160 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -6,7 +6,7 @@ log = logging.getLogger(__name__) class ControllerQueryService(GradingService): """ - Interface to staff grading backend. + Interface to controller query backend. """ def __init__(self, config, system): @@ -77,6 +77,50 @@ class ControllerQueryService(GradingService): return response +class MockControllerQueryService(object): + """ + Mock controller query service for testing + """ + + def __init__(self, config, system): + pass + + def check_if_name_is_unique(self, **params): + """ + Mock later if needed. Stub function for now. + @param params: + @return: + """ + pass + + def check_for_eta(self, **params): + """ + Mock later if needed. Stub function for now. + @param params: + @return: + """ + pass + + def check_combined_notifications(self, **params): + combined_notifications = '{"flagged_submissions_exist": false, "version": 1, "new_student_grading_to_view": false, "success": true, "staff_needs_to_grade": false, "student_needs_to_peer_grade": true, "overall_need_to_check": true}' + return combined_notifications + + def get_grading_status_list(self, **params): + grading_status_list = '{"version": 1, "problem_list": [{"problem_name": "Science Question -- Machine Assessed", "grader_type": "NA", "eta_available": true, "state": "Waiting to be Graded", "eta": 259200, "location": "i4x://MITx/oe101x/combinedopenended/Science_SA_ML"}, {"problem_name": "Humanities Question -- Peer Assessed", "grader_type": "NA", "eta_available": true, "state": "Waiting to be Graded", "eta": 259200, "location": "i4x://MITx/oe101x/combinedopenended/Humanities_SA_Peer"}], "success": true}' + return grading_status_list + + def get_flagged_problem_list(self, **params): + flagged_problem_list = '{"version": 1, "success": false, "error": "No flagged submissions exist for course: MITx/oe101x/2012_Fall"}' + return flagged_problem_list + + def take_action_on_flags(self, **params): + """ + Mock later if needed. Stub function for now. + @param params: + @return: + """ + pass + def convert_seconds_to_human_readable(seconds): if seconds < 60: human_string = "{0} seconds".format(seconds) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index ffc02608d5..76068dea70 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -5,19 +5,20 @@ django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/open """ import json -from mock import MagicMock +from mock import MagicMock, patch, Mock from django.core.urlresolvers import reverse from django.contrib.auth.models import Group +from django.http import HttpResponse from mitxmako.shortcuts import render_to_string -from xmodule.open_ended_grading_classes import peer_grading_service +from xmodule.open_ended_grading_classes import peer_grading_service, controller_query_service from xmodule import peer_grading_module from xmodule.modulestore.django import modulestore import xmodule.modulestore.django from xmodule.x_module import ModuleSystem -from open_ended_grading import staff_grading_service +from open_ended_grading import staff_grading_service, views from courseware.access import _course_staff_group_name from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user @@ -25,10 +26,11 @@ import logging log = logging.getLogger(__name__) from django.test.utils import override_settings -from django.http import QueryDict from xmodule.tests import test_util_open_ended +from courseware.tests import factories + @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestStaffGradingService(LoginEnrollmentTestCase): @@ -55,8 +57,8 @@ class TestStaffGradingService(LoginEnrollmentTestCase): def make_instructor(course): group_name = _course_staff_group_name(course.location) - g = Group.objects.create(name=group_name) - g.user_set.add(get_user(self.instructor)) + group = Group.objects.create(name=group_name) + group.user_set.add(get_user(self.instructor)) make_instructor(self.toy) @@ -76,30 +78,28 @@ class TestStaffGradingService(LoginEnrollmentTestCase): self.check_for_get_code(404, url) self.check_for_post_code(404, url) - def test_get_next(self): self.login(self.instructor, self.password) url = reverse('staff_grading_get_next', kwargs={'course_id': self.course_id}) data = {'location': self.location} - r = self.check_for_post_code(200, url, data) + response = self.check_for_post_code(200, url, data) - d = json.loads(r.content) + content = json.loads(response.content) - self.assertTrue(d['success']) - self.assertEquals(d['submission_id'], self.mock_service.cnt) - self.assertIsNotNone(d['submission']) - self.assertIsNotNone(d['num_graded']) - self.assertIsNotNone(d['min_for_ml']) - self.assertIsNotNone(d['num_pending']) - self.assertIsNotNone(d['prompt']) - self.assertIsNotNone(d['ml_error_info']) - self.assertIsNotNone(d['max_score']) - self.assertIsNotNone(d['rubric']) + self.assertTrue(content['success']) + self.assertEquals(content['submission_id'], self.mock_service.cnt) + self.assertIsNotNone(content['submission']) + self.assertIsNotNone(content['num_graded']) + self.assertIsNotNone(content['min_for_ml']) + self.assertIsNotNone(content['num_pending']) + self.assertIsNotNone(content['prompt']) + self.assertIsNotNone(content['ml_error_info']) + self.assertIsNotNone(content['max_score']) + self.assertIsNotNone(content['rubric']) - - def save_grade_base(self,skip=False): + def save_grade_base(self, skip=False): self.login(self.instructor, self.password) url = reverse('staff_grading_save_grade', kwargs={'course_id': self.course_id}) @@ -111,12 +111,12 @@ class TestStaffGradingService(LoginEnrollmentTestCase): 'submission_flagged': "true", 'rubric_scores[]': ['1', '2']} if skip: - data.update({'skipped' : True}) + data.update({'skipped': True}) - r = self.check_for_post_code(200, url, data) - d = json.loads(r.content) - self.assertTrue(d['success'], str(d)) - self.assertEquals(d['submission_id'], self.mock_service.cnt) + response = self.check_for_post_code(200, url, data) + content = json.loads(response.content) + self.assertTrue(content['success'], str(content)) + self.assertEquals(content['submission_id'], self.mock_service.cnt) def test_save_grade(self): self.save_grade_base(skip=False) @@ -130,11 +130,11 @@ class TestStaffGradingService(LoginEnrollmentTestCase): url = reverse('staff_grading_get_problem_list', kwargs={'course_id': self.course_id}) data = {} - r = self.check_for_post_code(200, url, data) - d = json.loads(r.content) + response = self.check_for_post_code(200, url, data) + content = json.loads(response.content) - self.assertTrue(d['success'], str(d)) - self.assertIsNotNone(d['problem_list']) + self.assertTrue(content['success'], str(content)) + self.assertIsNotNone(content['problem_list']) @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) @@ -181,14 +181,14 @@ class TestPeerGradingService(LoginEnrollmentTestCase): def test_get_next_submission_success(self): data = {'location': self.location} - r = self.peer_module.get_next_submission(data) - d = r + response = self.peer_module.get_next_submission(data) + content = response - self.assertTrue(d['success']) - self.assertIsNotNone(d['submission_id']) - self.assertIsNotNone(d['prompt']) - self.assertIsNotNone(d['submission_key']) - self.assertIsNotNone(d['max_score']) + self.assertTrue(content['success']) + self.assertIsNotNone(content['submission_id']) + self.assertIsNotNone(content['prompt']) + self.assertIsNotNone(content['submission_key']) + self.assertIsNotNone(content['max_score']) def test_get_next_submission_missing_location(self): data = {} @@ -216,10 +216,9 @@ class TestPeerGradingService(LoginEnrollmentTestCase): qdict.getlist = fake_get_item qdict.keys = data.keys - r = self.peer_module.save_grade(qdict) - d = r + response = self.peer_module.save_grade(qdict) - self.assertTrue(d['success']) + self.assertTrue(response['success']) def test_save_grade_missing_keys(self): data = {} @@ -229,37 +228,35 @@ class TestPeerGradingService(LoginEnrollmentTestCase): def test_is_calibrated_success(self): data = {'location': self.location} - r = self.peer_module.is_student_calibrated(data) - d = r + response = self.peer_module.is_student_calibrated(data) - self.assertTrue(d['success']) - self.assertTrue('calibrated' in d) + self.assertTrue(response['success']) + self.assertTrue('calibrated' in response) def test_is_calibrated_failure(self): data = {} - d = self.peer_module.is_student_calibrated(data) - self.assertFalse(d['success']) - self.assertFalse('calibrated' in d) + response = self.peer_module.is_student_calibrated(data) + self.assertFalse(response['success']) + self.assertFalse('calibrated' in response) def test_show_calibration_essay_success(self): data = {'location': self.location} - r = self.peer_module.show_calibration_essay(data) - d = r + response = self.peer_module.show_calibration_essay(data) - self.assertTrue(d['success']) - self.assertIsNotNone(d['submission_id']) - self.assertIsNotNone(d['prompt']) - self.assertIsNotNone(d['submission_key']) - self.assertIsNotNone(d['max_score']) + self.assertTrue(response['success']) + self.assertIsNotNone(response['submission_id']) + self.assertIsNotNone(response['prompt']) + self.assertIsNotNone(response['submission_key']) + self.assertIsNotNone(response['max_score']) def test_show_calibration_essay_missing_key(self): data = {} - d = self.peer_module.show_calibration_essay(data) + response = self.peer_module.show_calibration_essay(data) - self.assertFalse(d['success']) - self.assertEqual(d['error'], "Missing required keys: location") + self.assertFalse(response['success']) + self.assertEqual(response['error'], "Missing required keys: location") def test_save_calibration_essay_success(self): data = { @@ -281,13 +278,39 @@ class TestPeerGradingService(LoginEnrollmentTestCase): qdict.getlist = fake_get_item qdict.keys = data.keys - d = self.peer_module.save_calibration_essay(qdict) - self.assertTrue(d['success']) - self.assertTrue('actual_score' in d) + response = self.peer_module.save_calibration_essay(qdict) + self.assertTrue(response['success']) + self.assertTrue('actual_score' in response) def test_save_calibration_essay_missing_keys(self): data = {} - d = self.peer_module.save_calibration_essay(data) - self.assertFalse(d['success']) - self.assertTrue(d['error'].find('Missing required keys:') > -1) - self.assertFalse('actual_score' in d) + response = self.peer_module.save_calibration_essay(data) + self.assertFalse(response['success']) + self.assertTrue(response['error'].find('Missing required keys:') > -1) + self.assertFalse('actual_score' in response) + + +@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) +class TestPanel(LoginEnrollmentTestCase): + """Check the Table of Contents for a course""" + + def setUp(self): + # Toy courses should be loaded + self.course_name = 'edX/open_ended/2012_Fall' + self.course = modulestore().get_course(self.course_name) + self.user = factories.UserFactory() + + def test_open_ended_panel(self): + """ + Test to see if the peer grading module in the demo course is found + @return: + """ + found_module, peer_grading_module = views.find_peer_grading_module(self.course) + self.assertTrue(found_module) + + @patch('xmodule.open_ended_grading_classes.controller_query_service.ControllerQueryService', + controller_query_service.MockControllerQueryService) + def test_problem_list(self): + request = Mock(user=self.user) + response = views.student_problem_list(request, self.course.id) + self.assertTrue(isinstance(response, HttpResponse)) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index e6da717067..eac79aa85e 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -31,10 +31,10 @@ log = logging.getLogger(__name__) system = ModuleSystem( ajax_url=None, track_function=None, - get_module = None, + get_module=None, render_template=render_to_string, - replace_urls = None, - xblock_model_data= {} + replace_urls=None, + xblock_model_data={} ) controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system) @@ -90,6 +90,7 @@ def staff_grading(request, course_id): # Checked above 'staff_access': True, }) + def find_peer_grading_module(course): """ Given a course, finds the first peer grading module in it. @@ -103,14 +104,15 @@ def find_peer_grading_module(course): #Get the course id and split it course_id_parts = course.id.split("/") - #TODO: This will not work with multiple runs of a course. Make it work. The last key in the Location passed - #to get_items is called revision. Is this the same as run? - #Get the peer grading modules currently in the course - items = modulestore().get_items(['i4x', None, course_id_parts[1], 'peergrading', None]) + 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. + 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)] + items = [i for i in items if not getattr(i, "use_for_single_location", True)] #Get the first one - if len(items)>0: + 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) @@ -119,10 +121,12 @@ def find_peer_grading_module(course): return found_module, problem_url + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def peer_grading(request, course_id): ''' - Show a peer grading interface to the student. The interface is linked to from the button. + When a student clicks on the "peer grading" button in the open ended interface, link them to a peer grading + xmodule in the course. ''' #Get the current course @@ -138,6 +142,7 @@ def peer_grading(request, course_id): return HttpResponseRedirect(problem_url) + def generate_problem_url(problem_url_parts, base_course_url): """ From a list of problem url parts generated by search.path_to_location and a base course url, generates a url to a problem @@ -190,7 +195,9 @@ def student_problem_list(request, course_id): except ItemNotFoundError: #If the problem cannot be found at the location received from the grading controller server, it has been deleted by the course author. #Continue with the rest of the location to construct the list - error_message = "Could not find module for course {0} at location {1}".format(course.id, problem_list[i]['location']) + error_message = "Could not find module for course {0} at location {1}".format(course.id, + problem_list[i][ + 'location']) log.error(error_message) #Mark the problem for removal from the list list_to_remove.append(i) @@ -225,7 +232,7 @@ def student_problem_list(request, course_id): success = False #Remove problems that cannot be found in the courseware from the list - problem_list = [problem_list[i] for i in xrange(0,len(problem_list)) if i not in list_to_remove] + problem_list = [problem_list[i] for i in xrange(0, len(problem_list)) if i not in list_to_remove] ajax_url = _reverse_with_slash('open_ended_problems', course_id) return render_to_response('open_ended_problems/open_ended_problems.html', { @@ -329,6 +336,10 @@ def combined_notifications(request, course_id): 'description': description, 'alert_message': alert_message } + #The open ended panel will need to link the "peer grading" button in the panel to a peer grading + #xmodule defined in the course. This checks to see if the human name of the server notification + #that we are currently processing is "peer grading". If it is, it looks for a peer grading + #module in the course. If none exists, it removes the peer grading item from the panel. if human_name == "Peer Grading": found_module, problem_url = find_peer_grading_module(course) if found_module: @@ -345,9 +356,7 @@ def combined_notifications(request, course_id): 'ajax_url': ajax_url, } - return render_to_response('open_ended_problems/combined_notifications.html', - combined_dict - ) + return render_to_response('open_ended_problems/combined_notifications.html', combined_dict) @cache_control(no_cache=True, no_store=True, must_revalidate=True) From 96f145b8f60caf3a0a7712b1a3810231cd1d808b Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 22 May 2013 12:00:36 -0400 Subject: [PATCH 8/9] Update error message --- lms/djangoapps/open_ended_grading/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index eac79aa85e..2bb6f61491 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -135,7 +135,11 @@ def peer_grading(request, course_id): found_module, problem_url = find_peer_grading_module(course) if not found_module: #This is a student_facing_error - error_message = "Error with initializing peer grading. Centralized module does not exist. Please contact course staff." + error_message = """ + Error with initializing peer grading. + There has not been a peer grading module created in the courseware that would allow you to grade others. + Please check back later for this. + """ #This is a dev_facing_error log.exception(error_message + "Current course is: {0}".format(course_id)) return HttpResponse(error_message) From e9f05640f805c4a6650869b93c6df5e81b7720ca Mon Sep 17 00:00:00 2001 From: Vik Paruchuri Date: Wed, 22 May 2013 12:16:23 -0400 Subject: [PATCH 9/9] Update comments --- lms/djangoapps/open_ended_grading/tests.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 76068dea70..18ba863d69 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -292,7 +292,9 @@ class TestPeerGradingService(LoginEnrollmentTestCase): @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) class TestPanel(LoginEnrollmentTestCase): - """Check the Table of Contents for a course""" + """ + Run tests on the open ended panel + """ def setUp(self): # Toy courses should be loaded @@ -311,6 +313,10 @@ class TestPanel(LoginEnrollmentTestCase): @patch('xmodule.open_ended_grading_classes.controller_query_service.ControllerQueryService', controller_query_service.MockControllerQueryService) def test_problem_list(self): + """ + Ensure that the problem list from the grading controller server can be rendered properly locally + @return: + """ request = Mock(user=self.user) response = views.student_problem_list(request, self.course.id) self.assertTrue(isinstance(response, HttpResponse))