From 415ea63bc0780a49593910f4c0244db51dd8322c Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Wed, 12 Jun 2013 14:45:40 -0400 Subject: [PATCH 01/19] Working prototype of crowdsourced hinting module. Conflicts: common/static/coffee/src/logger.coffee --- common/lib/xmodule/setup.py | 1 + .../lib/xmodule/xmodule/crowdsource_hinter.py | 261 ++++++++++++++++++ .../xmodule/js/src/capa/display.coffee | 1 + .../js/src/crowdsource_hinter/display.coffee | 62 +++++ common/static/coffee/src/logger.coffee | 30 +- 5 files changed, 354 insertions(+), 1 deletion(-) create mode 100644 common/lib/xmodule/xmodule/crowdsource_hinter.py create mode 100644 common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 43d970d898..6b106dd94d 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -55,6 +55,7 @@ setup( "word_cloud = xmodule.word_cloud_module:WordCloudDescriptor", "hidden = xmodule.hidden_module:HiddenDescriptor", "raw = xmodule.raw_module:RawDescriptor", + "crowdsource_hinter = xmodule.crowdsource_hinter:CrowdsourceHinterDescriptor", ], 'console_scripts': [ 'xmodule_assets = xmodule.static_content:main', diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py new file mode 100644 index 0000000000..1d424b7fff --- /dev/null +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -0,0 +1,261 @@ +import logging +import copy +import json +import os +import re +import string +import random + +from pkg_resources import resource_listdir, resource_string, resource_isdir + +from lxml import etree + +from xmodule.modulestore import Location +from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.x_module import XModule +from xmodule.xml_module import XmlDescriptor +from xblock.core import XBlock, Scope, String, Integer, Float, Object, Boolean + +from django.utils.html import escape + +log = logging.getLogger(__name__) + + +class CrowdsourceHinterFields(object): + has_children = True + hints = Object(help='''A dictionary mapping answers to lists of [hint, number_of_votes] pairs. + ''', scope=Scope.content, default= { + '4': + [['This is a hint.', 5], + ['This is hint 2', 3], + ['This is hint 3', 2], + ['This is hint 4', 1]]}) + ''' + Testing data for hints: + + ''' + previous_answers = Object(help='''A list of previous answers this student made to this problem. + Of the form (answer, (hint_id_1, hint_id_2, hint_id_3)) for each problem. hint_id's are + None if the hint was not given.''', + scope=Scope.user_state, default=[]) + + user_voted = Boolean(help='Specifies if the user has voted on this problem or not.', + scope=Scope.user_state, default=False) + + +class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): + ''' An Xmodule that makes crowdsourced hints. + ''' + icon_class = 'crowdsource_hinter' + + js = {'coffee': [resource_string(__name__, 'js/src/crowdsource_hinter/display.coffee'), + ], + 'js': []} + js_module_name = "Hinter" + + + def __init__(self, system, location, descriptor, model_data): + XModule.__init__(self, system, location, descriptor, model_data) + + + def get_html(self): + ''' + Does a regular expression find and replace to change the AJAX url. + - Dependent on lon-capa problem. + ''' + # Reset the user vote, for debugging only! Remove for prod. + self.user_voted = False + for child in self.get_display_items(): + out = child.get_html() + # The event listener uses the ajax url to find the child. + child_url = child.system.ajax_url + break + # Wrap the module in a
. This lets us pass data attributes to the javascript. + out += '
' + return out + + def capa_make_answer_hashable(self, answer): + ''' + Capa answer format: dict[problem name] -> [list of answers] + Output format: ((problem name, (answers))) + ''' + out = [] + for problem, a in answer.items(): + out.append((problem, tuple(a))) + return str(tuple(sorted(out))) + + + def ans_to_text(self, answer): + ''' + Converts capa answer format to a string representation + of the answer. + -Lon-capa dependent. + ''' + return answer.values()[0][0] + + + def handle_ajax(self, dispatch, get): + ''' + This is the landing method for AJAX calls. + ''' + if dispatch == 'get_hint': + return self.get_hint(get) + if dispatch == 'get_feedback': + return self.get_feedback(get) + if dispatch == 'vote': + return self.tally_vote(get) + if dispatch == 'submit_hint': + return self.submit_hint(get) + + def get_hint(self, get): + ''' + The student got the incorrect answer found in get. Give him a hint. + ''' + print self.hints + answer = self.ans_to_text(get) + # Look for a hint to give. + if answer not in self.hints: + # No hints to give. Return. + self.previous_answers += [(answer, (None, None, None))] + return json.dumps({'contents': ' '}) + # Get the top hint, plus two random hints. + n_hints = len(self.hints[answer]) + best_hint_index = max(xrange(n_hints), key=lambda i:self.hints[answer][i][1]) + best_hint = self.hints[answer][best_hint_index][0] + if len(self.hints[answer]) == 1: + rand_hint_1 = '' + rand_hint_2 = '' + self.previous_answers += [(answer, (0, None, None))] + elif len(self.hints[answer]) == 2: + best_hint = self.hints[answer][0][0] + rand_hint_1 = self.hints[answer][1][0] + rand_hint_2 = '' + self.previous_answers += [(answer, (0, 1, None))] + else: + hint_index_1, hint_index_2 = random.sample(xrange(len(self.hints[answer])), 2) + rand_hint_1 = self.hints[answer][hint_index_1][0] + rand_hint_2 = self.hints[answer][hint_index_2][0] + self.previous_answers += [(answer, (best_hint_index, hint_index_1, hint_index_2))] + hint_text = best_hint + '
' + rand_hint_1 + '
' + rand_hint_2 + return json.dumps({'contents': hint_text}) + + def get_feedback(self, get): + ''' + The student got it correct. Ask him to vote on hints, or submit a hint. + ''' + # 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 json.dumps({'contents': out}) + # Make a hint-voting interface for each wrong answer. The student will only + # be allowed to make one vote / submission, but he can choose which wrong answer + # he wants to look at. + pretty_answers = [] + for i in xrange(len(self.previous_answers)): + answer, hints_offered = self.previous_answers[i] + pretty_answers.append(answer) + # If there are previous hints for this answer, ask the student to vote on one. + if answer in self.hints: + out += '' + + # Add preamble. + out2 = '''Help us improve our hinting system by voting on the hint that was most helpful + to you. Start by picking one of your previous incorrect answers from below:
+
' + return json.dumps({'contents': out2 + out}) + + + def tally_vote(self, get): + ''' + Tally a user's vote on his favorite hint. + get: + 'answer': ans_no (index in previous_answers) + 'hint': hint_no + ''' + if self.user_voted: + return json.dumps({'contents': 'Sorry, but you have already voted!'}) + ans_no = int(get['answer']) + hint_no = int(get['hint']) + answer = self.previous_answers[ans_no][0] + temp_dict = self.hints + temp_dict[answer][hint_no][1] += 1 + # Awkward, but you need to do a direct write for the database to update. + self.hints = temp_dict + # Don't let the user vote again! + self.user_voted = True + # Reset self.previous_answers. + self.previous_answers = [] + # In the future, return a list of how many votes each hint got, maybe? + return json.dumps({'contents': 'Congrats, you\'ve voted!'}) + + + def submit_hint(self, get): + ''' + Take a hint submission and add it to the database. + get: + 'answer': answer index in previous_answers + 'hint': text of the new hint that the user is adding + ''' + # Do html escaping. Perhaps in the future do profanity filtering, etc. as well. + hint = escape(get['hint']) + answer = self.previous_answers[int(get['answer'])][0] + # Add the new hint to self.hints. (Awkward because a direct write + # is necessary.) + temp_dict = self.hints + temp_dict[answer].append([hint, 1]) # With one vote (the user himself). + self.hints = temp_dict + # Mark the user has having voted; reset previous_answers + self.user_voted = True + self.previous_answers = [] + return json.dumps({'contents': 'Thank you for your hint!'}) + + +class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, XmlDescriptor): + module_class = CrowdsourceHinterModule + stores_state = True + + @classmethod + def definition_from_xml(cls, xml_object, system): + children = [] + for child in xml_object: + try: + children.append(system.process_xml(etree.tostring(child, encoding='unicode')).location.url()) + except Exception as e: + log.exception("Unable to load child when parsing CrowdsourceHinter. Continuing...") + if system.error_tracker is not None: + system.error_tracker("ERROR: " + str(e)) + continue + return {}, children + + def definition_to_xml(self, resource_fs): + xml_object = etree.Element('crowdsource_hinter') + for child in self.get_children(): + xml_object.append( + etree.fromstring(child.export_to_xml(resource_fs))) + return xml_object \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 1f3be9e5e9..4640f7555d 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -223,6 +223,7 @@ class @Problem @el.removeClass 'showed' else @gentle_alert response.success + Logger.log 'problem_graded', [@answers, response.contents], @url reset: => Logger.log 'problem_reset', @answers diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee new file mode 100644 index 0000000000..1e38ff0e82 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -0,0 +1,62 @@ +class @Hinter + + constructor: (element) -> + @el = $(element).find('.crowdsource-wrapper') + @url = @el.data('url') + Logger.listen('problem_graded', @el.data('child-url'), @capture_problem) + # The line below will eventually be generated by Python. + @render() + + capture_problem: (event_type, data, element) => + # After a problem gets graded, we get the info here. + # We want to send this info to the server in another AJAX + # request. + answers = data[0] + response = data[1] + if response.search(/class="correct "/) == -1 + # Incorrect. Get hints. + $.postWithPrefix "#{@url}/get_hint", answers, (response) => + @render(response.contents) + else + # Correct. Get feedback from students. + $.postWithPrefix "#{@url}/get_feedback", answers, (response) => + @render(response.contents) + + $: (selector) -> + $(selector, @el) + + bind: => + window.update_schematics() + @$('input.vote').click @vote + @$('#feedback-select').change @feedback_ui_change + @$('input.submit-hint').click @submit_hint + + + vote: (eventObj) => + target = @$(eventObj.currentTarget) + post_json = {'answer': target.data('answer'), 'hint': target.data('hintno')} + $.postWithPrefix "#{@url}/vote", post_json, (response) => + @render(response.contents) + + submit_hint: (eventObj) => + target = @$(eventObj.currentTarget) + textarea_id = '#custom-hint-' + target.data('answer') + console.debug(textarea_id) + post_json = {'answer': target.data('answer'), 'hint': @$(textarea_id).val()} + $.postWithPrefix "#{@url}/submit_hint",post_json, (response) => + @render(response.contents) + + 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 + @el.html(content) + JavascriptLoader.executeModuleScripts @el, () => + @bind() + @$('#previous-answer-0').css('display', 'inline') \ No newline at end of file diff --git a/common/static/coffee/src/logger.coffee b/common/static/coffee/src/logger.coffee index f2dfef5132..6eaa497255 100644 --- a/common/static/coffee/src/logger.coffee +++ b/common/static/coffee/src/logger.coffee @@ -1,8 +1,11 @@ class @Logger + # events we want sent to Segment.io for tracking SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev", "problem_check", "problem_reset", "problem_show", "problem_save"] - @log: (event_type, data) -> + # listeners[event_type][element] -> list of callbacks + listeners = {} + @log: (event_type, data, element = null) -> # Segment.io event tracking if event_type in SEGMENT_IO_WHITELIST # to avoid changing the format of data sent to our servers, we only massage it here @@ -11,11 +14,36 @@ class @Logger else analytics.track event_type, data + # Check to see if we're listening for the event type. + if event_type of listeners + # Cool. Do the elements also match? + # null element in the listener dictionary means any element will do. + # null element in the @log call means we don't know the element name. + if null of listeners[event_type] + # Make the callbacks. + for callback in listeners[event_type][null] + callback(event_type, data, element) + else if element of listeners[event_type] + for callback in listeners[event_type][element] + callback(event_type, data, element) + + # Regardless of whether any callbacks were made, log this event. $.getWithPrefix '/event', event_type: event_type event: JSON.stringify(data) page: window.location.href + @listen: (event_type, element, callback) -> + # Add a listener. If you want any element to trigger this listener, + # do element = null + if event_type not of listeners + listeners[event_type] = {} + if element not of listeners[event_type] + listeners[event_type][element] = [callback] + else + listeners[event_type][element].push callback + + @bind: -> window.onunload = -> $.ajaxWithPrefix From 0c0de20a2f0eadea9b3a3d2b1d9ef93457c0e8a6 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Thu, 20 Jun 2013 09:44:19 -0400 Subject: [PATCH 02/19] Began work on instructor view to hinting system. Added moderation feature - you can now choose to hold all hints for moderator approval before showing. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 65 ++++++++- lms/djangoapps/instructor/hint_manager.py | 138 ++++++++++++++++++ lms/templates/courseware/hint_manager.html | 89 +++++++++++ .../courseware/hint_manager_inner.html | 39 +++++ lms/urls.py | 3 + 5 files changed, 333 insertions(+), 1 deletion(-) create mode 100644 lms/djangoapps/instructor/hint_manager.py create mode 100644 lms/templates/courseware/hint_manager.html create mode 100644 lms/templates/courseware/hint_manager_inner.html diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 1d424b7fff..97120bbf1c 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -42,6 +42,14 @@ class CrowdsourceHinterFields(object): user_voted = Boolean(help='Specifies if the user has voted on this problem or not.', scope=Scope.user_state, default=False) + moderate = String(help='''If 'True', then all hints must be approved by staff before + becoming visible. + This field is automatically populated from the xml metadata.''', scope=Scope.settings, + default='False') + + mod_queue = Dict(help='''Contains hints that have not been approved by the staff yet. Structured + identically to the hints dictionary.''', scope=Scope.content, default={}) + class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): ''' An Xmodule that makes crowdsourced hints. @@ -115,7 +123,11 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): print self.hints answer = self.ans_to_text(get) # Look for a hint to give. +<<<<<<< HEAD if answer not in self.hints: +======= + if (answer not in self.hints) or (len(self.hints[answer]) == 0): +>>>>>>> Began work on instructor view to hinting system. # No hints to give. Return. self.previous_answers += [(answer, (None, None, None))] return json.dumps({'contents': ' '}) @@ -126,12 +138,23 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): if len(self.hints[answer]) == 1: rand_hint_1 = '' rand_hint_2 = '' +<<<<<<< HEAD self.previous_answers += [(answer, (0, None, None))] elif len(self.hints[answer]) == 2: best_hint = self.hints[answer][0][0] rand_hint_1 = self.hints[answer][1][0] rand_hint_2 = '' self.previous_answers += [(answer, (0, 1, None))] +======= + self.previous_answers += [[answer, [best_hint_index, None, None]]] + elif n_hints == 2: + best_hint = self.hints[answer].values()[0][0] + best_hint_index = self.hints[answer].keys()[0] + rand_hint_1 = self.hints[answer].values()[1][0] + hint_index_1 = self.hints[answer].keys()[1] + rand_hint_2 = '' + self.previous_answers += [[answer, [best_hint_index, hint_index_1, None]]] +>>>>>>> Began work on instructor view to hinting system. else: hint_index_1, hint_index_2 = random.sample(xrange(len(self.hints[answer])), 2) rand_hint_1 = self.hints[answer][hint_index_1][0] @@ -163,10 +186,20 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): '" style="display:none"> Which hint was most helpful when you got the wrong answer of '\ + answer + '?' # Add each hint to the html string, with a vote button. - for j, hint_id in enumerate(hints_offered): + for hint_id in hints_offered: if hint_id != None: +<<<<<<< HEAD out += '
' + self.hints[answer][hint_id][0] +======= + hint_id = str(hint_id) + try: + out += '
' + self.hints[answer][hint_id][0] + except KeyError: + # Sometimes, the hint that a user saw will have been deleted by the instructor. + continue +>>>>>>> Began work on instructor view to hinting system. # Or, let the student create his own hint @@ -227,15 +260,45 @@ What would you say to help someone who got this wrong answer? answer = self.previous_answers[int(get['answer'])][0] # Add the new hint to self.hints. (Awkward because a direct write # is necessary.) +<<<<<<< HEAD temp_dict = self.hints temp_dict[answer].append([hint, 1]) # With one vote (the user himself). self.hints = temp_dict +======= + if self.moderate: + temp_dict = self.mod_queue + else: + temp_dict = self.hints + if answer in temp_dict: + temp_dict[answer][self.hint_pk] = [hint, 1] # With one vote (the user himself). + else: + temp_dict[answer] = {self.hint_pk: [hint, 1]} + self.hint_pk += 1 + if self.moderate: + self.mod_queue = temp_dict + else: + self.hints = temp_dict +>>>>>>> Began work on instructor view to hinting system. # Mark the user has having voted; reset previous_answers self.user_voted = True self.previous_answers = [] return json.dumps({'contents': 'Thank you for your hint!'}) +<<<<<<< HEAD +======= + def delete_hint(self, answer, hint_id): + ''' + From the answer, delete the hint with hint_id. + Not designed to be accessed via POST request, for now. + -LIKELY DEPRECATED. + ''' + temp_hints = self.hints + del temp_hints[answer][str(hint_id)] + self.hints = temp_hints + + +>>>>>>> Began work on instructor view to hinting system. class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, XmlDescriptor): module_class = CrowdsourceHinterModule stores_state = True diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py new file mode 100644 index 0000000000..431d3f5d7c --- /dev/null +++ b/lms/djangoapps/instructor/hint_manager.py @@ -0,0 +1,138 @@ +''' +Views for hint management. +''' + +from collections import defaultdict +import csv +import json +import logging +from markupsafe import escape +import os +import re +import requests +from requests.status_codes import codes +import urllib +from collections import OrderedDict + +from StringIO import StringIO + +from django.conf import settings +from django.contrib.auth.models import User, Group +from django.http import HttpResponse, Http404 +from django_future.csrf import ensure_csrf_cookie +from django.views.decorators.cache import cache_control +from mitxmako.shortcuts import render_to_response, render_to_string +from django.core.urlresolvers import reverse + +from courseware.courses import get_course_with_access +from courseware.models import XModuleContentField + + +@ensure_csrf_cookie +def hint_manager(request, course_id): + try: + course = 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 + if request.method == 'GET': + out = get_hints(request, course_id, 'mod_queue') + return render_to_response('courseware/hint_manager.html', out) + field = request.POST['field'] + if not (field == 'mod_queue' or field == 'hints'): + # Invalid field. (Don't let users continue - they may overwrite other db's) + return + if request.POST['op'] == 'delete hints': + delete_hints(request, course_id, field) + if request.POST['op'] == 'switch fields': + pass + if request.POST['op'] == 'change votes': + change_votes(request, course_id, field) + rendered_html = render_to_string('courseware/hint_manager_inner.html', get_hints(request, course_id, field)) + return HttpResponse(json.dumps({'success': True, 'contents': rendered_html})) + + + +def get_hints(request, course_id, field): + # field indicates the database entry that we are modifying. + # Right now, the options are 'hints' or 'mod_queue'. + # DON'T TRUST field attributes that come from ajax. Use an if statement + # to make sure the field is valid before plugging into functions. + + out = '' + if field == 'mod_queue': + other_field = 'hints' + field_label = 'Hints Awaiting Moderation' + other_field_label = 'Approved Hints' + elif field == 'hints': + other_field = 'mod_queue' + field_label = 'Approved Hints' + other_field_label = 'Hints Awaiting Moderation' + 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) + for problem in all_hints: + out += '

Problem: ' + problem.definition_id + '

' + for answer, hint_dict in json.loads(problem.value).items(): + out += '

Answer: ' + answer + '

' + for pk, hint in hint_dict.items(): + out += '

' + out += '' + hint[0] + \ + '
Votes: ' + out += '

' + out += '''

