From f8b26e111d0566ee8cec645a7468664d51e82a22 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Tue, 23 May 2017 15:45:53 -0400 Subject: [PATCH 1/7] =?UTF-8?q?Temporary=20commit=20=E2=80=94=20the=20past?= =?UTF-8?q?=20answer=20logs=20appear=20to=20work=20=E2=80=94=20but=20need?= =?UTF-8?q?=20simplification=20and=20clean=20up.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../ProblemUtil/ProblemUtil.pm | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm index 0e6451532f..f26561c61c 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm @@ -80,6 +80,7 @@ sub process_and_log_answer{ if ($submitAnswers && !$authz->hasPermissions($effectiveUser, "dont_log_past_answers")) { my $answerString = ""; my $scores = ""; my %answerHash = %{ $pg->{answers} }; + my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; # FIXME this is the line 552 error. make sure original student ans is defined. # The fact that it is not defined is probably due to an error in some answer evaluator. # But I think it is useful to suppress this error message in the log. @@ -96,6 +97,33 @@ sub process_and_log_answer{ $answerString = '' unless defined($answerString); # insure string is defined. + # experimental fix for past answers + my %answersToStore2; + my @answer_order2; + my $scores2=''; + my $isEssay2=0; + my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; + foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { + $scores2.= ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{score}//0) >= 1 ? "1" : "0"; + $isEssay2 = 1 if ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{type}//'') eq 'essay'; + foreach my $response_id ($answerHash2{$ans_id}->response_obj->response_labels) { + $answersToStore2{$response_id} = $self->{formFields}->{$response_id}; + push @answer_order2, $response_id; + } + } + my $answerString2 = ''; + foreach my $response_id (@answer_order2) { + $answerString2.=$answersToStore2{$response_id}."\t"; + } + $answerString2=~s/\t$//; # remove last tab + warn "answerString1: $answerString"; + warn "answerString2: $answerString2"; + warn "scores1: $scores"; + warn "scores2: $scores2"; + warn "isEssay1: $isEssay"; + warn "isEssay2: $isEssay2"; + + # end experimental fix for past answers my $timestamp = time(); writeCourseLog($self->{ce}, "answer_log", join("", @@ -158,10 +186,12 @@ sub process_and_log_answer{ #my @answer_order = (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]}, @extra_answer_names); # %answerToStore and @answer_order are passed as references # because of profile for encodeAnswers + + # encodeAnswers creates a hash and uses Storage::nfreeze to serialize it my $answerString = encodeAnswers(%answersToStore, @answer_order); - # store last answer to database + # store last answer to database for use in "sticky" answers $problem->last_answer($answerString); $pureProblem->last_answer($answerString); $db->putUserProblem($pureProblem); From f08c38e83f4e5d075265c3a9cedb82bf636bf4ac Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Wed, 24 May 2017 08:57:07 -0400 Subject: [PATCH 2/7] Comment out warning messages and comparison between old and new ways of logging past answers --- .../ProblemUtil/ProblemUtil.pm | 112 ++++++++++-------- 1 file changed, 64 insertions(+), 48 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm index f26561c61c..5d0690794c 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm @@ -66,42 +66,26 @@ sub process_and_log_answer{ my $set = $self->{set}; my $urlpath = $r->urlpath; my $courseID = $urlpath->arg("courseID"); - - my $scoreRecordedMessage = ""; - my $pureProblem; - my $isEssay = 0; - $pureProblem = $db->getUserProblem($problem->user_id, $problem->set_id, $problem->problem_id); # checked + # logging student answers + my $pureProblem = $db->getUserProblem($problem->user_id, $problem->set_id, $problem->problem_id); # checked + my $answer_log = $self->{ce}->{courseFiles}->{logs}->{'answer_log'}; - # logging student answers + # my $isEssay = 0; + my $scores2=''; + my $isEssay2=0; + + my %answersToStore2; + my @answer_order2; + my $scoreRecordedMessage = ""; - my $answer_log = $self->{ce}->{courseFiles}->{logs}->{'answer_log'}; if ( defined($answer_log ) and defined($pureProblem)) { if ($submitAnswers && !$authz->hasPermissions($effectiveUser, "dont_log_past_answers")) { - my $answerString = ""; my $scores = ""; - my %answerHash = %{ $pg->{answers} }; - my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; - # FIXME this is the line 552 error. make sure original student ans is defined. - # The fact that it is not defined is probably due to an error in some answer evaluator. - # But I think it is useful to suppress this error message in the log. - foreach (sortByName(undef, keys %answerHash)) { - my $orig_ans = $answerHash{$_}->{original_student_ans}; - my $student_ans = defined $orig_ans ? $orig_ans : ''; - $answerString .= $student_ans."\t"; - # answer score *could* actually be a float, and this doesnt - # allow for fractional answers :( - $scores .= ($answerHash{$_}->{score}//0) >= 1 ? "1" : "0"; - $isEssay = 1 if ($answerHash{$_}->{type}//'') eq 'essay'; - - } + +################################################################ +# new code +######################################### - $answerString = '' unless defined($answerString); # insure string is defined. - - # experimental fix for past answers - my %answersToStore2; - my @answer_order2; - my $scores2=''; - my $isEssay2=0; my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { $scores2.= ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{score}//0) >= 1 ? "1" : "0"; @@ -116,35 +100,64 @@ sub process_and_log_answer{ $answerString2.=$answersToStore2{$response_id}."\t"; } $answerString2=~s/\t$//; # remove last tab - warn "answerString1: $answerString"; - warn "answerString2: $answerString2"; - warn "scores1: $scores"; - warn "scores2: $scores2"; - warn "isEssay1: $isEssay"; - warn "isEssay2: $isEssay2"; +# end new code +################################################################ +# my $answerString = ""; my $scores = ""; +# my %answerHash = %{ $pg->{answers} }; +# # FIXME this is the line 552 error. make sure original student ans is defined. +# # The fact that it is not defined is probably due to an error in some answer evaluator. +# # But I think it is useful to suppress this error message in the log. +# foreach (sortByName(undef, keys %answerHash)) { +# my $orig_ans = $answerHash{$_}->{original_student_ans}; +# my $student_ans = defined $orig_ans ? $orig_ans : ''; +# $answerString .= $student_ans."\t"; +# # answer score *could* actually be a float, and this doesnt +# # allow for fractional answers :( +# $scores .= ($answerHash{$_}->{score}//0) >= 1 ? "1" : "0"; +# $isEssay = 1 if ($answerHash{$_}->{type}//'') eq 'essay'; +# +# } +# +# $answerString = '' unless defined($answerString); # insure string is defined. + +############################################################################## +# check new code + # experimental fix for past answers + # notice that it grabs the student response from the html form fields rather than + # from "original_student_ans" in the answerHash + # The answer hash is inside ans_id.ans_eval.rh_ans + # +# warn "answerString1: $answerString"; +# warn "answerString2: $answerString2"; +# warn "scores1: $scores"; +# warn "scores2: $scores2"; +# warn "isEssay1: $isEssay"; +# warn "isEssay2: $isEssay2"; # end experimental fix for past answers +############################################################################## +# store in answer_log past answers file my $timestamp = time(); writeCourseLog($self->{ce}, "answer_log", join("", '|', $problem->user_id, '|', $problem->set_id, '|', $problem->problem_id, - '|', $scores, "\t", + '|', $scores2, "\t", $timestamp,"\t", - $answerString, + $answerString2, ), ); - #add to PastAnswer db +# add to PastAnswer db my $pastAnswer = $db->newPastAnswer(); $pastAnswer->course_id($courseID); $pastAnswer->user_id($problem->user_id); $pastAnswer->set_id($problem->set_id); $pastAnswer->problem_id($problem->problem_id); $pastAnswer->timestamp($timestamp); - $pastAnswer->scores($scores); - $pastAnswer->answer_string($answerString); + $pastAnswer->scores($scores2); + $pastAnswer->answer_string($answerString2); $pastAnswer->source_file($problem->source_file); $db->addPastAnswer($pastAnswer); @@ -153,6 +166,9 @@ sub process_and_log_answer{ } } +###################################################################### +# this stores previous answers to the problem to +# provide "sticky answers" if ($submitAnswers) { # get a "pure" (unmerged) UserProblem to modify @@ -188,12 +204,12 @@ sub process_and_log_answer{ # because of profile for encodeAnswers # encodeAnswers creates a hash and uses Storage::nfreeze to serialize it - my $answerString = encodeAnswers(%answersToStore, - @answer_order); + my $answerString3 = encodeAnswers(%answersToStore2, + @answer_order2); # store last answer to database for use in "sticky" answers - $problem->last_answer($answerString); - $pureProblem->last_answer($answerString); + $problem->last_answer($answerString3); + $pureProblem->last_answer($answerString3); $db->putUserProblem($pureProblem); # store state in DB if it makes sense @@ -214,16 +230,16 @@ sub process_and_log_answer{ # be flaged as needing grading # we shoudl also check for the appropriate flag in the global problem and set it - if ($isEssay && $pureProblem->{flags} !~ /needs_grading/) { + if ($isEssay2 && $pureProblem->{flags} !~ /needs_grading/) { $pureProblem->{flags} =~ s/graded,//; $pureProblem->{flags} .= "needs_grading,"; } my $globalProblem = $db->getGlobalProblem($problem->set_id, $problem->problem_id); - if ($isEssay && $globalProblem->{flags} !~ /essay/) { + if ($isEssay2 && $globalProblem->{flags} !~ /essay/) { $globalProblem->{flags} .= "essay,"; $db->putGlobalProblem($globalProblem); - } elsif (!$isEssay && $globalProblem->{flags} =~ /essay/) { + } elsif (!$isEssay2 && $globalProblem->{flags} =~ /essay/) { $globalProblem->{flags} =~ s/essay,//; $db->putGlobalProblem($globalProblem); } From 9ca5ced318f066f86719af32972bf362f84be065 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Fri, 26 May 2017 15:02:33 -0400 Subject: [PATCH 3/7] Make sure all of the answers are printed. Counting the number of scores gives the number of answer groups (or answer evaluators) but not the number of answers. --- .../ContentGenerator/Instructor/ShowAnswers.pm | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm b/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm index 8094b895c1..7f5a34ffa9 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ShowAnswers.pm @@ -479,7 +479,18 @@ sub body { } my $num_ans = $#scores; - + my $num_ans_blanks=$#answers; + my $upper_limit = ($num_ans > $num_ans_blanks)? $num_ans: $num_ans_blanks; + + #FIXME -- $num_ans is no longer the value needed -- $num_ans is the number of + # answer evaluators (or answer groups) each of which might have several + # answer blanks. On the other hand sometimes an answer blank has been left blank + # and there is no answer, but the score is zero. + # In other words sometimes number of scores is greater than the number of answers + # and sometimes it is less. + + #warn "checking number of answers ", scalar(@answers), " vs number of scores ",scalar(@scores), " and limit is $upper_limit"; + #warn "answers are ", join(" ", @answers); if ($record{time} - $previousTime > $ce->{sessionKeyTimeout}) { $rowOptions->{'class'} = 'table-rule'; } else { @@ -489,7 +500,7 @@ sub body { @row = (CGI::td({width=>10}),CGI::td({style=>"color:#808080"},CGI::small($time))); - for (my $i = 0; $i <= $num_ans; $i++) { + for (my $i = 0; $i <= $upper_limit; $i++) { my $td; my $answer = $answers[$i] // ''; my $answerType = defined($answerTypes[$i]) ? $answerTypes[$i] : ''; From e5bd698c3245c41c2feae7d30eb88e02e8774902 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sat, 27 May 2017 20:32:55 -0400 Subject: [PATCH 4/7] Refactor create_ans_str_from_responses() Also changed the calling hierarchal structure of WeBWorK::ContentGenerator::Problem. The previous one seemed devious. --- lib/WeBWorK/ContentGenerator/Problem.pm | 6 +- .../ProblemUtil/ProblemUtil.pm | 127 ++++++++++++------ 2 files changed, 89 insertions(+), 44 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index dcd4711a55..4f759ebca8 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -15,9 +15,9 @@ ################################################################################ package WeBWorK::ContentGenerator::Problem; -use base qw(WeBWorK); -#use base qw(WeBWorK::ContentGenerator); -use base qw(WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil); # not needed? +#use base qw(WeBWorK); +use base qw(WeBWorK::ContentGenerator); +use WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil; # not needed? =head1 NAME diff --git a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm index 5d0690794c..95f8efba57 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm @@ -47,12 +47,12 @@ use WeBWorK::Utils::Tasks qw(fake_set fake_problem); # process_and_log_answer subroutine. -# performs functions of processing and recording the answer given in the page. Also returns the appropriate scoreRecordedMessage. +# performs functions of processing and recording the answer given in the page. +# Also returns the appropriate scoreRecordedMessage. sub process_and_log_answer{ - my $self = shift; - + my $self = shift; #type is ref($self) eq 'WeBWorK::ContentGenerator::Problem' my $r = $self->r; my $db = $r->db; my $effectiveUser = $r->param('effectiveUser'); @@ -71,36 +71,43 @@ sub process_and_log_answer{ my $pureProblem = $db->getUserProblem($problem->user_id, $problem->set_id, $problem->problem_id); # checked my $answer_log = $self->{ce}->{courseFiles}->{logs}->{'answer_log'}; - # my $isEssay = 0; - my $scores2=''; - my $isEssay2=0; - - my %answersToStore2; - my @answer_order2; +# my $isEssay = 0; +# my $scores2=''; +# my $isEssay2=0; +# +# my %answersToStore2; +# my @answer_order2; + my ($encoded_answer_string, $scores2, $isEssay2); my $scoreRecordedMessage = ""; if ( defined($answer_log ) and defined($pureProblem)) { if ($submitAnswers && !$authz->hasPermissions($effectiveUser, "dont_log_past_answers")) { ################################################################ -# new code +# new code (input is $pg) ######################################### - - my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; - foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { - $scores2.= ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{score}//0) >= 1 ? "1" : "0"; - $isEssay2 = 1 if ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{type}//'') eq 'essay'; - foreach my $response_id ($answerHash2{$ans_id}->response_obj->response_labels) { - $answersToStore2{$response_id} = $self->{formFields}->{$response_id}; - push @answer_order2, $response_id; - } - } - my $answerString2 = ''; - foreach my $response_id (@answer_order2) { - $answerString2.=$answersToStore2{$response_id}."\t"; - } - $answerString2=~s/\t$//; # remove last tab -# end new code +# +# my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; +# foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { +# $scores2.= ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{score}//0) >= 1 ? "1" : "0"; +# $isEssay2 = 1 if ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{type}//'') eq 'essay'; +# foreach my $response_id ($answerHash2{$ans_id}->response_obj->response_labels) { +# $answersToStore2{$response_id} = $self->{formFields}->{$response_id}; +# push @answer_order2, $response_id; +# } +# } +# my $answerString2 = ''; +# foreach my $response_id (@answer_order2) { +# $answerString2.=($answersToStore2{$response_id}//'')."\t"; +# } +# $answerString2=~s/\t$//; # remove last tab + my ($answerString2); + ($answerString2,$encoded_answer_string, $scores2, $isEssay2) = + WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil::create_ans_str_from_responses( + $self, $pg + ); # ref($self) eq WeBWorK::ContentGenerator::Problem + # ref($pg) eq "PGcore"; +# end new code (output is answerString2, $scores, $isEssay) ################################################################ # my $answerString = ""; my $scores = ""; # my %answerHash = %{ $pg->{answers} }; @@ -136,7 +143,7 @@ sub process_and_log_answer{ # end experimental fix for past answers ############################################################################## -# store in answer_log past answers file +# store in answer_log past answers file (user_id,set_id,problem_id,courseID,answerString,scores,source_file) my $timestamp = time(); writeCourseLog($self->{ce}, "answer_log", join("", @@ -159,7 +166,6 @@ sub process_and_log_answer{ $pastAnswer->scores($scores2); $pastAnswer->answer_string($answerString2); $pastAnswer->source_file($problem->source_file); - $db->addPastAnswer($pastAnswer); @@ -183,15 +189,15 @@ sub process_and_log_answer{ # $answersToStore{$_} = $self->{formFields}->{$_} foreach (keys %answerHash); # $answerHash{$_}->{original_student_ans} -- this may have been modified for fields with multiple values. # Don't use it!! - my @answer_order; - my %answerHash = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; - foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { - foreach my $response_id ($answerHash{$ans_id}->response_obj->response_labels) { - $answersToStore{$response_id} = $self->{formFields}->{$response_id}; - push @answer_order, $response_id; - } - } - +# my @answer_order; +# my %answerHash = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; +# foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { +# foreach my $response_id ($answerHash{$ans_id}->response_obj->response_labels) { +# $answersToStore{$response_id} = $self->{formFields}->{$response_id}; +# push @answer_order, $response_id; +# } +# } +# # There may be some more answers to store -- one which are auxiliary entries to a primary answer. Evaluating # matrices works in this way, only the first answer triggers an answer evaluator, the rest are just inputs # however we need to store them. Fortunately they are still in the input form. @@ -204,12 +210,13 @@ sub process_and_log_answer{ # because of profile for encodeAnswers # encodeAnswers creates a hash and uses Storage::nfreeze to serialize it - my $answerString3 = encodeAnswers(%answersToStore2, - @answer_order2); + # replaced by $encoded_answer_string +# my $answerString3 = encodeAnswers(%answersToStore2, +# @answer_order2); # store last answer to database for use in "sticky" answers - $problem->last_answer($answerString3); - $pureProblem->last_answer($answerString3); + $problem->last_answer($encoded_answer_string); + $pureProblem->last_answer($encoded_answer_string); $db->putUserProblem($pureProblem); # store state in DB if it makes sense @@ -302,6 +309,44 @@ sub process_and_log_answer{ return $scoreRecordedMessage; } +# create answer string from responses hash +# ($ansString, $encoded_ans_string, $scores, $isEssay) = create_ans_str_from_responses($problem, $pg) +# +# input: ref($pg)eq 'WeBWorK::PG::Local' +# ref($problem)eq 'WeBWorK::ContentGenerator::Problem +# output: (str, str, str) + +sub create_ans_str_from_responses { + my $problem = shift; # ref($problem) eq 'WeBWorK::ContentGenerator::Problem' + # must contain $self->{formFields}->{$response_id} + my $pg = shift; # ref($pg) eq 'WeBWorK::PG::Local' + #warn "create_ans_str_from_responses pg has type ", ref($pg); + my $scores2=''; + my $isEssay2=0; + my %answersToStore2; + my @answer_order2; + + my %answerHash2 = %{ $pg->{pgcore}->{PG_ANSWERS_HASH}}; + foreach my $ans_id (@{$pg->{flags}->{ANSWER_ENTRY_ORDER}//[]} ) { + $scores2.= ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{score}//0) >= 1 ? "1" : "0"; + $isEssay2 = 1 if ($answerHash2{$ans_id}->{ans_eval}{rh_ans}{type}//'') eq 'essay'; + foreach my $response_id ($answerHash2{$ans_id}->response_obj->response_labels) { + $answersToStore2{$response_id} = $problem->{formFields}->{$response_id}; + push @answer_order2, $response_id; + } + } + my $answerString2 = ''; + foreach my $response_id (@answer_order2) { + $answerString2.=($answersToStore2{$response_id}//'')."\t"; + } + $answerString2=~s/\t$//; # remove last tab + + my $encoded_answer_string = encodeAnswers(%answersToStore2, + @answer_order2); + + return ($answerString2,$encoded_answer_string, $scores2,$isEssay2); +} + # process_editorLink subroutine # Creates and returns the proper editor link for the current website. Also checks for translation errors and prints an error message and returning a false value if one is detected. From c832b2f9373c5ae2a5eda9158cd3b9bb8dd04099 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sat, 27 May 2017 21:33:01 -0400 Subject: [PATCH 5/7] Update documentation --- lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm index 95f8efba57..b1c16cb122 100644 --- a/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm +++ b/lib/WeBWorK/ContentGenerator/ProblemUtil/ProblemUtil.pm @@ -106,7 +106,7 @@ sub process_and_log_answer{ WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil::create_ans_str_from_responses( $self, $pg ); # ref($self) eq WeBWorK::ContentGenerator::Problem - # ref($pg) eq "PGcore"; + # ref($pg) eq "WeBWorK::PG::Local"; # end new code (output is answerString2, $scores, $isEssay) ################################################################ # my $answerString = ""; my $scores = ""; From 09515e6b0205234d55a52ba45f58d4a376699649 Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Sat, 27 May 2017 21:35:54 -0400 Subject: [PATCH 6/7] Refactor: Use WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil::create_ans_str_from_responses in GatewayQuiz This is a first pass. The problems are gone through twice once to record sticky answers and a second time to record passed answers. These could probably be combined but at the moment the order of presentation is different. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 99 ++++++++++++++------- 1 file changed, 67 insertions(+), 32 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 2af7b2d0c5..8abe63d749 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1476,16 +1476,28 @@ sub body { } } - + + # The following arrays cache results obtained in the two passes through + # the collection of problems in the quiz + # This might save some time. + # This refactoring hasn't taken place yet + # because I don't yet understand why the ordering + # for creating past answers was chosen as it is, different from creating sticky answers +# my @answerString = (); +# my @encoded_ans_string = (); +# my @scores = (); +# my @isEssay = (); + my @pureProblems = $db->getAllProblemVersions($effectiveUser, $setName, $versionNumber); foreach my $i ( 0 .. $#problems ) { # process each problem # this code is essentially that from Problem.pm +# begin problem loop for sticky answers my $pureProblem = $pureProblems[$i]; # store answers in problem for sticky answers later - my %answersToStore; + # my %answersToStore; # we have to be a little careful about getting the # answers that we're saving, because we don't have @@ -1493,31 +1505,42 @@ sub body { # submitting my %answerHash = (); my @answer_order = (); + my $encoded_ans_string; if ( ref( $pg_results[$i] ) ) { - %answerHash = %{$pg_results[$i]->{answers}}; - $answersToStore{$_} = $self->{formFields}->{$_} - foreach (keys %answerHash); - # check for extra answers that slipped - # by---e.g. for matrices, and get them - # from the original input form - my @extra_answer_names = - @{ $pg_results[$i]->{flags}->{KEPT_EXTRA_ANSWERS} }; - $answersToStore{$_} = - $self->{formFields}->{$_} foreach (@extra_answer_names); - @answer_order = - ( @{$pg_results[$i]->{flags}->{ANSWER_ENTRY_ORDER}}, - @extra_answer_names ); +# %answerHash = %{$pg_results[$i]->{answers}}; +# $answersToStore{$_} = $self->{formFields}->{$_} +# foreach (keys %answerHash); +# # check for extra answers that slipped +# # by---e.g. for matrices, and get them +# # from the original input form +# my @extra_answer_names = +# @{ $pg_results[$i]->{flags}->{KEPT_EXTRA_ANSWERS} }; +# $answersToStore{$_} = +# $self->{formFields}->{$_} foreach (@extra_answer_names); +# @answer_order = +# ( @{$pg_results[$i]->{flags}->{ANSWER_ENTRY_ORDER}}, +# @extra_answer_names ); + my ($answerString, $scores,$isEssay); #not used here + ($answerString,$encoded_ans_string,$scores,$isEssay) = + WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil::create_ans_str_from_responses( + $self, $pg_results[$i] + ); # ref($self) eq WeBWorK::ContentGenerator::Problem + # ref($pg) eq "WeBWorK::PG::Local"; } else { my $prefix = sprintf('Q%04d_',$i+1); my @fields = sort grep {/^$prefix/} (keys %{$self->{formFields}}); - %answersToStore = map {$_ => $self->{formFields}->{$_}} @fields; - @answer_order = @fields; + my %answersToStore = map {$_ => $self->{formFields}->{$_}} @fields; + my @answer_order = @fields; + $encoded_ans_string = encodeAnswers( %answersToStore, + @answer_order ); + } - my $answerString = encodeAnswers( %answersToStore, - @answer_order ); +# my $answerString = encodeAnswers( %answersToStore, +# @answer_order ); +# # and get the last answer - $problems[$i]->last_answer( $answerString ); - $pureProblem->last_answer( $answerString ); + $problems[$i]->last_answer( $encoded_ans_string ); + $pureProblem->last_answer( $encoded_ans_string ); # next, store the state in the database if that makes # sense @@ -1584,7 +1607,7 @@ sub body { $db->putProblemVersion( $pureProblem ); } } # end loop through problems - +# end loop through problems for sticky answer #Try to update the student score on the LMS # if that option is enabled. @@ -1611,18 +1634,27 @@ sub body { # this is carried over from Problem.pm if ( defined( $answer_log ) ) { foreach my $i ( 0 .. $#problems ) { +# begin problem loop for passed answers my $answerString = ''; my $scores = ''; + my $isEssay = 0; # note that we store these answers in the # order that they are presented, not the # actual problem order if ( ref( $pg_results[$probOrder[$i]] ) ) { my %answerHash = %{ $pg_results[$probOrder[$i]]->{answers} }; - foreach ( sortByName(undef, keys %answerHash) ) { - my $sAns = defined($answerHash{$_}->{original_student_ans}) ? $answerHash{$_}->{original_student_ans} : ''; - $answerString .= $sAns . "\t"; - $scores .= $answerHash{$_}->{score}>=1 ? "1" : "0" if ( $submitAnswers ); - } +# foreach ( sortByName(undef, keys %answerHash) ) { +# my $sAns = defined($answerHash{$_}->{original_student_ans}) ? $answerHash{$_}->{original_student_ans} : ''; +# $answerString .= $sAns . "\t"; +# $scores .= $answerHash{$_}->{score}>=1 ? "1" : "0" if ( $submitAnswers ); +# } + my ($encoded_ans_string, ); #not used here + ($answerString,$encoded_ans_string,$scores,$isEssay) = + WeBWorK::ContentGenerator::ProblemUtil::ProblemUtil::create_ans_str_from_responses( + $self, $pg_results[$probOrder[$i]] + ); # ref($self) eq WeBWorK::ContentGenerator::Problem + # ref($pg) eq "WeBWorK::PG::Local"; + $answerString =~ s/\t+$/\t/; } else { my $prefix = sprintf('Q%04d_', ($probOrder[$i]+1)); my @fields = sort grep {/^$prefix/} (keys %{$self->{formFields}}); @@ -1630,9 +1662,11 @@ sub body { $answerString .= $self->{formFields}->{$_} . "\t"; $scores .= $self->{formFields}->{"probstatus" . ($probOrder[$i]+1)} >= 1 ? "1" : "0" if ( $submitAnswers ); } + $answerString =~ s/\t+$/\t/; } - $answerString =~ s/\t+$/\t/; - + + + # Prefix answer string with submission type my $answerPrefix; if ( $submitAnswers ) { $answerPrefix = "[submit] "; @@ -1650,7 +1684,8 @@ sub body { $answerString = "$answerPrefix" . "$answerString"; } - + + #Write to courseLog writeCourseLog( $self->{ce}, "answer_log", join("", '|', $problems[$i]->user_id, @@ -1659,7 +1694,7 @@ sub body { "\t$timeNow\t", "$answerString"), ); - #add to PastAnswer db + #add to PastAnswer db my $pastAnswer = $db->newPastAnswer(); $pastAnswer->course_id($courseID); $pastAnswer->user_id($problems[$i]->user_id); @@ -1676,7 +1711,7 @@ sub body { } } debug("end answer processing"); - +# end problem loop # additional set-level database manipulation: we want to save the time # that a set was submitted, and for proctored tests we want to reset # the assignment type after a set is submitted for the last time so From f826c8dc3a47f7bdb1bb079f54873cdc849b808e Mon Sep 17 00:00:00 2001 From: Michael Gage Date: Tue, 6 Jun 2017 11:24:27 -0400 Subject: [PATCH 7/7] Insure that $studentAnswer and $answerScore are defined. This eliminates error messages at the end of quizzes. I'm not sure if this just hides other errors or not, however initializing $answerScore=0 and $studentAnswer='' seems safe. --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 8abe63d749..4406024fe9 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -397,12 +397,12 @@ sub attemptResults { @row = (); my $answerResult = $pg->{answers}->{$name}; - my $studentAnswer = $answerResult->{student_ans}; # original_student_ans + my $studentAnswer = $answerResult->{student_ans}//''; # original_student_ans my $preview = ($showAttemptPreview ? $self->previewAnswer($answerResult, $imgGen) : ""); my $correctAnswer = $answerResult->{correct_ans}; - $answerScore = $answerResult->{score}; + $answerScore = $answerResult->{score}//0; my $answerMessage = $showMessages ? $answerResult->{ans_message} : ""; $numCorrect += $answerScore > 0; $numEssay += ($answerResult->{type}//'') eq 'essay';