Merge pull request #1080 from edx/fix/vik/oe-problems

Clean up open ended problems view and fix error.

lms/djangoapps/open_ended_grading/tests.py

@@ -18,8 +18,9 @@ from xmodule.modulestore.django import modulestore
 from xmodule.x_module import ModuleSystem
 from xblock.fields import ScopeIds
 
-from open_ended_grading import staff_grading_service, views
+from open_ended_grading import staff_grading_service, views, utils
 from courseware.access import _course_staff_group_name
+from student.models import unique_id_for_user
 
 import logging
 
@@ -46,6 +47,57 @@ class EmptyStaffGradingService(object):
         """
         return json.dumps({'success': True, 'error': 'No problems found.'})
 
+def make_instructor(course, user_email):
+    """
+    Makes a given user an instructor in a course.
+    """
+    group_name = _course_staff_group_name(course.location)
+    group = Group.objects.create(name=group_name)
+    group.user_set.add(User.objects.get(email=user_email))
+
+class StudentProblemListMockQuery(object):
+    """
+    Mock controller query service for testing student problem list functionality.
+    """
+    def get_grading_status_list(self, *args, **kwargs):
+        """
+        Get a mock grading status list with locations from the open_ended test course.
+        @returns: json formatted grading status message.
+        """
+        grading_status_list = json.dumps(
+            {
+                "version": 1,
+                "problem_list": [
+                    {
+                        "problem_name": "Test1",
+                        "grader_type": "IN",
+                        "eta_available": True,
+                        "state": "Finished",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion1Attempt"
+                    },
+                    {
+                        "problem_name": "Test2",
+                        "grader_type": "NA",
+                        "eta_available": True,
+                        "state": "Waiting to be Graded",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion"
+                    },
+                    {
+                        "problem_name": "Test3",
+                        "grader_type": "PE",
+                        "eta_available": True,
+                        "state": "Waiting to be Graded",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion454"
+                    },
+                ],
+                "success": True
+            }
+        )
+        return grading_status_list
+
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
     '''
@@ -67,12 +119,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.course_id = "edX/toy/2012_Fall"
         self.toy = modulestore().get_course(self.course_id)
 
-        def make_instructor(course):
-            group_name = _course_staff_group_name(course.location)
-            group = Group.objects.create(name=group_name)
-            group.user_set.add(User.objects.get(email=self.instructor))
-
-        make_instructor(self.toy)
+        make_instructor(self.toy, self.instructor)
 
         self.mock_service = staff_grading_service.staff_grading_service()
 
@@ -324,7 +371,7 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
-class TestPanel(ModuleStoreTestCase, LoginEnrollmentTestCase):
+class TestPanel(ModuleStoreTestCase):
     """
     Run tests on the open ended panel
     """
@@ -343,7 +390,15 @@ class TestPanel(ModuleStoreTestCase, LoginEnrollmentTestCase):
         found_module, peer_grading_module = views.find_peer_grading_module(self.course)
         self.assertTrue(found_module)
 
-    @patch('open_ended_grading.views.controller_qs', controller_query_service.MockControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, views.system))
+    @patch(
+        'open_ended_grading.utils.create_controller_query_service',
+        Mock(
+            return_value=controller_query_service.MockControllerQueryService(
+                settings.OPEN_ENDED_GRADING_INTERFACE,
+                utils.system
+            )
+        )
+    )
     def test_problem_list(self):
         """
         Ensure that the problem list from the grading controller server can be rendered properly locally
