diff --git a/common/lib/xmodule/xmodule/crowdsource_hinter.py b/common/lib/xmodule/xmodule/crowdsource_hinter.py index 6c868cb621..bac6982085 100644 --- a/common/lib/xmodule/xmodule/crowdsource_hinter.py +++ b/common/lib/xmodule/xmodule/crowdsource_hinter.py @@ -13,7 +13,7 @@ from pkg_resources import resource_string from lxml import etree from xmodule.x_module import XModule -from xmodule.xml_module import XmlDescriptor +from xmodule.raw_module import RawDescriptor from xblock.core import Scope, String, Integer, Boolean, Dict, List from capa.responsetypes import FormulaResponse, StudentInputError @@ -150,10 +150,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): # errors don't make a difference. out = str(responder.hash_answers(answer, self.formula_test_values)) except StudentInputError: - # I'm not sure what's the best thing to do here. - # I'll return the empty string, for now. - # That way, all invalid hints are clustered together. - return '' + # I'm not sure what's the best thing to do here. I'm returning + # None, for now, so that the calling function has a chance to catch + # the error without having to import StudentInputError. + return None return out def handle_ajax(self, dispatch, data): @@ -197,6 +197,10 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): return # Make a signature of the answer, for formula responses. signature = self.answer_signature(answer) + if signature == None: + # Sometimes, signature conversion may fail. + log.exception('Signature conversion failed: ' + str(answer)) + return # 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.) @@ -261,7 +265,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): if signature in self.hints: # Go through each hint, and add to index_to_hints for hint_id in hints_offered: - if (hint_id is not None) and (hint_id not in answer_to_hints[signature]): + if (hint_id is not None) and (hint_id not in answer_to_hints[answer]): try: answer_to_hints[answer][hint_id] = self.hints[signature][str(hint_id)][0] except KeyError: @@ -335,10 +339,7 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): temp_dict[signature] = {str(self.hint_pk): [hint, 1]} # Add the signature to signature_to_ans, if it's not there yet. # This allows instructors to see a human-readable answer that corresponds to each signature. - if answer not in self.signature_to_ans: - local_sta = self.signature_to_ans - local_sta[signature] = answer - self.signature_to_ans = local_sta + self.add_signature(signature, answer) self.hint_pk += 1 if self.moderate == 'True': self.mod_queue = temp_dict @@ -349,8 +350,18 @@ class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule): self.previous_answers = [] return {'message': 'Thank you for your hint!'} + def add_signature(self, signature, answer): + """ + Add a signature to self.signature_to_ans. If the signature already + exists, do nothing. + """ + if signature not in self.signature_to_ans: + local_sta = self.signature_to_ans + local_sta[signature] = answer + self.signature_to_ans = local_sta -class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, XmlDescriptor): + +class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, RawDescriptor): 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 3df970a618..2e4d0e2c3b 100644 --- a/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee @@ -48,8 +48,8 @@ class @Hinter vote: (eventObj) => target = @$(eventObj.currentTarget) - parent_div_selector = '#previous-answer-' + @jq_escape(target.attr('data-answer')) - all_pks = @$(parent_div_selector).attr('data-all-pks') + parent_div = $('.previous-answer[data-answer="'+target.attr('data-answer')+'"]') + all_pks = parent_div.attr('data-all-pks') console.debug(all_pks) post_json = {'answer': target.attr('data-answer'), 'hint': target.data('hintno'), 'pk_list': all_pks} $.postWithPrefix "#{@url}/vote", post_json, (response) => @@ -57,8 +57,9 @@ class @Hinter submit_hint: (eventObj) => target = @$(eventObj.currentTarget) - textarea_id = '#custom-hint-' + @jq_escape(target.attr('data-answer')) - post_json = {'answer': target.attr('data-answer'), 'hint': @$(textarea_id).val()} + textarea = $('.custom-hint[data-answer="'+target.attr('data-answer')+'"]') + console.debug(textarea) + post_json = {'answer': target.attr('data-answer'), 'hint': @$(textarea).val()} $.postWithPrefix "#{@url}/submit_hint",post_json, (response) => @render(response.contents) diff --git a/common/templates/hinter_display.html b/common/templates/hinter_display.html index d349a17fc5..4f40113265 100644 --- a/common/templates/hinter_display.html +++ b/common/templates/hinter_display.html @@ -23,10 +23,19 @@ Help your classmates by writing hints for this problem. Start by picking one of your previous incorrect answers from below:

+ <% + def unspace(string): + """ + HTML id's can't have spaces in them. This little function + removes spaces. + """ + return ''.join(string.split()) + %> +
@@ -35,7 +44,7 @@ import json all_pks = json.dumps(pk_dict.keys()) %> -
+
% if len(pk_dict) > 0:

@@ -54,7 +63,7 @@

What hint would you give a student who made the same mistake you did? Please don't give away the answer.

- diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index 899e386f70..b20cdc81e6 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -10,11 +10,14 @@ import re from django.http import HttpResponse, Http404 from django_future.csrf import ensure_csrf_cookie +from django.core.exceptions import ObjectDoesNotExist from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access from courseware.models import XModuleContentField +from courseware.module_render import get_module +from courseware.model_data import ModelDataCache from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore @@ -28,24 +31,29 @@ def hint_manager(request, course_id): return HttpResponse(out) if request.method == 'GET': out = get_hints(request, course_id, 'mod_queue') - return render_to_response('courseware/hint_manager.html', out) + out.update({'error': ''}) + return render_to_response('instructor/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) 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': - 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)) + switch_dict = { + 'delete hints': delete_hints, + 'switch fields': lambda *args: None, # Takes any number of arguments, returns None. + 'change votes': change_votes, + 'add hint': add_hint, + 'approve': approve, + } + + # Do the operation requested, and collect any error messages. + error_text = switch_dict[request.POST['op']](request, course_id, field) + if error_text is None: + error_text = '' + render_dict = get_hints(request, course_id, field) + render_dict.update({'error': error_text}) + rendered_html = render_to_string('instructor/hint_manager_inner.html', render_dict) return HttpResponse(json.dumps({'success': True, 'contents': rendered_html})) @@ -106,8 +114,27 @@ def get_hints(request, course_id, field): # Put all non-numerical answers first. return float('-inf') - # Answer list contains [answer, dict_of_hints] pairs. - answer_list = sorted(json.loads(hints_by_problem.value).items(), key=answer_sorter) + # Find the signature to answer converter for this problem. Sometimes, + # it doesn't exist; just assume that the signatures are the answers. + try: + signature_to_ans = XModuleContentField.objects.get( + field_name='signature_to_ans', + definition_id__regex=chopped_id + ) + signature_to_ans = json.loads(signature_to_ans.value) + except ObjectDoesNotExist: + signature_to_ans = {} + + signatures_dict = json.loads(hints_by_problem.value) + unsorted = [] + for signature, dict_of_hints in signatures_dict.items(): + if signature in signature_to_ans: + ans_txt = signature_to_ans[signature] + else: + ans_txt = signature + unsorted.append([signature, ans_txt, dict_of_hints]) + # Answer list contains [signature, answer, dict_of_hints] sub-lists. + answer_list = sorted(unsorted, key=answer_sorter) big_out_dict[hints_by_problem.definition_id] = answer_list render_dict = {'field': field, @@ -138,7 +165,7 @@ def delete_hints(request, course_id, field): 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. + [problem_defn_id, signature, pk] tuple. These tuples specify the hints to be deleted. Example `request.POST`: {'op': 'delete_hints', @@ -150,12 +177,12 @@ def delete_hints(request, course_id, field): for key in request.POST: if key == 'op' or key == 'field': continue - problem_id, answer, pk = request.POST.getlist(key) + problem_id, signature, 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] + del problem_dict[signature][pk] this_problem.value = json.dumps(problem_dict) this_problem.save() @@ -164,18 +191,18 @@ 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. + The numbered fields of `request.POST` contain [problem_id, signature, 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) + problem_id, signature, 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] = int(new_votes) + # problem_dict[signature][pk] points to a [hint_text, #votes] pair. + problem_dict[signature][pk][1] = int(new_votes) this_problem.value = json.dumps(problem_dict) this_problem.save() @@ -187,6 +214,7 @@ def add_hint(request, course_id, field): field problem - The problem id answer - The answer to which a hint will be added + - Needs to be converted into signature first. hint - The text of the hint """ @@ -200,10 +228,23 @@ def add_hint(request, course_id, field): hint_pk_entry.value = this_pk + 1 hint_pk_entry.save() + # Make signature. This is really annoying, but I don't see + # any alternative :( + loc = Location(problem_id) + descriptors = modulestore().get_items(loc) + m_d_c = ModelDataCache(descriptors, course_id, request.user) + hinter_module = get_module(request.user, request, loc, m_d_c, course_id) + signature = hinter_module.answer_signature(answer) + if signature is None: + # Signature generation failed. + # We should probably return an error message, too... working on that. + return 'Error - your answer could not be parsed as a formula expression.' + hinter_module.add_signature(signature, answer) + problem_dict = json.loads(this_problem.value) - if answer not in problem_dict: - problem_dict[answer] = {} - problem_dict[answer][this_pk] = [hint_text, 1] + if signature not in problem_dict: + problem_dict[signature] = {} + problem_dict[signature][this_pk] = [hint_text, 1] this_problem.value = json.dumps(problem_dict) this_problem.save() @@ -213,26 +254,26 @@ 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] + (some number) -> [problem, signature, pk] """ for key in request.POST: if key == 'op' or key == 'field': continue - problem_id, answer, pk = request.POST.getlist(key) + problem_id, signature, 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] + hint_to_move = problem_dict[signature][pk] + del problem_dict[signature][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 + if signature not in problem_dict: + problem_dict[signature] = {} + problem_dict[signature][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/instructor/hint_manager.html similarity index 98% rename from lms/templates/courseware/hint_manager.html rename to lms/templates/instructor/hint_manager.html index ebd7091a09..4a6e219560 100644 --- a/lms/templates/courseware/hint_manager.html +++ b/lms/templates/instructor/hint_manager.html @@ -1,6 +1,6 @@ <%inherit file="/main.html" /> <%namespace name='static' file='/static_content.html'/> -<%namespace name="content" file="/courseware/hint_manager_inner.html"/> +<%namespace name="content" file="/instructor/hint_manager_inner.html"/> <%block name="headextra"> diff --git a/lms/templates/courseware/hint_manager_inner.html b/lms/templates/instructor/hint_manager_inner.html similarity index 90% rename from lms/templates/courseware/hint_manager_inner.html rename to lms/templates/instructor/hint_manager_inner.html index c69539522f..3acf8102ac 100644 --- a/lms/templates/courseware/hint_manager_inner.html +++ b/lms/templates/instructor/hint_manager_inner.html @@ -4,15 +4,16 @@

${field_label}

Switch to ${other_field_label} +

${error}

% for definition_id in all_hints:

Problem: ${id_to_name[definition_id]}

- % for answer, hint_dict in all_hints[definition_id]: + % for signature, 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: @@ -36,6 +37,7 @@ Switch to ${other_field_label
% endfor +

${error}

% if field == 'mod_queue':