Add a hint to this problem

+ Answer (exact formatting): +
Hint:


' + + + out += ' ' + render_dict = {'out': out, + 'field': field, + 'other_field': other_field, + 'field_label': field_label, + 'other_field_label': other_field_label, + 'all_hints': all_hints} + return render_dict + +def delete_hints(request, course_id, field): + ''' + Deletes the hints specified by the [problem_defn_id, answer, pk] tuples in the numbered + fields of request.POST. + ''' + for key in request.POST: + if key == 'op' or key == 'field': + continue + problem_id, answer, pk = request.POST.getlist(key) + # Can be optimized - sort the delete list by problem_id, and load each problem + # from the database only once. + this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + problem_dict = json.loads(this_problem.value) + del problem_dict[answer][pk] + this_problem.value = json.dumps(problem_dict) + this_problem.save() + +def change_votes(request, course_id, field): + ''' + Updates the number of votes. The numbered fields of request.POST contain + [problem_id, answer, pk, new_votes] tuples. + - Very similar to delete_hints. Is there a way to merge them? Nah, too complicated. + ''' + for key in request.POST: + if key == 'op' or key == 'field': + continue + problem_id, answer, pk, new_votes = request.POST.getlist(key) + this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + problem_dict = json.loads(this_problem.value) + problem_dict[answer][pk][1] = new_votes + this_problem.value = json.dumps(problem_dict) + this_problem.save() + + + + + diff --git a/lms/templates/courseware/hint_manager.html b/lms/templates/courseware/hint_manager.html new file mode 100644 index 0000000000..94156d3d68 --- /dev/null +++ b/lms/templates/courseware/hint_manager.html @@ -0,0 +1,89 @@ +<%inherit file="/main.html" /> +<%namespace name='static' file='/static_content.html'/> +<%namespace name="content" file="/courseware/hint_manager_inner.html"/> + + +<%block name="headextra"> + <%static:css group='course'/> + + + + + + + + + + +
+
+ +
+ ${content.main()} +
+ +
+
diff --git a/lms/templates/courseware/hint_manager_inner.html b/lms/templates/courseware/hint_manager_inner.html new file mode 100644 index 0000000000..41e8d018c5 --- /dev/null +++ b/lms/templates/courseware/hint_manager_inner.html @@ -0,0 +1,39 @@ +<%block name="main"> + + +

${field_label}

+Switch to ${other_field_label} + + +% for problem in all_hints: +

Problem: ${problem.definition_id}

+ <% + import json + loaded_json = json.loads(problem.value).items() + %> + % for answer, hint_dict in loaded_json: +

Answer: ${answer}

+ % for pk, hint in hint_dict.items(): +

+ ${hint[0]} +
+ Votes: +

+ % endfor + % endfor + +

Add a hint to this problem

+ Answer (exact formatting): + +
+ Hint:
+ +
+ +
+% endfor + + + + + \ No newline at end of file diff --git a/lms/urls.py b/lms/urls.py index 52a7d99aaf..55d4efd8b8 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -264,6 +264,9 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/instructor$', 'instructor.views.instructor_dashboard', name="instructor_dashboard"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/hint_manager$', + 'instructor.hint_manager.hint_manager', name="hint_manager"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/gradebook$', 'instructor.views.gradebook', name='gradebook'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade_summary$', From aba99084f22c1b3aef544e86991a448dd9f4dca9 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Thu, 20 Jun 2013 17:09:00 -0400 Subject: [PATCH 03/19] Finished prototype of hint moderation view. Began re-writing tests of the crowdsource hinter module. (Old tests no longer cover all the code, now that moderation has been added.) --- .../xmodule/tests/test_crowdsource_hinter.py | 296 ++++++++++++++++++ lms/djangoapps/instructor/hint_manager.py | 129 ++++++-- lms/templates/courseware/hint_manager.html | 35 +++ .../courseware/hint_manager_inner.html | 40 ++- 4 files changed, 462 insertions(+), 38 deletions(-) create mode 100644 common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py new file mode 100644 index 0000000000..7fe890fa77 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -0,0 +1,296 @@ +from mock import Mock, patch +import unittest + +import xmodule +from xmodule.crowdsource_hinter import CrowdsourceHinterModule +from xmodule.modulestore import Location + +from django.http import QueryDict + +from . import test_system + +import json + +class CHModuleFactory(object): + ''' + Helps us make a CrowdsourceHinterModule with the specified internal + state. + ''' + + sample_problem_xml = ''' + + + +

