Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Made explanation for hints field in crowdsource_hinter.py more clear.

Browse Source 
Fixed various commenting things.

Removed an unused function in the crowdsourced module coffeescript.

Improved commenting in hint_manager.

Fixed pep and pylint violations.
This commit is contained in:
Felix Sun
2013-06-28 15:43:26 -04:00
parent 0b25e76194
commit 0c4d6ba6d9
6 changed files with 62 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
common/lib/xmodule/xmodule/crowdsource_hinter.py
View File

@@ -29,13 +29,16 @@ class CrowdsourceHinterFields(object):
default='False')
debug = String(help='String "True"/"False" - allows multiple voting', scope=Scope.content,
default='False')
# hints[answer] = {str(pk): [hint_text, #votes]}
# Usage: hints[answer] = {str(pk): [hint_text, #votes]}
# hints is a dictionary that takes answer keys.
# Each value is itself a dictionary, accepting hint_pk strings as keys,
# and returning [hint text, #votes] pairs as values
hints = Dict(help='A dictionary containing all the active hints.', scope=Scope.content, default={})
mod_queue = Dict(help='A dictionary containing hints still awaiting approval', scope=Scope.content,
default={})
hint_pk = Integer(help='Used to index hints.', scope=Scope.content, default=0)
# A list of previous answers this student made to this problem.
# Of the form (answer, (hint_pk_1, hint_pk_2, hint_pk_3)) for each problem. hint_pk's are
# Of the form [answer, [hint_pk_1, hint_pk_2, hint_pk_3]] for each problem. hint_pk's are
# None if the hint was not given.
previous_answers = List(help='A list of previous submissions.', scope=Scope.user_state, default=[])
user_voted = Boolean(help='Specifies if the user has voted on this problem or not.',
@@ -166,7 +169,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
random.sample(local_hints[answer].items(), 2)
rand_hint_1 = rand_hint_1[0]
rand_hint_2 = rand_hint_2[0]
self.previous_answers += [(answer, (best_hint_index, hint_index_1, hint_index_2))]
self.previous_answers += [[answer, [best_hint_index, hint_index_1, hint_index_2]]]
return {'best_hint': best_hint,
'rand_hint_1': rand_hint_1,
@@ -185,7 +188,6 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
"""
# The student got it right.
# Did he submit at least one wrong answer?
out = ''
if len(self.previous_answers) == 0:
# No. Nothing to do here.
return

7
common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee
View File

@@ -61,13 +61,6 @@ class @Hinter
target.val('')
target.data('cleared', true)
feedback_ui_change: =>
# Make all of the previous-answer divs hidden.
@$('.previous-answer').css('display', 'none')
# But, now find the selected div, and make it visible.
selector = '#previous-answer-' + @$('#feedback-select option:selected').attr('value')
@$(selector).css('display', 'inline')
render: (content) ->
if content
# Trim leading and trailing whitespace

94
common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py
View File

@@ -1,19 +1,19 @@
from mock import Mock, patch
"""
Tests the crowdsourced hinter xmodule.
"""
from mock import Mock
import unittest
import copy
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
from . import get_test_system
import json
class CHModuleFactory(object):
"""
Helps us make a CrowdsourceHinterModule with the specified internal
@@ -44,6 +44,9 @@ class CHModuleFactory(object):
@staticmethod
def next_num():
"""
Helps make unique names for our mock CrowdsourceHinterModule's
"""
CHModuleFactory.num += 1
return CHModuleFactory.num
@@ -53,23 +56,23 @@ class CHModuleFactory(object):
user_voted=None,
moderate=None,
mod_queue=None):
location = Location(["i4x", "edX", "capa_test", "problem",
"SampleProblem{0}".format(CHModuleFactory.next_num())])
"""
A factory method for making CHM's
"""
model_data = {'data': CHModuleFactory.sample_problem_xml}
if hints != None:
if hints is not None:
model_data['hints'] = hints
else:
model_data['hints'] = {
'24.0': {'0': ['Best hint', 40],
'3': ['Another hint', 30],
'4': ['A third hint', 20],
'6': ['A less popular hint', 3]},
'3': ['Another hint', 30],
'4': ['A third hint', 20],
'6': ['A less popular hint', 3]},
'25.0': {'1': ['Really popular hint', 100]}
}
if mod_queue != None:
if mod_queue is not None:
model_data['mod_queue'] = mod_queue
else:
model_data['mod_queue'] = {
@@ -77,7 +80,7 @@ class CHModuleFactory(object):
'26.0': {'5': ['Another non-approved hint']}
}
if previous_answers != None:
if previous_answers is not None:
model_data['previous_answers'] = previous_answers
else:
model_data['previous_answers'] = [
@@ -85,18 +88,19 @@ class CHModuleFactory(object):
['29.0', [None, None, None]]
]
if user_voted != None:
if user_voted is not None:
model_data['user_voted'] = user_voted
if moderate != None:
if moderate is not None:
model_data['moderate'] = moderate
descriptor = Mock(weight="1")
system = get_test_system()
module = CrowdsourceHinterModule(system, descriptor, model_data)
return module
class VerticalWithModulesFactory(object):
"""
Makes a vertical with several crowdsourced hinter modules inside.
@@ -147,8 +151,6 @@ class VerticalWithModulesFactory(object):
@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)
@@ -166,10 +168,12 @@ class FakeChild(object):
self.system.ajax_url = 'this/is/a/fake/ajax/url'
def get_html(self):
"""
Return a fake html string.
"""
return 'This is supposed to be test html.'
class CrowdsourceHinterTest(unittest.TestCase):
"""
In the below tests, '24.0' represents a wrong answer, and '42.5' represents
@@ -178,10 +182,11 @@ class CrowdsourceHinterTest(unittest.TestCase):
def test_gethtml(self):
"""
A simple test of get_html - make sure it returns the html of the inner
A simple test of get_html - make sure it returns the html of the inner
problem.
"""
m = CHModuleFactory.create()
def fake_get_display_items():
"""
A mock of get_display_items
@@ -198,6 +203,7 @@ class CrowdsourceHinterTest(unittest.TestCase):
error message.
"""
m = CHModuleFactory.create()
def fake_get_display_items():
"""
Returns no children.
@@ -207,13 +213,13 @@ class CrowdsourceHinterTest(unittest.TestCase):
out_html = m.get_html()
self.assertTrue('Error in loading crowdsourced hinter' in out_html)
@unittest.skip("Needs to be finished.")
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
@@ -229,7 +235,7 @@ class CrowdsourceHinterTest(unittest.TestCase):
m = CHModuleFactory.create()
json_in = {'problem_name': '26.0'}
out = m.get_hint(json_in)
self.assertTrue(out == None)
self.assertTrue(out is None)
self.assertTrue(['26.0', [None, None, None]] in m.previous_answers)
def test_gethint_1hint(self):
@@ -242,7 +248,6 @@ class CrowdsourceHinterTest(unittest.TestCase):
out = m.get_hint(json_in)
self.assertTrue(out['best_hint'] == 'Really popular hint')
def test_gethint_manyhints(self):
"""
Someone asks for a hint, with many matching hints in the database.
@@ -258,7 +263,6 @@ class CrowdsourceHinterTest(unittest.TestCase):
self.assertTrue('rand_hint_1' in out)
self.assertTrue('rand_hint_2' in out)
def test_getfeedback_0wronganswers(self):
"""
Someone has gotten the problem correct on the first try.
@@ -267,7 +271,7 @@ class CrowdsourceHinterTest(unittest.TestCase):
m = CHModuleFactory.create(previous_answers=[])
json_in = {'problem_name': '42.5'}
out = m.get_feedback(json_in)
self.assertTrue(out == None)
self.assertTrue(out is None)
def test_getfeedback_1wronganswer_nohints(self):
"""
@@ -275,29 +279,24 @@ class CrowdsourceHinterTest(unittest.TestCase):
answer. However, we don't actually have hints for this problem.
There should be a dialog to submit a new hint.
"""
m = CHModuleFactory.create(previous_answers=[['26.0',[None, None, None]]])
m = CHModuleFactory.create(previous_answers=[['26.0', [None, None, None]]])
json_in = {'problem_name': '42.5'}
out = m.get_feedback(json_in)
print out['index_to_answer']
self.assertTrue(out['index_to_hints'][0] == [])
self.assertTrue(out['index_to_answer'][0] == '26.0')
def test_getfeedback_1wronganswer_withhints(self):
"""
Same as above, except the user did see hints. There should be
a voting dialog, with the correct choices, plus a hint submission
dialog.
"""
m = CHModuleFactory.create(
previous_answers=[
['24.0', [0, 3, None]]],
)
m = CHModuleFactory.create(previous_answers=[['24.0', [0, 3, None]]])
json_in = {'problem_name': '42.5'}
out = m.get_feedback(json_in)
print out['index_to_hints']
self.assertTrue(len(out['index_to_hints'][0])==2)
self.assertTrue(len(out['index_to_hints'][0]) == 2)
def test_getfeedback_missingkey(self):
"""
@@ -308,7 +307,7 @@ class CrowdsourceHinterTest(unittest.TestCase):
previous_answers=[['24.0', [0, 100, None]]])
json_in = {'problem_name': '42.5'}
out = m.get_feedback(json_in)
self.assertTrue(len(out['index_to_hints'][0])==1)
self.assertTrue(len(out['index_to_hints'][0]) == 1)
def test_vote_nopermission(self):
"""
@@ -318,10 +317,9 @@ class CrowdsourceHinterTest(unittest.TestCase):
m = CHModuleFactory.create(user_voted=True)
json_in = {'answer': 0, 'hint': 1}
old_hints = copy.deepcopy(m.hints)
json_out = json.loads(m.tally_vote(json_in))['contents']
m.tally_vote(json_in)
self.assertTrue(m.hints == old_hints)
def test_vote_withpermission(self):
"""
A user votes for a hint.
@@ -336,7 +334,6 @@ class CrowdsourceHinterTest(unittest.TestCase):
self.assertTrue(['Best hint', 40] in dict_out['hint_and_votes'])
self.assertTrue(['Another hint', 31] in dict_out['hint_and_votes'])
def test_submithint_nopermission(self):
"""
A user tries to submit a hint, but he has already voted.
@@ -348,7 +345,6 @@ class CrowdsourceHinterTest(unittest.TestCase):
print m.hints
self.assertTrue('29.0' not in m.hints)
def test_submithint_withpermission_new(self):
"""
A user submits a hint to an answer for which no hints
@@ -359,21 +355,19 @@ class CrowdsourceHinterTest(unittest.TestCase):
m.submit_hint(json_in)
self.assertTrue('29.0' in m.hints)
def test_submithint_withpermission_existing(self):
"""
A user submits a hint to an answer that has other hints
already.
"""
m = CHModuleFactory.create(previous_answers = [['25.0', [1, None, None]]])
m = CHModuleFactory.create(previous_answers=[['25.0', [1, None, None]]])
json_in = {'answer': 0, 'hint': 'This is a new hint.'}
m.submit_hint(json_in)
# Make a hint request.
json_in = {'problem name': '25.0'}
out = m.get_hint(json_in)
self.assertTrue((out['best_hint'] == 'This is a new hint.')
or (out['rand_hint_1'] == 'This is a new hint.'))
or (out['rand_hint_1'] == 'This is a new hint.'))
def test_submithint_moderate(self):
"""
@@ -397,7 +391,6 @@ class CrowdsourceHinterTest(unittest.TestCase):
print m.hints
self.assertTrue(m.hints['29.0'][0][0] == u'<script> alert("Trololo"); </script>')
def test_template_gethint(self):
"""
Test the templates for get_hint.
@@ -421,12 +414,11 @@ class CrowdsourceHinterTest(unittest.TestCase):
self.assertTrue('A random hint' in out)
self.assertTrue('Another random hint' in out)
def test_template_feedback(self):
"""
Test the templates for get_feedback.
NOT FINISHED
from lxml import etree
m = CHModuleFactory.create()
@@ -445,11 +437,3 @@ class CrowdsourceHinterTest(unittest.TestCase):
"""
pass

