diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index c12fb1f160..1bb04654f0 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/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 = """ + + + +

Test numerical problem.

+ + + + +
+

Explanation

+

If you look at your hand, you can count that you have five fingers.

+
+ + + + + + +

Another test numerical problem.

+ + + + +
+

Explanation

+

If you look at your hand, you can count that you have five fingers.

+
+ + + + + """ + + 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. diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index 96ea91eabc..056784947d 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/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() diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py index 1da83dcc43..44e8458e19 100644 --- a/lms/djangoapps/instructor/tests/test_hint_manager.py +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -1,45 +1,20 @@ -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) + + + diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 813f9cf32c..2ceebf39b8 100644 --- a/lms/envs/dev.py +++ b/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