From beed394a8b40c1cba36c570c17b14cc8b73a54ce Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 7 Dec 2023 15:07:17 -0600 Subject: [PATCH] Remove the checkbox for showing correct answers. Correct answers are shown when they "can" be. However, there are two cases in which I don't think that it is appropriate to show the correct answers without any user interaction. This is where the corresponding PG pull request that adds a "Reveal" button for correct answers comes in. The first case is in "Show Me Another" problems when the course configuration is set to allow correct answers to be shown for these problems. Students should be able to see feedback messages and such without seeing the correct answer. The point of these problems is for students to obtain practice working problems without simply looking at the correct answers, so an additional step should be needed to see the correct answer. The second case is for instructors viewing problems before answers are available for students. The instructor may be viewing the problem with students present (for example, when working problems in class) and may want to be able to show messages and such without showing the correct answer. There is a new course configuration option in the "Optional Modules". If this option is true, then answer feedback is immediately available in problems after answers are available. Students do not even need to click "Submit Answers" to make this feedback appear. Furthermore, the $showPartialCorrectAnswers variable set in some problems that prevents showing which of the answers are correct is ignored if this option is true. This option is true by default, but if it is beleived that the majority won't want this, then it can be switched to false. This option was actually added as an afterthought. It was initially implemented with automatic feedback after answers are available with no option to disable that behavior. I would also be happy reverting to that if we feel that this is something everyone will want. This automatic feedback is particularly useful for tests that have multiple pages. Currently you need to change the page, click the show correct answers checkbox, then click check test, and that has to be repeated for each page. That is a real pain if each problem is on its own page. The check box is gone, but you still have to click check test after changing pages. With this option the feedback is there with each page change (the same as it was before when the problem grader is enabled). Also the %must hash has been removed. It was not used at all except in Problem.pm for one permission, and that usage was incorrect. It resulted in recordAnswers always being forced to be true with the default setting of avoid_recording_answers set to "nobody". The %must hash can not force an option to be disabled. It can only force it to be enabled. This permission was done correctly in the GatewayQuiz.pm module, so that approach is now used in Problem.pm also. --- conf/defaults.config | 19 ++++ .../js/ProblemSetDetail/problemsetdetail.js | 2 - lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 104 +++++++++--------- lib/WeBWorK/ContentGenerator/Problem.pm | 61 ++++------ lib/WeBWorK/ContentGenerator/ShowMeAnother.pm | 16 +-- lib/WebworkWebservice/RenderProblem.pm | 2 +- .../ContentGenerator/GatewayQuiz.html.ep | 11 +- .../Problem/checkboxes.html.ep | 13 +-- 8 files changed, 102 insertions(+), 126 deletions(-) diff --git a/conf/defaults.config b/conf/defaults.config index 3e9cd4ac28..0ebe0ceab3 100644 --- a/conf/defaults.config +++ b/conf/defaults.config @@ -205,6 +205,14 @@ $pg{options}{periodicRandomizationPeriod} = 5; # and before a new version is requested. $pg{options}{showCorrectOnRandomize} = 0; +############################################################################### +# automaticAnswerFeedback +################################################################################ +# If set, then answer feedback is automatically shown after answers are +# available for the assignment. This is present in problems when a student opens +# the problem without the student needing to click the "Check Answers" button. +$pg{options}{automaticAnswerFeedback} = 1; + ################################################################################ # Single Problem Grader ################################################################################ @@ -2044,6 +2052,17 @@ $ConfigValues = [ ), type => 'popuplist', values => ['preview', 'submit', 'conservative'] + }, + { + var => 'pg{options}{automaticAnswerFeedback}', + doc => x('Show automatic answer feedback when answers are available.'), + doc2 => x( + 'Answer feedback will be available in problems after answers are available. Students will not even ' + . 'need to click "Submit Answers" to make this feedback appear. Furthermore, the ' + . '$showPartialCorrectAnswers variable set in some problems that prevents showing which of the ' + . 'answers are correct is ignored.' + ), + type => 'boolean' } ], [ diff --git a/htdocs/js/ProblemSetDetail/problemsetdetail.js b/htdocs/js/ProblemSetDetail/problemsetdetail.js index 49a0c58d13..7d0a10668b 100644 --- a/htdocs/js/ProblemSetDetail/problemsetdetail.js +++ b/htdocs/js/ProblemSetDetail/problemsetdetail.js @@ -294,8 +294,6 @@ }, { passive: true }); // Send a request to the webwork webservice and render a problem. - const basicWebserviceURL = `${webworkConfig?.webwork_url ?? '/webwork2'}/render_rpc`; - const render = (id) => new Promise((resolve) => { const renderArea = document.getElementById(`psr_render_area_${id}`); diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 9a07e3e95a..c5ea5d4ccb 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -75,23 +75,41 @@ sub can_showCorrectAnswers ($c, $user, $permissionLevel, $effectiveUser, $set, $ my $attemptsPerVersion = $set->attempts_per_version || 0; my $attemptsUsed = $problem->num_correct + $problem->num_incorrect + ($c->{submitAnswers} ? 1 : 0); - # This is complicated by trying to address hiding scores by problem. That is, if $set->hide_score_by_problem and - # $set->hide_score are both set, then we should allow scores to be shown, but not show the score on any individual - # problem. To deal with this, we make can_showCorrectAnswers give the least restrictive view of hiding, and then - # filter scores for the problems themselves later. return ( ( - ( - after($set->answer_date, $c->submitTime) || ($attemptsUsed >= $attemptsPerVersion - && $attemptsPerVersion != 0 - && $set->due_date == $set->answer_date) - ) - || $authz->hasPermissions($user->user_id, 'show_correct_answers_before_answer_date') + $authz->hasPermissions($user->user_id, 'show_correct_answers_before_answer_date') + || ( + after($set->answer_date, $c->submitTime) + || ($attemptsUsed >= $attemptsPerVersion + && $attemptsPerVersion != 0 + && $set->due_date == $set->answer_date) + ) + ) + && $c->can_showProblemScores($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet) + ); +} + +# This version is the same as the above version except that it ignores elevated permisions. So it will be true if this +# set is in the state that anyone can show correct answers regardless of if they have the +# show_correct_answers_before_answer_date or view_hidden_work permissions. In this case, feedback is shown even without +# a form submission, and correct answers are shown in the feedback, if the $pg{options}{automaticAnswerFeedback} option +# is set in the course configuration. +sub can_showCorrectAnswersForAll ($c, $set, $problem, $tmplSet) { + my $attemptsPerVersion = $set->attempts_per_version || 0; + my $attemptsUsed = $problem->num_correct + $problem->num_incorrect + ($c->{submitAnswers} ? 1 : 0); + + return ( + ( + after($set->answer_date, $c->submitTime) || ($attemptsUsed >= $attemptsPerVersion + && $attemptsPerVersion != 0 + && $set->due_date == $set->answer_date) ) && ( - $authz->hasPermissions($user->user_id, 'view_hidden_work') - || $set->hide_score_by_problem eq 'N' && ($set->hide_score eq 'N' - || ($set->hide_score eq 'BeforeAnswerDate' && after($tmplSet->answer_date, $c->submitTime))) + ( + $set->hide_score eq 'N' + || ($set->hide_score eq 'BeforeAnswerDate' && after($tmplSet->answer_date, $c->submitTime)) + ) + && $set->hide_score_by_problem eq 'N' ) ); } @@ -109,10 +127,7 @@ sub can_showHints ($c) { return 1; } sub can_showSolutions ($c, $user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet) { my $authz = $c->authz; - return 1 if $authz->hasPermissions($user->user_id, 'always_show_solution'); - - # This is the same as can_showCorrectAnswers. return $c->can_showCorrectAnswers($user, $permissionLevel, $effectiveUser, $set, $problem, $tmplSet); } @@ -685,34 +700,15 @@ async sub pre_header_initialize ($c) { # What does the user want to do? my %want = ( - showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers}, - # showProblemGrader implies showCorrectAnswers. This is a convenience for grading. - showCorrectAnswers => ($c->param('showProblemGrader') || 0) - || ($c->param('showCorrectAnswers') && ($c->{submitAnswers} || $c->{checkAnswers})) - || 0, - showProblemGrader => $c->param('showProblemGrader') - || 0, - # Hints are not yet implemented in gateway quzzes. - showHints => 0, - showSolutions => 1, - recordAnswers => $c->{submitAnswers} && !$authz->hasPermissions($userID, 'avoid_recording_answers'), - # we also want to check answers if we were checking answers and are switching between pages - checkAnswers => $c->{checkAnswers}, - useMathView => $user->useMathView ne '' ? $user->useMathView : $ce->{pg}{options}{useMathView}, - useMathQuill => $user->useMathQuill ne '' ? $user->useMathQuill : $ce->{pg}{options}{useMathQuill}, - ); - - # Are certain options enforced? - my %must = ( - showOldAnswers => 0, - showCorrectAnswers => 0, - showProblemGrader => 0, - showHints => 0, - showSolutions => 0, - recordAnswers => 0, - checkAnswers => 0, - useMathView => 0, - useMathQuill => 0, + showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers}, + showCorrectAnswers => 1, + showProblemGrader => $c->param('showProblemGrader') || 0, + showHints => 0, # Hints are not yet implemented in gateway quzzes. + showSolutions => 1, + recordAnswers => $c->{submitAnswers} && !$authz->hasPermissions($userID, 'avoid_recording_answers'), + checkAnswers => $c->{checkAnswers}, + useMathView => $user->useMathView ne '' ? $user->useMathView : $ce->{pg}{options}{useMathView}, + useMathQuill => $user->useMathQuill ne '' ? $user->useMathQuill : $ce->{pg}{options}{useMathQuill}, ); # Does the user have permission to use certain options? @@ -735,10 +731,9 @@ async sub pre_header_initialize ($c) { ); # Final values for options - my %will = map { $_ => $can{$_} && ($must{$_} || $want{$_}) } keys %must; + my %will = map { $_ => $can{$_} && $want{$_} } keys %can; $c->{want} = \%want; - $c->{must} = \%must; $c->{can} = \%can; $c->{will} = \%will; @@ -1473,11 +1468,18 @@ async sub getProblemHTML ($c, $effectiveUser, $set, $formFields, $mergedProblem) showAttemptAnswers => $c->ce->{pg}{options}{showEvaluatedAnswers}, showAttemptPreviews => 1, showAttemptResults => !$c->{previewAnswers} && $c->{can}{showProblemScores}, - forceShowAttemptResults => $c->{will}{showProblemGrader}, - showMessages => 1, - showCorrectAnswers => ($c->{submitAnswers} || $c->{will}{checkAnswers} || $c->{will}{showProblemGrader}) - ? $c->{will}{showCorrectAnswers} - : 0, + forceShowAttemptResults => $c->{will}{showProblemGrader} + || ($c->ce->{pg}{options}{automaticAnswerFeedback} + && !$c->{previewAnswers} + && $c->can_showCorrectAnswersForAll($set, $c->{problem}, $c->{tmplSet})), + showMessages => 1, + showCorrectAnswers => ( + $c->{will}{showProblemGrader} + || (!$c->{previewAnswers} && $c->can_showCorrectAnswersForAll($set, $c->{problem}, $c->{tmplSet})) + ? 2 + : ($c->{submitAnswers} || $c->{will}{checkAnswers}) && $c->{will}{showCorrectAnswers} ? 1 + : 0 + ), debuggingOptions => getTranslatorDebuggingOptions($c->authz, $c->{userID}) }, ); diff --git a/lib/WeBWorK/ContentGenerator/Problem.pm b/lib/WeBWorK/ContentGenerator/Problem.pm index e596fe288d..985f2ee8d6 100644 --- a/lib/WeBWorK/ContentGenerator/Problem.pm +++ b/lib/WeBWorK/ContentGenerator/Problem.pm @@ -449,49 +449,29 @@ async sub pre_header_initialize ($c) { $showMeAnother{Count} = 0 unless $showMeAnother{Count} =~ /^[+-]?\d+$/; # Store the showMeAnother hash for the check to see if the button can be used - # (this hash is updated and re-stored after the can, must, will hashes) + # (this hash is updated and re-stored after the can and will hashes) $c->{showMeAnother} = \%showMeAnother; # Permissions # What does the user want to do? my %want = ( - showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers}, - # showProblemGrader implies showCorrectAnswers. This is a convenience for grading. - showCorrectAnswers => $c->param('showCorrectAnswers') || $c->param('showProblemGrader') || 0, - showProblemGrader => $c->param('showProblemGrader') || 0, - showAnsGroupInfo => $c->param('showAnsGroupInfo') || $ce->{pg}{options}{showAnsGroupInfo}, - showAnsHashInfo => $c->param('showAnsHashInfo') || $ce->{pg}{options}{showAnsHashInfo}, - showPGInfo => $c->param('showPGInfo') || $ce->{pg}{options}{showPGInfo}, - showResourceInfo => $c->param('showResourceInfo') || $ce->{pg}{options}{showResourceInfo}, + showOldAnswers => $user->showOldAnswers ne '' ? $user->showOldAnswers : $ce->{pg}{options}{showOldAnswers}, + showCorrectAnswers => 1, + showProblemGrader => $c->param('showProblemGrader') || 0, + showAnsGroupInfo => $c->param('showAnsGroupInfo') || $ce->{pg}{options}{showAnsGroupInfo}, + showAnsHashInfo => $c->param('showAnsHashInfo') || $ce->{pg}{options}{showAnsHashInfo}, + showPGInfo => $c->param('showPGInfo') || $ce->{pg}{options}{showPGInfo}, + showResourceInfo => $c->param('showResourceInfo') || $ce->{pg}{options}{showResourceInfo}, showHints => 1, showSolutions => 1, useMathView => $user->useMathView ne '' ? $user->useMathView : $ce->{pg}{options}{useMathView}, useMathQuill => $user->useMathQuill ne '' ? $user->useMathQuill : $ce->{pg}{options}{useMathQuill}, - recordAnswers => $c->{submitAnswers}, + recordAnswers => $c->{submitAnswers} && !$authz->hasPermissions($userID, 'avoid_recording_answers'), checkAnswers => $checkAnswers, getSubmitButton => 1, ); - # Are certain options enforced? - my %must = ( - showOldAnswers => 0, - showCorrectAnswers => 0, - showProblemGrader => 0, - showAnsGroupInfo => 0, - showAnsHashInfo => 0, - showPGInfo => 0, - showResourceInfo => 0, - showHints => 0, - showSolutions => 0, - recordAnswers => !$authz->hasPermissions($userID, 'avoid_recording_answers'), - checkAnswers => 0, - showMeAnother => 0, - getSubmitButton => 0, - useMathView => 0, - useMathQuill => 0, - ); - # Does the user have permission to use certain options? my @args = ($user, $effectiveUser, $c->{set}, $problem); @@ -548,11 +528,10 @@ async sub pre_header_initialize ($c) { } # Final values for options - my %will = map { $_ => $can{$_} && ($want{$_} || $must{$_}) } keys %must; + my %will = map { $_ => $can{$_} && $want{$_} } keys %can; if ($prEnabled && $problem->{prCount} >= $rerandomizePeriod && !after($c->{set}->due_date, $c->submitTime)) { $showMeAnother{active} = 0; - $must{requestNewSeed} = 1; $can{requestNewSeed} = 1; $want{requestNewSeed} = 1; $will{requestNewSeed} = 1; @@ -561,7 +540,6 @@ async sub pre_header_initialize ($c) { # being recorded and the number of attempts from being increased. if ($problem->{prCount} > $rerandomizePeriod) { $c->{resubmitDetected} = 1; - $must{recordAnswers} = 0; $can{recordAnswers} = 0; $want{recordAnswers} = 0; $will{recordAnswers} = 0; @@ -606,11 +584,19 @@ async sub pre_header_initialize ($c) { showAttemptAnswers => $ce->{pg}{options}{showEvaluatedAnswers}, showAttemptPreviews => 1, showAttemptResults => $c->{submitAnswers}, - forceShowAttemptResults => $will{checkAnswers} || $will{showProblemGrader}, - showMessages => 1, - showCorrectAnswers => $c->{submitAnswers} ? ($c->{showCorrectOnRandomize} // $will{showCorrectAnswers}) - : $will{checkAnswers} || $will{showProblemGrader} ? $will{showCorrectAnswers} - : 0, + forceShowAttemptResults => $will{checkAnswers} + || $will{showProblemGrader} + || ($ce->{pg}{options}{automaticAnswerFeedback} + && !$c->{previewAnswers} + && after($c->{set}->answer_date, $c->submitTime)), + showMessages => 1, + showCorrectAnswers => ( + $will{showProblemGrader} + || (!$c->{previewAnswers} && after($c->{set}->answer_date, $c->submitTime)) + || ($c->{submitAnswers} && $c->{showCorrectOnRandomize}) ? 2 + : (($c->{submitAnswers} || $will{checkAnswers}) && $will{showCorrectAnswers}) ? 1 + : 0 + ), debuggingOptions => getTranslatorDebuggingOptions($authz, $userID) } ); @@ -646,7 +632,6 @@ async sub pre_header_initialize ($c) { # Store fields $c->{want} = \%want; - $c->{must} = \%must; $c->{can} = \%can; $c->{will} = \%will; $c->{pg} = $pg; diff --git a/lib/WeBWorK/ContentGenerator/ShowMeAnother.pm b/lib/WeBWorK/ContentGenerator/ShowMeAnother.pm index d0bd45f011..1838a9be38 100644 --- a/lib/WeBWorK/ContentGenerator/ShowMeAnother.pm +++ b/lib/WeBWorK/ContentGenerator/ShowMeAnother.pm @@ -104,7 +104,7 @@ async sub pre_header_initialize ($c) { if ($showMeAnother{TriesNeeded} !~ /^[+-]?\d+$/ || $showMeAnother{TriesNeeded} == -2); # store the showMeAnother hash for the check to see if the button can be used - # (this hash is updated and re-stored after the can, must, will hashes) + # (this hash is updated and re-stored after the can and will hashes) $c->{showMeAnother} = \%showMeAnother; # Show a message if we aren't allowed to show me another here. @@ -116,14 +116,8 @@ async sub pre_header_initialize ($c) { my $want = $c->{want}; $want->{showMeAnother} = 1; - my $must = $c->{must}; - $must->{showMeAnother} = 0; - - # does the user have permission to use certain options? - my @args = ($user, $effectiveUser, $set, $problem); - my $can = $c->{can}; - $can->{showMeAnother} = $c->can_showMeAnother(@args, $submitAnswers); + $can->{showMeAnother} = $c->can_showMeAnother($user, $effectiveUser, $set, $problem, $submitAnswers); # store text of original problem for later comparison with text from problem with new seed my $showMeAnotherOriginalPG = await renderPG( @@ -277,9 +271,7 @@ async sub pre_header_initialize ($c) { # final values for options my $will = $c->{will}; - foreach (keys %$must) { - $will->{$_} = $can->{$_} && ($want->{$_} || $must->{$_}); - } + $will->{$_} = $can->{$_} && $want->{$_} for keys %$can; # PG problem translation # Unfortunately we have to do this over because we potentially picked a new problem seed. @@ -308,7 +300,7 @@ async sub pre_header_initialize ($c) { showAttemptPreviews => 1, showAttemptResults => $c->{checkAnswers}, showMessages => 1, - showCorrectAnswers => $will->{checkAnswers} ? $will->{showCorrectAnswers} : 0, + showCorrectAnswers => $will->{checkAnswers} && $will->{showCorrectAnswers} ? 1 : 0, debuggingOptions => getTranslatorDebuggingOptions($authz, $userName) } ); diff --git a/lib/WebworkWebservice/RenderProblem.pm b/lib/WebworkWebservice/RenderProblem.pm index f039322613..a77189af84 100644 --- a/lib/WebworkWebservice/RenderProblem.pm +++ b/lib/WebworkWebservice/RenderProblem.pm @@ -235,7 +235,7 @@ async sub renderProblem { showAttemptResults => $rh->{showAttemptResults} // ($rh->{WWsubmit} || $rh->{WWcorrectAns}), forceShowAttemptResults => $rh->{forceShowAttemptResults}, showMessages => $rh->{showMessages} // ($rh->{preview} || $rh->{WWsubmit} || $rh->{WWcorrectAns}), - showCorrectAnswers => $rh->{showCorrectAnswers} // $rh->{WWcorrectAns}, + showCorrectAnswers => $rh->{showCorrectAnswers} // ($rh->{WWcorrectAns} ? 2 : 0), debuggingOptions => { show_resource_info => $rh->{show_resource_info} // 0, view_problem_debugging_info => $rh->{view_problem_debugging_info} // 0, diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index 0869515197..94b3e583ef 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -345,7 +345,7 @@
- <%= maketext('The test (which is version [_1]) may no longer be submitted for a grade.', + <%= maketext('The test (which is version [_1]) may no longer be submitted for a grade.', $setVersionID) . ($c->{can}{showScore} ? (' ' . maketext('You may still check your answers.')) : '') %>
@@ -618,15 +618,6 @@
%
- % if ($c->{can}{showCorrectAnswers}) { -
- -
- % } % if ($c->{can}{showProblemGrader}) {