Fixed a small, but dangerous, string-to-integer casting bug in hint_manager.

Expanded tests of hint_manager.

Enabled the hint_manager by default in development environments.

common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py

@@ -5,6 +5,7 @@ import random
 
 import xmodule
 from xmodule.crowdsource_hinter import CrowdsourceHinterModule
+from xmodule.vertical_module import VerticalModule, VerticalDescriptor
 from xmodule.modulestore import Location
 
 from django.http import QueryDict
@@ -96,6 +97,65 @@ class CHModuleFactory(object):
 
         return module
 
+class VerticalWithModulesFactory(object):
+    """
+    Makes a vertical with several crowdsourced hinter modules inside.
+    Used to make sure that several crowdsourced hinter modules can co-exist
+    on one vertical.
+    """
+
+    sample_problem_xml = """<?xml version="1.0"?>
+    <vertical display_name="Test vertical">
+        <crowdsource_hinter>
+            <problem display_name="Numerical Input" markdown=" " rerandomize="never" showanswer="finished">
+              <p>Test numerical problem.</p>
+              <numericalresponse answer="5">
+                <textline/>
+              </numericalresponse>
+              <solution>
+                <div class="detailed-solution">
+                  <p>Explanation</p>
+                  <p>If you look at your hand, you can count that you have five fingers. </p>
+                </div>
+              </solution>
+            </problem>
+        </crowdsource_hinter>
+
+        <crowdsource_hinter>
+            <problem display_name="Numerical Input" markdown=" " rerandomize="never" showanswer="finished">
+              <p>Another test numerical problem.</p>
+              <numericalresponse answer="5">
+                <textline/>
+              </numericalresponse>
+              <solution>
+                <div class="detailed-solution">
+                  <p>Explanation</p>
+                  <p>If you look at your hand, you can count that you have five fingers. </p>
+                </div>
+              </solution>
+            </problem>
+        </crowdsource_hinter>
+    </vertical>
+    """
+
+    num = 0
+
+    @staticmethod
+    def next_num():
+        CHModuleFactory.num += 1
+        return CHModuleFactory.num
+
+    @staticmethod
+    def create():
+        location = Location(["i4x", "edX", "capa_test", "vertical",
+                             "SampleVertical{0}".format(CHModuleFactory.next_num())])
+        model_data = {'data': VerticalWithModulesFactory.sample_problem_xml}
+        system = get_test_system()
+        descriptor = VerticalDescriptor.from_xml(VerticalWithModulesFactory.sample_problem_xml, system)
+        module = VerticalModule(system, descriptor, model_data)
+
+        return module
+
 
 class FakeChild(object):
     """
@@ -132,6 +192,19 @@ class CrowdsourceHinterTest(unittest.TestCase):
         self.assertTrue('This is supposed to be test html.' in out_html)
         self.assertTrue('this/is/a/fake/ajax/url' in out_html)
 
+    def test_gethtml_multiple(self):
+        """
+        Makes sure that multiple crowdsourced hinters play nice, when get_html
+        is called.
+        NOT WORKING RIGHT NOW
+        """
+        return
+        m = VerticalWithModulesFactory.create()
+        out_html = m.get_html()
+        print out_html
+        self.assertTrue('Test numerical problem.' in out_html)
+        self.assertTrue('Another test numerical problem.' in out_html)
+
     def test_gethint_0hint(self):
         """
         Someone asks for a hint, when there's no hint to give.

lms/djangoapps/instructor/hint_manager.py

@@ -66,7 +66,6 @@ def get_hints(request, course_id, field):
           Sorted by answer.
         - id_to_name: A dictionary mapping problem id to problem name.
     """
-
     if field == 'mod_queue':
         other_field = 'hints'
         field_label = 'Hints Awaiting Moderation'
@@ -85,13 +84,10 @@ def get_hints(request, course_id, field):
 
     for hints_by_problem in all_hints:
         loc = Location(hints_by_problem.definition_id)
-        try:
-            descriptor = modulestore().get_items(loc)[0]
-        except IndexError:
-            # Sometimes, the problem is no longer in the course.  Just
-            # don't include said problem.
+        name = location_to_problem_name(loc)
+        if name is None:
             continue
-        id_to_name[hints_by_problem.definition_id] = descriptor.get_children()[0].display_name
+        id_to_name[hints_by_problem.definition_id] = name
         # Answer list contains (answer, dict_of_hints) tuples.
 
         def answer_sorter(thing):
@@ -117,6 +113,19 @@ def get_hints(request, course_id, field):
                    'id_to_name': id_to_name}
     return render_dict
 
+def location_to_problem_name(loc):
+    """
+    Given the location of a crowdsource_hinter module, try to return the name of the
+    problem it wraps around.  Return None if the hinter no longer exists.
+    """
+    try:
+        descriptor = modulestore().get_items(loc)[0]
+        return descriptor.get_children()[0].display_name
+    except IndexError:
+        # Sometimes, the problem is no longer in the course.  Just
+        # don't include said problem.
+        return None
+
 
 def delete_hints(request, course_id, field):
     """