@@ -370,4 +425,44 @@ class TestPeerGradingFound(ModuleStoreTestCase):
         """
 
         found, url = views.find_peer_grading_module(self.course)
-        self.assertEqual(found, False)
+        self.assertEqual(found, False)
\ No newline at end of file
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class TestStudentProblemList(ModuleStoreTestCase):
+    """
+    Test if the student problem list correctly fetches and parses problems.
+    """
+
+    def setUp(self):
+        # Load an open ended course with several problems.
+        self.course_name = 'edX/open_ended/2012_Fall'
+        self.course = modulestore().get_course(self.course_name)
+        self.user = factories.UserFactory()
+        # Enroll our user in our course and make them an instructor.
+        make_instructor(self.course, self.user.email)
+
+    @patch(
+        'open_ended_grading.utils.create_controller_query_service',
+        Mock(return_value=StudentProblemListMockQuery())
+    )
+    def test_get_problem_list(self):
+        """
+        Test to see if the StudentProblemList class can get and parse a problem list from ORA.
+        Mock the get_grading_status_list function using StudentProblemListMockQuery.
+        """
+        # Initialize a StudentProblemList object.
+        student_problem_list = utils.StudentProblemList(self.course.id, unique_id_for_user(self.user))
+        # Get the initial problem list from ORA.
+        success = student_problem_list.fetch_from_grading_service()
+        # Should be successful, and we should have three problems.  See mock class for details.
+        self.assertTrue(success)
+        self.assertEqual(len(student_problem_list.problem_list), 3)
+
+        # See if the problem locations are valid.
+        valid_problems = student_problem_list.add_problem_data(reverse('courses'))
+        # One location is invalid, so we should now have two.
+        self.assertEqual(len(valid_problems), 2)
+        # Ensure that human names are being set properly.
+        self.assertEqual(valid_problems[0]['grader_type_display_name'], "Instructor Assessment")
+
+

lms/djangoapps/open_ended_grading/utils.py

+import json
+
 from xmodule.modulestore import search
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
+from xmodule.x_module import ModuleSystem
+from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService
+from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError
+
+from django.utils.translation import ugettext as _
+from django.conf import settings
+
+from mitxmako.shortcuts import render_to_string
+
+from xblock.field_data import DictFieldData
+
 import logging
 
 log = logging.getLogger(__name__)
 
+GRADER_DISPLAY_NAMES = {
+    'ML': _("AI Assessment"),
+    'PE': _("Peer Assessment"),
+    'NA': _("Not yet available"),
+    'BC': _("Automatic Checker"),
+    'IN': _("Instructor Assessment"),
+    }
+
+STUDENT_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify course staff.")
+STAFF_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify your edX point of contact.")
+
+system = ModuleSystem(
+    ajax_url=None,
+    track_function=None,
+    get_module=None,
+    render_template=render_to_string,
+    replace_urls=None,
+    xmodule_field_data=DictFieldData({}),
+    )
+
+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
+    @param problem_url_parts: Output of search.path_to_location
+    @param base_course_url: Base url of a given course
+    @return: A path to the problem
+    """
+    problem_url = base_course_url + "/"
+    for i, part in enumerate(problem_url_parts):
+        if part is not None:
+            if i == 1:
+                problem_url += "courseware/"
+            problem_url += part + "/"
+    return problem_url
+
 def does_location_exist(course_id, location):
     """
     Checks to see if a valid module exists at a given location (ie has not been deleted)
@@ -25,3 +73,102 @@ def does_location_exist(course_id, location):
             log.warn(("Got an unexpected NoPathToItem error in staff grading with a non-draft location {0}. "
                       "Ensure that the location is valid.").format(location))
         return False
+
+def create_controller_query_service():
+    """
+    Return an instance of a service that can query edX ORA.
+    """
+
+    return ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
+
+class StudentProblemList(object):
+    """
+    Get a list of problems that the student has attempted from ORA.
+    Add in metadata as needed.
+    """
+    def __init__(self, course_id, user_id):
+        """
+        @param course_id: The id of a course object.  Get using course.id.
+        @param user_id: The anonymous id of the user, from the unique_id_for_user function.
+        """
+        self.course_id = course_id
+        self.user_id = user_id
+
+        # We want to append this string to all of our error messages.
+        self.course_error_ending = _("for course {0} and student {1}.").format(self.course_id, user_id)
+
+        # This is our generic error message.
+        self.error_text = STUDENT_ERROR_MESSAGE
+        self.success = False
+
+        # Create a service to query edX ORA.
+        self.controller_qs = create_controller_query_service()
+
+    def fetch_from_grading_service(self):
+        """
+        Fetch a list of problems that the student has answered from ORA.
+        Handle various error conditions.
+        @return: A boolean success indicator.
+        """
+        # In the case of multiple calls, ensure that success is false initially.
+        self.success = False
+        try:
+            #Get list of all open ended problems that the grading server knows about
+            problem_list_json = self.controller_qs.get_grading_status_list(self.course_id, self.user_id)
+        except GradingServiceError:
+            log.error("Problem contacting open ended grading service " + self.course_error_ending)
+            return self.success
+        try:
+            problem_list_dict = json.loads(problem_list_json)
+        except ValueError:
+            log.error("Problem with results from external grading service for open ended" + self.course_error_ending)
+            return self.success
+
+        success = problem_list_dict['success']
+        if 'error' in problem_list_dict:
+            self.error_text = problem_list_dict['error']
+            return success
+        if 'problem_list' not in problem_list_dict:
+            log.error("Did not receive a problem list in ORA response" + self.course_error_ending)
+            return success
+
+        self.problem_list = problem_list_dict['problem_list']
+
+        self.success = True
+        return self.success
+
+    def add_problem_data(self, base_course_url):
+        """
+        Add metadata to problems.
+        @param base_course_url: the base url for any course.  Can get with reverse('course')
+        @return: A list of valid problems in the course and their appended data.
+        """
+        # Our list of valid problems.
+        valid_problems = []
+
+        if not self.success or not isinstance(self.problem_list, list):
+            log.error("Called add_problem_data without a valid problem list" + self.course_error_ending)
+            return valid_problems
+
+        # Iterate through all of our problems and add data.
+        for problem in self.problem_list:
+            try:
+                # Try to load the problem.
+                problem_url_parts = search.path_to_location(modulestore(), self.course_id, problem['location'])
+            except (ItemNotFoundError, NoPathToItem):
+                # If the problem cannot be found at the location received from the grading controller server,
+                # it has been deleted by the course author. We should not display it.
+                error_message = "Could not find module for course {0} at location {1}".format(self.course_id,
+                                                                                              problem['location'])
+                log.error(error_message)
+                continue
+
+            # Get the problem url in the courseware.
+            problem_url = generate_problem_url(problem_url_parts, base_course_url)
+
+            # Map the grader name from ORA to a human readable version.
+            grader_type_display_name = GRADER_DISPLAY_NAMES.get(problem['grader_type'], "edX Assessment")
+            problem['actual_url'] = problem_url
+            problem['grader_type_display_name'] = grader_type_display_name
+            valid_problems.append(problem)
+        return valid_problems

lms/templates/open_ended_problems/open_ended_problems.html

@@ -19,36 +19,32 @@
         <h2>${_("Instructions")}</h2>
         <p>${_("Here is a list of open ended problems for this course.")}</p>
         % if success:
-            % if len(problem_list) == 0:
-                <div class="message-container">
-                  ${_("You have not attempted any open ended problems yet.")}
-                </div>
-            %else:
-                <table class="problem-list">
-                    <tr>
-                        <th>${_("Problem Name")}</th>
-                        <th>${_("Status")}</th>
-                        <th>${_("Grader Type")}</th>
-                        <th>${_("ETA")}</th>
-                    </tr>
-                    %for problem in problem_list:
-                    <tr>
-                        <td>
-                            <a href="${problem['actual_url']}">${problem['problem_name']}</a>
-                        </td>
-                        <td>
-                            ${problem['state']}
-                        </td>
-                        <td>
-                            ${problem['grader_type']}
-                        </td>
-                        <td>
-                            ${problem['eta_string']}
-                        </td>
-                    </tr>
-                    %endfor
-                </table>
-            %endif
+        % if len(problem_list) == 0:
+        <div class="message-container">
+          ${_("You have not attempted any open ended problems yet.")}
+        </div>
+        %else:
+        <table class="problem-list">
+            <tr>
+                <th>${_("Problem Name")}</th>
+                <th>${_("Status")}</th>
+                <th>${_("Grader Type")}</th>
+            </tr>
+            %for problem in problem_list:
+            <tr>
+                <td>
+                    <a href="${problem['actual_url']}">${problem['problem_name']}</a>
+                </td>
+                <td>
+                    ${problem['state']}
+                </td>
+                <td>
+                    ${problem['grader_type_display_name']}
+                </td>
+            </tr>
+            %endfor
+        </table>
+        %endif
         %endif
     </div>
 </section>