2
common/templates/hinter_display.html
View File

@@ -60,7 +60,7 @@ What would you say to help someone who got this wrong answer?
% endfor
</div>
<p>Read about <a class="expand-goodhint" href="javascript:;">what makes a good hint</a>.</p>
<p>Read about <a class="expand-goodhint" href="javascript:void(0);">what makes a good hint</a>.</p>
<div class="goodhint" style="display:none">
<h4>What makes a good hint?</h4>

14
lms/djangoapps/instructor/hint_manager.py
View File

@@ -22,7 +22,7 @@ from xmodule.modulestore.django import modulestore
@ensure_csrf_cookie
def hint_manager(request, course_id):
try:
course = get_course_with_access(request.user, course_id, 'staff', depth=None)
get_course_with_access(request.user, course_id, 'staff', depth=None)
except Http404:
out = 'Sorry, but students are not allowed to access the hint manager!'
return HttpResponse(out)
@@ -74,12 +74,17 @@ def get_hints(request, course_id, field):
other_field = 'mod_queue'
field_label = 'Approved Hints'
other_field_label = 'Hints Awaiting Moderation'
# The course_id is of the form school/number/classname.
# We want to use the course_id to find all matching definition_id's.
# To do this, just take the school/number part - leave off the classname.
chopped_id = '/'.join(course_id.split('/')[:-1])
chopped_id = re.escape(chopped_id)
all_hints = XModuleContentField.objects.filter(field_name=field, definition_id__regex=chopped_id)
# big_out_dict[problem id] = [[answer, {pk: [hint, votes]}], sorted by answer]
# big_out_dict maps a problem id to a list of [answer, hints] pairs, sorted in order of answer.
big_out_dict = {}
# name_dict[problem id] = Display name of problem
# id_to name maps a problem id to the name of the problem.
# id_to_name[problem id] = Display name of problem
id_to_name = {}
for hints_by_problem in all_hints:
@@ -88,7 +93,6 @@ def get_hints(request, course_id, field):
if name is None:
continue
id_to_name[hints_by_problem.definition_id] = name
# Answer list contains (answer, dict_of_hints) tuples.
def answer_sorter(thing):
"""
@@ -102,6 +106,7 @@ def get_hints(request, course_id, field):
# Put all non-numerical answers first.
return float('-inf')
# Answer list contains [answer, dict_of_hints] pairs.
answer_list = sorted(json.loads(hints_by_problem.value).items(), key=answer_sorter)
big_out_dict[hints_by_problem.definition_id] = answer_list
@@ -113,6 +118,7 @@ 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
@@ -229,4 +235,4 @@ def approve(request, course_id, field):
problem_dict[answer] = {}
problem_dict[answer][pk] = hint_to_move
problem_in_hints.value = json.dumps(problem_dict)
problem_in_hints.save()
problem_in_hints.save()

20
lms/djangoapps/instructor/tests/test_hint_manager.py
View File

@@ -1,21 +1,17 @@
import unittest
import nose.tools
import json
from django.http import Http404
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 student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
class HintManagerTest(ModuleStoreTestCase):
@@ -36,7 +32,7 @@ class HintManagerTest(ModuleStoreTestCase):
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]}}))
@@ -48,13 +44,12 @@ class HintManagerTest(ModuleStoreTestCase):
# (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.
"""
c = Client()
user = UserFactory.create(username='student', email='student@edx.org', password='test')
UserFactory.create(username='student', email='student@edx.org', password='test')
c.login(username='student', password='test')
out = c.get(self.url)
print out
@@ -70,7 +65,7 @@ class HintManagerTest(ModuleStoreTestCase):
def test_invalid_field_access(self):
"""
Makes sure that field names other than 'mod_queue' and 'hints' are
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'})
@@ -110,7 +105,7 @@ class HintManagerTest(ModuleStoreTestCase):
'3': ['Hint 3', 12]}),
('2.0', {'4': ['Hint 4', 3]})
]}
self.assertTrue(out['all_hints'] == expected)
self.assertTrue(out['all_hints'] == expected)
def test_deletehints(self):
"""
@@ -167,6 +162,3 @@ class HintManagerTest(ModuleStoreTestCase):
problem_hints = XModuleContentField.objects.get(field_name='hints', definition_id=self.problem_id).value
self.assertTrue(json.loads(problem_hints)['2.0']['2'] == ['Hint 2', 1])
self.assertTrue(len(json.loads(problem_hints)['2.0']) == 2)