A numerical input problem accepts a line of text input from the student, and evaluates the input for correctness based on its numerical value.

+

The answer is correct if it is within a specified numerical tolerance of the expected answer.

+

Enter the number of fingers on a human hand:

+ + + + +
+

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(hints=None, + previous_answers=None, + user_voted=None, + moderate=None, + mod_queue=None): + + location = Location(["i4x", "edX", "capa_test", "problem", + "SampleProblem{0}".format(CHModuleFactory.next_num())]) + model_data = {'data': CHModuleFactory.sample_problem_xml} + + if hints != 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]}, + '25.0': {'1': ['Really popular hint', 100]} + } + + if mod_queue != None: + model_data['mod_queue'] = mod_queue + else: + model_data['mod_queue'] = { + '24.0': {'2': ['A non-approved hint']}, + '26.0': {'5': ['Another non-approved hint']} + } + + if previous_answers != None: + model_data['previous_answers'] = previous_answers + else: + model_data['previous_answers'] = [ + ['24.0', [0, 3, 4]], + ['29.0', [None, None, None]] + ] + + if user_voted != None: + model_data['user_voted'] = user_voted + + if moderate != None: + model_data['moderate'] = moderate + + descriptor = Mock(weight="1") + system = test_system() + system.render_template = Mock(return_value="
Test Template HTML
") + module = CrowdsourceHinterModule(system, descriptor, model_data) + + return module + +class CrowdsourceHinterTest(unittest.TestCase): + ''' + In the below tests, '24.0' represents a wrong answer, and '42.5' represents + a correct answer. + ''' + + def test_gethint_0hint(self): + ''' + Someone asks for a hint, when there's no hint to give. + - Output should be blank. + - New entry should be added to previous_answers + ''' + m = CHModuleFactory.create() + json_in = {'problem_name': '26.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue(json_out == ' ') + self.assertTrue(['26.0', [None, None, None]] in m.previous_answers) + + def test_gethint_1hint(self): + ''' + Someone asks for a hint, with exactly one hint in the database. + Output should contain that hint. + ''' + m = CHModuleFactory.create() + json_in = {'problem_name': '25.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue('Really popular hint' in json_out) + + + def test_gethint_manyhints(self): + ''' + Someone asks for a hint, with many matching hints in the database. + - The top-rated hint should be returned. + - Two other random hints should be returned. + Currently, the best hint could be returned twice - need to fix this + in implementation. + ''' + m = CHModuleFactory.create() + json_in = {'problem_name': '24.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue('Best hint' in json_out) + self.assertTrue(json_out.count('hint') == 3) + + + def test_getfeedback_0wronganswers(self): + ''' + Someone has gotten the problem correct on the first try. + Output should be empty. + ''' + m = CHModuleFactory.create(previous_answers=[]) + json_in = {'problem_name': '42.5'} + json_out = json.loads(m.get_feedback(json_in))['contents'] + self.assertTrue(json_out == ' ') + + def test_getfeedback_1wronganswer_nohints(self): + ''' + Someone has gotten the problem correct, with one previous wrong + 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]]]) + json_in = {'problem_name': '42.5'} + json_out = json.loads(m.get_feedback(json_in))['contents'] + self.assertTrue('textarea' in json_out) + self.assertTrue('Vote' not in json_out) + + + 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(hints={ + '24.0': {'0': ['a hint', 42], + '1': ['another hint', 35], + '2': ['irrelevent hint', 25.0]} + }, + previous_answers=[ + ['24.0', [0, 1, None]]], + ) + json_in = {'problem_name': '42.5'} + json_out = json.loads(m.get_feedback(json_in))['contents'] + self.assertTrue('a hint' in json_out) + self.assertTrue('another hint' in json_out) + self.assertTrue('irrelevent hint' not in json_out) + self.assertTrue('textarea' in json_out) + + + def test_vote_nopermission(self): + ''' + A user tries to vote for a hint, but he has already voted! + Should not change any vote tallies. + ''' + m = CHModuleFactory.create(hints={ + '24.0': {'0': ['a hint', 42], + '1': ['another hint', 35], + '2': ['irrelevent hint', 25.0]} + }, + previous_answers=[ + ['24.0', [0, 1, None]]], + user_voted=True + ) + json_in = {'answer': 0, 'hint': 1} + json_out = json.loads(m.tally_vote(json_in))['contents'] + self.assertTrue(m.hints['24.0']['0'][1] == 42) + self.assertTrue(m.hints['24.0']['1'][1] == 35) + self.assertTrue(m.hints['24.0']['2'][1] == 25.0) + + + def test_vote_withpermission(self): + ''' + A user votes for a hint. + ''' + m = CHModuleFactory.create(hints={ + '24.0': {'0': ['a hint', 42], + '1': ['another hint', 35], + '2': ['irrelevent hint', 25.0]} + }, + previous_answers=[ + ['24.0', [0, 1, None]]], + ) + json_in = {'answer': 0, 'hint': 1} + json_out = json.loads(m.tally_vote(json_in))['contents'] + self.assertTrue(m.hints['24.0']['0'][1] == 42) + self.assertTrue(m.hints['24.0']['1'][1] == 36) + self.assertTrue(m.hints['24.0']['2'][1] == 25.0) + + + def test_submithint_nopermission(self): + ''' + A user tries to submit a hint, but he has already voted. + ''' + m = CHModuleFactory.create(previous_answers=[ + ['24.0', [None, None, None]]], + user_voted=True) + json_in = {'answer': 0, 'hint': 'This is a new hint.'} + m.submit_hint(json_in) + self.assertTrue('24.0' not in m.hints) + + + def test_submithint_withpermission_new(self): + ''' + A user submits a hint to an answer for which no hints + exist yet. + ''' + m = CHModuleFactory.create(previous_answers=[ + ['24.0', [None, 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': '24.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue('This is a new hint.' in json_out) + + + def test_submithint_withpermission_existing(self): + ''' + A user submits a hint to an answer that has other hints + already. + ''' + m = CHModuleFactory.create(previous_answers=[ + ['24.0', [0, None, None]]], + hints={'24.0': {'0': ['Existing hint.', 1]}} + ) + json_in = {'answer': 0, 'hint': 'This is a new hint.'} + m.submit_hint(json_in) + # Make a hint request. + json_in = {'problem name': '24.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue('This is a new hint.' in json_out) + + def test_deletehint(self): + ''' + An admin / instructor deletes a hint. + ''' + m = CHModuleFactory.create(hints={ + '24.0': {'0': ['Deleted hint', 5], + '1': ['Safe hint', 4]} + }) + m.delete_hint('24.0', '0') + json_in = {'problem name': '24.0'} + json_out = json.loads(m.get_hint(json_in))['contents'] + self.assertTrue('Deleted hint' not in json_out) + self.assertTrue('Safe hint' in json_out) + + + + + + + + + + + + + diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index 431d3f5d7c..5d722b1e79 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -26,6 +26,8 @@ from django.core.urlresolvers import reverse from courseware.courses import get_course_with_access from courseware.models import XModuleContentField +from xmodule.modulestore import Location +from xmodule.modulestore.django import modulestore @ensure_csrf_cookie @@ -48,6 +50,10 @@ def hint_manager(request, course_id): pass if request.POST['op'] == 'change votes': change_votes(request, course_id, field) + if request.POST['op'] == 'add hint': + add_hint(request, course_id, field) + if request.POST['op'] == 'approve': + approve(request, course_id, field) rendered_html = render_to_string('courseware/hint_manager_inner.html', get_hints(request, course_id, field)) return HttpResponse(json.dumps({'success': True, 'contents': rendered_html})) @@ -59,7 +65,6 @@ def get_hints(request, course_id, field): # DON'T TRUST field attributes that come from ajax. Use an if statement # to make sure the field is valid before plugging into functions. - out = '' if field == 'mod_queue': other_field = 'hints' field_label = 'Hints Awaiting Moderation' @@ -71,32 +76,40 @@ def get_hints(request, course_id, field): 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 = {} + name_dict = {} for problem in all_hints: - out += '

Problem: ' + problem.definition_id + '

' - for answer, hint_dict in json.loads(problem.value).items(): - out += '

Answer: ' + answer + '

' - for pk, hint in hint_dict.items(): - out += '

' - out += '' + hint[0] + \ - '
Votes: ' - out += '

' - out += '''

Add a hint to this problem

- Answer (exact formatting): -
Hint:


' + loc = Location(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. + continue + name_dict[problem.definition_id] = descriptor.get_children()[0].display_name + # Answer list contains (answer, dict_of_hints) tuples. + def answer_sorter(thing): + ''' + thing is a tuple, where thing[0] contains an answer, and thing[1] contains + a dict of hints. This function returns an index based on thing[0], which + is used as a key to sort the list of things. + ''' + try: + return float(thing[0]) + except ValueError: + # Put all non-numerical answers first. + return float('-inf') - out += ' ' - render_dict = {'out': out, - 'field': field, + answer_list = sorted(json.loads(problem.value).items(), key=answer_sorter) + big_out_dict[problem.definition_id] = answer_list + + render_dict = {'field': field, 'other_field': other_field, 'field_label': field_label, 'other_field_label': other_field_label, - 'all_hints': all_hints} + 'all_hints': big_out_dict, + 'id_to_name': name_dict} return render_dict def delete_hints(request, course_id, field): @@ -131,6 +144,80 @@ def change_votes(request, course_id, field): problem_dict[answer][pk][1] = new_votes this_problem.value = json.dumps(problem_dict) this_problem.save() + +def add_hint(request, course_id, field): + ''' + Add a new hint. POST: + op + field + problem - The problem id + answer - The answer to which a hint will be added + hint - The text of the hint + ''' + problem_id = request.POST['problem'] + answer = request.POST['answer'] + hint_text = request.POST['hint'] + this_problem = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + + hint_pk_entry = XModuleContentField.objects.get(field_name='hint_pk', definition_id=problem_id) + this_pk = int(hint_pk_entry.value) + hint_pk_entry.value = this_pk + 1 + hint_pk_entry.save() + + problem_dict = json.loads(this_problem.value) + if answer not in problem_dict: + problem_dict[answer] = {} + problem_dict[answer][this_pk] = [hint_text, 1] + this_problem.value = json.dumps(problem_dict) + this_problem.save() + +def approve(request, course_id, field): + ''' + Approve a list of hints, moving them from the mod_queue to the real + hint list. POST: + op, field + (some number) -> [problem, answer, pk] + ''' + for key in request.POST: + if key == 'op' or key == 'field': + continue + problem_id, answer, pk = request.POST.getlist(key) + # Can be optimized - sort the delete list by problem_id, and load each problem + # from the database only once. + problem_in_mod = XModuleContentField.objects.get(field_name=field, definition_id=problem_id) + problem_dict = json.loads(problem_in_mod.value) + hint_to_move = problem_dict[answer][pk] + del problem_dict[answer][pk] + problem_in_mod.value = json.dumps(problem_dict) + problem_in_mod.save() + + problem_in_hints = XModuleContentField.objects.get(field_name='hints', definition_id=problem_id) + problem_dict = json.loads(problem_in_hints.value) + if answer not in problem_dict: + problem_dict[answer] = {} + problem_dict[answer][pk] = hint_to_move + problem_in_hints.value = json.dumps(problem_dict) + problem_in_hints.save() + + + + + + + + + + + + + + + + + + + + diff --git a/lms/templates/courseware/hint_manager.html b/lms/templates/courseware/hint_manager.html index 94156d3d68..394792f892 100644 --- a/lms/templates/courseware/hint_manager.html +++ b/lms/templates/courseware/hint_manager.html @@ -28,6 +28,7 @@ data_dict[i] = [$(this).parent().attr("data-problem"), $(this).parent().attr("data-answer"), $(this).parent().attr("data-pk")]; + i += 1 } }); $.ajax(window.location.pathname, { @@ -64,6 +65,40 @@ }); }); + $(".submit-new-hint").click(function(){ + problem_name = $(this).data("problem"); + hint_text = $(".submit-hint-text").filter('*[data-problem="'+problem_name+'"]').val(); + hint_answer = $(".submit-hint-answer").filter('*[data-problem="'+problem_name+'"]').val(); + data_dict = {'op': 'add hint', + 'field': field, + 'problem': problem_name, + 'answer': hint_answer, + 'hint': hint_text}; + $.ajax(window.location.pathname, { + type: "POST", + data: data_dict, + success: update_contents + }); + }); + + $("#approve").click(function(){ + var data_dict = {'op': 'approve', + 'field': field} + var i = 1 + $(".hint-select").each(function(){ + if ($(this).is(":checked")) { + data_dict[i] = [$(this).parent().attr("data-problem"), + $(this).parent().attr("data-answer"), + $(this).parent().attr("data-pk")]; + i += 1 + } + }); + $.ajax(window.location.pathname, { + type: "POST", + data: data_dict, + success: update_contents + }); + }); } $(document).ready(setup); diff --git a/lms/templates/courseware/hint_manager_inner.html b/lms/templates/courseware/hint_manager_inner.html index 41e8d018c5..c69539522f 100644 --- a/lms/templates/courseware/hint_manager_inner.html +++ b/lms/templates/courseware/hint_manager_inner.html @@ -1,39 +1,45 @@ <%block name="main"> - +

${field_label}

-Switch to ${other_field_label} +Switch to ${other_field_label} -% for problem in all_hints: -

Problem: ${problem.definition_id}

- <% - import json - loaded_json = json.loads(problem.value).items() - %> - % for answer, hint_dict in loaded_json: -

Answer: ${answer}

+% for definition_id in all_hints: +

Problem: ${id_to_name[definition_id]}

+ % for answer, hint_dict in all_hints[definition_id]: + % if len(hint_dict) > 0: +

Answer: ${answer}

+ % endif % for pk, hint in hint_dict.items(): -

+

${hint[0]}
- Votes: + Votes: +

% endfor + % if len(hint_dict) > 0: +

+ % endif % endfor

Add a hint to this problem

- Answer (exact formatting): - +

Answer:

+ + (Be sure to format your answer in the same way as the other answers you see here.)
Hint:
- +
- +
% endfor - + +% if field == 'mod_queue': + +% endif \ No newline at end of file From 2088da4edcf70c56b719218e6856da65065927ef Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Fri, 21 Jun 2013 12:15:03 -0400 Subject: [PATCH 04/19] Made tests of the crowdsource hinter module more standardized and easier to read. Fixed database non-initialization bug in crowdsource hinter module. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 32 +++---- .../xmodule/tests/test_crowdsource_hinter.py | 92 +++++++------------ 2 files changed, 44 insertions(+), 80 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 97120bbf1c..5fc9cab09b 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -42,9 +42,9 @@ class CrowdsourceHinterFields(object): user_voted = Boolean(help='Specifies if the user has voted on this problem or not.', scope=Scope.user_state, default=False) - moderate = String(help='''If 'True', then all hints must be approved by staff before + moderate = String(help='''If True, then all hints must be approved by staff before becoming visible. - This field is automatically populated from the xml metadata.''', scope=Scope.settings, + This field is automatically populated from the xml metadata.''', scope=Scope.content, default='False') mod_queue = Dict(help='''Contains hints that have not been approved by the staff yet. Structured @@ -73,6 +73,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): ''' # Reset the user vote, for debugging only! Remove for prod. self.user_voted = False + # You are invited to guess what the lines below do :) + if self.hints == {}: + self.hints = {} + for child in self.get_display_items(): out = child.get_html() # The event listener uses the ajax url to find the child. @@ -123,11 +127,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): print self.hints answer = self.ans_to_text(get) # Look for a hint to give. -<<<<<<< HEAD - if answer not in self.hints: -======= if (answer not in self.hints) or (len(self.hints[answer]) == 0): ->>>>>>> Began work on instructor view to hinting system. # No hints to give. Return. self.previous_answers += [(answer, (None, None, None))] return json.dumps({'contents': ' '}) @@ -138,14 +138,6 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): if len(self.hints[answer]) == 1: rand_hint_1 = '' rand_hint_2 = '' -<<<<<<< HEAD - self.previous_answers += [(answer, (0, None, None))] - elif len(self.hints[answer]) == 2: - best_hint = self.hints[answer][0][0] - rand_hint_1 = self.hints[answer][1][0] - rand_hint_2 = '' - self.previous_answers += [(answer, (0, 1, None))] -======= self.previous_answers += [[answer, [best_hint_index, None, None]]] elif n_hints == 2: best_hint = self.hints[answer].values()[0][0] @@ -154,7 +146,6 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): hint_index_1 = self.hints[answer].keys()[1] rand_hint_2 = '' self.previous_answers += [[answer, [best_hint_index, hint_index_1, None]]] ->>>>>>> Began work on instructor view to hinting system. else: hint_index_1, hint_index_2 = random.sample(xrange(len(self.hints[answer])), 2) rand_hint_1 = self.hints[answer][hint_index_1][0] @@ -188,10 +179,6 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): # Add each hint to the html string, with a vote button. for hint_id in hints_offered: if hint_id != None: -<<<<<<< HEAD - out += '
' + self.hints[answer][hint_id][0] -======= hint_id = str(hint_id) try: out += '
' + rand_hint_1 + '
' + rand_hint_2 return json.dumps({'contents': hint_text}) @@ -172,9 +166,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): answer, hints_offered = self.previous_answers[i] pretty_answers.append(answer) # If there are previous hints for this answer, ask the student to vote on one. + out += ' From 696cc3a4db90a404b7d20dd270d3b0dafde15fd7 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Wed, 26 Jun 2013 15:53:40 -0400 Subject: [PATCH 10/19] Fixed numerous code-formatting issues and pep8 violations. Began enforcing one-vote-per-person. This can be disabled with debug="True" in the tag. Started tests of the hint manager. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 178 ++++++++++-------- .../js/src/crowdsource_hinter/display.coffee | 9 +- .../xmodule/tests/test_crowdsource_hinter.py | 70 ++++++- common/templates/hinter_display.html | 2 +- lms/djangoapps/instructor/hint_manager.py | 142 +++++++------- .../instructor/tests/test_hint_manager.py | 79 ++++++++ 6 files changed, 313 insertions(+), 167 deletions(-) create mode 100644 lms/djangoapps/instructor/tests/test_hint_manager.py diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index c5d5dd7f80..664cf85f1a 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -1,20 +1,20 @@ +""" +Adds crowdsourced hinting functionality to lon-capa numerical response problems. + +Currently experimental - not for instructor use, yet. +""" + import logging -import copy import json -import os -import re -import string import random -from pkg_resources import resource_listdir, resource_string, resource_isdir +from pkg_resources import resource_string from lxml import etree -from xmodule.modulestore import Location -from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModule from xmodule.xml_module import XmlDescriptor -from xblock.core import XBlock, Scope, String, Integer, Float, Boolean, Dict, List +from xblock.core import Scope, String, Integer, Boolean, Dict, List from django.utils.html import escape @@ -22,112 +22,123 @@ log = logging.getLogger(__name__) class CrowdsourceHinterFields(object): + """Defines fields for the crowdsource hinter module.""" has_children = True - hints = Dict(help="""A dictionary mapping answers to lists of [hint, number_of_votes] pairs. - """, scope=Scope.content, default= {}) - - previous_answers = List(help="""A list of previous answers this student made to this problem. - Of the form (answer, (hint_id_1, hint_id_2, hint_id_3)) for each problem. hint_id's are - None if the hint was not given.""", - scope=Scope.user_state, default=[]) - - user_voted = Boolean(help='Specifies if the user has voted on this problem or not.', - scope=Scope.user_state, default=False) - - moderate = String(help="""If True, then all hints must be approved by staff before - becoming visible. - This field is automatically populated from the xml metadata.""", scope=Scope.content, - default='False') - - mod_queue = Dict(help="""Contains hints that have not been approved by the staff yet. Structured - identically to the hints dictionary.""", scope=Scope.content, default={}) + moderate = String(help='String "True"/"False" - activates moderation', scope=Scope.content, + default='False') + debug = String(help='String "True"/"False" - allows multiple voting', scope=Scope.content, + default='False') + # hints[answer] = {str(pk): [hint_text, #votes]} + 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 + # 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.', + scope=Scope.user_state, default=False) class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): - """ An Xmodule that makes crowdsourced hints. + """ + An Xmodule that makes crowdsourced hints. + Currently, only works on capa problems with exactly one numerical response, + and no other parts. + + Example usage: + + + + + XML attributes: + -moderate="True" will not display hints until staff approve them in the hint manager. + -debug="True" will let users vote as often as they want. """ icon_class = 'crowdsource_hinter' - - js = {'coffee': [resource_string(__name__, 'js/src/crowdsource_hinter/display.coffee'), - ], - 'js': []} + js = {'coffee': [resource_string(__name__, 'js/src/crowdsource_hinter/display.coffee')], + 'js': []} js_module_name = "Hinter" - def __init__(self, *args, **kwargs): XModule.__init__(self, *args, **kwargs) - def get_html(self): """ - Does a regular expression find and replace to change the AJAX url. + Puts a wrapper around the problem html. This wrapper includes ajax urls of the + hinter and of the problem. - Dependent on lon-capa problem. """ - # Reset the user vote, for debugging only! Remove for prod. - self.user_voted = False - # You are invited to guess what the lines below do :) + if self.debug == 'True': + # Reset the user vote, for debugging only! + self.user_voted = False if self.hints == {}: + # Force self.hints to be written into the database. (When an xmodule is initialized, + # fields are not added to the db until explicitly changed at least once.) self.hints = {} - for child in self.get_display_items(): + try: + child = self.get_display_items()[0] out = child.get_html() # The event listener uses the ajax url to find the child. child_url = child.system.ajax_url - break + except IndexError: + out = 'Error in loading crowdsourced hinter - can\'t find child problem.' + child_url = '' + # Wrap the module in a
. This lets us pass data attributes to the javascript. out += '
' return out - def capa_make_answer_hashable(self, answer): - """ - Capa answer format: dict[problem name] -> [list of answers] - Output format: ((problem name, (answers))) - """ - out = [] - for problem, a in answer.items(): - out.append((problem, tuple(a))) - return str(tuple(sorted(out))) - - - def ans_to_text(self, answer): + def capa_answer_to_str(self, answer): """ Converts capa answer format to a string representation of the answer. -Lon-capa dependent. + -Assumes that the problem only has one part. """ return str(float(answer.values()[0])) - def handle_ajax(self, dispatch, get): """ This is the landing method for AJAX calls. """ if dispatch == 'get_hint': out = self.get_hint(get) - if dispatch == 'get_feedback': + elif dispatch == 'get_feedback': out = self.get_feedback(get) - if dispatch == 'vote': + elif dispatch == 'vote': out = self.tally_vote(get) - if dispatch == 'submit_hint': + elif dispatch == 'submit_hint': out = self.submit_hint(get) + else: + return json.dumps({'contents': 'Error - invalid operation.'}) - if out == None: + if out is None: out = {'op': 'empty'} else: out.update({'op': dispatch}) return json.dumps({'contents': self.system.render_template('hinter_display.html', out)}) - def get_hint(self, get): """ The student got the incorrect answer found in get. Give him a hint. + + Called by hinter javascript after a problem is graded as incorrect. + Args: + get -- must be interpretable by capa_answer_to_str. + Output keys: + - best_hint is the hint text with the most votes. + - rand_hint_1 and rand_hint_2 are two random hints to the answer in get. + - answer is the parsed answer that was submitted. """ - answer = self.ans_to_text(get) + answer = self.capa_answer_to_str(get) # Look for a hint to give. # Make a local copy of self.hints - this means we only need to do one json unpacking. + # (This is because xblocks storage makes the following command a deep copy.) local_hints = self.hints if (answer not in local_hints) or (len(local_hints[answer]) == 0): # No hints to give. Return. @@ -156,13 +167,19 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): self.previous_answers += [(answer, (best_hint_index, hint_index_1, hint_index_2))] return {'best_hint': best_hint, - 'rand_hint_1': rand_hint_1, - 'rand_hint_2': rand_hint_2, + 'rand_hint_1': rand_hint_1, + 'rand_hint_2': rand_hint_2, 'answer': answer} def get_feedback(self, get): """ The student got it correct. Ask him to vote on hints, or submit a hint. + + Args: + get -- not actually used. (It is assumed that the answer is correct.) + Output keys: + - index_to_hints maps previous answer indices to hints that the user saw earlier. + - index_to_answer maps previous answer indices to the actual answer submitted. """ # The student got it right. # Did he submit at least one wrong answer? @@ -178,13 +195,15 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): # index_to_answer[previous answer #] = answer text index_to_answer = {} + # Go through each previous answer, and populate index_to_hints and index_to_answer. for i in xrange(len(self.previous_answers)): answer, hints_offered = self.previous_answers[i] index_to_hints[i] = [] index_to_answer[i] = answer if answer in self.hints: + # Go through each hint, and add to index_to_hints for hint_id in hints_offered: - if hint_id != None: + if hint_id is None: try: index_to_hints[i].append((self.hints[answer][str(hint_id)][0], hint_id)) except KeyError: @@ -193,22 +212,24 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): return {'index_to_hints': index_to_hints, 'index_to_answer': index_to_answer} - def tally_vote(self, get): """ Tally a user's vote on his favorite hint. - get: + + Args: + get -- expected to have the following keys: 'answer': ans_no (index in previous_answers) - 'hint': hint_no + 'hint': hint_pk + Returns key hint_and_votes, a list of (hint_text, #votes) pairs. """ if self.user_voted: - return json.dumps({'contents': 'Sorry, but you have already voted!'}) - ans_no = int(get['answer']) + return json.dumps({'contents': 'Sorry, but you have already voted!'}) + ans_no = int(get['answer']) hint_no = str(get['hint']) answer = self.previous_answers[ans_no][0] + # We use temp_dict because we need to do a direct write for the database to update. temp_dict = self.hints temp_dict[answer][hint_no][1] += 1 - # Awkward, but you need to do a direct write for the database to update. self.hints = temp_dict # Don't let the user vote again! self.user_voted = True @@ -216,7 +237,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): # Return a list of how many votes each hint got. hint_and_votes = [] for hint_no in self.previous_answers[ans_no][1]: - if hint_no == None: + if hint_no is None: continue hint_and_votes.append(temp_dict[answer][str(hint_no)]) @@ -227,16 +248,20 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): def submit_hint(self, get): """ Take a hint submission and add it to the database. - get: + + Args: + get -- expected to have the following keys: 'answer': answer index in previous_answers 'hint': text of the new hint that the user is adding + Returns a thank-you message. """ # Do html escaping. Perhaps in the future do profanity filtering, etc. as well. hint = escape(get['hint']) answer = self.previous_answers[int(get['answer'])][0] + # Only allow a student to vote or submit a hint once. if self.user_voted: - return {'message': 'Sorry, but you have already voted!'} - # Add the new hint to self.hints. (Awkward because a direct write + return {'message': 'Sorry, but you have already voted!'} + # Add the new hint to self.hints or self.mod_queue. (Awkward because a direct write # is necessary.) if self.moderate == 'True': temp_dict = self.mod_queue @@ -257,17 +282,6 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): return {'message': 'Thank you for your hint!'} - def delete_hint(self, answer, hint_id): - """ - From the answer, delete the hint with hint_id. - Not designed to be accessed via POST request, for now. - -LIKELY DEPRECATED. - """ - temp_hints = self.hints - del temp_hints[answer][str(hint_id)] - self.hints = temp_hints - - class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, XmlDescriptor): module_class = CrowdsourceHinterModule stores_state = True diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee index 8eeab4cb02..ea42601622 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -1,10 +1,13 @@ class @Hinter + # The client side code for the crowdsource_hinter. + # Contains code for capturing problem checks and making ajax calls to + # the server component. Also contains styling code to clear default + # text on a textarea. constructor: (element) -> @el = $(element).find('.crowdsource-wrapper') @url = @el.data('url') Logger.listen('problem_graded', @el.data('child-url'), @capture_problem) - # The line below will eventually be generated by Python. @render() capture_problem: (event_type, data, element) => @@ -32,7 +35,6 @@ class @Hinter @$('.custom-hint').click @clear_default_text @$('#answer-tabs').tabs({active: 0}) - vote: (eventObj) => target = @$(eventObj.currentTarget) post_json = {'answer': target.data('answer'), 'hint': target.data('hintno')} @@ -42,7 +44,6 @@ class @Hinter submit_hint: (eventObj) => target = @$(eventObj.currentTarget) textarea_id = '#custom-hint-' + target.data('answer') - console.debug(textarea_id) post_json = {'answer': target.data('answer'), 'hint': @$(textarea_id).val()} $.postWithPrefix "#{@url}/submit_hint",post_json, (response) => @render(response.contents) @@ -53,7 +54,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') @@ -61,7 +61,6 @@ class @Hinter selector = '#previous-answer-' + @$('#feedback-select option:selected').attr('value') @$(selector).css('display', 'inline') - render: (content) -> if content @el.html(content) diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index 350abe9c8f..31614c4849 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -97,6 +97,17 @@ class CHModuleFactory(object): return module +class FakeChild(object): + """ + A fake Xmodule. + """ + def __init__(self): + self.system = Mock() + self.system.ajax_url = 'this/is/a/fake/ajax/url' + + def get_html(self): + return 'This is supposed to be test html.' + class CrowdsourceHinterTest(unittest.TestCase): @@ -105,6 +116,22 @@ class CrowdsourceHinterTest(unittest.TestCase): a correct answer. """ + def test_gethtml(self): + """ + 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 + """ + return [FakeChild()] + m.get_display_items = fake_get_display_items + out_html = m.get_html() + 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_gethint_0hint(self): """ Someone asks for a hint, when there's no hint to give. @@ -182,6 +209,18 @@ class CrowdsourceHinterTest(unittest.TestCase): out = m.get_feedback(json_in) self.assertTrue(len(out['index_to_hints'][0])==2) + + def test_getfeedback_missingkey(self): + """ + Someone gets a problem correct, but one of the hints that he saw + earlier (pk=100) has been deleted. Should just skip that hint. + """ + m = CHModuleFactory.create( + 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) + def test_vote_nopermission(self): """ A user tries to vote for a hint, but he has already voted! @@ -197,13 +236,16 @@ class CrowdsourceHinterTest(unittest.TestCase): def test_vote_withpermission(self): """ A user votes for a hint. + Also tests vote result rendering. """ - m = CHModuleFactory.create() + m = CHModuleFactory.create( + previous_answers=[['24.0', [0, 3, None]]]) json_in = {'answer': 0, 'hint': 3} - m.tally_vote(json_in) + dict_out = m.tally_vote(json_in) self.assertTrue(m.hints['24.0']['0'][1] == 40) self.assertTrue(m.hints['24.0']['3'][1] == 31) - self.assertTrue(m.hints['24.0']['4'][1] == 20) + 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): @@ -256,6 +298,16 @@ class CrowdsourceHinterTest(unittest.TestCase): self.assertTrue('29.0' not in m.hints) self.assertTrue('29.0' in m.mod_queue) + def test_submithint_escape(self): + """ + Make sure that hints are being html-escaped. + """ + m = CHModuleFactory.create() + json_in = {'answer': 1, 'hint': ''} + m.submit_hint(json_in) + print m.hints + self.assertTrue(m.hints['29.0'][0][0] == u'<script> alert("Trololo"); </script>') + def test_template_gethint(self): """ @@ -284,7 +336,9 @@ class CrowdsourceHinterTest(unittest.TestCase): def test_template_feedback(self): """ Test the templates for get_feedback. - """ + NOT FINISHED + + from lxml import etree m = CHModuleFactory.create() def fake_get_feedback(get): @@ -297,9 +351,11 @@ class CrowdsourceHinterTest(unittest.TestCase): m.get_feedback = fake_get_feedback json_in = {'problem_name': '42.5'} out = json.loads(m.handle_ajax('get_feedback', json_in))['contents'] - - - + html_tree = etree.XML(out) + # To be continued... + + """ + pass diff --git a/common/templates/hinter_display.html b/common/templates/hinter_display.html index a253f9f639..f05bb34c40 100644 --- a/common/templates/hinter_display.html +++ b/common/templates/hinter_display.html @@ -4,7 +4,7 @@ <%def name="get_hint()"> % if best_hint != '': -

Other students who arrvied at the wrong answer of ${answer} recommend the following hints:

+

Other students who arrived at the wrong answer of ${answer} recommend the following hints:

  • ${best_hint}
    • % endif diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index 520255a8fc..96ea91eabc 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -1,28 +1,17 @@ -''' +""" Views for hint management. -''' -from collections import defaultdict -import csv +Along with the crowdsource_hinter xmodule, this code is still +experimental, and should not be used in new courses, yet. +""" + import json -import logging -from markupsafe import escape -import os import re -import requests -from requests.status_codes import codes -import urllib -from collections import OrderedDict -from StringIO import StringIO - -from django.conf import settings -from django.contrib.auth.models import User, Group from django.http import HttpResponse, Http404 from django_future.csrf import ensure_csrf_cookie -from django.views.decorators.cache import cache_control + from mitxmako.shortcuts import render_to_response, render_to_string -from django.core.urlresolvers import reverse from courseware.courses import get_course_with_access from courseware.models import XModuleContentField @@ -43,7 +32,9 @@ def hint_manager(request, course_id): field = request.POST['field'] if not (field == 'mod_queue' or field == 'hints'): # Invalid field. (Don't let users continue - they may overwrite other db's) - return + out = 'Error in hint manager - an invalid field was accessed.' + return HttpResponse(out) + if request.POST['op'] == 'delete hints': delete_hints(request, course_id, field) if request.POST['op'] == 'switch fields': @@ -58,12 +49,23 @@ def hint_manager(request, course_id): return HttpResponse(json.dumps({'success': True, 'contents': rendered_html})) - def get_hints(request, course_id, field): - # field indicates the database entry that we are modifying. - # Right now, the options are 'hints' or 'mod_queue'. - # DON'T TRUST field attributes that come from ajax. Use an if statement - # to make sure the field is valid before plugging into functions. + """ + Load all of the hints submitted to the course. + + Args: + request -- Django request object. + course_id -- The course id, like 'Me/19.002/test_course' + field -- Either 'hints' or 'mod_queue'; specifies which set of hints to load. + + Keys in returned dict: + - field: Same as input + - other_field: 'mod_queue' if field == 'hints'; and vice-versa. + - field_label, other_field_label: English name for the above. + - all_hints: A list of [answer, pk dict] pairs, representing all hints. + Sorted by answer. + - id_to_name: A dictionary mapping problem id to problem name. + """ if field == 'mod_queue': other_field = 'hints' @@ -76,47 +78,60 @@ def get_hints(request, course_id, field): 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 = {} - name_dict = {} - for problem in all_hints: - loc = Location(problem.definition_id) + # name_dict[problem id] = Display name of problem + id_to_name = {} + + 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. continue - name_dict[problem.definition_id] = descriptor.get_children()[0].display_name + id_to_name[hints_by_problem.definition_id] = descriptor.get_children()[0].display_name # Answer list contains (answer, dict_of_hints) tuples. def answer_sorter(thing): - ''' + """ thing is a tuple, where thing[0] contains an answer, and thing[1] contains - a dict of hints. This function returns an index based on thing[0], which + a dict of hints. This function returns an index based on thing[0], which is used as a key to sort the list of things. - ''' + """ try: return float(thing[0]) except ValueError: # Put all non-numerical answers first. return float('-inf') - answer_list = sorted(json.loads(problem.value).items(), key=answer_sorter) - big_out_dict[problem.definition_id] = answer_list + answer_list = sorted(json.loads(hints_by_problem.value).items(), key=answer_sorter) + big_out_dict[hints_by_problem.definition_id] = answer_list render_dict = {'field': field, 'other_field': other_field, 'field_label': field_label, 'other_field_label': other_field_label, 'all_hints': big_out_dict, - 'id_to_name': name_dict} + 'id_to_name': id_to_name} return render_dict + def delete_hints(request, course_id, field): - ''' - Deletes the hints specified by the [problem_defn_id, answer, pk] tuples in the numbered - fields of request.POST. - ''' + """ + Deletes the hints specified. + + request.POST contains some fields keyed by integers. Each such field contains a + [problem_defn_id, answer, pk] tuple. These tuples specify the hints to be deleted. + + Example request.POST: + {'op': 'delete_hints', + 'field': 'mod_queue', + 1: ['problem_whatever', '42.0', 3], + 2: ['problem_whatever', '32.5', 12]} + """ + for key in request.POST: if key == 'op' or key == 'field': continue @@ -129,31 +144,37 @@ def delete_hints(request, course_id, field): this_problem.value = json.dumps(problem_dict) this_problem.save() + def change_votes(request, course_id, field): - ''' - Updates the number of votes. The numbered fields of request.POST contain - [problem_id, answer, pk, new_votes] tuples. + """ + Updates the number of votes. + + The numbered fields of request.POST contain [problem_id, answer, pk, new_votes] tuples. - Very similar to delete_hints. Is there a way to merge them? Nah, too complicated. - ''' + """ + for key in request.POST: if key == 'op' or key == 'field': continue problem_id, answer, pk, new_votes = request.POST.getlist(key) 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 this_problem.value = json.dumps(problem_dict) this_problem.save() + def add_hint(request, course_id, field): - ''' - Add a new hint. POST: + """ + Add a new hint. request.POST: op field problem - The problem id answer - The answer to which a hint will be added hint - The text of the hint - ''' + """ + problem_id = request.POST['problem'] answer = request.POST['answer'] hint_text = request.POST['hint'] @@ -171,13 +192,15 @@ def add_hint(request, course_id, field): this_problem.value = json.dumps(problem_dict) this_problem.save() + def approve(request, course_id, field): - ''' + """ Approve a list of hints, moving them from the mod_queue to the real hint list. POST: op, field (some number) -> [problem, answer, pk] - ''' + """ + for key in request.POST: if key == 'op' or key == 'field': continue @@ -197,29 +220,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() \ No newline at end of file diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py new file mode 100644 index 0000000000..44e676dd83 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -0,0 +1,79 @@ +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.utils import override_settings +import mitxmako.middleware + +from courseware.models import XModuleContentField +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 + + +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): + + def setUp(self): + """ + 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') + # mitxmako.middleware.MakoMiddleware() + + + def test_student_block(self): + """ + Makes sure that students cannot see the hint management view. + """ + 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') + print out + self.assertTrue('Sorry, but students are not allowed to access the hint manager!' in out.content) + + def test_staff_access(self): + """ + 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') + print out + self.assertTrue('Hints Awaiting Moderation' in out.content) + + + From bc2cab2fda546cbc48b6a1c83a5da36c17c34ff0 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Wed, 26 Jun 2013 16:25:24 -0400 Subject: [PATCH 11/19] Made the hint management instructor view off by default. --- lms/envs/common.py | 3 +++ lms/urls.py | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 141bc127be..8b2a1f28cf 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -141,6 +141,9 @@ MITX_FEATURES = { # Enable instructor dash to submit background tasks 'ENABLE_INSTRUCTOR_BACKGROUND_TASKS': True, + + # Allow use of the hint managment instructor view. + 'ENABLE_HINTER_INSTRUCTOR_VIEW': False, } # Used for A/B testing diff --git a/lms/urls.py b/lms/urls.py index 55d4efd8b8..776a518599 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -264,9 +264,6 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/instructor$', 'instructor.views.instructor_dashboard', name="instructor_dashboard"), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/hint_manager$', - 'instructor.hint_manager.hint_manager', name="hint_manager"), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/gradebook$', 'instructor.views.gradebook', name='gradebook'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade_summary$', @@ -433,6 +430,13 @@ if settings.MITX_FEATURES.get('ENABLE_DEBUG_RUN_PYTHON'): url(r'^debug/run_python', 'debug.views.run_python'), ) +# Crowdsourced hinting instructor manager. +if settings.MITX_FEATURES.get('ENABLE_HINTER_INSTRUCTOR_VIEW'): + urlpatterns += ( + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/hint_manager$', + 'instructor.hint_manager.hint_manager', name="hint_manager"), + ) + urlpatterns = patterns(*urlpatterns) if settings.DEBUG: From 4071a57f68fe78f26d244fbe870ae909abaad09e Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Thu, 27 Jun 2013 10:23:12 -0400 Subject: [PATCH 12/19] Fixed broken tests. Made the hint manager enabled in testing environments - this lets us test the hint manager. --- common/lib/xmodule/xmodule/crowdsource_hinter.py | 2 +- .../lib/xmodule/xmodule/tests/test_crowdsource_hinter.py | 1 + lms/djangoapps/instructor/tests/test_hint_manager.py | 2 +- lms/envs/test.py | 2 ++ lms/templates/courseware/hint_manager.html | 8 ++++---- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 664cf85f1a..8a238a1779 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -203,7 +203,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): if answer in self.hints: # Go through each hint, and add to index_to_hints for hint_id in hints_offered: - if hint_id is None: + if hint_id is not None: try: index_to_hints[i].append((self.hints[answer][str(hint_id)][0], hint_id)) except KeyError: diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index 31614c4849..c12fb1f160 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -207,6 +207,7 @@ class CrowdsourceHinterTest(unittest.TestCase): ) 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) diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py index 44e676dd83..1da83dcc43 100644 --- a/lms/djangoapps/instructor/tests/test_hint_manager.py +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -50,13 +50,13 @@ class HintManagerTest(ModuleStoreTestCase): Set up mako middleware, which is necessary for template rendering to happen. """ course = CourseFactory.create(org='Me', number='19.002', display_name='test_course') - # mitxmako.middleware.MakoMiddleware() 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') diff --git a/lms/envs/test.py b/lms/envs/test.py index e9b683487e..d335fcd600 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -27,6 +27,8 @@ MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = False MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True +MITX_FEATURES['ENABLE_HINTER_INSTRUCTOR_VIEW'] = True + # Need wiki for courseware views to work. TODO (vshnayder): shouldn't need it. WIKI_ENABLED = True diff --git a/lms/templates/courseware/hint_manager.html b/lms/templates/courseware/hint_manager.html index 394792f892..ebd7091a09 100644 --- a/lms/templates/courseware/hint_manager.html +++ b/lms/templates/courseware/hint_manager.html @@ -15,7 +15,7 @@ function setup() { field = $("#field-label").html() changed_votes = [] - $(".votes").live('input', function() { + $(".votes").on('input', function() { changed_votes.push($(this)) }); @@ -43,9 +43,9 @@ 'field': field} for (var i=0; i Date: Thu, 27 Jun 2013 15:20:08 -0400 Subject: [PATCH 13/19] 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. --- .../xmodule/tests/test_crowdsource_hinter.py | 73 +++++++++++ lms/djangoapps/instructor/hint_manager.py | 29 +++-- .../instructor/tests/test_hint_manager.py | 122 ++++++++++++------ lms/envs/dev.py | 1 + 4 files changed, 177 insertions(+), 48 deletions(-) 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 From 728ccd4bcac2daf56bd98f6d15a3a2e986018018 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Thu, 27 Jun 2013 16:57:48 -0400 Subject: [PATCH 14/19] Fixed some docstring formatting things. Expanded test coverage a little. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 20 +++---- .../xmodule/tests/test_crowdsource_hinter.py | 15 +++++ lms/djangoapps/instructor/hint_manager.py | 30 +++++----- .../instructor/tests/test_hint_manager.py | 57 +++++++++++++++++-- 4 files changed, 92 insertions(+), 30 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 8a238a1779..341a2598ef 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -129,11 +129,11 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): Called by hinter javascript after a problem is graded as incorrect. Args: - get -- must be interpretable by capa_answer_to_str. + `get` -- must be interpretable by capa_answer_to_str. Output keys: - - best_hint is the hint text with the most votes. - - rand_hint_1 and rand_hint_2 are two random hints to the answer in get. - - answer is the parsed answer that was submitted. + - 'best_hint' is the hint text with the most votes. + - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `get`. + - 'answer' is the parsed answer that was submitted. """ answer = self.capa_answer_to_str(get) # Look for a hint to give. @@ -176,10 +176,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): The student got it correct. Ask him to vote on hints, or submit a hint. Args: - get -- not actually used. (It is assumed that the answer is correct.) + `get` -- not actually used. (It is assumed that the answer is correct.) Output keys: - - index_to_hints maps previous answer indices to hints that the user saw earlier. - - index_to_answer maps previous answer indices to the actual answer submitted. + - 'index_to_hints' maps previous answer indices to hints that the user saw earlier. + - 'index_to_answer' maps previous answer indices to the actual answer submitted. """ # The student got it right. # Did he submit at least one wrong answer? @@ -217,10 +217,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): Tally a user's vote on his favorite hint. Args: - get -- expected to have the following keys: + `get` -- expected to have the following keys: 'answer': ans_no (index in previous_answers) 'hint': hint_pk - Returns key hint_and_votes, a list of (hint_text, #votes) pairs. + Returns key 'hint_and_votes', a list of (hint_text, #votes) pairs. """ if self.user_voted: return json.dumps({'contents': 'Sorry, but you have already voted!'}) @@ -250,7 +250,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): Take a hint submission and add it to the database. Args: - get -- expected to have the following keys: + `get` -- expected to have the following keys: 'answer': answer index in previous_answers 'hint': text of the new hint that the user is adding Returns a thank-you message. diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index 1bb04654f0..b97fb34d9b 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -192,6 +192,21 @@ 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_nochild(self): + """ + get_html, except the module has no child :( Should return a polite + error message. + """ + m = CHModuleFactory.create() + def fake_get_display_items(): + """ + Returns no children. + """ + return [] + m.get_display_items = fake_get_display_items + out_html = m.get_html() + self.assertTrue('Error in loading crowdsourced hinter' in out_html) + def test_gethtml_multiple(self): """ Makes sure that multiple crowdsourced hinters play nice, when get_html diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index 056784947d..4d5b35356b 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -54,17 +54,17 @@ def get_hints(request, course_id, field): Load all of the hints submitted to the course. Args: - request -- Django request object. - course_id -- The course id, like 'Me/19.002/test_course' - field -- Either 'hints' or 'mod_queue'; specifies which set of hints to load. + `request` -- Django request object. + `course_id` -- The course id, like 'Me/19.002/test_course' + `field` -- Either 'hints' or 'mod_queue'; specifies which set of hints to load. Keys in returned dict: - - field: Same as input - - other_field: 'mod_queue' if field == 'hints'; and vice-versa. - - field_label, other_field_label: English name for the above. - - all_hints: A list of [answer, pk dict] pairs, representing all hints. + - 'field': Same as input + - 'other_field': 'mod_queue' if `field` == 'hints'; and vice-versa. + - 'field_label', 'other_field_label': English name for the above. + - 'all_hints': A list of [answer, pk dict] pairs, representing all hints. Sorted by answer. - - id_to_name: A dictionary mapping problem id to problem name. + - 'id_to_name': A dictionary mapping problem id to problem name. """ if field == 'mod_queue': other_field = 'hints' @@ -92,8 +92,8 @@ def get_hints(request, course_id, field): def answer_sorter(thing): """ - thing is a tuple, where thing[0] contains an answer, and thing[1] contains - a dict of hints. This function returns an index based on thing[0], which + `thing` is a tuple, where `thing[0]` contains an answer, and `thing[1]` contains + a dict of hints. This function returns an index based on `thing[0]`, which is used as a key to sort the list of things. """ try: @@ -131,10 +131,10 @@ def delete_hints(request, course_id, field): """ Deletes the hints specified. - request.POST contains some fields keyed by integers. Each such field contains a + `request.POST` contains some fields keyed by integers. Each such field contains a [problem_defn_id, answer, pk] tuple. These tuples specify the hints to be deleted. - Example request.POST: + Example `request.POST`: {'op': 'delete_hints', 'field': 'mod_queue', 1: ['problem_whatever', '42.0', '3'], @@ -158,8 +158,8 @@ def change_votes(request, course_id, field): """ Updates the number of votes. - The numbered fields of request.POST contain [problem_id, answer, pk, new_votes] tuples. - - Very similar to delete_hints. Is there a way to merge them? Nah, too complicated. + The numbered fields of `request.POST` contain [problem_id, answer, pk, new_votes] tuples. + - Very similar to `delete_hints`. Is there a way to merge them? Nah, too complicated. """ for key in request.POST: @@ -176,7 +176,7 @@ def change_votes(request, course_id, field): def add_hint(request, course_id, field): """ - Add a new hint. request.POST: + Add a new hint. `request.POST`: op field problem - The problem id diff --git a/lms/djangoapps/instructor/tests/test_hint_manager.py b/lms/djangoapps/instructor/tests/test_hint_manager.py index 44e8458e19..39227f93d6 100644 --- a/lms/djangoapps/instructor/tests/test_hint_manager.py +++ b/lms/djangoapps/instructor/tests/test_hint_manager.py @@ -74,13 +74,17 @@ class HintManagerTest(ModuleStoreTestCase): 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_switchfields(self): + """ + Checks that the op: 'switch fields' POST request works. + """ + out = self.c.post(self.url, {'op': 'switch fields', 'field': 'mod_queue'}) + print out + self.assertTrue('Hint 2' in out.content) + def test_gethints(self): """ Checks that gethints returns the right data. @@ -93,6 +97,21 @@ class HintManagerTest(ModuleStoreTestCase): expected = {self.problem_id: [(u'2.0', {u'2': [u'Hint 2', 1]})]} self.assertTrue(out['all_hints'] == expected) + def test_gethints_other(self): + """ + Same as above, with hints instead of mod_queue + """ + request = RequestFactory() + post = request.post(self.url, {'field': 'hints'}) + out = view.get_hints(post, self.course_id, 'hints') + print out + self.assertTrue(out['other_field'] == 'mod_queue') + expected = {self.problem_id: [('1.0', {'1': ['Hint 1', 2], + '3': ['Hint 3', 12]}), + ('2.0', {'4': ['Hint 4', 3]}) + ]} + self.assertTrue(out['all_hints'] == expected) + def test_deletehints(self): """ Checks that delete_hints deletes the right stuff. @@ -119,7 +138,35 @@ class HintManagerTest(ModuleStoreTestCase): print json.loads(problem_hints)['1.0']['1'] self.assertTrue(json.loads(problem_hints)['1.0']['1'][1] == 5) - + def test_addhint(self): + """ + Check that instructors can add new hints. + """ + request = RequestFactory() + post = request.post(self.url, {'field': 'mod_queue', + 'op': 'add hint', + 'problem': self.problem_id, + 'answer': '3.14', + 'hint': 'This is a new hint.'}) + view.add_hint(post, self.course_id, 'mod_queue') + problem_hints = XModuleContentField.objects.get(field_name='mod_queue', definition_id=self.problem_id).value + self.assertTrue('3.14' in json.loads(problem_hints)) + + def test_approve(self): + """ + Check that instructors can approve hints. (Move them + from the mod_queue to the hints.) + """ + request = RequestFactory() + post = request.post(self.url, {'field': 'mod_queue', + 'op': 'approve', + 1: [self.problem_id, '2.0', '2']}) + view.approve(post, self.course_id, 'mod_queue') + problem_hints = XModuleContentField.objects.get(field_name='mod_queue', definition_id=self.problem_id).value + self.assertTrue('2.0' not in json.loads(problem_hints) or len(json.loads(problem_hints)['2.0']) == 0) + 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) From 77e4e2a009db9a9fd81e89c4be2ddeb4bfd74532 Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Fri, 28 Jun 2013 09:27:47 -0400 Subject: [PATCH 15/19] Edited text of crowdsourced hinter template to Piotr's suggestions. Added some HTML formatting to same. --- .../js/src/crowdsource_hinter/display.coffee | 7 +++ common/templates/hinter_display.html | 54 +++++++++++++++---- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee index ea42601622..cbc5c6edd1 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -34,6 +34,13 @@ class @Hinter @$('input.submit-hint').click @submit_hint @$('.custom-hint').click @clear_default_text @$('#answer-tabs').tabs({active: 0}) + @$('.expand-goodhint').click @expand_goodhint + + expand_goodhint: => + if @$('.goodhint').css('display') == 'none' + @$('.goodhint').css('display', 'block') + else + @$('.goodhint').css('display', 'none') vote: (eventObj) => target = @$(eventObj.currentTarget) diff --git a/common/templates/hinter_display.html b/common/templates/hinter_display.html index f05bb34c40..830f69c382 100644 --- a/common/templates/hinter_display.html +++ b/common/templates/hinter_display.html @@ -4,7 +4,7 @@ <%def name="get_hint()"> % if best_hint != '': -

    Other students who arrived at the wrong answer of ${answer} recommend the following hints:

    +

    Hints from students who made similar mistakes:

    • ${best_hint}
      • % endif @@ -65,11 +65,12 @@ +

      Participation in the hinting system is strictly optional, and will not influence your grade.
      - Help us improve our hinting system. Start by picking one of your previous incorrect answers from below: -

      + Help your classmates by writing hints for this problem. Start by picking one of your previous incorrect answers from below: +

        @@ -82,19 +83,54 @@
        % if index in index_to_hints and len(index_to_hints[index]) > 0: - Which hint was most helpful when you got the wrong answer of ${answer}? -
        +

        + Which hint would be most effective to show a student who also got ${answer}? +

        % for hint_text, hint_pk in index_to_hints[index]: +

        ${hint_text} -
        +

        % endfor +

        Don't like any of the hints above? You can also submit your own. % else: - Write a hint for other students who get the wrong answer of ${answer}. +

        % endif - Try to describe what concepts you misunderstood, or what mistake you made. Please don't - give away the answer. + What hint would you give a student who made the same mistake you did? Please don't give away the answer. + Read about what makes a good hint. +

        + - +

        % endfor
      +

      Read about what makes a good hint.

      + + <%def name="show_votes()"> @@ -172,6 +123,3 @@ What would you say to help someone who got this wrong answer? % if op == "vote": ${show_votes()} % endif - - - From 15317de252228e45f5c2b6c0300e5c3dc2d5f04c Mon Sep 17 00:00:00 2001 From: Felix Sun Date: Fri, 28 Jun 2013 15:43:26 -0400 Subject: [PATCH 17/19] Made explanation for hints field in crowdsource_hinter.py more clear. Fixed various commenting things. Removed an unused function in the crowdsourced module coffeescript. Improved commenting in hint_manager. Fixed pep and pylint violations. --- .../lib/xmodule/xmodule/crowdsource_hinter.py | 10 +- .../js/src/crowdsource_hinter/display.coffee | 7 -- .../xmodule/tests/test_crowdsource_hinter.py | 94 ++++++++----------- common/templates/hinter_display.html | 2 +- lms/djangoapps/instructor/hint_manager.py | 14 ++- .../instructor/tests/test_hint_manager.py | 20 ++-- 6 files changed, 62 insertions(+), 85 deletions(-) diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 19c447d014..f84b366d2c 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -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 diff --git a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee index 81872a5ef4..f8bc6037db 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -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 diff --git a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py index b97fb34d9b..f57e28ef46 100644 --- a/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py @@ -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 - - - - - - - - diff --git a/common/templates/hinter_display.html b/common/templates/hinter_display.html index 5ae3d3e9b4..bc49bf18bd 100644 --- a/common/templates/hinter_display.html +++ b/common/templates/hinter_display.html @@ -60,7 +60,7 @@ What would you say to help someone who got this wrong answer? % endfor -

      Read about what makes a good hint.

      +

      Read about what makes a good hint.