@@ -128,8 +137,8 @@ def delete_hints(request, course_id, field):
     Example request.POST:
     {'op': 'delete_hints',
      'field': 'mod_queue',
-      1: ['problem_whatever', '42.0', 3],
-      2: ['problem_whatever', '32.5', 12]}
+      1: ['problem_whatever', '42.0', '3'],
+      2: ['problem_whatever', '32.5', '12']}
     """
 
     for key in request.POST:
@@ -160,7 +169,7 @@ def change_votes(request, course_id, field):
         this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id)
         problem_dict = json.loads(this_problem.value)
         # problem_dict[answer][pk] points to a [hint_text, #votes] pair.
-        problem_dict[answer][pk][1] = new_votes
+        problem_dict[answer][pk][1] = int(new_votes)
         this_problem.value = json.dumps(problem_dict)
         this_problem.save()
 

lms/djangoapps/instructor/tests/test_hint_manager.py

-from factory import DjangoModelFactory
 import unittest
 import nose.tools
 import json
 
 from django.http import Http404
-from django.test.client import Client
+from django.test.client import Client, RequestFactory
 from django.test.utils import override_settings
 import mitxmako.middleware
 
 from courseware.models import XModuleContentField
+from courseware.tests.factories import ContentFactory
+from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 import instructor.hint_manager as view
 from student.tests.factories import UserFactory, AdminFactory
-from xmodule.modulestore.tests.factories import CourseFactory
-from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from xmodule.modulestore.tests.factories import CourseFactory
 
-
-class HintsFactory(DjangoModelFactory):
-    FACTORY_FOR = XModuleContentField
-    definition_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001'
-    field_name = 'hints'
-    value = json.dumps({'1.0': 
-                               {'1': ['Hint 1', 2],
-                                '3': ['Hint 3', 12]},
-                        '2.0':
-                               {'4': ['Hint 4', 3]}
-                        })
-
-class ModQueueFactory(DjangoModelFactory):
-    FACTORY_FOR = XModuleContentField
-    definition_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001'
-    field_name = 'mod_queue'
-    value = json.dumps({'2.0':
-                               {'2': ['Hint 2', 1]}
-                        })
-
-class PKFactory(DjangoModelFactory):
-    FACTORY_FOR = XModuleContentField
-    definition_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001'
-    field_name = 'hint_pk'
-    value = 5
             
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 class HintManagerTest(ModuleStoreTestCase):
@@ -49,18 +24,39 @@ class HintManagerTest(ModuleStoreTestCase):
         Makes a course, which will be the same for all tests.
         Set up mako middleware, which is necessary for template rendering to happen.
         """
-        course = CourseFactory.create(org='Me', number='19.002', display_name='test_course')
+        self.course = CourseFactory.create(org='Me', number='19.002', display_name='test_course')
+        self.url = '/courses/Me/19.002/test_course/hint_manager'
+        self.user = UserFactory.create(username='robot', email='robot@edx.org', password='test', is_staff=True)
+        self.c = Client()
+        self.c.login(username='robot', password='test')
+        self.problem_id = 'i4x://Me/19.002/crowdsource_hinter/crowdsource_hinter_001'
+        self.course_id = 'Me/19.002/test_course'
+        ContentFactory.create(field_name='hints',
+                              definition_id=self.problem_id,
+                              value=json.dumps({'1.0': {'1': ['Hint 1', 2],
+                                                        '3': ['Hint 3', 12]},
+                                                '2.0': {'4': ['Hint 4', 3]}
+                              }))
+        ContentFactory.create(field_name='mod_queue',
+                              definition_id=self.problem_id,
+                              value=json.dumps({'2.0': {'2': ['Hint 2', 1]}}))
+
+        ContentFactory.create(field_name='hint_pk',
+                              definition_id=self.problem_id,
+                              value=5)
+        # Mock out location_to_problem_name, which ordinarily accesses the modulestore.
+        # (I can't figure out how to get fake structures into the modulestore.)
+        view.location_to_problem_name = lambda loc: "Test problem"
 
 
     def test_student_block(self):
         """
         Makes sure that students cannot see the hint management view.
         """
-        nose.tools.set_trace()
         c = Client()
-        user = UserFactory.create(username='robot', email='robot@edx.org', password='test')
-        c.login(username='robot', password='test')
-        out = c.get('/courses/Me/19.002/test_course/hint_manager')
+        user = UserFactory.create(username='student', email='student@edx.org', password='test')
+        c.login(username='student', password='test')
+        out = c.get(self.url)
         print out
         self.assertTrue('Sorry, but students are not allowed to access the hint manager!' in out.content)
 
@@ -68,12 +64,62 @@ class HintManagerTest(ModuleStoreTestCase):
         """
         Makes sure that staff can access the hint management view.
         """
-        c = Client()
-        user = UserFactory.create(username='robot', email='robot@edx.org', password='test', is_staff=True)
-        c.login(username='robot', password='test')
-        out = c.get('/courses/Me/19.002/test_course/hint_manager')
+        out = self.c.get('/courses/Me/19.002/test_course/hint_manager')
         print out
         self.assertTrue('Hints Awaiting Moderation' in out.content)
 
+    def test_invalid_field_access(self):
+        """
+        Makes sure that field names other than 'mod_queue' and 'hints' are 
+        rejected.
+        """
+        out = self.c.post(self.url, {'op': 'delete hints', 'field': 'all your private data'})
+        # Keep this around for reference - might be useful later.
+        # request = RequestFactory()
+        # post = request.post(self.url, {'op': 'delete hints', 'field': 'all your private data'})
+        # out = view.hint_manager(post, 'Me/19.002/test_course')
+        print out
+        self.assertTrue('an invalid field was accessed' in out.content)
+
+    def test_gethints(self):
+        """
+        Checks that gethints returns the right data.
+        """
+        request = RequestFactory()
+        post = request.post(self.url, {'field': 'mod_queue'})
+        out = view.get_hints(post, self.course_id, 'mod_queue')
+        print out
+        self.assertTrue(out['other_field'] == 'hints')
+        expected = {self.problem_id: [(u'2.0', {u'2': [u'Hint 2', 1]})]}
+        self.assertTrue(out['all_hints'] == expected)
+
+    def test_deletehints(self):
+        """
+        Checks that delete_hints deletes the right stuff.
+        """
+        request = RequestFactory()
+        post = request.post(self.url, {'field': 'hints',
+                                       'op': 'delete hints',
+                                       1: [self.problem_id, '1.0', '1']})
+        view.delete_hints(post, self.course_id, 'hints')
+        problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value
+        self.assertTrue('1' not in json.loads(problem_hints)['1.0'])
+
+    def test_changevotes(self):
+        """
+        Checks that vote changing works.
+        """
+        request = RequestFactory()
+        post = request.post(self.url, {'field': 'hints',
+                                       'op': 'change votes',
+                                       1: [self.problem_id, '1.0', '1', 5]})
+        view.change_votes(post, self.course_id, 'hints')
+        problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value
+        # hints[answer][hint_pk (string)] = [hint text, vote count]
+        print json.loads(problem_hints)['1.0']['1']
+        self.assertTrue(json.loads(problem_hints)['1.0']['1'][1] == 5)
+
+
+
 
 

lms/envs/dev.py

@@ -28,6 +28,7 @@ MITX_FEATURES['ENABLE_MANUAL_GIT_RELOAD'] = True
 MITX_FEATURES['ENABLE_PSYCHOMETRICS'] = False    # real-time psychometrics (eg item response theory analysis in instructor dashboard)
 MITX_FEATURES['ENABLE_INSTRUCTOR_ANALYTICS'] = True
 MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True
+MITX_FEATURES['ENABLE_HINTER_INSTRUCTOR_VIEW'] = True
 
 WIKI_ENABLED = True