From 9938ca895bdb5f25647f2b3f9ebc5a8387a6d537 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 8 Nov 2024 14:53:52 -0800 Subject: [PATCH 01/25] add controls for when grades are sent to the LMS --- conf/authen_LTI.conf.dist | 63 ++++++++++++--- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 54 +++++++++---- .../Authen/LTIAdvantage/SubmitGrade.pm | 48 ++++++++--- lib/WeBWorK/ConfigValues.pm | 55 ++++++++++++- lib/WeBWorK/Utils/Sets.pm | 81 ++++++++++++++++--- 5 files changed, 257 insertions(+), 44 deletions(-) diff --git a/conf/authen_LTI.conf.dist b/conf/authen_LTI.conf.dist index 8e901ece14..e889d862fd 100644 --- a/conf/authen_LTI.conf.dist +++ b/conf/authen_LTI.conf.dist @@ -135,18 +135,58 @@ $LTIGradeMode = ''; #$LTIGradeMode = 'course'; #$LTIGradeMode = 'homework'; -# When set this variable sends grades back to the LMS every time a user submits an answer. This -# keeps students grades up to date but can be a drain on the server. -$LTIGradeOnSubmit = 1; - -# If $LTICheckPrior is set to 1 then the current LMS grade will be checked first, and if the grade -# has not changed then the grade will not be updated. This is intended to reduce changes to LMS -# records when no real grade change occurred. It requires a 2 round process, first querying the -# current grade from the LMS and then when needed making the grade submission. +# Controls for when to report grades to the LMS. + +# This variable can be set to 'open_date', 'reduced_scoring_date', 'close_date', 'answer_date', +# or 'never', corresponding to the open, reduced scoring, close, and answer dates for a set, or +# never. After this type of date has passed, WeBWorK will send grades to the LMS, subject to the +# other controls described below. Prior to this date, WeBWorK will only send grades to the LMS if +# the set has met the condition set with $LTISendGradesEarlyThreshold. For example, if it is set +# to 'close', then once the close date is reached for a set and something triggers a potential +# grade passback, scores will be sent to the LMS regardless of whether or not the set has been +# attempted or has reached a threshold score. Prior to the close date, if something triggers a +# potential grade passback, whether or not the set's scores are passed depends on the value of +# $LTISendGradesEarlyThreshold. In 'course' grade mode, this is controlling whether or not the +# set is included in the overall course grade computation. In 'homework' grade mode, it is only +# about that set's score. For a test, the date that matters is the earliest date among the versions +# or the set template. +$LTISendScoresAfterDate = 'open_date'; + +# This variable can be set to a number from 0 to 1 inclusive, or set to the string 'attempted'. +# When something triggers a potential grade passback, if it is earlier than $LTISendScoresAfterDate, +# the condition described by this variable must be met or else no grade will be sent. (In 'course' +# grade mode, the set will not be included in the overall grade calculation.) If this variable is +# a number, then the set will need to have a score that reaches or exceeds this number for its +# score to be sent to the LMS (or included in the 'course' grade calcuation). If this variable is +# set to 'attempted', then the set needs to have been attempted for its score to be sent to the LMS +# (or included in the 'course' grade calcuation). For a regular or jitar set, 'attempted' means that +# at least one exercise was attempted. For a test, 'attempted' means that either multiple versions +# exist or there is one version with a graded submission. +$LTISendGradesEarlyThreshold = 0; + +# When this variable is set, then each time WeBWorK would send a grade to the LMS, it first +# requests the current score from the LMS. If there is not a significant difference between the +# current score in the LMS and the score that WeBWorK is possibly going to send, then WeBWorK will +# not send a score to the LMS. This is intended to avoid frequent insignificant updates to the +# student's grades in the LMS. With some LMSs, some students receive notifications each time there +# is a grade update, and setting this variable will prevent too many notifications for them. This +# does create a two-phase process, first querying the current grade from the LMS and then when +# needed making the grade submission. $LTICheckPrior = 0; -# The system periodically updates student grades on the LMS. This variable controls how often -# that happens. Set to -1 to disable. +# When this variable is set then every time a user submits an answer, WeBWorK may send grades back +# to the LMS. This is subject to the above settings. If using 'course' grade mode, WeBWorK will +# possibly send a new overall score for this user. If using 'homework' grade mode, WeBWorK will +# only send a new score for this user for this set. Since this may put a strain on the server, it +# can be turned off and grades will only be sent to the LMS according to the $LTIMassUpdateInterval +# or when an instructor initiates a mass grade passback using LTI Grade Update. +$LTIGradeOnSubmit = 1; + +# The system periodically updates student grades on the LMS. This variable controls how often +# that happens. Set this to -1 to disable mass passback. These mass passback events must be triggered +# by someone in the course doing something after enough time has passed, even as simple as loading a +# page. So for example this cannot be set in such a way as to make sure the passback events happen +# at a particular time each day. $LTIMassUpdateInterval = 86400; #in seconds ################################################################################################ @@ -170,6 +210,9 @@ $LTIMassUpdateInterval = 86400; #in seconds #'LTI{v1p3}{LMS_url}', #'external_auth', #'LTIGradeMode', + #'LTISendScoresAfterDate', + #'LTISendGradesEarlyThreshold', + #'LTICheckPrior', #'LTIGradeOnSubmit', #'LTIMassUpdateInterval', #'LMSManageUserData', diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index a3ec411f47..fb4d0541a6 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -30,7 +30,7 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway gateway_attempted earliest_gateway_date grade_all_sets get_date); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -128,7 +128,8 @@ async sub submit_course_grade ($self, $userID) { $self->warning("lis_source_did is not available for user: $userID") if !$user->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - return await $self->submit_grade($user->lis_source_did, scalar(grade_all_sets($db, $userID))); + return await $self->submit_grade($user->lis_source_did, + scalar(grade_all_sets($db, $userID, $ce->{LTISendScoresAfterDate}, $ce->{LTISendGradesEarlyThreshold}))); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. @@ -147,18 +148,37 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('lis_source_did is not available for this set.') if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - return await $self->submit_grade( - $userSet->lis_source_did, - scalar( - $userSet->assignment_type =~ /gateway/ - ? grade_gateway($db, $userSet, $userSet->set_id, $userID) - : grade_set($db, $userSet, $userID, 0) - ) - ); + my $score; + my $incorrect_attempts = []; + my $attempted; + my $date; + + if ($userSet->assignment_type =~ /gateway/) { + $score = scalar(grade_gateway($db, $userSet, $userSet->set_id, $userID)); + $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); + $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); + } else { + my $totalRight; + my $total; + ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $userID, 0, 1))[ 0, 1, 3 ]; + $score = ($total == 0) ? 0 : $totalRight / $total; + $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); + $date = get_date($userSet, $ce->{LTISendScoresAfterDate}); + } + + my $beforeSendScoresAfterDate = $ce->{LTISendScoresAfterDate} eq 'never' || before($date); + if ($beforeSendScoresAfterDate) { + return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !$attempted); + return if ($score < $ce->{LTISendGradesEarlyThreshold}); + } + +# $beforeSendScoresAfterDate needs to be passed so that if LTICheckPrior is set, submit_grade() can decide between calling +# an empty grade equivalent to 0 before the SendScoresAfterDate versus not equivalent after the SendScoresAfterDate + return await $self->submit_grade($userSet->lis_source_did, $score, $beforeSendScoresAfterDate); } # Submits a score of $score to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $sourcedid, $score) { +async sub submit_grade ($self, $sourcedid, $score, $beforeSendScoresAfterDate = 1) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; @@ -301,11 +321,15 @@ EOS # See: https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5002 debug("LMS grade will be updated. sourcedid: $sourcedid; Old score: $oldScore; New score: $score") if $ce->{debug_lti_grade_passback}; - } elsif ($oldScore ne '' && abs($score - $oldScore) < 0.001) { + } elsif (abs($score - $oldScore) < 0.001 + && ($score != 1 || $oldScore == 1) + && ($score != 0 || $oldScore ne '' || $beforeSendScoresAfterDate)) + { # LMS has essentially the same score, no reason to update it - debug("LMS grade will NOT be updated - grade unchanges. Old score: $oldScore; New score: $score") - if $ce->{debug_lti_grade_passback}; - $self->warning('LMS grade will NOT be updated - grade unchanged. ' + debug( + "LMS grade will NOT be updated - grade has not significantly changed. Old score: $oldScore; New score: $score" + ) if $ce->{debug_lti_grade_passback}; + $self->warning('LMS grade will NOT be updated - grade has not significantly changed. ' . "Old score: $oldScore; New score: $score") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; return 1; diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 688c94d36e..89dda49de0 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -39,7 +39,7 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway gateway_attempted earliest_gateway_date grade_all_sets get_date); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -207,7 +207,8 @@ async sub submit_course_grade ($self, $userID) { $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for the course.') unless $lineitem; - return await $self->submit_grade($user->lis_source_did, $lineitem, grade_all_sets($db, $userID)); + return await $self->submit_grade($user->lis_source_did, $lineitem, + grade_all_sets($db, $userID, $ce->{LTISendScoresAfterDate}, $ce->{LTISendGradesEarlyThreshold})); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. @@ -225,14 +226,36 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; - return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, - $userSet->assignment_type =~ /gateway/ - ? grade_gateway($db, $userSet, $userSet->set_id, $userID) - : (grade_set($db, $userSet, $userID, 0))[ 0, 1 ]); + my $totalRight; + my $total; + my $incorrect_attempts = []; + my $attempted; + my $date; + + if ($userSet->assignment_type =~ /gateway/) { + ($totalRight, $total) = grade_gateway($db, $userSet, $userSet->set_id, $userID); + $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); + $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); + } else { + ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $userID, 0, 1))[ 0, 1, 3 ]; + $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); + $date = get_date($userSet, $ce->{LTISendScoresAfterDate}); + } + + my $beforeSendScoresAfterDate = $ce->{LTISendScoresAfterDate} eq 'never' || before($date); + if ($beforeSendScoresAfterDate) { + return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !$attempted); + return if ($total > 0 && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); + } + +# $beforeSendScoresAfterDate needs to be passed so that if LTICheckPrior is set, submit_grade() can decide between calling +# an empty grade equivalent to 0 before the SendScoresAfterDate versus not equivalent after the SendScoresAfterDate + return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $totalRight, $total, + $beforeSendScoresAfterDate); } # Submits scoreGiven and scoreMaximum to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum) { +async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum, $beforeSendScoresAfterDate = 1) { my $c = $self->{c}; my $ce = $c->{ce}; @@ -275,9 +298,16 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum && $priorData->[0]{resultMaximum} ? $priorData->[0]{resultScore} / $priorData->[0]{resultMaximum} : 0; my $score = $scoreGiven / $scoreMaximum; - if (abs($score - $priorScore) < 0.001) { + # we want to update the LMS score if the difference is significant, + # or if the new score is 1 but the LMS score was not 1 (but possibly insignificantly different) + # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate + if (abs($score - $priorScore) < 0.001 + && ($score != 1 || $priorScore == 1) + && ($score != 0 || @$priorData && $priorData->[0]{resultScore} ne '' || $beforeSendScoresAfterDate)) + { $self->warning( - "LMS grade will NOT be updated as the grade is unchanged. Old score: $priorScore, New score: $score."); + "LMS grade will NOT be updated as the grade has not significantly changed. Old score: $priorScore, New score: $score." + ); return 1; } diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 24cb77116c..2c5895af19 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -874,7 +874,7 @@ sub getConfigValues ($ce) { }, 'LTI{v1p1}{LMS_url}' => { var => 'LTI{v1p1}{LMS_url}', - doc => x('A URL for the LMS'), + doc => x('URL for the LMS'), doc2 => x( 'An address that can be used to log in to the LMS. This is used in messages to users ' . 'that direct them to go back to the LMS to access something in the WeBWorK course.' @@ -912,6 +912,59 @@ sub getConfigValues ($ce) { labels => { '' => 'None', 'course' => 'Course', 'homework' => 'Homework' }, type => 'popuplist' }, + LTISendScoresAfterDate => { + var => 'LTISendScoresAfterDate', + doc => x('When to send scores to the LMS regardless of status'), + doc2 => x( + 'After this type of date has passed, WeBWorK will send grades to the LMS, subject to the other ' + . 'controls for grade passback. Prior to this date, WeBWorK will only send grades to the LMS if ' + . "the set has met the condition set by \$LTISendGradesEarlyThreshold. In 'course' grade mode, " + . 'this is controlling whether or not the set is included in the overall course grade computation. ' + . "In 'homework' grade mode, it is only about that set's score. For a test, the date that matters " + . 'is the earliest date among the versions or the set template.' + ), + values => [qw(open_date reduced_scoring_date close_date answer_date never)], + labels => { + open_date => 'After the open date', + reduced_scoring_date => 'After the reduced scoring date', + close_date => 'After the close date', + answer_date => 'After the answer date', + never_date => 'Never' + }, + type => 'popuplist' + }, + LTISendGradesEarlyThreshold => { + var => 'LTISendGradesEarlyThreshold', + doc => x('Condition under which scores can be sent to an LMS early'), + doc2 => x( + "This can be set to a number from 0 to 1 inclusive, or set to the string 'attempted'. When something " + . 'triggers a potential grade passback, if it is earlier than $LTISendScoresAfterDate, the ' + . "condition described by this variable must be met or else no grade will be sent. (In 'course' " + . 'grade mode, the set will not be included in the overall grade calculation.) If this variable is ' + . 'a number, then the set will need to have a score that reaches or exceeds this number for its ' + . "score to be sent to the LMS (or included in the 'course' grade calcuation). If this variable is " + . "set to 'attempted', then the set needs to have been attempted for its score to be sent to the " + . "LMS (or included in the 'course' grade calcuation). For a regular or jitar set, 'attempted' " + . "means that at least one exercise was attempted. For a test, 'attempted' means that either " + . 'multiple versions exist or there is one version with a graded submission.' + ), + type => 'text', + }, + LTICheckPrior => { + var => 'LTICheckPrior', + doc => x('Check a score in the LMS before sending a score to the LMS'), + doc2 => x( + 'When this is set, then each time WeBWorK would send a grade to the LMS, it first requests the current ' + . 'score from the LMS. If there is not a significant difference between the current score in the ' + . 'LMS and the score that WeBWorK is possibly going to send, then WeBWorK will not send a score to ' + . "the LMS. This is intended to avoid frequent insignificant updates to the student's grades in " + . 'the LMS. With some LMSs, some students receive notifications each time there is a grade update, ' + . 'and setting this variable will prevent too many notifications for them. This does create a ' + . 'two-step process, first querying the current grade from the LMS and then when needed making ' + . 'the grade submission.' + ), + type => 'boolean' + }, LTIGradeOnSubmit => { var => 'LTIGradeOnSubmit', doc => x('Update LMS Grade Each Submit'), diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 5c2d69c868..fca65219f3 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -20,7 +20,7 @@ use Carp; use PGrandom; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::DateTime qw(after); +use WeBWorK::Utils::DateTime qw(after before); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq jitar_problem_adjusted_status); our @EXPORT_OK = qw( @@ -28,6 +28,9 @@ our @EXPORT_OK = qw( format_set_name_display grade_set grade_gateway + gateway_attempted + get_date + earliest_gateway_date grade_all_sets is_restricted get_test_problem_position @@ -146,7 +149,45 @@ sub grade_gateway ($db, $set, $setName, $studentName) { } } -sub grade_all_sets ($db, $studentName) { +sub gateway_attempted ($db, $setName, $studentName) { + my @versionNums = $db->listSetVersions($studentName, $setName); + + # it counts as "attempted" if there is more than one version + return 1 if (1 < @versionNums); + + # if there is one version, check for an attempted problem + if (@versionNums) { + my $versionedSet = $db->getSetVersion($studentName, $setName, $versionNums[0]); + + my @problemRecords = + $db->getAllMergedProblemVersions($studentName, $versionedSet->set_id, $versionedSet->version_id); + for my $problemRecord (@problemRecords) { + return 1 if $problemRecord->attempted; + } + } + + return 0; +} + +sub earliest_gateway_date ($db, $setName, $studentName, $dateType) { + return 'never' if ($dateType eq 'never'); + my @versionNums = $db->listSetVersions($studentName, $setName); + + # if there are no versions, use the template's date + unless (@versionNums) { + return get_date($db->getMergedSet($studentName, $setName), $dateType); + } + + # otherwise, use the earliest date among versions + my $date = get_date($db->getSetVersion($studentName, $setName, $versionNums[0]), $dateType); + for my $i (@versionNums) { + my $versionedSetDate = get_date($db->getSetVersion($studentName, $setName, $i), $dateType); + $date = $versionedSetDate if ($versionedSetDate < $date); + } + return $date; +} + +sub grade_all_sets ($db, $studentName, $sendScoresAfterDate = 'open_date', $sendGradesEarlyThreshold = 0) { my @setIDs = $db->listUserSets($studentName); my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; my @userSets = $db->getMergedSets(@userSetIDs); @@ -156,17 +197,29 @@ sub grade_all_sets ($db, $studentName) { for my $userSet (@userSets) { next unless (after($userSet->open_date())); - if ($userSet->assignment_type() =~ /gateway/) { + my $totalRight; + my $total; + my $incorrect_attempts = []; + my $attempted; + my $date; - my ($totalRight, $total) = grade_gateway($db, $userSet, $userSet->set_id, $studentName); - $courseTotalRight += $totalRight; - $courseTotal += $total; + if ($userSet->assignment_type() =~ /gateway/) { + ($totalRight, $total) = grade_gateway($db, $userSet, $userSet->set_id, $studentName); + $attempted = gateway_attempted($db, $userSet->set_id, $studentName); + $date = earliest_gateway_date($db, $userSet->set_id, $studentName, $sendScoresAfterDate); } else { - my ($totalRight, $total) = grade_set($db, $userSet, $studentName, 0); + ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $studentName, 0, 1))[ 0, 1, 3 ]; + $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); + $date = get_date($userSet, $sendScoresAfterDate); + } - $courseTotalRight += $totalRight; - $courseTotal += $total; + if ($sendScoresAfterDate eq 'never' || before($date)) { + next if ($sendGradesEarlyThreshold eq 'attempted' && !$attempted); + next if ($total > 0 && $totalRight / $total < $sendGradesEarlyThreshold); } + + $courseTotalRight += $totalRight; + $courseTotal += $total; } if (wantarray) { @@ -178,6 +231,16 @@ sub grade_all_sets ($db, $studentName) { } +sub get_date ($set, $type) { + return { + open_date => $set->open_date(), + reduced_scoring_date => $set->reduced_scoring_date() // $set->close_date(), + close_date => $set->close_date(), + answer_date => $set->answer_date(), + never => 'never' + }->{$type}; +} + sub is_restricted ($db, $set, $studentName) { # all sets open after the due date return () if after($set->due_date()); From 4fd4a57c404b4199e59d51d7f5cbba4df7a75e4e Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 8 Nov 2024 15:35:28 -0800 Subject: [PATCH 02/25] remove unused argument to grade_gateway() --- lib/Caliper/Entity.pm | 2 +- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 2 +- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 2 +- lib/WeBWorK/Utils/Sets.pm | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Caliper/Entity.pm b/lib/Caliper/Entity.pm index 13d9561a79..78883b765e 100644 --- a/lib/Caliper/Entity.pm +++ b/lib/Caliper/Entity.pm @@ -372,7 +372,7 @@ sub problem_set_attempt { my $extensions = { 'attempt_score' => $score, }; if ($version_id) { - $extensions->{'gateway_score'} = grade_gateway($db, $problem_set_user, $problem_set_user->set_id, $user_id); + $extensions->{'gateway_score'} = grade_gateway($db, $problem_set_user->set_id, $user_id); } my $problem_set_attempt = { diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index fb4d0541a6..13d227efca 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -154,7 +154,7 @@ async sub submit_set_grade ($self, $userID, $setID) { my $date; if ($userSet->assignment_type =~ /gateway/) { - $score = scalar(grade_gateway($db, $userSet, $userSet->set_id, $userID)); + $score = scalar(grade_gateway($db, $userSet->set_id, $userID)); $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); } else { diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 89dda49de0..ce8c52baf4 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -233,7 +233,7 @@ async sub submit_set_grade ($self, $userID, $setID) { my $date; if ($userSet->assignment_type =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $userSet, $userSet->set_id, $userID); + ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $userID); $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); } else { diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index fca65219f3..b4aa5d0c04 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -123,7 +123,7 @@ sub grade_set ($db, $set, $studentName, $setIsVersioned = 0, $wantProblemDetails } } -sub grade_gateway ($db, $set, $setName, $studentName) { +sub grade_gateway ($db, $setName, $studentName) { my @versionNums = $db->listSetVersions($studentName, $setName); my $bestTotalRight = 0; @@ -204,7 +204,7 @@ sub grade_all_sets ($db, $studentName, $sendScoresAfterDate = 'open_date', $send my $date; if ($userSet->assignment_type() =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $userSet, $userSet->set_id, $studentName); + ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $studentName); $attempted = gateway_attempted($db, $userSet->set_id, $studentName); $date = earliest_gateway_date($db, $userSet->set_id, $studentName, $sendScoresAfterDate); } else { @@ -378,7 +378,7 @@ In scalar context this returns the percentage correct. =head2 grade_gateway -Usage: C +Usage: C All arguments are required. From 94d38bae064cfc6cf32369db45276f9bbf9c043d Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 8 Nov 2024 16:27:04 -0800 Subject: [PATCH 03/25] simplify check if gateway set has been attempted --- lib/WeBWorK/Utils/Sets.pm | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index b4aa5d0c04..0ff5a346f5 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -157,13 +157,9 @@ sub gateway_attempted ($db, $setName, $studentName) { # if there is one version, check for an attempted problem if (@versionNums) { - my $versionedSet = $db->getSetVersion($studentName, $setName, $versionNums[0]); - - my @problemRecords = - $db->getAllMergedProblemVersions($studentName, $versionedSet->set_id, $versionedSet->version_id); - for my $problemRecord (@problemRecords) { - return 1 if $problemRecord->attempted; - } + my @problemNums = $db->listUserProblems($studentName, $setName); + my $problem = $db->getMergedProblemVersion($studentName, $setName, $versionNums[0], $problemNums[0]); + return defined $problem ? $problem->attempted : 0; } return 0; From 147a96145edab24cc6f75c6a6b64cb28f78b9a38 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 8 Nov 2024 19:30:51 -0800 Subject: [PATCH 04/25] fix division by zero bug --- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index ce8c52baf4..66ae0e66eb 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -297,7 +297,7 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum my $priorScore = @$priorData && $priorData->[0]{resultMaximum} ? $priorData->[0]{resultScore} / $priorData->[0]{resultMaximum} : 0; - my $score = $scoreGiven / $scoreMaximum; + my $score = $scoreMaximum ? $scoreGiven / $scoreMaximum : 0; # we want to update the LMS score if the difference is significant, # or if the new score is 1 but the LMS score was not 1 (but possibly insignificantly different) # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate From 1b7c7075c0f971ade41f38694edce965793fb5b2 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Sun, 10 Nov 2024 10:18:15 -0800 Subject: [PATCH 05/25] modularize more with new date and threshold considerations for LTI grade passback --- conf/authen_LTI.conf.dist | 136 +++++++++++------- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 64 ++++++--- .../Authen/LTIAdvantage/SubmitGrade.pm | 63 +++++--- lib/WeBWorK/ConfigValues.pm | 123 ++++++++++------ lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 2 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 4 +- lib/WeBWorK/Utils/Sets.pm | 81 ++++++----- 7 files changed, 299 insertions(+), 174 deletions(-) diff --git a/conf/authen_LTI.conf.dist b/conf/authen_LTI.conf.dist index e889d862fd..3b8e41db68 100644 --- a/conf/authen_LTI.conf.dist +++ b/conf/authen_LTI.conf.dist @@ -135,59 +135,93 @@ $LTIGradeMode = ''; #$LTIGradeMode = 'course'; #$LTIGradeMode = 'homework'; -# Controls for when to report grades to the LMS. - -# This variable can be set to 'open_date', 'reduced_scoring_date', 'close_date', 'answer_date', -# or 'never', corresponding to the open, reduced scoring, close, and answer dates for a set, or -# never. After this type of date has passed, WeBWorK will send grades to the LMS, subject to the -# other controls described below. Prior to this date, WeBWorK will only send grades to the LMS if -# the set has met the condition set with $LTISendGradesEarlyThreshold. For example, if it is set -# to 'close', then once the close date is reached for a set and something triggers a potential -# grade passback, scores will be sent to the LMS regardless of whether or not the set has been -# attempted or has reached a threshold score. Prior to the close date, if something triggers a -# potential grade passback, whether or not the set's scores are passed depends on the value of -# $LTISendGradesEarlyThreshold. In 'course' grade mode, this is controlling whether or not the -# set is included in the overall course grade computation. In 'homework' grade mode, it is only -# about that set's score. For a test, the date that matters is the earliest date among the versions -# or the set template. -$LTISendScoresAfterDate = 'open_date'; - -# This variable can be set to a number from 0 to 1 inclusive, or set to the string 'attempted'. -# When something triggers a potential grade passback, if it is earlier than $LTISendScoresAfterDate, -# the condition described by this variable must be met or else no grade will be sent. (In 'course' -# grade mode, the set will not be included in the overall grade calculation.) If this variable is -# a number, then the set will need to have a score that reaches or exceeds this number for its -# score to be sent to the LMS (or included in the 'course' grade calcuation). If this variable is -# set to 'attempted', then the set needs to have been attempted for its score to be sent to the LMS -# (or included in the 'course' grade calcuation). For a regular or jitar set, 'attempted' means that -# at least one exercise was attempted. For a test, 'attempted' means that either multiple versions -# exist or there is one version with a graded submission. -$LTISendGradesEarlyThreshold = 0; - -# When this variable is set, then each time WeBWorK would send a grade to the LMS, it first -# requests the current score from the LMS. If there is not a significant difference between the -# current score in the LMS and the score that WeBWorK is possibly going to send, then WeBWorK will -# not send a score to the LMS. This is intended to avoid frequent insignificant updates to the -# student's grades in the LMS. With some LMSs, some students receive notifications each time there -# is a grade update, and setting this variable will prevent too many notifications for them. This -# does create a two-phase process, first querying the current grade from the LMS and then when -# needed making the grade submission. +# There are several controls for when to report grades to the LMS. Sometimes these controls +# interplay with each other, and the details of how they work may depend on whether +# $LTIGradeMode is set to 'course' or 'homework'. So it is recommended to understand all of +# them and then decide how to set them. + +# If $LTICheckPrior is 1, then any time WeBWorK is about to send a grade to the LMS, it will +# first request from the LMS what that grade currently is. Then if there is no significant +# difference between the LMS grade and the WeBWorK grade, WeBWorK will not follow through with +# updating the LMS grade. This is to avoid frequent insignificant updates to a student's grades +# in the LMS. With some LMSs, students may receive notifications each time a grade is updated, +# and setting this variable will prevent too many notifications for them. This does create a +# two-phase process, first querying the current grade from the LMS and then actually updating +# the grade (if there is a significant difference). + +# Additional details: +# - If the LMS score is not 100%, but the WeBWorK score is, then even if the LMS score is only +# insignificantly less than 100%, it will be updated anyway. +# - If the LMS score is null and the WeBWorK score is 0, this is considered an insignificant +# difference and the LMS score will not be updated to 0. However if it is after the +# $LTISendScoresAfterDate (described below), then the null score will be updated to 0 anyway. +# - "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not +# configrable. $LTICheckPrior = 0; -# When this variable is set then every time a user submits an answer, WeBWorK may send grades back -# to the LMS. This is subject to the above settings. If using 'course' grade mode, WeBWorK will -# possibly send a new overall score for this user. If using 'homework' grade mode, WeBWorK will -# only send a new score for this user for this set. Since this may put a strain on the server, it -# can be turned off and grades will only be sent to the LMS according to the $LTIMassUpdateInterval -# or when an instructor initiates a mass grade passback using LTI Grade Update. +# If $LTIGradeOnSubmit is set to 1, then each time a user submits an answer or grades a test, +# that will trigger WeBWorK possibly reporting a score to the LMS. See $LTICheckPrior for one +# reason that WeBWorK might not ultimately send a grade. But there are other reasons too. +# WeBWorK will send the grade (the assignment's score if $LTIGradeMode is 'homework' or the +# overall course grade if $LTIGradeMode is 'course') to the LMS only if either the assignment's +# $LTISendGradesEarlyThreshold (described below) has been met or if it is past that assignment's +# $LTISendScoresAfterDate (also described below). + +# If $LTIGradeOnSubmit is set to 'homework_always', then +# - If $LTIGradeMode is 'homework', then WeBWorK will send that assignment's score to the LMS +# regardless of having met the $LTISendGradesEarlyThreshold or being past the +# $LTISendScoresAfterDate +# - If $LTIGradeMode is 'course', the behavior will be no different as if $LTIGradeOnSubmit were +# set to 1. + +# If sending grades upon each submission is not desired, you can set $LTIGradeOnSubmit to 0. + $LTIGradeOnSubmit = 1; +#$LTIGradeOnSubmit = 'homework_always'; +#$LTIGradeOnSubmit = 0; + +# In addition to scores possibly being sent to the LMS upon submission, they can be sent by an +# instructor or admin user using the LTI Grades Update Tool. And thirdly, the system can +# periodically update student grades on the LMS on its own. For all three possible triggers for +# grades to be passed to the LMS, $LTISendScoresAfterDate and $LTISendGradesEarlyThreshold can +# affect what is sent. $LTISendScoresAfterDate can be set to 'open_date', 'reduced_scoring_date', +# 'close_date', 'answer_date', or 'never'. For a given assignment, if it is *after* the +# $LTISendScoresAfterDate, then WeBWorK will send grades. +# - For 'course' grade passback mode, the assignment will be included in the overall course +# grade calculation. +# - For 'homework' grade passback mode, the assignemnt's score will be sent. + +# If $LTISendScoresAfterDate = 'reduced_scoring_date' and an assignent has no reduced_scoring_date +# or reduced scoring is disabled for that assignment, the fallback is to use the close_date. + +# For a given assignment, if it is *before* the $LTISendScoresAfterDate, WeBWorK *may* send a +# score to the LMS depending on $LTISendGradesEarlyThreshold. This variable can either be the +# string 'attempted' or a number from 0 to 1. If this variable is 'attempted', a given set must +# have been attempted for the threshold to have been met, and then the score can be used even if +# it is before the $LTISendScoresAfterDate. For a regular or jitar set, 'attempted' just means +# that some exercise in the set was attempted. For a test, 'attempted means that either there +# is one version with a graded submission, or there are at least two versions. + +# If $LTISendGradesEarlyThreshold is a number from 0 to 1, the score for an assignment needs to +# have reached that number for the threshold to be met, and then the score can be used even if +# it is before the $LTISendScoresAfterDate. + +#$LTISendScoresAfterDate = 'open_date'; +$LTISendScoresAfterDate = 'reduced_scoring_date'; +#$LTISendScoresAfterDate = 'close_date'; +#$LTISendScoresAfterDate = 'answer_date'; +#$LTISendScoresAfterDate = 'never'; + +$LTISendGradesEarlyThreshold = 'attempted'; +#$LTISendGradesEarlyThreshold = 0; +#$LTISendGradesEarlyThreshold = 0.7; +#$LTISendGradesEarlyThreshold = 1; + +# The system periodically updates student grades on the LMS. If it has been at least this many +# seconds since the last mass passback event and someone in the course does anything to load a +# page, then a new mass passback job will begin. Set this to -1 to disable mass passback. +$LTIMassUpdateInterval = 86400; -# The system periodically updates student grades on the LMS. This variable controls how often -# that happens. Set this to -1 to disable mass passback. These mass passback events must be triggered -# by someone in the course doing something after enough time has passed, even as simple as loading a -# page. So for example this cannot be set in such a way as to make sure the passback events happen -# at a particular time each day. -$LTIMassUpdateInterval = 86400; #in seconds ################################################################################################ # Add an 'LTI' tab to the Course Configuration page @@ -210,10 +244,10 @@ $LTIMassUpdateInterval = 86400; #in seconds #'LTI{v1p3}{LMS_url}', #'external_auth', #'LTIGradeMode', - #'LTISendScoresAfterDate', - #'LTISendGradesEarlyThreshold', #'LTICheckPrior', #'LTIGradeOnSubmit', + #'LTISendScoresAfterDate', + #'LTISendGradesEarlyThreshold', #'LTIMassUpdateInterval', #'LMSManageUserData', #'LTI{v1p1}{BasicConsumerSecret}', diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 13d227efca..e628b052ca 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -30,7 +30,8 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway gateway_attempted earliest_gateway_date grade_all_sets get_date); +use WeBWorK::Utils::DateTime qw(after before); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets get_set_date); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -113,6 +114,31 @@ sub update_sourcedid ($self, $userID) { return; } +# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold +sub can_submit_LMS_score ($db, $ce, $userID, $setID) { + my $userSet = $db->getMergedSet($userID, $setID); + + if ($ce->{LTISendScoresAfterDate} != 'never') { + my $critical_date; + if ($userSet->assignment_type() =~ /gateway/) { + $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); + } else { + $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); + } + return 1 if after($critical_date); + } + + return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); + + my $score; + if ($userSet->assignment_type() =~ /gateway/) { + $score = grade_gateway($db, $setID, $userID); + } else { + $score = grade_set($db, $setID, $userID); + } + return ($score >= $ce->{LTISendGradesEarlyThreshold}); +} + # Computes and submits the course grade for userID to the LMS. # The course grade is the average of all sets assigned to the user. async sub submit_course_grade ($self, $userID) { @@ -149,36 +175,30 @@ async sub submit_set_grade ($self, $userID, $setID) { if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); my $score; - my $incorrect_attempts = []; - my $attempted; - my $date; + my $criticalDate; if ($userSet->assignment_type =~ /gateway/) { - $score = scalar(grade_gateway($db, $userSet->set_id, $userID)); - $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); - $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); + $score = scalar(grade_gateway($db, $setID, $userID)); + $criticalDate = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}) + unless ($ce->{LTISendScoresAfterDate} eq 'never'); } else { - my $totalRight; - my $total; - ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $userID, 0, 1))[ 0, 1, 3 ]; - $score = ($total == 0) ? 0 : $totalRight / $total; - $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); - $date = get_date($userSet, $ce->{LTISendScoresAfterDate}); + $score = grade_set($db, $userSet, $userID); + $criticalDate = get_set_date($userSet, $ce->{LTISendScoresAfterDate}) + unless ($ce->{LTISendScoresAfterDate} eq 'never'); } - my $beforeSendScoresAfterDate = $ce->{LTISendScoresAfterDate} eq 'never' || before($date); - if ($beforeSendScoresAfterDate) { - return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !$attempted); - return if ($score < $ce->{LTISendGradesEarlyThreshold}); + if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { + return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !set_attempted($db, $userID, $setID)); + return if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' && $score < $ce->{LTISendGradesEarlyThreshold}); } -# $beforeSendScoresAfterDate needs to be passed so that if LTICheckPrior is set, submit_grade() can decide between calling -# an empty grade equivalent to 0 before the SendScoresAfterDate versus not equivalent after the SendScoresAfterDate - return await $self->submit_grade($userSet->lis_source_did, $score, $beforeSendScoresAfterDate); + return await $self->submit_grade($userSet->lis_source_did, $score, + ($ce->{LTISendScoresAfterDate} eq 'never' || before($criticalDate)) + && ($self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always')); } # Submits a score of $score to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $sourcedid, $score, $beforeSendScoresAfterDate = 1) { +async sub submit_grade ($self, $sourcedid, $score, $nullEqualsZero = 1) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; @@ -323,7 +343,7 @@ EOS if $ce->{debug_lti_grade_passback}; } elsif (abs($score - $oldScore) < 0.001 && ($score != 1 || $oldScore == 1) - && ($score != 0 || $oldScore ne '' || $beforeSendScoresAfterDate)) + && ($score != 0 || $oldScore ne '' || $nullEqualsZero)) { # LMS has essentially the same score, no reason to update it debug( diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 66ae0e66eb..db9e68efc4 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -39,7 +39,8 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway gateway_attempted earliest_gateway_date grade_all_sets get_date); +use WeBWorK::Utils::DateTime qw(after before); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets get_set_date); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -191,6 +192,31 @@ async sub get_access_token ($self) { return; } +# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold +sub can_submit_LMS_score ($db, $ce, $userID, $setID) { + my $userSet = $db->getMergedSet($userID, $setID); + + if ($ce->{LTISendScoresAfterDate} != 'never') { + my $critical_date; + if ($userSet->assignment_type() =~ /gateway/) { + $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); + } else { + $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); + } + return 1 if after($critical_date); + } + + return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); + + my $score; + if ($userSet->assignment_type() =~ /gateway/) { + $score = grade_gateway($db, $setID, $userID); + } else { + $score = grade_set($db, $setID, $userID); + } + return ($score >= $ce->{LTISendGradesEarlyThreshold}); +} + # Computes and submits the course grade for userID to the LMS. # The course grade is the sum of all (weighted) problems assigned to the user. async sub submit_course_grade ($self, $userID) { @@ -228,34 +254,33 @@ async sub submit_set_grade ($self, $userID, $setID) { my $totalRight; my $total; - my $incorrect_attempts = []; - my $attempted; - my $date; + my $criticalDate; if ($userSet->assignment_type =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $userID); - $attempted = gateway_attempted($db, $userSet, $userSet->set_id, $userID); - $date = earliest_gateway_date($db, $userSet->set_id, $userID, $ce->{LTISendScoresAfterDate}); + ($totalRight, $total) = grade_gateway($db, $setID, $userID); + $criticalDate = earliest_gateway_date($db, $userSet, $userID, $ce->{LTISendScoresAfterDate}) + unless ($ce->{LTISendScoresAfterDate} eq 'never'); } else { - ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $userID, 0, 1))[ 0, 1, 3 ]; - $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); - $date = get_date($userSet, $ce->{LTISendScoresAfterDate}); + ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; + $criticalDate = get_set_date($userSet, $ce->{LTISendScoresAfterDate}) + unless ($ce->{LTISendScoresAfterDate} eq 'never'); } - my $beforeSendScoresAfterDate = $ce->{LTISendScoresAfterDate} eq 'never' || before($date); - if ($beforeSendScoresAfterDate) { - return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !$attempted); - return if ($total > 0 && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); + if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { + return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !set_attempted($db, $userID, $setID)); + return + if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' + && $total + && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); } -# $beforeSendScoresAfterDate needs to be passed so that if LTICheckPrior is set, submit_grade() can decide between calling -# an empty grade equivalent to 0 before the SendScoresAfterDate versus not equivalent after the SendScoresAfterDate return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $totalRight, $total, - $beforeSendScoresAfterDate); + ($ce->{LTISendScoresAfterDate} eq 'never' || before($criticalDate)) + && ($self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always')); } # Submits scoreGiven and scoreMaximum to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum, $beforeSendScoresAfterDate = 1) { +async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum, $nullEqualsZero = 1) { my $c = $self->{c}; my $ce = $c->{ce}; @@ -303,7 +328,7 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate if (abs($score - $priorScore) < 0.001 && ($score != 1 || $priorScore == 1) - && ($score != 0 || @$priorData && $priorData->[0]{resultScore} ne '' || $beforeSendScoresAfterDate)) + && ($score != 0 || @$priorData && $priorData->[0]{resultScore} ne '' || $nullEqualsZero)) { $self->warning( "LMS grade will NOT be updated as the grade has not significantly changed. Old score: $priorScore, New score: $score." diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 2c5895af19..5ff37ebace 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -912,16 +912,64 @@ sub getConfigValues ($ce) { labels => { '' => 'None', 'course' => 'Course', 'homework' => 'Homework' }, type => 'popuplist' }, + LTICheckPrior => { + var => 'LTICheckPrior', + doc => x('Check a score in the LMS actually needs updating before updating it'), + doc2 => x( + '

When this is true, any time WeBWorK is about to send a grade to the LMS, it will first request ' + . 'from the LMS what that grade currently is. Then if there is no significant difference between ' + . 'the LMS grade and the WeBWorK grade, WeBWorK will not follow through with updating the LMS ' + . 'grade. This is to avoid frequent insignificant updates to a student grade in the LMS. With some ' + . 'LMSs, students may receive notifications each time a grade is updated, and setting this ' + . 'variable will prevent too many notifications for them. This does create a two-step process, ' + . 'first querying the current grade from the LMS and then actually updating the grade (if there is ' + . 'a significant difference). Additional details:

  • If the LMS score is not 100%, but the ' + . 'WeBWorK score is, then even if the LMS score is only insignificantly less than 100%, it will be ' + . 'updated anyway.
  • If the LMS score is null and the WeBWorK score is 0, this is considered ' + . 'an insignificant difference and the LMS score will not be updated to 0. However if it is after ' + . 'the $LTISendScoresAfterDate (described below), then the null score will be updated to 0 anyway.' + . '
  • "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not ' + . 'configrable.
' + ), + type => 'boolean' + }, + LTIGradeOnSubmit => { + var => 'LTIGradeOnSubmit', + doc => x('Update LMS Grade Each Submit'), + doc2 => x( + '

If this is set to Conditionally, then each time a user submits an answer or grades a test, that ' + . 'will trigger WeBWorK possibly reporting a score to the LMS. See $LTICheckPrior for one reason ' + . 'that WeBWorK might not ultimately send a grade. But there are other reasons too. WeBWorK will ' + . "send the grade (the assignment's score if \$LTIGradeMode is 'homework' or the overall course " + . "grade if \$LTIGradeMode is 'course') to the LMS only if either the assignment's " + . "\$LTISendGradesEarlyThreshold has been met or if it is past that assignment's " + . '$LTISendScoresAfterDate.

If $LTIGradeOnSubmit is set to Always, then:

  • If ' + . "\$LTIGradeMode is 'homework', then WeBWorK will send that assignment's score to the LMS " + . 'regardless of having met the $LTISendGradesEarlyThreshold or being past the ' + . "\$LTISendScoresAfterDate.
  • If \$LTIGradeMode is 'course', the behavior will be no " + . 'different as if $LTIGradeOnSubmit were set to Conditionally.

If sending grades upon ' + . 'each submission is not desired, you can set $LTIGradeOnSubmit to 0.

' + ), + values => [qw(0 1 homework_always)], + labels => { + 0 => 'Never', + 1 => 'Conditionally', + homework_always => 'Always' + }, + type => 'popuplist' + }, LTISendScoresAfterDate => { var => 'LTISendScoresAfterDate', doc => x('When to send scores to the LMS regardless of status'), doc2 => x( - 'After this type of date has passed, WeBWorK will send grades to the LMS, subject to the other ' - . 'controls for grade passback. Prior to this date, WeBWorK will only send grades to the LMS if ' - . "the set has met the condition set by \$LTISendGradesEarlyThreshold. In 'course' grade mode, " - . 'this is controlling whether or not the set is included in the overall course grade computation. ' - . "In 'homework' grade mode, it is only about that set's score. For a test, the date that matters " - . 'is the earliest date among the versions or the set template.' + '

This can be set to one of the dates assciated with assignments. For a given assignment, if it is after ' + . "the \$LTISendScoresAfterDate, then WeBWorK will send grades.

  • For 'course' grade " + . 'passback mode, the assignment will be included in the overall course grade calculation.
  • ' + . "For 'homework' grade passback mode, the assignment's score will be sent.

If " + . '$LTISendScoresAfterDate is set to "After the reduced scoring date" and an assignent has no ' + . 'reduced scoring date or reduced scoring is disabled for that assignment, the fallback is to use ' + . 'the close date.

For a given assignment, if it is before the $LTISendScoresAfterDate, ' + . 'WeBWorK will still send a score to the LMS if the $LTISendGradesEarlyThreshold has been met.

' ), values => [qw(open_date reduced_scoring_date close_date answer_date never)], labels => { @@ -937,42 +985,32 @@ sub getConfigValues ($ce) { var => 'LTISendGradesEarlyThreshold', doc => x('Condition under which scores can be sent to an LMS early'), doc2 => x( - "This can be set to a number from 0 to 1 inclusive, or set to the string 'attempted'. When something " - . 'triggers a potential grade passback, if it is earlier than $LTISendScoresAfterDate, the ' - . "condition described by this variable must be met or else no grade will be sent. (In 'course' " - . 'grade mode, the set will not be included in the overall grade calculation.) If this variable is ' - . 'a number, then the set will need to have a score that reaches or exceeds this number for its ' - . "score to be sent to the LMS (or included in the 'course' grade calcuation). If this variable is " - . "set to 'attempted', then the set needs to have been attempted for its score to be sent to the " - . "LMS (or included in the 'course' grade calcuation). For a regular or jitar set, 'attempted' " - . "means that at least one exercise was attempted. For a test, 'attempted' means that either " - . 'multiple versions exist or there is one version with a graded submission.' + "

This can either be set to a score or set to Attempted. When something triggers a potential grade " + . 'passback, if it is earlier than $LTISendScoresAfterDate, the condition described by this ' + . 'variable must be met or else no grade will be sent. Although if $LTIGradeOnSubmit is set to ' + . "'homework_always', then users submitting an answer or grading a test will still cause there to " + . 'be a grade passback regardless of this setting.

If this variable is a score, then the set ' + . 'will need to have a score that reaches or exceeds this score for its score to be sent to the ' + . "LMS (or included in the 'course' grade calcuation). If this variable is set to Attempted, then " + . 'the set needs to have been attempted for its score to be sent to the LMS (or included in the ' + . "'course' grade calcuation).

For a regular or jitar set, 'attempted' means that at least one " + . "exercise was attempted. For a test, 'attempted' means that either multiple versions exist or " + . 'there is one version with a graded submission.

' ), - type => 'text', - }, - LTICheckPrior => { - var => 'LTICheckPrior', - doc => x('Check a score in the LMS before sending a score to the LMS'), - doc2 => x( - 'When this is set, then each time WeBWorK would send a grade to the LMS, it first requests the current ' - . 'score from the LMS. If there is not a significant difference between the current score in the ' - . 'LMS and the score that WeBWorK is possibly going to send, then WeBWorK will not send a score to ' - . "the LMS. This is intended to avoid frequent insignificant updates to the student's grades in " - . 'the LMS. With some LMSs, some students receive notifications each time there is a grade update, ' - . 'and setting this variable will prevent too many notifications for them. This does create a ' - . 'two-step process, first querying the current grade from the LMS and then when needed making ' - . 'the grade submission.' - ), - type => 'boolean' - }, - LTIGradeOnSubmit => { - var => 'LTIGradeOnSubmit', - doc => x('Update LMS Grade Each Submit'), - doc2 => x( - 'Sets if webwork sends grades back to the LMS every time a user submits an answer. ' - . 'This keeps students grades up to date, but can cause additional server load.' - ), - type => 'boolean' + values => [qw(attempted 0 0.5 0.7 0.75 0.8 0.85 0.9 0.95 1)], + labels => { + attempted => 'Attempted', + 0 => '0%', + 0.5 => '50%', + 0.7 => '70%', + 0.75 => '75%', + 0.8 => '80%', + 0.85 => '85%', + 0.9 => '90%', + 0.95 => '95%', + 1 => '100%', + }, + type => 'popuplist' }, LTIMassUpdateInterval => { var => 'LTIMassUpdateInterval', @@ -980,7 +1018,8 @@ sub getConfigValues ($ce) { doc2 => x( 'Sets the time in seconds to periodically update the LMS grades. WeBWorK will update all grades on ' . 'the LMS if it has been longer than this time since the completion of the last update. This is ' - . 'only an approximate time. 86400 seconds is one day. -1 disables periodic updates.' + . 'only an approximate time. Mass updates of this nature may put significant strain on the server, ' + . 'and should not be set to happen too frequently. -1 disables these periodic updates.' ), type => 'number' }, diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 3f89af5ff5..614620ab17 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1026,7 +1026,7 @@ async sub pre_header_initialize ($c) { # Try to update the student score on the LMS if that option is enabled. if ($c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} && $ce->{LTIGradeOnSubmit}) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course') { + if ($ce->{LTIGradeMode} eq 'course' && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) { $LTIGradeResult = await $grader->submit_course_grade($effectiveUserID); } elsif ($ce->{LTIGradeMode} eq 'homework') { $LTIGradeResult = await $grader->submit_set_grade($effectiveUserID, $setID); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index b2755a5ed9..9e03318df8 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -255,7 +255,9 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeOnSubmit}) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course') { + if ($ce->{LTIGradeMode} eq 'course' + && can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) + { $LTIGradeResult = await $grader->submit_course_grade($problem->user_id); } elsif ($ce->{LTIGradeMode} eq 'homework') { $LTIGradeResult = await $grader->submit_set_grade($problem->user_id, $problem->set_id); diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 0ff5a346f5..8e10a4b286 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -28,8 +28,8 @@ our @EXPORT_OK = qw( format_set_name_display grade_set grade_gateway - gateway_attempted - get_date + set_attempted + get_set_date earliest_gateway_date grade_all_sets is_restricted @@ -149,41 +149,51 @@ sub grade_gateway ($db, $setName, $studentName) { } } -sub gateway_attempted ($db, $setName, $studentName) { - my @versionNums = $db->listSetVersions($studentName, $setName); +sub set_attempted ($db, $userID, $setID) { + my $userSet = $db->getMergedSet($userID, $setID); - # it counts as "attempted" if there is more than one version - return 1 if (1 < @versionNums); + if ($userSet->assignment_type() =~ /gateway/) { + my @versionNums = $db->listSetVersions($userID, $setID); - # if there is one version, check for an attempted problem - if (@versionNums) { - my @problemNums = $db->listUserProblems($studentName, $setName); - my $problem = $db->getMergedProblemVersion($studentName, $setName, $versionNums[0], $problemNums[0]); - return defined $problem ? $problem->attempted : 0; - } + # it counts as "attempted" if there is more than one version + return 1 if (1 < @versionNums); + + # if there is one version, check for an attempted problem + if (@versionNums) { + my @problemNums = $db->listUserProblems($userID, $setID); + my $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $problemNums[0]); + return defined $problem ? $problem->attempted : 0; + } - return 0; + # if there are no versions + return 0; + } else { + my @problemNums = $db->listUserProblems($userID, $setID); + for (@problemNums) { + my $problem = $db->getMergedProblem($userID, $setID, $_); + return 1 if $problem->attempted; + } + return 0; + } } -sub earliest_gateway_date ($db, $setName, $studentName, $dateType) { - return 'never' if ($dateType eq 'never'); - my @versionNums = $db->listSetVersions($studentName, $setName); +sub earliest_gateway_date ($db, $userSet, $dateType) { + my @versionNums = $db->listSetVersions($userSet->user_id, $userSet->set_id); # if there are no versions, use the template's date - unless (@versionNums) { - return get_date($db->getMergedSet($studentName, $setName), $dateType); - } + return get_set_date($userSet, $dateType) unless (@versionNums); # otherwise, use the earliest date among versions - my $date = get_date($db->getSetVersion($studentName, $setName, $versionNums[0]), $dateType); + my $earliest_date = + get_set_date($db->getSetVersion($userSet->user_id, $userSet->set_id, $versionNums[0]), $dateType); for my $i (@versionNums) { - my $versionedSetDate = get_date($db->getSetVersion($studentName, $setName, $i), $dateType); - $date = $versionedSetDate if ($versionedSetDate < $date); + my $versionedSetDate = get_set_date($db->getSetVersion($userSet->user_id, $userSet->set_id, $i), $dateType); + $earliest_date = $versionedSetDate if ($versionedSetDate < $earliest_date); } - return $date; + return $earliest_date; } -sub grade_all_sets ($db, $studentName, $sendScoresAfterDate = 'open_date', $sendGradesEarlyThreshold = 0) { +sub grade_all_sets ($db, $studentName, $dateType = 'reduced_scoring_date', $threshold = 'attempted') { my @setIDs = $db->listUserSets($studentName); my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; my @userSets = $db->getMergedSets(@userSetIDs); @@ -195,23 +205,19 @@ sub grade_all_sets ($db, $studentName, $sendScoresAfterDate = 'open_date', $send next unless (after($userSet->open_date())); my $totalRight; my $total; - my $incorrect_attempts = []; - my $attempted; - my $date; + my $criticalDate; if ($userSet->assignment_type() =~ /gateway/) { ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $studentName); - $attempted = gateway_attempted($db, $userSet->set_id, $studentName); - $date = earliest_gateway_date($db, $userSet->set_id, $studentName, $sendScoresAfterDate); + $criticalDate = earliest_gateway_date($db, $userSet, $dateType) unless ($dateType eq 'never'); } else { - ($totalRight, $total, $incorrect_attempts) = (grade_set($db, $userSet, $studentName, 0, 1))[ 0, 1, 3 ]; - $attempted = 1 if ($totalRight || grep $_ > 0, @$incorrect_attempts); - $date = get_date($userSet, $sendScoresAfterDate); + ($totalRight, $total) = grade_set($db, $userSet, $studentName); + $criticalDate = get_set_date($userSet, $dateType) unless ($dateType eq 'never'); } - if ($sendScoresAfterDate eq 'never' || before($date)) { - next if ($sendGradesEarlyThreshold eq 'attempted' && !$attempted); - next if ($total > 0 && $totalRight / $total < $sendGradesEarlyThreshold); + if ($dateType eq 'never' || $criticalDate && before($criticalDate)) { + next if ($threshold eq 'attempted' && !set_attempted($db, $studentName, $userSet->set_id)); + next if ($threshold ne 'attempted' && $total > 0 && $totalRight / $total < $threshold); } $courseTotalRight += $totalRight; @@ -227,14 +233,13 @@ sub grade_all_sets ($db, $studentName, $sendScoresAfterDate = 'open_date', $send } -sub get_date ($set, $type) { +sub get_set_date ($set, $dateType) { return { open_date => $set->open_date(), reduced_scoring_date => $set->reduced_scoring_date() // $set->close_date(), close_date => $set->close_date(), answer_date => $set->answer_date(), - never => 'never' - }->{$type}; + }->{$dateType}; } sub is_restricted ($db, $set, $studentName) { From f6efa8c5b08b1dbf0f9e66c16d80d120eff96266 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Sun, 10 Nov 2024 15:21:47 -0800 Subject: [PATCH 06/25] debugging PR 2617 --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 6 ++-- .../Authen/LTIAdvantage/SubmitGrade.pm | 6 ++-- lib/WeBWorK/ConfigValues.pm | 4 +-- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 2 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 2 +- lib/WeBWorK/Utils/Sets.pm | 31 +++++++++++++------ 6 files changed, 32 insertions(+), 19 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index e628b052ca..c7a6285c69 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -115,10 +115,10 @@ sub update_sourcedid ($self, $userID) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($db, $ce, $userID, $setID) { +sub can_submit_LMS_score ($self, $db, $ce, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); - if ($ce->{LTISendScoresAfterDate} != 'never') { + if ($ce->{LTISendScoresAfterDate} ne 'never') { my $critical_date; if ($userSet->assignment_type() =~ /gateway/) { $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); @@ -134,7 +134,7 @@ sub can_submit_LMS_score ($db, $ce, $userID, $setID) { if ($userSet->assignment_type() =~ /gateway/) { $score = grade_gateway($db, $setID, $userID); } else { - $score = grade_set($db, $setID, $userID); + $score = grade_set($db, $userSet, $userID); } return ($score >= $ce->{LTISendGradesEarlyThreshold}); } diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index db9e68efc4..ba33c706ea 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -193,10 +193,10 @@ async sub get_access_token ($self) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($db, $ce, $userID, $setID) { +sub can_submit_LMS_score ($self, $db, $ce, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); - if ($ce->{LTISendScoresAfterDate} != 'never') { + if ($ce->{LTISendScoresAfterDate} ne 'never') { my $critical_date; if ($userSet->assignment_type() =~ /gateway/) { $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); @@ -212,7 +212,7 @@ sub can_submit_LMS_score ($db, $ce, $userID, $setID) { if ($userSet->assignment_type() =~ /gateway/) { $score = grade_gateway($db, $setID, $userID); } else { - $score = grade_set($db, $setID, $userID); + $score = grade_set($db, $userSet, $userID); } return ($score >= $ce->{LTISendGradesEarlyThreshold}); } diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 5ff37ebace..72c702b770 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -971,11 +971,11 @@ sub getConfigValues ($ce) { . 'the close date.

For a given assignment, if it is before the $LTISendScoresAfterDate, ' . 'WeBWorK will still send a score to the LMS if the $LTISendGradesEarlyThreshold has been met.

' ), - values => [qw(open_date reduced_scoring_date close_date answer_date never)], + values => [qw(open_date reduced_scoring_date due_date answer_date never)], labels => { open_date => 'After the open date', reduced_scoring_date => 'After the reduced scoring date', - close_date => 'After the close date', + due_date => 'After the close date', answer_date => 'After the answer date', never_date => 'Never' }, diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 614620ab17..0b68889269 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1026,7 +1026,7 @@ async sub pre_header_initialize ($c) { # Try to update the student score on the LMS if that option is enabled. if ($c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} && $ce->{LTIGradeOnSubmit}) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course' && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) { + if ($ce->{LTIGradeMode} eq 'course' && $grader->can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) { $LTIGradeResult = await $grader->submit_course_grade($effectiveUserID); } elsif ($ce->{LTIGradeMode} eq 'homework') { $LTIGradeResult = await $grader->submit_set_grade($effectiveUserID, $setID); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 9e03318df8..3cda3c1f9e 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -256,7 +256,7 @@ async sub process_and_log_answer ($c) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); if ($ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) + && $grader->can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) { $LTIGradeResult = await $grader->submit_course_grade($problem->user_id); } elsif ($ce->{LTIGradeMode} eq 'homework') { diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 8e10a4b286..3029926b02 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -159,10 +159,17 @@ sub set_attempted ($db, $userID, $setID) { return 1 if (1 < @versionNums); # if there is one version, check for an attempted problem + # there could also be no actual attempted problems, but something like an + # achievement item has awarded credit for one exercise somewhere in the test if (@versionNums) { my @problemNums = $db->listUserProblems($userID, $setID); my $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $problemNums[0]); - return defined $problem ? $problem->attempted : 0; + return 1 if defined $problem && $problem->attempted; + for (@problemNums) { + $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $_); + return 1 if defined $problem && $problem->status > 0; + } + return 0; } # if there are no versions @@ -171,7 +178,7 @@ sub set_attempted ($db, $userID, $setID) { my @problemNums = $db->listUserProblems($userID, $setID); for (@problemNums) { my $problem = $db->getMergedProblem($userID, $setID, $_); - return 1 if $problem->attempted; + return 1 if ($problem->attempted || $problem->status > 0); } return 0; } @@ -234,12 +241,18 @@ sub grade_all_sets ($db, $studentName, $dateType = 'reduced_scoring_date', $thre } sub get_set_date ($set, $dateType) { - return { - open_date => $set->open_date(), - reduced_scoring_date => $set->reduced_scoring_date() // $set->close_date(), - close_date => $set->close_date(), - answer_date => $set->answer_date(), - }->{$dateType}; + my $date; + if ($dateType eq 'open_date') { + $date = $set->open_date; + } elsif ($dateType eq 'reduced_scoring_date') { + $date = + ($set->enable_reduced_scoring && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; + } elsif ($dateType eq 'due_date') { + $date = $set->due_date; + } elsif ($dateType eq 'answer_date') { + $date = $set->answer_date; + } + return $date; } sub is_restricted ($db, $set, $studentName) { @@ -271,7 +284,7 @@ sub is_restricted ($db, $set, $studentName) { $r_score = $v_score if ($v_score > $r_score); } } else { - $r_score = grade_set($db, $restrictor_set, $studentName, 0); + $r_score = grade_set($db, $restrictor_set, $studentName); } # round to evade machine rounding error From 02c9ba992187c0a502860e0c876ac7bac0524755 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Sun, 10 Nov 2024 15:29:49 -0800 Subject: [PATCH 07/25] put x() around config value labels --- lib/WeBWorK/ConfigValues.pm | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 72c702b770..8c7f96f7fe 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -821,9 +821,9 @@ sub getConfigValues ($ce) { . '
  • Debug: as in Standard, plus the problem environment (debugging data)
    • ' ), labels => { - '0' => 'Simple', - '1' => 'Standard', - '2' => 'Debug' + '0' => x('Simple'), + '1' => x('Standard'), + '2' => x('Debug') }, values => [qw(0 1 2)], type => 'popuplist' @@ -909,7 +909,7 @@ sub getConfigValues ($ce) { . 'of the LMS course.' ), values => [ '', qw(course homework) ], - labels => { '' => 'None', 'course' => 'Course', 'homework' => 'Homework' }, + labels => { '' => x('None'), 'course' => x('Course'), 'homework' => x('Homework') }, type => 'popuplist' }, LTICheckPrior => { @@ -952,9 +952,9 @@ sub getConfigValues ($ce) { ), values => [qw(0 1 homework_always)], labels => { - 0 => 'Never', - 1 => 'Conditionally', - homework_always => 'Always' + 0 => x('Never'), + 1 => x('Conditionally'), + homework_always => x('Always') }, type => 'popuplist' }, @@ -973,11 +973,11 @@ sub getConfigValues ($ce) { ), values => [qw(open_date reduced_scoring_date due_date answer_date never)], labels => { - open_date => 'After the open date', - reduced_scoring_date => 'After the reduced scoring date', - due_date => 'After the close date', - answer_date => 'After the answer date', - never_date => 'Never' + open_date => x('After the open date'), + reduced_scoring_date => x('After the reduced scoring date'), + due_date => x('After the close date'), + answer_date => x('After the answer date'), + never_date => x('Never') }, type => 'popuplist' }, @@ -999,7 +999,7 @@ sub getConfigValues ($ce) { ), values => [qw(attempted 0 0.5 0.7 0.75 0.8 0.85 0.9 0.95 1)], labels => { - attempted => 'Attempted', + attempted => x('Attempted'), 0 => '0%', 0.5 => '50%', 0.7 => '70%', From 10ad001076b6b3c592a78d78c14983f3d75fb352 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Sun, 10 Nov 2024 18:27:25 -0800 Subject: [PATCH 08/25] simplify how can_submit_LMS_score is used, and use it earlier --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 34 ++++++---------- .../Authen/LTIAdvantage/SubmitGrade.pm | 39 +++++++------------ lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 13 ++++++- lib/WeBWorK/Utils/ProblemProcessing.pm | 10 +++-- 4 files changed, 42 insertions(+), 54 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index c7a6285c69..da1dd7ea56 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -115,7 +115,7 @@ sub update_sourcedid ($self, $userID) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($self, $db, $ce, $userID, $setID) { +sub can_submit_LMS_score ($invocant, $db, $ce, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); if ($ce->{LTISendScoresAfterDate} ne 'never') { @@ -164,6 +164,8 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; + return unless $self->can_submit_LMS_score($db, $ce, $userID, $setID); + my $user = $db->getUser($userID); return 0 unless $user; @@ -174,27 +176,15 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('lis_source_did is not available for this set.') if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - my $score; - my $criticalDate; - - if ($userSet->assignment_type =~ /gateway/) { - $score = scalar(grade_gateway($db, $setID, $userID)); - $criticalDate = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}) - unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } else { - $score = grade_set($db, $userSet, $userID); - $criticalDate = get_set_date($userSet, $ce->{LTISendScoresAfterDate}) - unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } - - if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { - return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !set_attempted($db, $userID, $setID)); - return if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' && $score < $ce->{LTISendGradesEarlyThreshold}); - } - - return await $self->submit_grade($userSet->lis_source_did, $score, - ($ce->{LTISendScoresAfterDate} eq 'never' || before($criticalDate)) - && ($self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always')); + return await $self->submit_grade( + $userSet->lis_source_did, + scalar( + $userSet->assignment_type =~ /gateway/ + ? grade_gateway($db, $setID, $userID) + : grade_set($db, $userSet, $userID) + ), + $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always' + ); } # Submits a score of $score to the lms with $sourcedid as the identifier. diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index ba33c706ea..19f0b3e841 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -193,7 +193,7 @@ async sub get_access_token ($self) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($self, $db, $ce, $userID, $setID) { +sub can_submit_LMS_score ($invocant, $db, $ce, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); if ($ce->{LTISendScoresAfterDate} ne 'never') { @@ -243,6 +243,8 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; + return unless $self->can_submit_LMS_score($db, $ce, $userID, $setID); + my $user = $db->getUser($userID); return 0 unless $user; @@ -252,31 +254,16 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; - my $totalRight; - my $total; - my $criticalDate; - - if ($userSet->assignment_type =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $setID, $userID); - $criticalDate = earliest_gateway_date($db, $userSet, $userID, $ce->{LTISendScoresAfterDate}) - unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } else { - ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; - $criticalDate = get_set_date($userSet, $ce->{LTISendScoresAfterDate}) - unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } - - if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { - return if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && !set_attempted($db, $userID, $setID)); - return - if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' - && $total - && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); - } - - return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $totalRight, $total, - ($ce->{LTISendScoresAfterDate} eq 'never' || before($criticalDate)) - && ($self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always')); + return await $self->submit_grade( + $user->lis_source_did, + $userSet->lis_source_did, + ( + $userSet->assignment_type =~ /gateway/ + ? grade_gateway($db, $setID, $userID) + : (grade_set($db, $userSet, $userID))[ 0, 1 ] + ), + $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always' + ); } # Submits scoreGiven and scoreMaximum to the lms with $sourcedid as the identifier. diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 0b68889269..f320a736c1 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1024,9 +1024,18 @@ async sub pre_header_initialize ($c) { } # Try to update the student score on the LMS if that option is enabled. - if ($c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} && $ce->{LTIGradeOnSubmit}) { + if ( + $c->{submitAnswers} + && $will{recordAnswers} + && $ce->{LTIGradeMode} + && ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' + || $ce->{LTIGradeOnSubmit} + && $ce->{LTI}{ $ce->{LTIVersion} }{grader}->can_submit_LMS_score($db, $ce, $effectiveUserID, $setID) + ) + ) + { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course' && $grader->can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) { + if ($ce->{LTIGradeMode} eq 'course') { $LTIGradeResult = await $grader->submit_course_grade($effectiveUserID); } elsif ($ce->{LTIGradeMode} eq 'homework') { $LTIGradeResult = await $grader->submit_set_grade($effectiveUserID, $setID); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 3cda3c1f9e..f030ee541b 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -252,12 +252,14 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeMode}) { my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; my $LTIGradeResult = -1; - if ($ce->{LTIGradeOnSubmit}) { + if ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' + || $ce->{LTIGradeOnSubmit} + && $ce->{LTI}{ $ce->{LTIVersion} }{grader} + ->can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) + { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course' - && $grader->can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) - { + if ($ce->{LTIGradeMode} eq 'course') { $LTIGradeResult = await $grader->submit_course_grade($problem->user_id); } elsif ($ce->{LTIGradeMode} eq 'homework') { $LTIGradeResult = await $grader->submit_set_grade($problem->user_id, $problem->set_id); From 118b7322439dfea5df497d55d6ef0a08c5c6af45 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Tue, 12 Nov 2024 12:09:45 -0800 Subject: [PATCH 09/25] move can_submit_LMS_score to general set utilities --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 30 ++----------------- .../Authen/LTIAdvantage/SubmitGrade.pm | 30 ++----------------- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 6 ++-- lib/WeBWorK/Utils/ProblemProcessing.pm | 4 +-- lib/WeBWorK/Utils/Sets.pm | 26 ++++++++++++++++ 5 files changed, 36 insertions(+), 60 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index da1dd7ea56..7f7fdc0a83 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -31,7 +31,8 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after before); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets get_set_date); +use WeBWorK::Utils::Sets + qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -114,31 +115,6 @@ sub update_sourcedid ($self, $userID) { return; } -# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($invocant, $db, $ce, $userID, $setID) { - my $userSet = $db->getMergedSet($userID, $setID); - - if ($ce->{LTISendScoresAfterDate} ne 'never') { - my $critical_date; - if ($userSet->assignment_type() =~ /gateway/) { - $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); - } else { - $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); - } - return 1 if after($critical_date); - } - - return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); - - my $score; - if ($userSet->assignment_type() =~ /gateway/) { - $score = grade_gateway($db, $setID, $userID); - } else { - $score = grade_set($db, $userSet, $userID); - } - return ($score >= $ce->{LTISendGradesEarlyThreshold}); -} - # Computes and submits the course grade for userID to the LMS. # The course grade is the average of all sets assigned to the user. async sub submit_course_grade ($self, $userID) { @@ -164,7 +140,7 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return unless $self->can_submit_LMS_score($db, $ce, $userID, $setID); + return unless can_submit_LMS_score($db, $ce, $userID, $setID); my $user = $db->getUser($userID); return 0 unless $user; diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 19f0b3e841..d8a5e72437 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -40,7 +40,8 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after before); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets get_set_date); +use WeBWorK::Utils::Sets + qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -192,31 +193,6 @@ async sub get_access_token ($self) { return; } -# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold -sub can_submit_LMS_score ($invocant, $db, $ce, $userID, $setID) { - my $userSet = $db->getMergedSet($userID, $setID); - - if ($ce->{LTISendScoresAfterDate} ne 'never') { - my $critical_date; - if ($userSet->assignment_type() =~ /gateway/) { - $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); - } else { - $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); - } - return 1 if after($critical_date); - } - - return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); - - my $score; - if ($userSet->assignment_type() =~ /gateway/) { - $score = grade_gateway($db, $setID, $userID); - } else { - $score = grade_set($db, $userSet, $userID); - } - return ($score >= $ce->{LTISendGradesEarlyThreshold}); -} - # Computes and submits the course grade for userID to the LMS. # The course grade is the sum of all (weighted) problems assigned to the user. async sub submit_course_grade ($self, $userID) { @@ -243,7 +219,7 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return unless $self->can_submit_LMS_score($db, $ce, $userID, $setID); + return unless can_submit_LMS_score($db, $ce, $userID, $setID); my $user = $db->getUser($userID); return 0 unless $user; diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index f320a736c1..e8117ddc0c 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -33,7 +33,7 @@ use WeBWorK::Utils::Instructor qw(assignSetVersionToUser); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); use WeBWorK::Utils::ProblemProcessing qw/create_ans_str_from_responses compute_reduced_score/; use WeBWorK::Utils::Rendering qw(getTranslatorDebuggingOptions renderPG); -use WeBWorK::Utils::Sets qw(is_restricted); +use WeBWorK::Utils::Sets qw(is_restricted can_submit_LMS_score); use WeBWorK::DB::Utils qw(global2user fake_set fake_set_version fake_problem); use WeBWorK::Debug; use WeBWorK::Authen::LTIAdvanced::SubmitGrade; @@ -1029,9 +1029,7 @@ async sub pre_header_initialize ($c) { && $will{recordAnswers} && $ce->{LTIGradeMode} && ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} - && $ce->{LTI}{ $ce->{LTIVersion} }{grader}->can_submit_LMS_score($db, $ce, $effectiveUserID, $setID) - ) + || $ce->{LTIGradeOnSubmit} && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) ) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index f030ee541b..76836dd861 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -33,6 +33,7 @@ use WeBWorK::Utils qw(encodeAnswers createEmailSenderTransportSMTP); use WeBWorK::Utils::DateTime qw(before after); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq jitar_problem_adjusted_status); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); +use WeBWorK::Utils::Sets qw(can_submit_LMS_score); use WeBWorK::Authen::LTIAdvanced::SubmitGrade; use WeBWorK::Authen::LTIAdvantage::SubmitGrade; use Caliper::Sensor; @@ -254,8 +255,7 @@ async sub process_and_log_answer ($c) { my $LTIGradeResult = -1; if ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' || $ce->{LTIGradeOnSubmit} - && $ce->{LTI}{ $ce->{LTIVersion} }{grader} - ->can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) + && can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 3029926b02..dba927ec41 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -35,6 +35,7 @@ our @EXPORT_OK = qw( is_restricted get_test_problem_position list_set_versions + can_submit_LMS_score ); sub format_set_name_internal ($set_name) { @@ -255,6 +256,31 @@ sub get_set_date ($set, $dateType) { return $date; } +# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold +sub can_submit_LMS_score ($db, $ce, $userID, $setID) { + my $userSet = $db->getMergedSet($userID, $setID); + + if ($ce->{LTISendScoresAfterDate} ne 'never') { + my $critical_date; + if ($userSet->assignment_type() =~ /gateway/) { + $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); + } else { + $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); + } + return 1 if after($critical_date); + } + + return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); + + my $score; + if ($userSet->assignment_type() =~ /gateway/) { + $score = grade_gateway($db, $setID, $userID); + } else { + $score = grade_set($db, $userSet, $userID); + } + return ($score >= $ce->{LTISendGradesEarlyThreshold}); +} + sub is_restricted ($db, $set, $studentName) { # all sets open after the due date return () if after($set->due_date()); From b879a980aa42e382b20ef945fb3a7d275065251d Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Tue, 12 Nov 2024 14:35:12 -0800 Subject: [PATCH 10/25] check that reduced scoring date is enabled or else move on to the close (due) date for the LTI critical date --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 6 +-- .../Authen/LTIAdvantage/SubmitGrade.pm | 6 +-- lib/WeBWorK/Utils/Sets.pm | 41 +++++++++++-------- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 7f7fdc0a83..35ee84beef 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -31,8 +31,7 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after before); -use WeBWorK::Utils::Sets - qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -130,8 +129,7 @@ async sub submit_course_grade ($self, $userID) { $self->warning("lis_source_did is not available for user: $userID") if !$user->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - return await $self->submit_grade($user->lis_source_did, - scalar(grade_all_sets($db, $userID, $ce->{LTISendScoresAfterDate}, $ce->{LTISendGradesEarlyThreshold}))); + return await $self->submit_grade($user->lis_source_did, scalar(grade_all_sets($db, $ce, $userID))); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index d8a5e72437..0d1191a4d0 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -40,8 +40,7 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); use WeBWorK::Utils::DateTime qw(after before); -use WeBWorK::Utils::Sets - qw(grade_set grade_gateway set_attempted earliest_gateway_date grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -209,8 +208,7 @@ async sub submit_course_grade ($self, $userID) { $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for the course.') unless $lineitem; - return await $self->submit_grade($user->lis_source_did, $lineitem, - grade_all_sets($db, $userID, $ce->{LTISendScoresAfterDate}, $ce->{LTISendGradesEarlyThreshold})); + return await $self->submit_grade($user->lis_source_did, $lineitem, grade_all_sets($db, $ce, $userID)); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index dba927ec41..e7c0c9b809 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -29,8 +29,6 @@ our @EXPORT_OK = qw( grade_set grade_gateway set_attempted - get_set_date - earliest_gateway_date grade_all_sets is_restricted get_test_problem_position @@ -185,23 +183,24 @@ sub set_attempted ($db, $userID, $setID) { } } -sub earliest_gateway_date ($db, $userSet, $dateType) { +sub earliest_gateway_date ($db, $ce, $userSet) { my @versionNums = $db->listSetVersions($userSet->user_id, $userSet->set_id); # if there are no versions, use the template's date - return get_set_date($userSet, $dateType) unless (@versionNums); + return get_LTISendScoresAfterDate($userSet, $ce) unless (@versionNums); # otherwise, use the earliest date among versions my $earliest_date = - get_set_date($db->getSetVersion($userSet->user_id, $userSet->set_id, $versionNums[0]), $dateType); + get_LTISendScoresAfterDate($db->getSetVersion($userSet->user_id, $userSet->set_id, $versionNums[0]), $ce); for my $i (@versionNums) { - my $versionedSetDate = get_set_date($db->getSetVersion($userSet->user_id, $userSet->set_id, $i), $dateType); + my $versionedSetDate = + get_LTISendScoresAfterDate($db->getSetVersion($userSet->user_id, $userSet->set_id, $i), $ce); $earliest_date = $versionedSetDate if ($versionedSetDate < $earliest_date); } return $earliest_date; } -sub grade_all_sets ($db, $studentName, $dateType = 'reduced_scoring_date', $threshold = 'attempted') { +sub grade_all_sets ($db, $ce, $studentName) { my @setIDs = $db->listUserSets($studentName); my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; my @userSets = $db->getMergedSets(@userSetIDs); @@ -217,15 +216,20 @@ sub grade_all_sets ($db, $studentName, $dateType = 'reduced_scoring_date', $thre if ($userSet->assignment_type() =~ /gateway/) { ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $studentName); - $criticalDate = earliest_gateway_date($db, $userSet, $dateType) unless ($dateType eq 'never'); + $criticalDate = earliest_gateway_date($db, $ce, $userSet) unless ($ce->{LTISendScoresAfterDate} eq 'never'); } else { ($totalRight, $total) = grade_set($db, $userSet, $studentName); - $criticalDate = get_set_date($userSet, $dateType) unless ($dateType eq 'never'); + $criticalDate = get_LTISendScoresAfterDate($userSet, $ce) unless ($ce->{LTISendScoresAfterDate} eq 'never'); } - if ($dateType eq 'never' || $criticalDate && before($criticalDate)) { - next if ($threshold eq 'attempted' && !set_attempted($db, $studentName, $userSet->set_id)); - next if ($threshold ne 'attempted' && $total > 0 && $totalRight / $total < $threshold); + if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { + next + if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' + && !set_attempted($db, $studentName, $userSet->set_id)); + next + if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' + && $total > 0 + && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); } $courseTotalRight += $totalRight; @@ -241,13 +245,16 @@ sub grade_all_sets ($db, $studentName, $dateType = 'reduced_scoring_date', $thre } -sub get_set_date ($set, $dateType) { +sub get_LTISendScoresAfterDate ($set, $ce) { + my $dateType = $ce->{LTISendScoresAfterDate}; my $date; if ($dateType eq 'open_date') { $date = $set->open_date; } elsif ($dateType eq 'reduced_scoring_date') { $date = - ($set->enable_reduced_scoring && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; + ($ce->{pg}{ansEvalDefaults}{enableReducedScoring} + && $set->enable_reduced_scoring + && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; } elsif ($dateType eq 'due_date') { $date = $set->due_date; } elsif ($dateType eq 'answer_date') { @@ -263,9 +270,9 @@ sub can_submit_LMS_score ($db, $ce, $userID, $setID) { if ($ce->{LTISendScoresAfterDate} ne 'never') { my $critical_date; if ($userSet->assignment_type() =~ /gateway/) { - $critical_date = earliest_gateway_date($db, $userSet, $ce->{LTISendScoresAfterDate}); + $critical_date = earliest_gateway_date($db, $ce, $userSet); } else { - $critical_date = get_set_date($userSet, $ce->{LTISendScoresAfterDate}); + $critical_date = get_LTISendScoresAfterDate($userSet, $ce); } return 1 if after($critical_date); } @@ -431,7 +438,7 @@ version of this test. =head2 grade_all_sets -Usage: C +Usage: C All arguments listed are required. From 8964a0c60544e40c3b70fcc05642dcac5de0ae1a Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Tue, 12 Nov 2024 14:39:17 -0800 Subject: [PATCH 11/25] fix logic for homework_always with LTIGradeOnSubmit --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 4 +++- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 35ee84beef..8af7145682 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -138,7 +138,9 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return unless can_submit_LMS_score($db, $ce, $userID, $setID); + return + unless (can_submit_LMS_score($db, $ce, $userID, $setID) + || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); my $user = $db->getUser($userID); return 0 unless $user; diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 0d1191a4d0..c3ffcd8d6b 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -217,7 +217,9 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return unless can_submit_LMS_score($db, $ce, $userID, $setID); + return + unless (can_submit_LMS_score($db, $ce, $userID, $setID) + || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); my $user = $db->getUser($userID); return 0 unless $user; From 22457bc3c3233f9e4dda1dc30078a7b6c8bce8ca Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Wed, 13 Nov 2024 22:11:55 -0800 Subject: [PATCH 12/25] various feedback from PR#2617 --- conf/authen_LTI.conf.dist | 6 +++--- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 6 +++--- .../Authen/LTIAdvantage/SubmitGrade.pm | 1 - lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 6 ++++-- lib/WeBWorK/Utils/ProblemProcessing.pm | 3 ++- lib/WeBWorK/Utils/Sets.pm | 20 ++++++++----------- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/conf/authen_LTI.conf.dist b/conf/authen_LTI.conf.dist index 3b8e41db68..f7d57d6986 100644 --- a/conf/authen_LTI.conf.dist +++ b/conf/authen_LTI.conf.dist @@ -185,14 +185,14 @@ $LTIGradeOnSubmit = 1; # periodically update student grades on the LMS on its own. For all three possible triggers for # grades to be passed to the LMS, $LTISendScoresAfterDate and $LTISendGradesEarlyThreshold can # affect what is sent. $LTISendScoresAfterDate can be set to 'open_date', 'reduced_scoring_date', -# 'close_date', 'answer_date', or 'never'. For a given assignment, if it is *after* the +# 'due_date', 'answer_date', or 'never'. For a given assignment, if it is *after* the # $LTISendScoresAfterDate, then WeBWorK will send grades. # - For 'course' grade passback mode, the assignment will be included in the overall course # grade calculation. # - For 'homework' grade passback mode, the assignemnt's score will be sent. # If $LTISendScoresAfterDate = 'reduced_scoring_date' and an assignent has no reduced_scoring_date -# or reduced scoring is disabled for that assignment, the fallback is to use the close_date. +# or reduced scoring is disabled for that assignment, the fallback is to use the due_date. # For a given assignment, if it is *before* the $LTISendScoresAfterDate, WeBWorK *may* send a # score to the LMS depending on $LTISendGradesEarlyThreshold. This variable can either be the @@ -208,7 +208,7 @@ $LTIGradeOnSubmit = 1; #$LTISendScoresAfterDate = 'open_date'; $LTISendScoresAfterDate = 'reduced_scoring_date'; -#$LTISendScoresAfterDate = 'close_date'; +#$LTISendScoresAfterDate = 'due_date'; #$LTISendScoresAfterDate = 'answer_date'; #$LTISendScoresAfterDate = 'never'; diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 8af7145682..b90fe02388 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -30,7 +30,6 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::DateTime qw(after before); use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS @@ -307,8 +306,9 @@ EOS # See: https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5002 debug("LMS grade will be updated. sourcedid: $sourcedid; Old score: $oldScore; New score: $score") if $ce->{debug_lti_grade_passback}; - } elsif (abs($score - $oldScore) < 0.001 - && ($score != 1 || $oldScore == 1) + } elsif ($oldScore ne '' + || abs($score - $oldScore) < 0.001 + && ($score != 1 || $oldScore ne '' && $oldScore == 1) && ($score != 0 || $oldScore ne '' || $nullEqualsZero)) { # LMS has essentially the same score, no reason to update it diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index c3ffcd8d6b..b41f15ffbe 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -39,7 +39,6 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::DateTime qw(after before); use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS via LTI 1.3. diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index e8117ddc0c..5d5516f537 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1028,8 +1028,10 @@ async sub pre_header_initialize ($c) { $c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} - && ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) + && ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' + || $ce->{LTIGradeOnSubmit} + && $ce->{LTIGradeMode} eq 'course' + && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) ) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 76836dd861..dd6a8c752f 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -253,8 +253,9 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeMode}) { my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; my $LTIGradeResult = -1; - if ($ce->{LTIGradeOnSubmit} eq 'homework_always' && $ce->{LTIGradeMode} eq 'homework' + if ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' || $ce->{LTIGradeOnSubmit} + && $ce->{LTIGradeMode} eq 'course' && can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) { $LTIGradeResult = 0; diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index e7c0c9b809..b71f17379b 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -246,21 +246,17 @@ sub grade_all_sets ($db, $ce, $studentName) { } sub get_LTISendScoresAfterDate ($set, $ce) { - my $dateType = $ce->{LTISendScoresAfterDate}; - my $date; - if ($dateType eq 'open_date') { - $date = $set->open_date; - } elsif ($dateType eq 'reduced_scoring_date') { - $date = - ($ce->{pg}{ansEvalDefaults}{enableReducedScoring} + if ($ce->{LTISendScoresAfterDate} eq 'open_date') { + return $set->open_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'reduced_scoring_date') { + return ($ce->{pg}{ansEvalDefaults}{enableReducedScoring} && $set->enable_reduced_scoring && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; - } elsif ($dateType eq 'due_date') { - $date = $set->due_date; - } elsif ($dateType eq 'answer_date') { - $date = $set->answer_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'due_date') { + return $set->due_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'answer_date') { + return $set->answer_date; } - return $date; } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold From f4373dd7780f1c7509658d77a8d38f3128209f9a Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Fri, 15 Nov 2024 19:03:12 -0800 Subject: [PATCH 13/25] minimize calls to grade_set --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 22 +++----- .../Authen/LTIAdvantage/SubmitGrade.pm | 23 +++----- lib/WeBWorK/Utils/Sets.pm | 55 +++++++++---------- 3 files changed, 39 insertions(+), 61 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index b90fe02388..14eb40d6be 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -30,7 +30,7 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -137,12 +137,11 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return - unless (can_submit_LMS_score($db, $ce, $userID, $setID) - || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); - my $user = $db->getUser($userID); - return 0 unless $user; + return unless $user; + + my $score = can_submit_LMS_score($db, $ce, $userID, $setID); + return unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); my $userSet = $db->getMergedSet($userID, $setID); @@ -151,15 +150,8 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('lis_source_did is not available for this set.') if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - return await $self->submit_grade( - $userSet->lis_source_did, - scalar( - $userSet->assignment_type =~ /gateway/ - ? grade_gateway($db, $setID, $userID) - : grade_set($db, $userSet, $userID) - ), - $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always' - ); + return await $self->submit_grade($userSet->lis_source_did, $score->{score}, + $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always'); } # Submits a score of $score to the lms with $sourcedid as the identifier. diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index b41f15ffbe..fb3614e6a2 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -39,7 +39,7 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_set grade_gateway grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_all_sets can_submit_LMS_score); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -216,12 +216,11 @@ async sub submit_set_grade ($self, $userID, $setID) { my $ce = $c->{ce}; my $db = $c->{db}; - return - unless (can_submit_LMS_score($db, $ce, $userID, $setID) - || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); - my $user = $db->getUser($userID); - return 0 unless $user; + return unless $user; + + my $score = can_submit_LMS_score($db, $ce, $userID, $setID); + return unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); my $userSet = $db->getMergedSet($userID, $setID); @@ -229,16 +228,8 @@ async sub submit_set_grade ($self, $userID, $setID) { $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; - return await $self->submit_grade( - $user->lis_source_did, - $userSet->lis_source_did, - ( - $userSet->assignment_type =~ /gateway/ - ? grade_gateway($db, $setID, $userID) - : (grade_set($db, $userSet, $userID))[ 0, 1 ] - ), - $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always' - ); + return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $score->{totalRight}, + $score->{total}, $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always'); } # Submits scoreGiven and scoreMaximum to the lms with $sourcedid as the identifier. diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index b71f17379b..dfc7da414c 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -28,7 +28,6 @@ our @EXPORT_OK = qw( format_set_name_display grade_set grade_gateway - set_attempted grade_all_sets is_restricted get_test_problem_position @@ -201,31 +200,20 @@ sub earliest_gateway_date ($db, $ce, $userSet) { } sub grade_all_sets ($db, $ce, $studentName) { - my @setIDs = $db->listUserSets($studentName); - my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; - my @userSets = $db->getMergedSets(@userSetIDs); - - my $courseTotal = 0; + my @setIDs = $db->listUserSets($studentName); my $courseTotalRight = 0; + my $courseTotal = 0; - for my $userSet (@userSets) { - next unless (after($userSet->open_date())); - my $totalRight; - my $total; - my $criticalDate; - - if ($userSet->assignment_type() =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $studentName); - $criticalDate = earliest_gateway_date($db, $ce, $userSet) unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } else { - ($totalRight, $total) = grade_set($db, $userSet, $studentName); - $criticalDate = get_LTISendScoresAfterDate($userSet, $ce) unless ($ce->{LTISendScoresAfterDate} eq 'never'); - } + for my $setID (@setIDs) { + my $score = can_submit_LMS_score($db, $ce, $studentName, $setID); + my $totalRight = $score->{totalRight}; + my $total = $score->{total}; + my $criticalDate = $score->{criticalDate} // ''; if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { next if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' - && !set_attempted($db, $studentName, $userSet->set_id)); + && !set_attempted($db, $studentName, $setID)); next if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' && $total > 0 @@ -260,9 +248,21 @@ sub get_LTISendScoresAfterDate ($set, $ce) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold +# Returns a reference to hash with keys totalRight, total, score, criticalDate if the set has met +# either condition and undef if not. sub can_submit_LMS_score ($db, $ce, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); + my $totalRight; + my $total; + if ($userSet->assignment_type() =~ /gateway/) { + ($totalRight, $total) = grade_gateway($db, $setID, $userID); + } else { + ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; + } + my $score = $total > 1 ? $totalRight / $total : 1; + my $return = { totalRight => $totalRight, total => $total, score => $score }; + if ($ce->{LTISendScoresAfterDate} ne 'never') { my $critical_date; if ($userSet->assignment_type() =~ /gateway/) { @@ -270,18 +270,13 @@ sub can_submit_LMS_score ($db, $ce, $userID, $setID) { } else { $critical_date = get_LTISendScoresAfterDate($userSet, $ce); } - return 1 if after($critical_date); + $return->{critical_date} = $critical_date; + return $return if after($critical_date); } - return set_attempted($db, $userID, $setID) if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted'); - - my $score; - if ($userSet->assignment_type() =~ /gateway/) { - $score = grade_gateway($db, $setID, $userID); - } else { - $score = grade_set($db, $userSet, $userID); - } - return ($score >= $ce->{LTISendGradesEarlyThreshold}); + return $return + if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && set_attempted($db, $userID, $setID) + || $score >= $ce->{LTISendGradesEarlyThreshold}); } sub is_restricted ($db, $set, $studentName) { From f251b0405a85ac2bbd628e55ac2687546fc0e730 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 18 Nov 2024 14:58:23 -0800 Subject: [PATCH 14/25] feedback from PR#2617 --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 8 ++-- .../Authen/LTIAdvantage/SubmitGrade.pm | 24 ++++++----- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 2 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 10 +++-- lib/WeBWorK/Utils/Sets.pm | 43 +++++++++---------- 5 files changed, 45 insertions(+), 42 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 14eb40d6be..a68e591e1d 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -138,13 +138,13 @@ async sub submit_set_grade ($self, $userID, $setID) { my $db = $c->{db}; my $user = $db->getUser($userID); - return unless $user; - - my $score = can_submit_LMS_score($db, $ce, $userID, $setID); - return unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + return 0 unless $user; my $userSet = $db->getMergedSet($userID, $setID); + my $score = can_submit_LMS_score($db, $ce, $userID, $userSet); + return 0 unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + $self->warning("Submitting grade for user $userID and set $setID.") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; $self->warning('lis_source_did is not available for this set.') diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index fb3614e6a2..df91eb7656 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -217,13 +217,13 @@ async sub submit_set_grade ($self, $userID, $setID) { my $db = $c->{db}; my $user = $db->getUser($userID); - return unless $user; - - my $score = can_submit_LMS_score($db, $ce, $userID, $setID); - return unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + return 0 unless $user; my $userSet = $db->getMergedSet($userID, $setID); + my $score = can_submit_LMS_score($db, $ce, $userID, $userSet); + return 0 unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + $self->warning("Submitting grade for user $userID and set $setID."); $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; @@ -271,17 +271,19 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum return 0; } - my $priorData = decode_json($response->body); - my $priorScore = @$priorData - && $priorData->[0]{resultMaximum} ? $priorData->[0]{resultScore} / $priorData->[0]{resultMaximum} : 0; + my $priorData = decode_json($response->body); + my $priorScore = + (@$priorData && $priorData->[0]{resultMaximum} && defined $priorData->[0]{resultScore}) + ? $priorData->[0]{resultScore} / $priorData->[0]{resultMaximum} + : 0; my $score = $scoreMaximum ? $scoreGiven / $scoreMaximum : 0; - # we want to update the LMS score if the difference is significant, - # or if the new score is 1 but the LMS score was not 1 (but possibly insignificantly different) - # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate + # We want to update the LMS score if the difference is significant, + # or if the new score is 1 but the LMS score was not 1 but possibly insignificantly different, + # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate. if (abs($score - $priorScore) < 0.001 && ($score != 1 || $priorScore == 1) - && ($score != 0 || @$priorData && $priorData->[0]{resultScore} ne '' || $nullEqualsZero)) + && ($score != 0 || @$priorData && defined $priorData->[0]{resultScore} || $nullEqualsZero)) { $self->warning( "LMS grade will NOT be updated as the grade has not significantly changed. Old score: $priorScore, New score: $score." diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 5d5516f537..5e20bf4909 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1031,7 +1031,7 @@ async sub pre_header_initialize ($c) { && ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' || $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $effectiveUserID, $setID)) + && can_submit_LMS_score($db, $ce, $effectiveUserID, $db->getMergedSet($effectiveUserID, $setID))) ) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index dd6a8c752f..70c699fafb 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -253,10 +253,12 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeMode}) { my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; my $LTIGradeResult = -1; - if ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} - && $ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $problem->user_id, $problem->set_id)) + if ( + $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' + || $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'course' && can_submit_LMS_score( + $db, $ce, $problem->user_id, $db->getMergedSet($problem->user_id, $problem->set_id) + ) + ) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index dfc7da414c..84e2771cf3 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -147,18 +147,17 @@ sub grade_gateway ($db, $setName, $studentName) { } } -sub set_attempted ($db, $userID, $setID) { - my $userSet = $db->getMergedSet($userID, $setID); +sub set_attempted ($db, $userID, $userSet) { + my $setID = $userSet->set_id; if ($userSet->assignment_type() =~ /gateway/) { my @versionNums = $db->listSetVersions($userID, $setID); - # it counts as "attempted" if there is more than one version + # It counts as "attempted" if there is more than one version. return 1 if (1 < @versionNums); - # if there is one version, check for an attempted problem - # there could also be no actual attempted problems, but something like an - # achievement item has awarded credit for one exercise somewhere in the test + # If there is one version, check for an attempted problem. There could also + # be no actual attempted problems, but a score was manually overridden. if (@versionNums) { my @problemNums = $db->listUserProblems($userID, $setID); my $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $problemNums[0]); @@ -170,7 +169,7 @@ sub set_attempted ($db, $userID, $setID) { return 0; } - # if there are no versions + # If there are no versions, the test was not attempted. return 0; } else { my @problemNums = $db->listUserProblems($userID, $setID); @@ -185,27 +184,29 @@ sub set_attempted ($db, $userID, $setID) { sub earliest_gateway_date ($db, $ce, $userSet) { my @versionNums = $db->listSetVersions($userSet->user_id, $userSet->set_id); - # if there are no versions, use the template's date + # If there are no versions, use the template's date. return get_LTISendScoresAfterDate($userSet, $ce) unless (@versionNums); - # otherwise, use the earliest date among versions - my $earliest_date = - get_LTISendScoresAfterDate($db->getSetVersion($userSet->user_id, $userSet->set_id, $versionNums[0]), $ce); + # Otherwise, use the earliest date among versions. + my $earliest_date = -1; for my $i (@versionNums) { my $versionedSetDate = get_LTISendScoresAfterDate($db->getSetVersion($userSet->user_id, $userSet->set_id, $i), $ce); - $earliest_date = $versionedSetDate if ($versionedSetDate < $earliest_date); + $earliest_date = $versionedSetDate if $earliest_date == -1 || $versionedSetDate < $earliest_date; } return $earliest_date; } sub grade_all_sets ($db, $ce, $studentName) { - my @setIDs = $db->listUserSets($studentName); + my @setIDs = $db->listUserSets($studentName); + my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; + my @userSets = $db->getMergedSets(@userSetIDs); + my $courseTotalRight = 0; my $courseTotal = 0; - for my $setID (@setIDs) { - my $score = can_submit_LMS_score($db, $ce, $studentName, $setID); + for my $userSet (@userSets) { + my $score = can_submit_LMS_score($db, $ce, $studentName, $userSet); my $totalRight = $score->{totalRight}; my $total = $score->{total}; my $criticalDate = $score->{criticalDate} // ''; @@ -213,7 +214,7 @@ sub grade_all_sets ($db, $ce, $studentName) { if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { next if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' - && !set_attempted($db, $studentName, $setID)); + && !set_attempted($db, $studentName, $userSet)); next if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' && $total > 0 @@ -247,16 +248,14 @@ sub get_LTISendScoresAfterDate ($set, $ce) { } } -# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold +# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold. # Returns a reference to hash with keys totalRight, total, score, criticalDate if the set has met # either condition and undef if not. -sub can_submit_LMS_score ($db, $ce, $userID, $setID) { - my $userSet = $db->getMergedSet($userID, $setID); - +sub can_submit_LMS_score ($db, $ce, $userID, $userSet) { my $totalRight; my $total; if ($userSet->assignment_type() =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $setID, $userID); + ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $userID); } else { ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; } @@ -275,7 +274,7 @@ sub can_submit_LMS_score ($db, $ce, $userID, $setID) { } return $return - if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && set_attempted($db, $userID, $setID) + if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && set_attempted($db, $userID, $userSet) || $score >= $ce->{LTISendGradesEarlyThreshold}); } From 8f0955fb3f2ecbbced5ac1adb758b8159be00346 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 18 Nov 2024 20:01:09 -0800 Subject: [PATCH 15/25] more feedback from PR#2617 --- lib/WeBWorK/ConfigValues.pm | 4 ++-- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 2 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 10 ++++------ lib/WeBWorK/Utils/Sets.pm | 16 +++++----------- 4 files changed, 12 insertions(+), 20 deletions(-) diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 8c7f96f7fe..03c1b50ea3 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -960,7 +960,7 @@ sub getConfigValues ($ce) { }, LTISendScoresAfterDate => { var => 'LTISendScoresAfterDate', - doc => x('When to send scores to the LMS regardless of status'), + doc => x('Date after which scores will be sent to the LMS'), doc2 => x( '

    This can be set to one of the dates assciated with assignments. For a given assignment, if it is after ' . "the \$LTISendScoresAfterDate, then WeBWorK will send grades.

    • For 'course' grade " @@ -983,7 +983,7 @@ sub getConfigValues ($ce) { }, LTISendGradesEarlyThreshold => { var => 'LTISendGradesEarlyThreshold', - doc => x('Condition under which scores can be sent to an LMS early'), + doc => x('Condition under which scores will be sent early to an LMS'), doc2 => x( "

      This can either be set to a score or set to Attempted. When something triggers a potential grade " . 'passback, if it is earlier than $LTISendScoresAfterDate, the condition described by this ' diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 5e20bf4909..f604e69d6c 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1031,7 +1031,7 @@ async sub pre_header_initialize ($c) { && ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' || $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $effectiveUserID, $db->getMergedSet($effectiveUserID, $setID))) + && can_submit_LMS_score($db, $ce, $effectiveUserID, $c->{set})) ) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 70c699fafb..2556dbc6de 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -253,12 +253,10 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeMode}) { my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; my $LTIGradeResult = -1; - if ( - $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'course' && can_submit_LMS_score( - $db, $ce, $problem->user_id, $db->getMergedSet($problem->user_id, $problem->set_id) - ) - ) + if ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' + || $ce->{LTIGradeOnSubmit} + && $ce->{LTIGradeMode} eq 'course' + && can_submit_LMS_score($db, $ce, $problem->user_id, $c->{set})) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 84e2771cf3..5f1c812007 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -156,17 +156,11 @@ sub set_attempted ($db, $userID, $userSet) { # It counts as "attempted" if there is more than one version. return 1 if (1 < @versionNums); - # If there is one version, check for an attempted problem. There could also - # be no actual attempted problems, but a score was manually overridden. + # If there is one version, check for an attempted problem. if (@versionNums) { - my @problemNums = $db->listUserProblems($userID, $setID); + my @problemNums = $db->listProblemVersions($userID, $setID, $versionNums[0]); my $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $problemNums[0]); - return 1 if defined $problem && $problem->attempted; - for (@problemNums) { - $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $_); - return 1 if defined $problem && $problem->status > 0; - } - return 0; + return defined $problem && $problem->attempted ? 1 : 0; } # If there are no versions, the test was not attempted. @@ -259,7 +253,7 @@ sub can_submit_LMS_score ($db, $ce, $userID, $userSet) { } else { ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; } - my $score = $total > 1 ? $totalRight / $total : 1; + my $score = $total ? $totalRight / $total : 0; my $return = { totalRight => $totalRight, total => $total, score => $score }; if ($ce->{LTISendScoresAfterDate} ne 'never') { @@ -269,7 +263,7 @@ sub can_submit_LMS_score ($db, $ce, $userID, $userSet) { } else { $critical_date = get_LTISendScoresAfterDate($userSet, $ce); } - $return->{critical_date} = $critical_date; + $return->{criticalDate} = $critical_date; return $return if after($critical_date); } From 00c7234730e4b7f9606c909679cab7f036182a6a Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 18 Nov 2024 20:21:42 -0800 Subject: [PATCH 16/25] handling "0/0" scores for the LMS --- lib/WeBWorK/Utils/Sets.pm | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 5f1c812007..5c3185b95b 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -150,7 +150,7 @@ sub grade_gateway ($db, $setName, $studentName) { sub set_attempted ($db, $userID, $userSet) { my $setID = $userSet->set_id; - if ($userSet->assignment_type() =~ /gateway/) { + if ($userSet->assignment_type =~ /gateway/) { my @versionNums = $db->listSetVersions($userID, $setID); # It counts as "attempted" if there is more than one version. @@ -211,8 +211,7 @@ sub grade_all_sets ($db, $ce, $studentName) { && !set_attempted($db, $studentName, $userSet)); next if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' - && $total > 0 - && $totalRight / $total < $ce->{LTISendGradesEarlyThreshold}); + && $score->{score} < $ce->{LTISendGradesEarlyThreshold}); } $courseTotalRight += $totalRight; @@ -245,25 +244,35 @@ sub get_LTISendScoresAfterDate ($set, $ce) { # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold. # Returns a reference to hash with keys totalRight, total, score, criticalDate if the set has met # either condition and undef if not. +# If the set is a test, the score "0/0" is treated as 0. +# If the set is not a test, the score "0/0" is treated as 0 if we have not yet reached the +# LTISendScoresAfterDate and have not attempted the set. Otherwise, "0/0" is treated as 1. sub can_submit_LMS_score ($db, $ce, $userID, $userSet) { my $totalRight; my $total; - if ($userSet->assignment_type() =~ /gateway/) { + if ($userSet->assignment_type =~ /gateway/) { ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $userID); } else { ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; } + my $score = $total ? $totalRight / $total : 0; my $return = { totalRight => $totalRight, total => $total, score => $score }; if ($ce->{LTISendScoresAfterDate} ne 'never') { my $critical_date; - if ($userSet->assignment_type() =~ /gateway/) { + if ($userSet->assignment_type =~ /gateway/) { $critical_date = earliest_gateway_date($db, $ce, $userSet); } else { $critical_date = get_LTISendScoresAfterDate($userSet, $ce); } $return->{criticalDate} = $critical_date; + if ($total == 0) { + $return->{score} = ( + $userSet->assignment_type =~ /gateway/ || ($ce->{LTISendScoresAfterDate} eq 'never' + || before($critical_date) && !set_attempted($db, $userID, $userSet)) + ) ? 0 : 1; + } return $return if after($critical_date); } From 16bbf2bbcf05ff64cd5c5fa5cce6c09a58f5ca1a Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Wed, 20 Nov 2024 20:11:15 -0800 Subject: [PATCH 17/25] efficiency overhaul of grade passback controls, and other feedback from PR#2617 --- conf/authen_LTI.conf.dist | 81 ++++++------ lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 47 ++++--- .../Authen/LTIAdvantage/SubmitGrade.pm | 16 ++- lib/WeBWorK/ConfigValues.pm | 76 +++++------ lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 13 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 7 +- lib/WeBWorK/Utils/Sets.pm | 121 +++++++++++------- 7 files changed, 183 insertions(+), 178 deletions(-) diff --git a/conf/authen_LTI.conf.dist b/conf/authen_LTI.conf.dist index f7d57d6986..9e2ab3ee76 100644 --- a/conf/authen_LTI.conf.dist +++ b/conf/authen_LTI.conf.dist @@ -135,19 +135,19 @@ $LTIGradeMode = ''; #$LTIGradeMode = 'course'; #$LTIGradeMode = 'homework'; -# There are several controls for when to report grades to the LMS. Sometimes these controls -# interplay with each other, and the details of how they work may depend on whether -# $LTIGradeMode is set to 'course' or 'homework'. So it is recommended to understand all of -# them and then decide how to set them. - -# If $LTICheckPrior is 1, then any time WeBWorK is about to send a grade to the LMS, it will -# first request from the LMS what that grade currently is. Then if there is no significant -# difference between the LMS grade and the WeBWorK grade, WeBWorK will not follow through with -# updating the LMS grade. This is to avoid frequent insignificant updates to a student's grades -# in the LMS. With some LMSs, students may receive notifications each time a grade is updated, +# There are several controls for when to report scores to the LMS. Sometimes these controls +# interact with each other, and the details of how they work may depend on whether $LTIGradeMode +# is set to 'course' or 'homework'. So it is recommended to understand all of them and then +# decide how to set them. + +# If $LTICheckPrior is 1, then any time WeBWorK is about to send a score to the LMS, it will +# first request from the LMS what that score currently is. Then if there is no significant +# difference between the LMS score and the WeBWorK score, WeBWorK will not follow through with +# updating the LMS score. This is to avoid frequent insignificant updates to a student's scores +# in the LMS. With some LMSs, students may receive notifications each time a score is updated, # and setting this variable will prevent too many notifications for them. This does create a -# two-phase process, first querying the current grade from the LMS and then actually updating -# the grade (if there is a significant difference). +# two-phase process, first querying the current score from the LMS and then actually updating +# the score (if there is a significant difference). # Additional details: # - If the LMS score is not 100%, but the WeBWorK score is, then even if the LMS score is only @@ -156,51 +156,42 @@ $LTIGradeMode = ''; # difference and the LMS score will not be updated to 0. However if it is after the # $LTISendScoresAfterDate (described below), then the null score will be updated to 0 anyway. # - "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not -# configrable. +# configurable. $LTICheckPrior = 0; -# If $LTIGradeOnSubmit is set to 1, then each time a user submits an answer or grades a test, +# If $LTIGradeOnSubmit is set to 1, then each time a user submits an answer or scores a test, # that will trigger WeBWorK possibly reporting a score to the LMS. See $LTICheckPrior for one -# reason that WeBWorK might not ultimately send a grade. But there are other reasons too. -# WeBWorK will send the grade (the assignment's score if $LTIGradeMode is 'homework' or the -# overall course grade if $LTIGradeMode is 'course') to the LMS only if either the assignment's +# reason that WeBWorK might not ultimately send a score. But there are other reasons too. +# WeBWorK will send the score (the assignment's score if $LTIGradeMode is 'homework' or the +# overall course score if $LTIGradeMode is 'course') to the LMS only if either the assignment's # $LTISendGradesEarlyThreshold (described below) has been met or if it is past that assignment's # $LTISendScoresAfterDate (also described below). - -# If $LTIGradeOnSubmit is set to 'homework_always', then -# - If $LTIGradeMode is 'homework', then WeBWorK will send that assignment's score to the LMS -# regardless of having met the $LTISendGradesEarlyThreshold or being past the -# $LTISendScoresAfterDate -# - If $LTIGradeMode is 'course', the behavior will be no different as if $LTIGradeOnSubmit were -# set to 1. - -# If sending grades upon each submission is not desired, you can set $LTIGradeOnSubmit to 0. - $LTIGradeOnSubmit = 1; -#$LTIGradeOnSubmit = 'homework_always'; -#$LTIGradeOnSubmit = 0; # In addition to scores possibly being sent to the LMS upon submission, they can be sent by an # instructor or admin user using the LTI Grades Update Tool. And thirdly, the system can -# periodically update student grades on the LMS on its own. For all three possible triggers for -# grades to be passed to the LMS, $LTISendScoresAfterDate and $LTISendGradesEarlyThreshold can -# affect what is sent. $LTISendScoresAfterDate can be set to 'open_date', 'reduced_scoring_date', -# 'due_date', 'answer_date', or 'never'. For a given assignment, if it is *after* the -# $LTISendScoresAfterDate, then WeBWorK will send grades. +# periodically update student scores on the LMS on its own. For all three possible triggers for +# scores to be passed to the LMS, $LTISendScoresAfterDate and $LTISendGradesEarlyThreshold can +# affect what is sent. $LTISendScoresAfterDate can be 'open_date', 'reduced_scoring_date', +# 'due_date', 'answer_date', or 'never'. For a given assignment, if it is after the +# $LTISendScoresAfterDate, then WeBWorK will send scores. If $LTISendScoresAfterDate is 'never', +# then there is no date after which WeBWorK is guaranteed to send scores. In that case, scores +# are only sent when a set's $LTISendGradesEarlyThreshold is met (see below). # - For 'course' grade passback mode, the assignment will be included in the overall course # grade calculation. -# - For 'homework' grade passback mode, the assignemnt's score will be sent. +# - For 'homework' grade passback mode, the assignment's score will be sent. -# If $LTISendScoresAfterDate = 'reduced_scoring_date' and an assignent has no reduced_scoring_date -# or reduced scoring is disabled for that assignment, the fallback is to use the due_date. +# If $LTISendScoresAfterDate is 'reduced_scoring_date' and an assignment has no reduced scoring +# date or reduced scoring is disabled for that assignment, the fallback is to use the due date. -# For a given assignment, if it is *before* the $LTISendScoresAfterDate, WeBWorK *may* send a -# score to the LMS depending on $LTISendGradesEarlyThreshold. This variable can either be the -# string 'attempted' or a number from 0 to 1. If this variable is 'attempted', a given set must -# have been attempted for the threshold to have been met, and then the score can be used even if -# it is before the $LTISendScoresAfterDate. For a regular or jitar set, 'attempted' just means -# that some exercise in the set was attempted. For a test, 'attempted means that either there -# is one version with a graded submission, or there are at least two versions. +# For a given assignment, if $LTISendScoresAfterDate is 'never' or if it is before the date +# specified by $LTISendScoresAfterDate, WeBWorK may send a score to the LMS depending on the +# value of $LTISendGradesEarlyThreshold. This variable can either be the string 'attempted' or a +# number from 0 to 1. If this variable is 'attempted', a given set must have been attempted for +# the threshold to have been met, and then the score can be used even if it is before the +# $LTISendScoresAfterDate. For a non-test set, 'attempted' just means that some exercise in the +# set was attempted using the Submit button. For a test, 'attempted' means that either there is +# one version with a graded submission, or there are at least two versions. # If $LTISendGradesEarlyThreshold is a number from 0 to 1, the score for an assignment needs to # have reached that number for the threshold to be met, and then the score can be used even if @@ -217,7 +208,7 @@ $LTISendGradesEarlyThreshold = 'attempted'; #$LTISendGradesEarlyThreshold = 0.7; #$LTISendGradesEarlyThreshold = 1; -# The system periodically updates student grades on the LMS. If it has been at least this many +# The system periodically updates student scores on the LMS. If it has been at least this many # seconds since the last mass passback event and someone in the course does anything to load a # page, then a new mass passback job will begin. Set this to -1 to disable mass passback. $LTIMassUpdateInterval = 86400; diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index a68e591e1d..c16addae5c 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -120,6 +120,12 @@ async sub submit_course_grade ($self, $userID) { my $ce = $c->{ce}; my $db = $c->{db}; + # Before the costly act of calculating the course grade, if this LMS submission was intitated because + # of $LTIGradeOnSubmit, then check if the set from which a problem was submitted meets the criteria to + # be included in a course grade calculation. If not, we can skip the rest because the course grade will + # not differ from what it previously was. + return 0 unless ($self->{post_processing_mode} || can_submit_LMS_score($db, $ce, $userID, $c->{set}, 1)); + my $user = $db->getUser($userID); return 0 unless $user; @@ -142,20 +148,19 @@ async sub submit_set_grade ($self, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); - my $score = can_submit_LMS_score($db, $ce, $userID, $userSet); - return 0 unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + my $score = can_submit_LMS_score($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); + return 0 unless $score; $self->warning("Submitting grade for user $userID and set $setID.") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; $self->warning('lis_source_did is not available for this set.') if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); - return await $self->submit_grade($userSet->lis_source_did, $score->{score}, - $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always'); + return await $self->submit_grade($userSet->lis_source_did, $score->{score}); } # Submits a score of $score to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $sourcedid, $score, $nullEqualsZero = 1) { +async sub submit_grade ($self, $sourcedid, $score) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; @@ -284,35 +289,35 @@ EOS . $message); return 0; } else { - my $oldScore; + my $priorScore; # Possibly no score yet. if ($content =~ //) { - $oldScore = ''; + $priorScore = ''; } else { $content =~ /\s*(\S+)\s*<\/textString>/; - $oldScore = $1; + $priorScore = $1; } - # Do not update the score if no change. - if ($oldScore eq 'success') { - # Blackboard seems to return this when there is no prior grade. - # See: https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5002 - debug("LMS grade will be updated. sourcedid: $sourcedid; Old score: $oldScore; New score: $score") - if $ce->{debug_lti_grade_passback}; - } elsif ($oldScore ne '' - || abs($score - $oldScore) < 0.001 - && ($score != 1 || $oldScore ne '' && $oldScore == 1) - && ($score != 0 || $oldScore ne '' || $nullEqualsZero)) + # Blackboard seems to return this when there is no prior grade. + # See: https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5002 + $priorScore = '' if $priorScore eq 'success'; + + # Do not update the score if there is no significant change. Note that the cases where the webwork score + # is exactly 1 and the LMS score is not exactly 1, and the case where the webwork score is 0 and the LMS + # score is not set are considered significant changes. + if (abs($score - ($priorScore || 0)) < 0.001 + && ($score != 1 || $priorScore == 1) + && ($score != 0 || $priorScore ne '')) { # LMS has essentially the same score, no reason to update it debug( - "LMS grade will NOT be updated - grade has not significantly changed. Old score: $oldScore; New score: $score" + "LMS grade will NOT be updated - grade has not significantly changed. Old score: $priorScore; New score: $score" ) if $ce->{debug_lti_grade_passback}; $self->warning('LMS grade will NOT be updated - grade has not significantly changed. ' - . "Old score: $oldScore; New score: $score") + . "Old score: $priorScore; New score: $score") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; return 1; } else { - debug("LMS grade will be updated. sourcedid: $sourcedid; Old score: $oldScore; New score: $score") + debug("LMS grade will be updated. sourcedid: $sourcedid; Old score: $priorScore; New score: $score") if $ce->{debug_lti_grade_passback}; } } diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index df91eb7656..104e30c449 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -198,6 +198,12 @@ async sub submit_course_grade ($self, $userID) { my $ce = $c->{ce}; my $db = $c->{db}; + # Before the costly act of calculating the course grade, if this LMS submission was intitated because + # of $LTIGradeOnSubmit, then check if the set from which a problem was submitted meets the criteria to + # be included in a course grade calculation. If not, we can skip the rest because the course grade will + # not differ from what it previously was. + return 0 unless ($self->{post_processing_mode} || can_submit_LMS_score($db, $ce, $userID, $c->{set}, 1)); + my $user = $db->getUser($userID); return 0 unless $user; @@ -221,19 +227,19 @@ async sub submit_set_grade ($self, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); - my $score = can_submit_LMS_score($db, $ce, $userID, $userSet); - return 0 unless ($score || !$self->{post_processing_mode} && $ce->{LTIGradeOnSubmit} eq 'homework_always'); + my $score = can_submit_LMS_score($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); + return 0 unless $score; $self->warning("Submitting grade for user $userID and set $setID."); $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $score->{totalRight}, - $score->{total}, $self->{post_processing_mode} || $ce->{LTIGradeOnSubmit} ne 'homework_always'); + $score->{total}); } # Submits scoreGiven and scoreMaximum to the lms with $sourcedid as the identifier. -async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum, $nullEqualsZero = 1) { +async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum) { my $c = $self->{c}; my $ce = $c->{ce}; @@ -283,7 +289,7 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate. if (abs($score - $priorScore) < 0.001 && ($score != 1 || $priorScore == 1) - && ($score != 0 || @$priorData && defined $priorData->[0]{resultScore} || $nullEqualsZero)) + && ($score != 0 || (@$priorData && defined $priorData->[0]{resultScore}))) { $self->warning( "LMS grade will NOT be updated as the grade has not significantly changed. Old score: $priorScore, New score: $score." diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 03c1b50ea3..3e40e11792 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -916,20 +916,20 @@ sub getConfigValues ($ce) { var => 'LTICheckPrior', doc => x('Check a score in the LMS actually needs updating before updating it'), doc2 => x( - '

      When this is true, any time WeBWorK is about to send a grade to the LMS, it will first request ' - . 'from the LMS what that grade currently is. Then if there is no significant difference between ' - . 'the LMS grade and the WeBWorK grade, WeBWorK will not follow through with updating the LMS ' - . 'grade. This is to avoid frequent insignificant updates to a student grade in the LMS. With some ' - . 'LMSs, students may receive notifications each time a grade is updated, and setting this ' + '

      When this is true, any time WeBWorK is about to send a score to the LMS, it will first request ' + . 'from the LMS what that score currently is. Then if there is no significant difference between ' + . 'the LMS score and the WeBWorK score, WeBWorK will not follow through with updating the LMS ' + . 'score. This is to avoid frequent insignificant updates to a student score in the LMS. With some ' + . 'LMSs, students may receive notifications each time a score is updated, and setting this ' . 'variable will prevent too many notifications for them. This does create a two-step process, ' - . 'first querying the current grade from the LMS and then actually updating the grade (if there is ' + . 'first querying the current score from the LMS and then actually updating the score (if there is ' . 'a significant difference). Additional details:

      • If the LMS score is not 100%, but the ' . 'WeBWorK score is, then even if the LMS score is only insignificantly less than 100%, it will be ' . 'updated anyway.
      • If the LMS score is null and the WeBWorK score is 0, this is considered ' . 'an insignificant difference and the LMS score will not be updated to 0. However if it is after ' . 'the $LTISendScoresAfterDate (described below), then the null score will be updated to 0 anyway.' . '
      • "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not ' - . 'configrable.
      ' + . 'configurable.
    ' ), type => 'boolean' }, @@ -937,39 +937,31 @@ sub getConfigValues ($ce) { var => 'LTIGradeOnSubmit', doc => x('Update LMS Grade Each Submit'), doc2 => x( - '

    If this is set to Conditionally, then each time a user submits an answer or grades a test, that ' - . 'will trigger WeBWorK possibly reporting a score to the LMS. See $LTICheckPrior for one reason ' - . 'that WeBWorK might not ultimately send a grade. But there are other reasons too. WeBWorK will ' - . "send the grade (the assignment's score if \$LTIGradeMode is 'homework' or the overall course " - . "grade if \$LTIGradeMode is 'course') to the LMS only if either the assignment's " + 'If this is set to true, then each time a user submits an answer or grades a test, that will trigger ' + . 'WeBWorK possibly reporting a score to the LMS. See $LTICheckPrior for one reason that WeBWorK ' + . 'might not ultimately send a score. But there are other reasons too. WeBWorK will send the score ' + . "(the assignment's score if \$LTIGradeMode is 'homework' or the overall course score if " + . "\$LTIGradeMode is 'course') to the LMS only if either the assignment's " . "\$LTISendGradesEarlyThreshold has been met or if it is past that assignment's " - . '$LTISendScoresAfterDate.

    If $LTIGradeOnSubmit is set to Always, then:

    • If ' - . "\$LTIGradeMode is 'homework', then WeBWorK will send that assignment's score to the LMS " - . 'regardless of having met the $LTISendGradesEarlyThreshold or being past the ' - . "\$LTISendScoresAfterDate.
    • If \$LTIGradeMode is 'course', the behavior will be no " - . 'different as if $LTIGradeOnSubmit were set to Conditionally.

    If sending grades upon ' - . 'each submission is not desired, you can set $LTIGradeOnSubmit to 0.

    ' + . '$LTISendScoresAfterDate.' ), - values => [qw(0 1 homework_always)], - labels => { - 0 => x('Never'), - 1 => x('Conditionally'), - homework_always => x('Always') - }, - type => 'popuplist' + type => 'boolean' }, LTISendScoresAfterDate => { var => 'LTISendScoresAfterDate', doc => x('Date after which scores will be sent to the LMS'), doc2 => x( - '

    This can be set to one of the dates assciated with assignments. For a given assignment, if it is after ' - . "the \$LTISendScoresAfterDate, then WeBWorK will send grades.

    • For 'course' grade " - . 'passback mode, the assignment will be included in the overall course grade calculation.
    • ' - . "For 'homework' grade passback mode, the assignment's score will be sent.

    If " - . '$LTISendScoresAfterDate is set to "After the reduced scoring date" and an assignent has no ' - . 'reduced scoring date or reduced scoring is disabled for that assignment, the fallback is to use ' - . 'the close date.

    For a given assignment, if it is before the $LTISendScoresAfterDate, ' - . 'WeBWorK will still send a score to the LMS if the $LTISendGradesEarlyThreshold has been met.

    ' + '

    This can be set to one of the dates associated with assignments, or "Never". For each assignment, ' + . 'if this setting is "After the ... " then if it is after the indicated date, WeBWorK will send ' + . 'scores. If this setting is "Never" then there is no date that will force WeBWorK to send scores ' + . 'and only the $LTISendGradesEarlyThreshold can cause scores to be sent. If scores are sent:

    ' + . "
    • For 'course' grade passback mode, the assignment will be included in the overall course " + . "score calculation.
    • For 'homework' grade passback mode, the assignment's score itself " + . 'will be sent.

    If $LTISendScoresAfterDate is set to "After the reduced scoring date" ' + . 'and an assignment has no reduced scoring date or reduced scoring is disabled, the fallback is ' + . 'to use the close date.

    For a given assignment, WeBWorK will still send a score to the LMS ' + . 'if the $LTISendGradesEarlyThreshold has been met, regardless of how $LTISendScoresAfterDate is ' + . 'set.

    ' ), values => [qw(open_date reduced_scoring_date due_date answer_date never)], labels => { @@ -987,15 +979,13 @@ sub getConfigValues ($ce) { doc2 => x( "

    This can either be set to a score or set to Attempted. When something triggers a potential grade " . 'passback, if it is earlier than $LTISendScoresAfterDate, the condition described by this ' - . 'variable must be met or else no grade will be sent. Although if $LTIGradeOnSubmit is set to ' - . "'homework_always', then users submitting an answer or grading a test will still cause there to " - . 'be a grade passback regardless of this setting.

    If this variable is a score, then the set ' - . 'will need to have a score that reaches or exceeds this score for its score to be sent to the ' - . "LMS (or included in the 'course' grade calcuation). If this variable is set to Attempted, then " - . 'the set needs to have been attempted for its score to be sent to the LMS (or included in the ' - . "'course' grade calcuation).

    For a regular or jitar set, 'attempted' means that at least one " - . "exercise was attempted. For a test, 'attempted' means that either multiple versions exist or " - . 'there is one version with a graded submission.

    ' + . 'variable must be met or else no score will be sent.

    If this variable is a score, then the ' + . 'set will need to have a score that reaches or exceeds this score for its score to be sent to ' + . "the LMS (or included in the 'course' score calculation). If this variable is set to Attempted, " + . 'then the set needs to have been attempted for its score to be sent to the LMS (or included in ' + . "the 'course' score calculation).

    For a regular or jitar set, 'attempted' means that at " + . "least one exercise was attempted. For a test, 'attempted' means that either multiple versions " + . 'exercise or there is one version with a graded submission.

    ' ), values => [qw(attempted 0 0.5 0.7 0.75 0.8 0.85 0.9 0.95 1)], labels => { @@ -1016,7 +1006,7 @@ sub getConfigValues ($ce) { var => 'LTIMassUpdateInterval', doc => x('Time in seconds to periodically update LMS grades (-1 to disable)'), doc2 => x( - 'Sets the time in seconds to periodically update the LMS grades. WeBWorK will update all grades on ' + 'Sets the time in seconds to periodically update the LMS scores. WeBWorK will update all scores on ' . 'the LMS if it has been longer than this time since the completion of the last update. This is ' . 'only an approximate time. Mass updates of this nature may put significant strain on the server, ' . 'and should not be set to happen too frequently. -1 disables these periodic updates.' diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index f604e69d6c..3f89af5ff5 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -33,7 +33,7 @@ use WeBWorK::Utils::Instructor qw(assignSetVersionToUser); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); use WeBWorK::Utils::ProblemProcessing qw/create_ans_str_from_responses compute_reduced_score/; use WeBWorK::Utils::Rendering qw(getTranslatorDebuggingOptions renderPG); -use WeBWorK::Utils::Sets qw(is_restricted can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(is_restricted); use WeBWorK::DB::Utils qw(global2user fake_set fake_set_version fake_problem); use WeBWorK::Debug; use WeBWorK::Authen::LTIAdvanced::SubmitGrade; @@ -1024,16 +1024,7 @@ async sub pre_header_initialize ($c) { } # Try to update the student score on the LMS if that option is enabled. - if ( - $c->{submitAnswers} - && $will{recordAnswers} - && $ce->{LTIGradeMode} - && ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} - && $ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $effectiveUserID, $c->{set})) - ) - { + if ($c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} && $ce->{LTIGradeOnSubmit}) { my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); if ($ce->{LTIGradeMode} eq 'course') { $LTIGradeResult = await $grader->submit_course_grade($effectiveUserID); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 2556dbc6de..b2755a5ed9 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -33,7 +33,6 @@ use WeBWorK::Utils qw(encodeAnswers createEmailSenderTransportSMTP); use WeBWorK::Utils::DateTime qw(before after); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq jitar_problem_adjusted_status); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); -use WeBWorK::Utils::Sets qw(can_submit_LMS_score); use WeBWorK::Authen::LTIAdvanced::SubmitGrade; use WeBWorK::Authen::LTIAdvantage::SubmitGrade; use Caliper::Sensor; @@ -253,11 +252,7 @@ async sub process_and_log_answer ($c) { if ($ce->{LTIGradeMode}) { my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; my $LTIGradeResult = -1; - if ($ce->{LTIGradeOnSubmit} && $ce->{LTIGradeMode} eq 'homework' - || $ce->{LTIGradeOnSubmit} - && $ce->{LTIGradeMode} eq 'course' - && can_submit_LMS_score($db, $ce, $problem->user_id, $c->{set})) - { + if ($ce->{LTIGradeOnSubmit}) { $LTIGradeResult = 0; my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); if ($ce->{LTIGradeMode} eq 'course') { diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 5c3185b95b..027356a025 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -147,6 +147,12 @@ sub grade_gateway ($db, $setName, $studentName) { } } +sub grade_set_or_gateway ($db, $userSet, $userID) { + return ($userSet->assignment_type =~ /gateway/) + ? grade_gateway($db, $userSet->set_id, $userID) + : (grade_set($db, $userSet, $userID))[ 0, 1 ]; +} + sub set_attempted ($db, $userID, $userSet) { my $setID = $userSet->set_id; @@ -200,22 +206,10 @@ sub grade_all_sets ($db, $ce, $studentName) { my $courseTotal = 0; for my $userSet (@userSets) { - my $score = can_submit_LMS_score($db, $ce, $studentName, $userSet); - my $totalRight = $score->{totalRight}; - my $total = $score->{total}; - my $criticalDate = $score->{criticalDate} // ''; - - if ($ce->{LTISendScoresAfterDate} eq 'never' || $criticalDate && before($criticalDate)) { - next - if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' - && !set_attempted($db, $studentName, $userSet)); - next - if ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' - && $score->{score} < $ce->{LTISendGradesEarlyThreshold}); - } - - $courseTotalRight += $totalRight; - $courseTotal += $total; + my $score = can_submit_LMS_score($db, $ce, $studentName, $userSet); + next unless $score; + $courseTotalRight += $score->{totalRight}; + $courseTotal += $score->{total}; } if (wantarray) { @@ -242,43 +236,76 @@ sub get_LTISendScoresAfterDate ($set, $ce) { } # Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold. -# Returns a reference to hash with keys totalRight, total, score, criticalDate if the set has met -# either condition and undef if not. -# If the set is a test, the score "0/0" is treated as 0. -# If the set is not a test, the score "0/0" is treated as 0 if we have not yet reached the -# LTISendScoresAfterDate and have not attempted the set. Otherwise, "0/0" is treated as 1. -sub can_submit_LMS_score ($db, $ce, $userID, $userSet) { +# Returns a reference to hash with keys totalRight, total, score, if the set has met either condition +# and undef if not. If the score is "0/0", that is treated as 0 if we have not yet reached the +# LTISendScoresAfterDate and have not attempted the set. Or if the set is a test with no completed +# version, that is also treated as 0. Otherwise, "0/0" is treated as 1. +sub can_submit_LMS_score ($db, $ce, $userID, $userSet, $set_attempted = 0) { my $totalRight; my $total; - if ($userSet->assignment_type =~ /gateway/) { - ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $userID); - } else { - ($totalRight, $total) = (grade_set($db, $userSet, $userID))[ 0, 1 ]; - } - - my $score = $total ? $totalRight / $total : 0; - my $return = { totalRight => $totalRight, total => $total, score => $score }; - - if ($ce->{LTISendScoresAfterDate} ne 'never') { - my $critical_date; - if ($userSet->assignment_type =~ /gateway/) { - $critical_date = earliest_gateway_date($db, $ce, $userSet); + my $score; + my $critical_date; + + # Complicated logic below is intended to exit this subroutine with minimal computational cost. + if ($ce->{LTISendScoresAfterDate} eq 'never') { + if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted') { + $set_attempted = $set_attempted || set_attempted($db, $userID, $userSet); + return unless $set_attempted; + ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); + $score = $total ? $totalRight / $total : 0; + $score = 1 if (!$total && $set_attempted); + return { totalRight => $totalRight, total => $total, score => $score }; } else { - $critical_date = get_LTISendScoresAfterDate($userSet, $ce); + ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); + $score = $total ? $totalRight / $total : 0; + $score = 1 if (!$total && $set_attempted); + return { totalRight => $totalRight, total => $total, score => $score } + if ($score >= $ce->{LTISendGradesEarlyThreshold}); } - $return->{criticalDate} = $critical_date; - if ($total == 0) { - $return->{score} = ( - $userSet->assignment_type =~ /gateway/ || ($ce->{LTISendScoresAfterDate} eq 'never' - || before($critical_date) && !set_attempted($db, $userID, $userSet)) - ) ? 0 : 1; + } elsif ($ce->{LTISendGradesEarlyThreshold} eq 'attempted') { + # We may have been sent $set_attempted = 1, and immediately know we can assess score and send it. + if ($set_attempted) { + ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); + $score = $total ? $totalRight / $total : 0; + $score = 1 unless $total; + return { totalRight => $totalRight, total => $total, score => $score }; + } else { + # Next cheapest assessment is assessing whether or not we have passed the critical date + # But if we are not, we must actually assess if set was attempted + $critical_date = + ($userSet->assignment_type =~ /gateway/) + ? earliest_gateway_date($db, $ce, $userSet) + : get_LTISendScoresAfterDate($userSet, $ce); + if (after($critical_date) || ($set_attempted = set_attempted($db, $userID, $userSet))) { + ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); + $score = $total ? $totalRight / $total : 0; + $score = 1 + if (!$total && ($userSet->assignment_type !~ /gateway/ && after($critical_date) || $set_attempted)); + return { totalRight => $totalRight, total => $total, score => $score }; + } } - return $return if after($critical_date); + } else { + # No matter what, we need to know score now + ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); + $score = $total ? $totalRight / $total : 0; + if (!$total) { + $critical_date = + ($userSet->assignment_type =~ /gateway/) + ? earliest_gateway_date($db, $ce, $userSet) + : get_LTISendScoresAfterDate($userSet, $ce); + $set_attempted = $set_attempted || set_attempted($db, $userID, $userSet); + $score = 1 if ($userSet->assignment_type !~ /gateway/ && after($critical_date) || $set_attempted); + } + return { totalRight => $totalRight, total => $total, score => $score } + if ($score >= $ce->{LTISendGradesEarlyThreshold}); + $critical_date = + ($userSet->assignment_type =~ /gateway/) + ? earliest_gateway_date($db, $ce, $userSet) + : get_LTISendScoresAfterDate($userSet, $ce) + unless $critical_date; + return { totalRight => $totalRight, total => $total, score => $score } + if after($critical_date); } - - return $return - if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && set_attempted($db, $userID, $userSet) - || $score >= $ce->{LTISendGradesEarlyThreshold}); } sub is_restricted ($db, $set, $studentName) { From 8f50372d1ba91b1d8edc4e4c398520cd15592518 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 18 Nov 2024 19:47:23 -0600 Subject: [PATCH 18/25] Rework of LTI grade pass back. Restructure LTI grade pass back so that there are no additional database queries with the new restrictions on when grades are passed back to the LMS. With this restructuring the only database queries are made while grading the sets the same as before. The grade_set and grade_gateway methods return the things they pull from the database so that the database records can be culled for the additional information needed to determine if grade pass back should occur. The usage of these methods elsewhere is the same. The new data that is passed back will simply be ignored in those cases. The `lib/WeBWorK/Authen/LTI/MassUpdate.pm` file has been renamed to `lib/WeBWorK/Authen/LTI/GradePassback.pm` and contains the previous `mass_update` method as well as all methods needed for the new processing. The `grade_all_sets` method that was in `lib/WeBWorK/Utils/Sets.pm` now takes `$getSetGradeConditionally` as an optional last argument. That argument should be a reference to a subroutine that takes the arguments $db, $ce, $studentName, and $userSet, and either returns a reference to a hash containing the keys totalRight and total with the grade for the set, or returns `undef`. If it returns `undef` then the set will not be included in the grade computation. Otherwise the totalRight and total will be added into the grade for all sets. If the optional last arugment is not provided, then a default method will be used that does the same thing as before. Note that the `$LTI{v1p1}{grader}` option the `authen_LTI_1_1.conf.dist` and `authen_LTI_1_3.conf.dist` files has been removed. Instead the correct module is determined directly from the `$LTIVersion` as was previously done in the `lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm` file. This way a system adiminstrator cannot accidentally delete that or modify it in some way and mess things up. Since this is only done in these two places and not needed elsewhere the convenience of the course environment variable containing the module name is not needed. --- conf/authen_LTI_1_1.conf.dist | 3 - conf/authen_LTI_1_3.conf.dist | 3 - lib/WeBWorK/Authen/LTI/GradePassback.pm | 185 ++++++++++++++ lib/WeBWorK/Authen/LTI/MassUpdate.pm | 71 ------ lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 71 +++--- .../Authen/LTIAdvantage/SubmitGrade.pm | 63 +++-- lib/WeBWorK/ConfigValues.pm | 18 +- lib/WeBWorK/ContentGenerator.pm | 4 +- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 21 +- .../ContentGenerator/Instructor/LTIUpdate.pm | 5 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 37 +-- lib/WeBWorK/Utils/Sets.pm | 232 ++++-------------- .../ContentGenerator/GatewayQuiz.html.ep | 6 +- 13 files changed, 338 insertions(+), 381 deletions(-) create mode 100644 lib/WeBWorK/Authen/LTI/GradePassback.pm delete mode 100644 lib/WeBWorK/Authen/LTI/MassUpdate.pm diff --git a/conf/authen_LTI_1_1.conf.dist b/conf/authen_LTI_1_1.conf.dist index e260eae69d..e6daaaf385 100644 --- a/conf/authen_LTI_1_1.conf.dist +++ b/conf/authen_LTI_1_1.conf.dist @@ -227,7 +227,4 @@ $LTI{v1p1}{LMSrolesToWeBWorKroles} = { # $userSet->answer_date($niceAnswerTime); #}; -# Do not change this. -$LTI{v1p1}{grader} = 'WeBWorK::Authen::LTIAdvanced::SubmitGrade'; - 1; # final line of the file to reassure perl that it was read properly. diff --git a/conf/authen_LTI_1_3.conf.dist b/conf/authen_LTI_1_3.conf.dist index 60e2cf0b31..b3fab6ec46 100644 --- a/conf/authen_LTI_1_3.conf.dist +++ b/conf/authen_LTI_1_3.conf.dist @@ -189,7 +189,4 @@ $LTI{v1p3}{LMSrolesToWeBWorKroles} = { # $userSet->answer_date($niceAnswerTime); #}; -# Do not change this. -$LTI{v1p3}{grader} = 'WeBWorK::Authen::LTIAdvantage::SubmitGrade'; - 1; # final line of the file to reassure perl that it was read properly. diff --git a/lib/WeBWorK/Authen/LTI/GradePassback.pm b/lib/WeBWorK/Authen/LTI/GradePassback.pm new file mode 100644 index 0000000000..9ee61b50c0 --- /dev/null +++ b/lib/WeBWorK/Authen/LTI/GradePassback.pm @@ -0,0 +1,185 @@ +############################################################################### +# WeBWorK Online Homework Delivery System +# Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork +# +# This program is free software; you can redistribute it and/or modify it under +# the terms of either: (a) the GNU General Public License as published by the +# Free Software Foundation; either version 2, or (at your option) any later +# version, or (b) the "Artistic License" which comes with this package. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the +# Artistic License for more details. +################################################################################ + +package WeBWorK::Authen::LTI::GradePassback; +use Mojo::Base 'Exporter', -signatures, -async_await; + +=head1 NAME + +WeBWorK::Authen::LTI::GradePassback - Grade passback utilities for LTI authentication + +=cut + +use WeBWorK::Utils::DateTime qw(after before); +use WeBWorK::Utils::Sets qw(grade_set grade_gateway); + +our @EXPORT_OK = qw(massUpdate passbackGradeOnSubmit getSetPassbackScore); + +# These must be required and not used, and must be after the exports are defined above. +# Otherwise this will create a circular dependency with the SubmitGrade modules. +require WeBWorK::Authen::LTIAdvanced::SubmitGrade; +require WeBWorK::Authen::LTIAdvantage::SubmitGrade; + +# Perform a mass update of all grades. This is all user grades for course grade mode and all user set grades for +# homework grade mode if $manual_update is false. Otherwise what is updated is determined by a combination of the grade +# mode and the useriD and setID parameters. Note that the only required parameter is $c which should be a +# WeBWorK::Controller object with a valid course environment and database. +sub massUpdate ($c, $manual_update = 0, $userID = undef, $setID = undef) { + my $ce = $c->ce; + my $db = $c->db; + + # Sanity check. + unless (ref($ce)) { + warn('course environment is not defined'); + return; + } + unless (ref($db)) { + warn('database reference is not defined'); + return; + } + + # Only run an automatic update if the time interval has passed. + if (!$manual_update) { + my $lastUpdate = $db->getSettingValue('LTILastUpdate') || 0; + my $updateInterval = $ce->{LTIMassUpdateInterval} // -1; + return unless ($updateInterval != -1 && time - $lastUpdate > $updateInterval); + $db->setSettingValue('LTILastUpdate', time); + } + + # Send warning if debug_lti_grade_passback is set. + if ($ce->{debug_lti_grade_passback}) { + if ($setID && $userID && $ce->{LTIGradeMode} eq 'homework') { + warn "LTI Mass Update: Queueing grade update for user $userID and set $setID.\n"; + } elsif ($setID && $ce->{LTIGradeMode} eq 'homework') { + warn "LTI Mass Update: Queueing grade update for all users assigned to set $setID.\n"; + } elsif ($userID) { + warn "LTI Mass Update: Queueing grade update of all sets assigned to user $userID.\n"; + } else { + warn "LTI Mass Update: Queueing grade update for all sets and users.\n"; + } + } + + $c->minion->enqueue(lti_mass_update => [ $userID, $setID ], { notes => { courseID => $ce->{courseName} } }); + + return; +} + +async sub passbackGradeOnSubmit ($c, $userID, $set) { + my $ce = $c->ce; + + my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; + + if ($ce->{LTIGradeOnSubmit}) { + my $LTIGradeResult = 0; + + my $grader = + $ce->{LTIVersion} eq 'v1p1' + ? WeBWorK::Authen::LTIAdvanced::SubmitGrade->new($c) + : WeBWorK::Authen::LTIAdvantage::SubmitGrade->new($c); + + if ($ce->{LTIGradeMode} eq 'course') { + $LTIGradeResult = await $grader->submit_course_grade($userID, $set); + } elsif ($ce->{LTIGradeMode} eq 'homework') { + $LTIGradeResult = await $grader->submit_set_grade($userID, $set->set_id, $set); + } + if ($LTIGradeResult == 0) { + return $c->maketext('Your score was not successfully sent to [_1].', $LMSname); + } elsif ($LTIGradeResult > 0) { + return $c->maketext('Your score was successfully sent to [_1].', $LMSname); + } elsif ($LTIGradeResult < 0) { + return $c->maketext('Your score will be sent to [_1] at a later time.', $LMSname); + } + } elsif ($ce->{LTIMassUpdateInterval} > 0) { + if ($ce->{LTIMassUpdateInterval} < 120) { + return $c->maketext('Scores are sent to [_1] every [quant,_2,second].', + $LMSname, $ce->{LTIMassUpdateInterval}); + } elsif ($ce->{LTIMassUpdateInterval} < 7200) { + return $c->maketext('Scores are sent to [_1] every [quant,_2,minute].', + $LMSname, int($ce->{LTIMassUpdateInterval} / 60 + 0.99)); + } else { + return $c->maketext('Scores are sent to [_1] every [quant,_2,hour].', + $LMSname, int($ce->{LTIMassUpdateInterval} / 3600 + 0.9999)); + } + } +} + +sub setAttempted ($problems, $setVersions = undef) { + return 0 unless ref($problems) eq 'ARRAY'; + + # If this is a test with set versions, then it counts as "attempted" if there is more than one set version. + return 1 if ref($setVersions) eq 'ARRAY' && @$setVersions > 1; + + for (@$problems) { + return 1 if $_->attempted || $_->status > 0; + } + return 0; +} + +sub earliestGatewayDate ($ce, $userSet, $setVersions) { + # If there are no versions, use the template's date. + return getLTISendScoresAfterDate($userSet, $ce) unless ref($setVersions) eq 'ARRAY'; + + # Otherwise, use the earliest date among versions. + my $earliest_date = -1; + for (@$setVersions) { + my $versionedSetDate = getLTISendScoresAfterDate($_, $ce); + $earliest_date = $versionedSetDate if $earliest_date == -1 || $versionedSetDate < $earliest_date; + } + return $earliest_date; +} + +sub getLTISendScoresAfterDate ($set, $ce) { + if ($ce->{LTISendScoresAfterDate} eq 'open_date') { + return $set->open_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'reduced_scoring_date') { + return ($ce->{pg}{ansEvalDefaults}{enableReducedScoring} + && $set->enable_reduced_scoring + && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'due_date') { + return $set->due_date; + } elsif ($ce->{LTISendScoresAfterDate} eq 'answer_date') { + return $set->answer_date; + } +} + +# Returns a reference to hash with the keys totalRight, total, and score if the +# set has met the conditions for grade pass back to occur, and undef otherwise. +sub getSetPassbackScore ($db, $ce, $userID, $userSet, $gradingSubmission = 0) { + my ($totalRight, $total, $problemRecords, $setVersions) = + $userSet->assignment_type =~ /gateway/ + ? grade_gateway($db, $userSet->set_id, $userID) + : grade_set($db, $userSet, $userID); + + my $return = { totalRight => $totalRight, total => $total, score => $total ? $totalRight / $total : 0 }; + + return $return if $gradingSubmission && $ce->{LTISendGradesEarlyThreshold} eq 'attempted'; + + my $criticalDate = + $ce->{LTISendScoresAfterDate} ne 'never' + ? ($userSet->assignment_type =~ /gateway/ + ? earliestGatewayDate($ce, $userSet, $setVersions) + : getLTISendScoresAfterDate($userSet, $ce)) + : undef; + + return $return + if ($criticalDate && after($criticalDate)) + || ($ce->{LTISendGradesEarlyThreshold} eq 'attempted' && setAttempted($problemRecords, $setVersions)) + || ($ce->{LTISendGradesEarlyThreshold} ne 'attempted' + && $return->{score} >= $ce->{LTISendGradesEarlyThreshold}); + + return; +} + +1; diff --git a/lib/WeBWorK/Authen/LTI/MassUpdate.pm b/lib/WeBWorK/Authen/LTI/MassUpdate.pm deleted file mode 100644 index 103498a765..0000000000 --- a/lib/WeBWorK/Authen/LTI/MassUpdate.pm +++ /dev/null @@ -1,71 +0,0 @@ -############################################################################### -# WeBWorK Online Homework Delivery System -# Copyright © 2000-2024 The WeBWorK Project, https://github.com/openwebwork -# -# This program is free software; you can redistribute it and/or modify it under -# the terms of either: (a) the GNU General Public License as published by the -# Free Software Foundation; either version 2, or (at your option) any later -# version, or (b) the "Artistic License" which comes with this package. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See either the GNU General Public License or the -# Artistic License for more details. -################################################################################ - -package WeBWorK::Authen::LTI::MassUpdate; -use Mojo::Base 'Exporter', -signatures; - -=head1 NAME - -WeBWorK::Authen::LTI::MassUpdate - Mass update grades to the LMS with LTI authentication - -=cut - -our @EXPORT_OK = qw(mass_update); - -# Perform a mass update of all grades. This is all user grades for course grade mode and all user set grades for -# homework grade mode if $manual_update is false. Otherwise what is updated is determined by a combination of the grade -# mode and the useriD and setID parameters. Note that the only required parameter is $c which should be a -# WeBWorK::Controller object with a valid course environment and database. -sub mass_update ($c, $manual_update = 0, $userID = undef, $setID = undef) { - my $ce = $c->ce; - my $db = $c->db; - - # Sanity check. - unless (ref($ce)) { - warn('course environment is not defined'); - return; - } - unless (ref($db)) { - warn('database reference is not defined'); - return; - } - - # Only run an automatic update if the time interval has passed. - if (!$manual_update) { - my $lastUpdate = $db->getSettingValue('LTILastUpdate') || 0; - my $updateInterval = $ce->{LTIMassUpdateInterval} // -1; - return unless ($updateInterval != -1 && time - $lastUpdate > $updateInterval); - $db->setSettingValue('LTILastUpdate', time); - } - - # Send warning if debug_lti_grade_passback is set. - if ($ce->{debug_lti_grade_passback}) { - if ($setID && $userID && $ce->{LTIGradeMode} eq 'homework') { - warn "LTI Mass Update: Queueing grade update for user $userID and set $setID.\n"; - } elsif ($setID && $ce->{LTIGradeMode} eq 'homework') { - warn "LTI Mass Update: Queueing grade update for all users assigned to set $setID.\n"; - } elsif ($userID) { - warn "LTI Mass Update: Queueing grade update of all sets assigned to user $userID.\n"; - } else { - warn "LTI Mass Update: Queueing grade update for all sets and users.\n"; - } - } - - $c->minion->enqueue(lti_mass_update => [ $userID, $setID ], { notes => { courseID => $ce->{courseName} } }); - - return; -} - -1; diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index c16addae5c..63ebc49626 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -30,7 +30,8 @@ use Digest::SHA qw(sha1_base64); use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_all_sets); +use WeBWorK::Authen::LTI::GradePassback qw(getSetPassbackScore); # This package contains utilities for submitting grades to the LMS sub new ($invocant, $c, $post_processing_mode = 0) { @@ -115,30 +116,30 @@ sub update_sourcedid ($self, $userID) { # Computes and submits the course grade for userID to the LMS. # The course grade is the average of all sets assigned to the user. -async sub submit_course_grade ($self, $userID) { +async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; - # Before the costly act of calculating the course grade, if this LMS submission was intitated because - # of $LTIGradeOnSubmit, then check if the set from which a problem was submitted meets the criteria to - # be included in a course grade calculation. If not, we can skip the rest because the course grade will - # not differ from what it previously was. - return 0 unless ($self->{post_processing_mode} || can_submit_LMS_score($db, $ce, $userID, $c->{set}, 1)); - my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("submitting all grades for user: $userID") + $self->warning("submitting course grade for user: $userID") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; - $self->warning("lis_source_did is not available for user: $userID") - if !$user->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); + unless ($user->lis_source_did) { + $self->warning("lis_source_did is not available for user: $userID") + if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; + return 0; + } + + return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); - return await $self->submit_grade($user->lis_source_did, scalar(grade_all_sets($db, $ce, $userID))); + return await $self->submit_grade($user->lis_source_did, + scalar(grade_all_sets($db, $ce, $userID, \&getSetPassbackScore))); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. -async sub submit_set_grade ($self, $userID, $setID) { +async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; @@ -146,15 +147,18 @@ async sub submit_set_grade ($self, $userID, $setID) { my $user = $db->getUser($userID); return 0 unless $user; - my $userSet = $db->getMergedSet($userID, $setID); - - my $score = can_submit_LMS_score($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); - return 0 unless $score; - $self->warning("Submitting grade for user $userID and set $setID.") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; - $self->warning('lis_source_did is not available for this set.') - if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); + + my $userSet = $submittedSet // $db->getMergedSet($userID, $setID); + unless ($userSet->lis_source_did) { + $self->warning('lis_source_did is not available for this set.') + if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; + return 0; + } + + my $score = getSetPassbackScore($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); + return -1 unless $score; return await $self->submit_grade($userSet->lis_source_did, $score->{score}); } @@ -167,9 +171,6 @@ async sub submit_grade ($self, $sourcedid, $score) { $score = wwRound(2, $score); - # Fail gracefully. Some users, like instructors, may not actually have a sourcedid. - return 0 if !$sourcedid; - my $request_url = $db->getSettingValue('lis_outcome_service_url'); if (!$request_url) { $self->warning('Cannot send/retrieve grades to/from the LMS, no lis_outcome_service_url'); @@ -280,13 +281,8 @@ EOS $content =~ /\s*(\w+)\s*<\/imsx_codeMajor>/; my $message = $1; if ($message ne 'success') { - $self->warning( - 'Unable to retrieve prior grade from LMS. Note that if your server time is not correct, ' - . 'this may fail for reasons which are less than obvious from the error messages. Error: ' - . $message); - debug('Unable to retrieve prior grade from LMS. Note that if your server time is not correct, ' - . 'this may fail for reasons which are less than obvious from the error messages. Error: ' - . $message); + $self->warning('Unable to retrieve prior grade from LMS. Error: ' . $message); + debug('Unable to retrieve prior grade from LMS. Error: ' . $message); return 0; } else { my $priorScore; @@ -297,6 +293,7 @@ EOS $content =~ /\s*(\S+)\s*<\/textString>/; $priorScore = $1; } + # Blackboard seems to return this when there is no prior grade. # See: https://webwork.maa.org/moodle/mod/forum/discuss.php?d=5002 $priorScore = '' if $priorScore eq 'success'; @@ -309,9 +306,9 @@ EOS && ($score != 0 || $priorScore ne '')) { # LMS has essentially the same score, no reason to update it - debug( - "LMS grade will NOT be updated - grade has not significantly changed. Old score: $priorScore; New score: $score" - ) if $ce->{debug_lti_grade_passback}; + debug('LMS grade will NOT be updated - grade has not significantly changed. ' + . "Old score: $priorScore; New score: $score") + if $ce->{debug_lti_grade_passback}; $self->warning('LMS grade will NOT be updated - grade has not significantly changed. ' . "Old score: $priorScore; New score: $score") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; @@ -322,13 +319,9 @@ EOS } } } else { - $self->warning('Unable to retrieve prior grade from LMS. Note that if your server time is not correct, ' - . 'this may fail for reasons which are less than obvious from the error messages. Error: ' - . $response->message) + $self->warning('Unable to retrieve prior grade from LMS. Error: ' . $response->message) if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; - debug('Unable to retrieve prior grade from LMS. Note that if your server time is not correct, ' - . 'this may fail for reasons which are less than obvious from the error messages. Error: ' - . $response->message); + debug('Unable to retrieve prior grade from LMS. Error: ' . $response->message); debug($response->body); return 0; } diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 104e30c449..757b58840c 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -39,7 +39,8 @@ use Time::HiRes; use WeBWorK::Debug; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::Sets qw(grade_all_sets can_submit_LMS_score); +use WeBWorK::Utils::Sets qw(grade_all_sets); +use WeBWorK::Authen::LTI::GradePassback qw(getSetPassbackScore); # This package contains utilities for submitting grades to the LMS via LTI 1.3. sub new ($invocant, $c, $post_processing_mode = 0) { @@ -193,31 +194,35 @@ async sub get_access_token ($self) { # Computes and submits the course grade for userID to the LMS. # The course grade is the sum of all (weighted) problems assigned to the user. -async sub submit_course_grade ($self, $userID) { +async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; - # Before the costly act of calculating the course grade, if this LMS submission was intitated because - # of $LTIGradeOnSubmit, then check if the set from which a problem was submitted meets the criteria to - # be included in a course grade calculation. If not, we can skip the rest because the course grade will - # not differ from what it previously was. - return 0 unless ($self->{post_processing_mode} || can_submit_LMS_score($db, $ce, $userID, $c->{set}, 1)); - my $user = $db->getUser($userID); return 0 unless $user; + $self->warning("Submitting all grades for user $userID"); + my $lineitem = $db->getSettingValue('LTIAdvantageCourseLineitem'); + unless ($lineitem) { + $self->warning('LMS lineitem is not available for the course.'); + return 0; + } - $self->warning("Submitting all grades for user $userID"); - $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; - $self->warning('LMS lineitem is not available for the course.') unless $lineitem; + unless ($user->lis_source_did) { + $self->warning('LMS user id is not available for this user.'); + return 0; + } - return await $self->submit_grade($user->lis_source_did, $lineitem, grade_all_sets($db, $ce, $userID)); + return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); + + return await $self->submit_grade($user->lis_source_did, $lineitem, + grade_all_sets($db, $ce, $userID, \&getSetPassbackScore)); } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. -async sub submit_set_grade ($self, $userID, $setID) { +async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { my $c = $self->{c}; my $ce = $c->{ce}; my $db = $c->{db}; @@ -225,14 +230,20 @@ async sub submit_set_grade ($self, $userID, $setID) { my $user = $db->getUser($userID); return 0 unless $user; - my $userSet = $db->getMergedSet($userID, $setID); + $self->warning("Submitting grade for user $userID and set $setID."); + unless ($user->lis_source_did) { + $self->warning('LMS user id is not available for this user.'); + return 0; + } - my $score = can_submit_LMS_score($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); - return 0 unless $score; + my $userSet = $submittedSet // $db->getMergedSet($userID, $setID); + unless ($userSet->lis_source_did) { + $self->warning('LMS lineitem is not available for this set.'); + return 0; + } - $self->warning("Submitting grade for user $userID and set $setID."); - $self->warning('LMS user id is not available for this user.') unless $user->lis_source_did; - $self->warning('LMS lineitem is not available for this set.') unless $userSet->lis_source_did; + my $score = getSetPassbackScore($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); + return -1 unless $score; return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $score->{totalRight}, $score->{total}); @@ -243,7 +254,7 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum my $c = $self->{c}; my $ce = $c->{ce}; - return 0 unless $LMSuserID && $lineitem && (my $access_token = await $self->get_access_token); + return 0 unless (my $access_token = await $self->get_access_token); $self->warning('Found data required for submitting grades to LMS.'); @@ -284,16 +295,16 @@ async sub submit_grade ($self, $LMSuserID, $lineitem, $scoreGiven, $scoreMaximum : 0; my $score = $scoreMaximum ? $scoreGiven / $scoreMaximum : 0; - # We want to update the LMS score if the difference is significant, - # or if the new score is 1 but the LMS score was not 1 but possibly insignificantly different, - # or if the new score is 0 and the LMS score was empty and it is past the SendScoresAfterDate. + + # Do not update the score if there is no significant change. Note that the cases where the webwork score + # is exactly 1 and the LMS score is not exactly 1, and the case where the webwork score is 0 and the LMS + # score is not set are considered significant changes. if (abs($score - $priorScore) < 0.001 && ($score != 1 || $priorScore == 1) && ($score != 0 || (@$priorData && defined $priorData->[0]{resultScore}))) { - $self->warning( - "LMS grade will NOT be updated as the grade has not significantly changed. Old score: $priorScore, New score: $score." - ); + $self->warning('LMS grade will NOT be updated as the grade has not significantly changed. ' + . "Old score: $priorScore, New score: $score."); return 1; } diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 3e40e11792..274e5ce73e 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -985,13 +985,27 @@ sub getConfigValues ($ce) { . 'then the set needs to have been attempted for its score to be sent to the LMS (or included in ' . "the 'course' score calculation).

    For a regular or jitar set, 'attempted' means that at " . "least one exercise was attempted. For a test, 'attempted' means that either multiple versions " - . 'exercise or there is one version with a graded submission.

    ' + . 'exist or there is one version with a graded submission.

    ' ), - values => [qw(attempted 0 0.5 0.7 0.75 0.8 0.85 0.9 0.95 1)], + values => [ qw( + attempted 0 0.05 0.1 0.15 0.2 0.25 0.3 0.35 0.4 0.45 0.5 0.55 0.6 0.65 0.7 0.75 0.8 0.85 0.9 0.95 1 + ) ], labels => { attempted => x('Attempted'), 0 => '0%', + 0.05 => '5%', + 0.1 => '10%', + 0.15 => '15%', + 0.2 => '20%', + 0.25 => '25%', + 0.3 => '30%', + 0.35 => '35%', + 0.4 => '40%', + 0.45 => '45%', 0.5 => '50%', + 0.55 => '55%', + 0.6 => '60%', + 0.65 => '65%', 0.7 => '70%', 0.75 => '75%', 0.8 => '80%', diff --git a/lib/WeBWorK/ContentGenerator.pm b/lib/WeBWorK/ContentGenerator.pm index 86789dbf9c..664926f34c 100644 --- a/lib/WeBWorK/ContentGenerator.pm +++ b/lib/WeBWorK/ContentGenerator.pm @@ -56,7 +56,7 @@ use WeBWorK::Utils::LanguageAndDirection qw(get_lang_and_dir); use WeBWorK::Utils::Logs qw(writeCourseLog); use WeBWorK::Utils::Routes qw(route_title route_navigation_is_restricted); use WeBWorK::Utils::Sets qw(format_set_name_display); -use WeBWorK::Authen::LTI::MassUpdate qw(mass_update); +use WeBWorK::Authen::LTI::GradePassback qw(massUpdate); =head1 INVOCATION @@ -111,7 +111,7 @@ async sub go ($c) { # If grades are being passed back to the lti, then peroidically update all of the # grades because things can get out of sync if instructors add or modify sets. - mass_update($c) if $c->stash('courseID') && ref($c->db) && $ce->{LTIGradeMode}; + massUpdate($c) if $c->stash('courseID') && ref($c->db) && $ce->{LTIGradeMode}; # Check to determine if this is a problem set response. Individual content generators must check # $c->{invalidSet} and react appropriately. diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 3f89af5ff5..8bf699b607 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -36,9 +36,8 @@ use WeBWorK::Utils::Rendering qw(getTranslatorDebuggingOptions renderPG); use WeBWorK::Utils::Sets qw(is_restricted); use WeBWorK::DB::Utils qw(global2user fake_set fake_set_version fake_problem); use WeBWorK::Debug; -use WeBWorK::Authen::LTIAdvanced::SubmitGrade; -use WeBWorK::Authen::LTIAdvantage::SubmitGrade; use PGrandom; +use WeBWorK::Authen::LTI::GradePassback qw(passbackGradeOnSubmit); use Caliper::Sensor; use Caliper::Entity; @@ -880,7 +879,7 @@ async sub pre_header_initialize ($c) { debug('begin answer processing'); my @scoreRecordedMessage = ('') x scalar(@problems); - my $LTIGradeResult = -1; + my $ltiGradePassbackMessage; # Save results to database as appropriate if ($c->{submitAnswers} || (($c->{previewAnswers} || $c->param('newPage')) && $can{recordAnswers})) { @@ -1023,15 +1022,9 @@ async sub pre_header_initialize ($c) { } } - # Try to update the student score on the LMS if that option is enabled. - if ($c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode} && $ce->{LTIGradeOnSubmit}) { - my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course') { - $LTIGradeResult = await $grader->submit_course_grade($effectiveUserID); - } elsif ($ce->{LTIGradeMode} eq 'homework') { - $LTIGradeResult = await $grader->submit_set_grade($effectiveUserID, $setID); - } - } + # Send the score for this set to the LMS if enabled. + $ltiGradePassbackMessage = await passbackGradeOnSubmit($c, $effectiveUserID, $c->{set}) + if $c->{submitAnswers} && $will{recordAnswers} && $ce->{LTIGradeMode}; # Finally, log student answers that are being submitted, provided that answers can be recorded. Note that # this will log an overtime submission (or any case where someone submits the test, or spoofs a request to @@ -1184,8 +1177,8 @@ async sub pre_header_initialize ($c) { } debug('end answer processing'); - $c->{scoreRecordedMessage} = \@scoreRecordedMessage; - $c->{LTIGradeResult} = $LTIGradeResult; + $c->{scoreRecordedMessage} = \@scoreRecordedMessage; + $c->{ltiGradePassbackMessage} = $ltiGradePassbackMessage; # 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 that it's possible to diff --git a/lib/WeBWorK/ContentGenerator/Instructor/LTIUpdate.pm b/lib/WeBWorK/ContentGenerator/Instructor/LTIUpdate.pm index 9b2e820ca4..9c2ce76483 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/LTIUpdate.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/LTIUpdate.pm @@ -18,9 +18,8 @@ package WeBWorK::ContentGenerator::Instructor::LTIUpdate; use Mojo::Base 'WeBWorK::ContentGenerator', -signatures; -use WeBWorK::Utils(qw(getAssetURL)); use WeBWorK::Utils::Sets qw(format_set_name_display); -use WeBWorK::Authen::LTI::MassUpdate qw(mass_update); +use WeBWorK::Authen::LTI::GradePassback qw(massUpdate); sub initialize ($c) { my $db = $c->db; @@ -68,7 +67,7 @@ sub initialize ($c) { # Note that if somehow this point is reached with a setID and grade mode is "course", # then the setID will be ignored by the job. - mass_update($c, 1, $userID, $setID); + massUpdate($c, 1, $userID, $setID); return; } diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index b2755a5ed9..c4d79b4247 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -33,8 +33,7 @@ use WeBWorK::Utils qw(encodeAnswers createEmailSenderTransportSMTP); use WeBWorK::Utils::DateTime qw(before after); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq jitar_problem_adjusted_status); use WeBWorK::Utils::Logs qw(writeLog writeCourseLog); -use WeBWorK::Authen::LTIAdvanced::SubmitGrade; -use WeBWorK::Authen::LTIAdvantage::SubmitGrade; +use WeBWorK::Authen::LTI::GradePassback qw(passbackGradeOnSubmit); use Caliper::Sensor; use Caliper::Entity; @@ -248,38 +247,10 @@ async sub process_and_log_answer ($c) { $c->param('startTime', ''); } - # Messages about passing the score back to the LMS + # Send the score for this set to the LMS if enabled. if ($ce->{LTIGradeMode}) { - my $LMSname = $ce->{LTI}{ $ce->{LTIVersion} }{LMS_name}; - my $LTIGradeResult = -1; - if ($ce->{LTIGradeOnSubmit}) { - $LTIGradeResult = 0; - my $grader = $ce->{LTI}{ $ce->{LTIVersion} }{grader}->new($c); - if ($ce->{LTIGradeMode} eq 'course') { - $LTIGradeResult = await $grader->submit_course_grade($problem->user_id); - } elsif ($ce->{LTIGradeMode} eq 'homework') { - $LTIGradeResult = await $grader->submit_set_grade($problem->user_id, $problem->set_id); - } - if ($LTIGradeResult == 0) { - $scoreRecordedMessage .= - $c->tag('br') . $c->maketext('Your score was not successfully sent to [_1].', $LMSname); - } elsif ($LTIGradeResult > 0) { - $scoreRecordedMessage .= - $c->tag('br') . $c->maketext('Your score was successfully sent to [_1].', $LMSname); - } - } elsif ($ce->{LTIMassUpdateInterval} > 0) { - $scoreRecordedMessage .= $c->tag('br'); - if ($ce->{LTIMassUpdateInterval} < 120) { - $scoreRecordedMessage .= $c->maketext('Scores are sent to [_1] every [quant,_2,second].', - $LMSname, $ce->{LTIMassUpdateInterval}); - } elsif ($ce->{LTIMassUpdateInterval} < 7200) { - $scoreRecordedMessage .= $c->maketext('Scores are sent to [_1] every [quant,_2,minute].', - $LMSname, int($ce->{LTIMassUpdateInterval} / 60 + 0.99)); - } else { - $scoreRecordedMessage .= $c->maketext('Scores are sent to [_1] every [quant,_2,hour].', - $LMSname, int($ce->{LTIMassUpdateInterval} / 3600 + 0.9999)); - } - } + my $message = await passbackGradeOnSubmit($c, $problem->user_id, $c->{set}); + $scoreRecordedMessage .= $c->tag('br') . $message if $message; } } else { # The "sticky" answers get saved here when $will{recordAnswers} is false diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 027356a025..280490829a 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -20,7 +20,7 @@ use Carp; use PGrandom; use WeBWorK::Utils qw(wwRound); -use WeBWorK::Utils::DateTime qw(after before); +use WeBWorK::Utils::DateTime qw(after); use WeBWorK::Utils::JITAR qw(jitar_id_to_seq jitar_problem_adjusted_status); our @EXPORT_OK = qw( @@ -32,7 +32,6 @@ our @EXPORT_OK = qw( is_restricted get_test_problem_position list_set_versions - can_submit_LMS_score ); sub format_set_name_internal ($set_name) { @@ -115,197 +114,54 @@ sub grade_set ($db, $set, $studentName, $setIsVersioned = 0, $wantProblemDetails } if (wantarray) { - return ($totalRight, $total, $problem_scores, $problem_incorrect_attempts); + return ($totalRight, $total, $wantProblemDetails ? ($problem_scores, $problem_incorrect_attempts) : (), + \@problemRecords); } else { return $total ? $totalRight / $total : 0; } } sub grade_gateway ($db, $setName, $studentName) { - my @versionNums = $db->listSetVersions($studentName, $setName); + my $bestSetData = [ 0, 0 ]; - my $bestTotalRight = 0; - my $bestTotal = 0; - - if (@versionNums) { - for my $i (@versionNums) { - my $versionedSet = $db->getSetVersion($studentName, $setName, $i); - - my ($totalRight, $total) = grade_set($db, $versionedSet, $studentName, 1); - if ($totalRight > $bestTotalRight) { - $bestTotalRight = $totalRight; - $bestTotal = $total; - } - } + my @setVersions = $db->getSetVersionsWhere({ user_id => $studentName, set_id => { like => "$setName,v\%" } }); + for (@setVersions) { + my @setData = grade_set($db, $_, $studentName, 1); + $bestSetData = \@setData if $setData[0] > $bestSetData->[0]; } - if (wantarray) { - return ($bestTotalRight, $bestTotal); - } else { - return 0 unless $bestTotal; - return $bestTotalRight / $bestTotal; - } -} - -sub grade_set_or_gateway ($db, $userSet, $userID) { - return ($userSet->assignment_type =~ /gateway/) - ? grade_gateway($db, $userSet->set_id, $userID) - : (grade_set($db, $userSet, $userID))[ 0, 1 ]; + return wantarray ? (@$bestSetData, \@setVersions) : ($bestSetData->[1] ? $bestSetData->[0] / $bestSetData->[1] : 0); } -sub set_attempted ($db, $userID, $userSet) { - my $setID = $userSet->set_id; - - if ($userSet->assignment_type =~ /gateway/) { - my @versionNums = $db->listSetVersions($userID, $setID); - - # It counts as "attempted" if there is more than one version. - return 1 if (1 < @versionNums); - - # If there is one version, check for an attempted problem. - if (@versionNums) { - my @problemNums = $db->listProblemVersions($userID, $setID, $versionNums[0]); - my $problem = $db->getMergedProblemVersion($userID, $setID, $versionNums[0], $problemNums[0]); - return defined $problem && $problem->attempted ? 1 : 0; - } - - # If there are no versions, the test was not attempted. - return 0; - } else { - my @problemNums = $db->listUserProblems($userID, $setID); - for (@problemNums) { - my $problem = $db->getMergedProblem($userID, $setID, $_); - return 1 if ($problem->attempted || $problem->status > 0); +sub grade_all_sets ( + $db, $ce, + $studentName, + $getSetGradeConditionally = sub ($db, $ce, $studentName, $userSet) { + return unless after($userSet->open_date); + if ($userSet->assignment_type =~ /gateway/) { + my ($totalRight, $total) = grade_gateway($db, $userSet->set_id, $studentName); + return { totalRight => $totalRight, total => $total }; + } else { + my ($totalRight, $total) = grade_set($db, $userSet, $studentName, 0); + return { totalRight => $totalRight, total => $total }; } - return 0; } -} - -sub earliest_gateway_date ($db, $ce, $userSet) { - my @versionNums = $db->listSetVersions($userSet->user_id, $userSet->set_id); - - # If there are no versions, use the template's date. - return get_LTISendScoresAfterDate($userSet, $ce) unless (@versionNums); - - # Otherwise, use the earliest date among versions. - my $earliest_date = -1; - for my $i (@versionNums) { - my $versionedSetDate = - get_LTISendScoresAfterDate($db->getSetVersion($userSet->user_id, $userSet->set_id, $i), $ce); - $earliest_date = $versionedSetDate if $earliest_date == -1 || $versionedSetDate < $earliest_date; - } - return $earliest_date; -} - -sub grade_all_sets ($db, $ce, $studentName) { - my @setIDs = $db->listUserSets($studentName); - my @userSetIDs = map { [ $studentName, $_ ] } @setIDs; - my @userSets = $db->getMergedSets(@userSetIDs); + ) +{ + croak 'grade_all_sets requires a code reference for its last argument' + unless ref($getSetGradeConditionally) eq 'CODE'; my $courseTotalRight = 0; my $courseTotal = 0; - for my $userSet (@userSets) { - my $score = can_submit_LMS_score($db, $ce, $studentName, $userSet); + for my $userSet ($db->getMergedSetsWhere({ user_id => $studentName })) { + my $score = $getSetGradeConditionally->($db, $ce, $studentName, $userSet); next unless $score; $courseTotalRight += $score->{totalRight}; $courseTotal += $score->{total}; } - if (wantarray) { - return ($courseTotalRight, $courseTotal); - } else { - return 0 unless $courseTotal; - return $courseTotalRight / $courseTotal; - } - -} - -sub get_LTISendScoresAfterDate ($set, $ce) { - if ($ce->{LTISendScoresAfterDate} eq 'open_date') { - return $set->open_date; - } elsif ($ce->{LTISendScoresAfterDate} eq 'reduced_scoring_date') { - return ($ce->{pg}{ansEvalDefaults}{enableReducedScoring} - && $set->enable_reduced_scoring - && $set->reduced_scoring_date) ? $set->reduced_scoring_date : $set->due_date; - } elsif ($ce->{LTISendScoresAfterDate} eq 'due_date') { - return $set->due_date; - } elsif ($ce->{LTISendScoresAfterDate} eq 'answer_date') { - return $set->answer_date; - } -} - -# Checks if the set is past the LTISendScoresAfterDate or has met the LTISendGradesEarlyThreshold. -# Returns a reference to hash with keys totalRight, total, score, if the set has met either condition -# and undef if not. If the score is "0/0", that is treated as 0 if we have not yet reached the -# LTISendScoresAfterDate and have not attempted the set. Or if the set is a test with no completed -# version, that is also treated as 0. Otherwise, "0/0" is treated as 1. -sub can_submit_LMS_score ($db, $ce, $userID, $userSet, $set_attempted = 0) { - my $totalRight; - my $total; - my $score; - my $critical_date; - - # Complicated logic below is intended to exit this subroutine with minimal computational cost. - if ($ce->{LTISendScoresAfterDate} eq 'never') { - if ($ce->{LTISendGradesEarlyThreshold} eq 'attempted') { - $set_attempted = $set_attempted || set_attempted($db, $userID, $userSet); - return unless $set_attempted; - ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); - $score = $total ? $totalRight / $total : 0; - $score = 1 if (!$total && $set_attempted); - return { totalRight => $totalRight, total => $total, score => $score }; - } else { - ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); - $score = $total ? $totalRight / $total : 0; - $score = 1 if (!$total && $set_attempted); - return { totalRight => $totalRight, total => $total, score => $score } - if ($score >= $ce->{LTISendGradesEarlyThreshold}); - } - } elsif ($ce->{LTISendGradesEarlyThreshold} eq 'attempted') { - # We may have been sent $set_attempted = 1, and immediately know we can assess score and send it. - if ($set_attempted) { - ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); - $score = $total ? $totalRight / $total : 0; - $score = 1 unless $total; - return { totalRight => $totalRight, total => $total, score => $score }; - } else { - # Next cheapest assessment is assessing whether or not we have passed the critical date - # But if we are not, we must actually assess if set was attempted - $critical_date = - ($userSet->assignment_type =~ /gateway/) - ? earliest_gateway_date($db, $ce, $userSet) - : get_LTISendScoresAfterDate($userSet, $ce); - if (after($critical_date) || ($set_attempted = set_attempted($db, $userID, $userSet))) { - ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); - $score = $total ? $totalRight / $total : 0; - $score = 1 - if (!$total && ($userSet->assignment_type !~ /gateway/ && after($critical_date) || $set_attempted)); - return { totalRight => $totalRight, total => $total, score => $score }; - } - } - } else { - # No matter what, we need to know score now - ($totalRight, $total) = grade_set_or_gateway($db, $userSet, $userID); - $score = $total ? $totalRight / $total : 0; - if (!$total) { - $critical_date = - ($userSet->assignment_type =~ /gateway/) - ? earliest_gateway_date($db, $ce, $userSet) - : get_LTISendScoresAfterDate($userSet, $ce); - $set_attempted = $set_attempted || set_attempted($db, $userID, $userSet); - $score = 1 if ($userSet->assignment_type !~ /gateway/ && after($critical_date) || $set_attempted); - } - return { totalRight => $totalRight, total => $total, score => $score } - if ($score >= $ce->{LTISendGradesEarlyThreshold}); - $critical_date = - ($userSet->assignment_type =~ /gateway/) - ? earliest_gateway_date($db, $ce, $userSet) - : get_LTISendScoresAfterDate($userSet, $ce) - unless $critical_date; - return { totalRight => $totalRight, total => $total, score => $score } - if after($critical_date); - } + return wantarray ? ($courseTotalRight, $courseTotal) : $courseTotal ? $courseTotalRight / $courseTotal : 0; } sub is_restricted ($db, $set, $studentName) { @@ -430,16 +286,17 @@ This formats set names for display, converting underscores back into spaces. =head2 grade_set -Usage: C +Usage: C The arguments C<$db>, C<$set>, and C<$studentName> are required. If C<$setIsVersioned> is true, then the given set is assumed to be a set version. In list context this returns a list containing the total number of correct -problems, and the total number of problems in the set. If -C<$wantProblemDetails> is true, then a reference to an array of the scores for -each problem, and a reference to the array of the number of incorrect attempts -for each problem are also included in the returned list. +problems, the total number of problems in the set, and a reference to an array +of merged user problem records from the set. If C<$wantProblemDetails> is true, +then a reference to an array of the scores for each problem, and a reference to +the array of the number of incorrect attempts for each problem are also included +in the returned list before the reference to the array of problem records. In scalar context this returns the percentage correct. @@ -449,18 +306,31 @@ Usage: C All arguments are required. -In list context this returns a list fo the total number of correct problems for -the highest scoring version of this test, and the total number of problems in -that version. +In list context this returns a list of the total number of correct problems for +the highest scoring version of this test, the total number of problems in that +version, a reference to an array of merged user problem records from that +version, and a reference to an array of merged user set versions for this user +and set. In scalar context this returns the percentage correct for the highest scoring version of this test. =head2 grade_all_sets -Usage: C +Usage: C + +The arguments C<$db>, C<$ce>, and C<$studentName> are rrequired. -All arguments listed are required. +The C<$getSetGradeConditionally> is an optional argument that if provided should +be a reference to a subroutine that will be passed the arguments $db, $ce, +$studentName listed above, and $userSet which is a merged user set record from +the database, and must either return a reference to a hash containing the keys +totalRight and total with the grade for the set, or C. If it returns +C then the set will not be included in the grade computation. Otherwise +the values for totalRight and total that are returned will be added into the +grade. If the optional last arugment is not provided, then a default method +will be used that returns the set grade if after the open date, and C +otherwise. In list context this returns the total course score for all sets and the maximum possible course score. diff --git a/templates/ContentGenerator/GatewayQuiz.html.ep b/templates/ContentGenerator/GatewayQuiz.html.ep index 1c1329ffdb..85947e155a 100644 --- a/templates/ContentGenerator/GatewayQuiz.html.ep +++ b/templates/ContentGenerator/GatewayQuiz.html.ep @@ -153,11 +153,9 @@ % } % } % # Print a message when submitting the score to an LMS. - % if ($c->{LTIGradeResult} != -1) { + % if ($c->{ltiGradePassbackMessage}) {
    - <%= $c->{LTIGradeResult} - ? maketext('Your score was successfully sent to the LMS.') - : maketext('Your score was not successfully sent to the LMS.') =%> + <%= $c->{ltiGradePassbackMessage} =%> % } % } From 5d7062ae4d5accce15b90d6fe5146a2314626905 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Thu, 21 Nov 2024 11:06:21 -0600 Subject: [PATCH 19/25] Tweak the wording of the `LTICheckPrior` configuration value documentation. --- lib/WeBWorK/ConfigValues.pm | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 274e5ce73e..672cc82ef6 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -925,10 +925,11 @@ sub getConfigValues ($ce) { . 'first querying the current score from the LMS and then actually updating the score (if there is ' . 'a significant difference). Additional details:

    • If the LMS score is not 100%, but the ' . 'WeBWorK score is, then even if the LMS score is only insignificantly less than 100%, it will be ' - . 'updated anyway.
    • If the LMS score is null and the WeBWorK score is 0, this is considered ' - . 'an insignificant difference and the LMS score will not be updated to 0. However if it is after ' - . 'the $LTISendScoresAfterDate (described below), then the null score will be updated to 0 anyway.' - . '
    • "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not ' + . 'updated anyway.
    • If the LMS score is not set and the WeBWorK score is 0, this is ' + . 'considered a significant difference and the LMS score will updated to 0. However, the ' + . 'constraints of the $LTISendScoresAfterDate and the $LTISendGradesEarlyThreshold variables ' + . '(described below) might apply, and the score may still not be updated in this case.
      • ' + . '
    • "Significant" means an absolute difference of 0.001, or 0.1%. At this time this is not ' . 'configurable.
    ' ), type => 'boolean' From 734246bce95c3a12c1c8115a2f7901c69801a388 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 25 Nov 2024 17:04:58 -0800 Subject: [PATCH 20/25] extend grade_all_sets to return a triple including references to the user sets that were included in the scoring --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 15 +++++++++++---- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 13 +++++++++---- lib/WeBWorK/Utils/Sets.pm | 11 ++++++----- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 63ebc49626..8481254ae2 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -124,8 +124,6 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("submitting course grade for user: $userID") - if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; unless ($user->lis_source_did) { $self->warning("lis_source_did is not available for user: $userID") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; @@ -134,8 +132,17 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); - return await $self->submit_grade($user->lis_source_did, - scalar(grade_all_sets($db, $ce, $userID, \&getSetPassbackScore))); + my ($courseTotalRight, $courseTotal, $includedSets) = grade_all_sets($db, $ce, $userID, \&getSetPassbackScore); + if (@$includedSets) { + $self->warning( + "Submitting overall score for user $userID for sets: " . join(', ', map { $_->set_id } @$includedSets)) + if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; + my $score = $courseTotal ? $courseTotalRight / $courseTotal : 0; + return await $self->submit_grade($user->lis_source_did, $score); + } else { + $self->warning("No sets for user $userID meet criteria to be included in course grade calculation."); + return 0; + } } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 757b58840c..9f24db4aeb 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -202,8 +202,6 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("Submitting all grades for user $userID"); - my $lineitem = $db->getSettingValue('LTIAdvantageCourseLineitem'); unless ($lineitem) { $self->warning('LMS lineitem is not available for the course.'); @@ -217,8 +215,15 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); - return await $self->submit_grade($user->lis_source_did, $lineitem, - grade_all_sets($db, $ce, $userID, \&getSetPassbackScore)); + my ($courseTotalRight, $courseTotal, $includedSets) = grade_all_sets($db, $ce, $userID, \&getSetPassbackScore); + if (@$includedSets) { + $self->warning("Submitting overall score for user $userID for sets: " + . join(', ', map { $_->set_id } (@$includedSets))); + return await $self->submit_grade($user->lis_source_did, $lineitem, $courseTotalRight, $courseTotal); + } else { + $self->warning("No sets for user $userID meet criteria to be included in course grade calculation."); + return 0; + } } # Computes and submits the set grade for $userID and $setID to the LMS. For gateways the best score is used. diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index 280490829a..b77ffc0f18 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -153,15 +153,17 @@ sub grade_all_sets ( my $courseTotalRight = 0; my $courseTotal = 0; + my $includedSets = []; for my $userSet ($db->getMergedSetsWhere({ user_id => $studentName })) { my $score = $getSetGradeConditionally->($db, $ce, $studentName, $userSet); next unless $score; $courseTotalRight += $score->{totalRight}; $courseTotal += $score->{total}; + push @$includedSets, $userSet; } - return wantarray ? ($courseTotalRight, $courseTotal) : $courseTotal ? $courseTotalRight / $courseTotal : 0; + return ($courseTotalRight, $courseTotal, $includedSets); } sub is_restricted ($db, $set, $studentName) { @@ -332,10 +334,9 @@ grade. If the optional last arugment is not provided, then a default method will be used that returns the set grade if after the open date, and C otherwise. -In list context this returns the total course score for all sets and the maximum -possible course score. - -In scalar context this returns the percentage score for all sets in the course. +This returns the total course score for all sets, the maximum possible course score, +and an array reference containing references to the user sets that were included in +those two tallies. =head2 is_restricted From 3067000a962a43ea665f2f01fa3e9310f8921282 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 25 Nov 2024 17:05:36 -0800 Subject: [PATCH 21/25] fix a mistake in LTISendScoresAfterDate config options --- lib/WeBWorK/ConfigValues.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/WeBWorK/ConfigValues.pm b/lib/WeBWorK/ConfigValues.pm index 672cc82ef6..a30a0785ff 100644 --- a/lib/WeBWorK/ConfigValues.pm +++ b/lib/WeBWorK/ConfigValues.pm @@ -970,7 +970,7 @@ sub getConfigValues ($ce) { reduced_scoring_date => x('After the reduced scoring date'), due_date => x('After the close date'), answer_date => x('After the answer date'), - never_date => x('Never') + never => x('Never') }, type => 'popuplist' }, From ac5953b65e916d4271ea762d91ee486cb487face Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 25 Nov 2024 23:35:28 -0800 Subject: [PATCH 22/25] Add log entries for when grade passback does is blocked --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 13 ++++++++++--- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 11 +++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 8481254ae2..ce89b77c4a 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -124,8 +124,11 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; + $self->warning("Preparing to submit overall course grade to LMS for user $userID.") + if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; + unless ($user->lis_source_did) { - $self->warning("lis_source_did is not available for user: $userID") + $self->warning("lis_source_did is not available for this user") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; return 0; } @@ -154,7 +157,7 @@ async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("Submitting grade for user $userID and set $setID.") + $self->warning("Preparing to submit grade to LMS for user $userID and set $setID.") if $ce->{debug_lti_grade_passback} || $self->{post_processing_mode}; my $userSet = $submittedSet // $db->getMergedSet($userID, $setID); @@ -165,7 +168,11 @@ async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { } my $score = getSetPassbackScore($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); - return -1 unless $score; + unless ($score) { + $self->warning("Set's critical date has not yet passed, and user has not yet met the threshold to send set's " + . 'score early. Not submitting grade.'); + return -1; + } return await $self->submit_grade($userSet->lis_source_did, $score->{score}); } diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 9f24db4aeb..6509f40b24 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -202,6 +202,8 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; + $self->warning("Preparing to submit overall course grade to LMS for user $userID."); + my $lineitem = $db->getSettingValue('LTIAdvantageCourseLineitem'); unless ($lineitem) { $self->warning('LMS lineitem is not available for the course.'); @@ -235,7 +237,8 @@ async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("Submitting grade for user $userID and set $setID."); + $self->warning("Preparing to submit grade to LMS for user $userID and set $setID."); + unless ($user->lis_source_did) { $self->warning('LMS user id is not available for this user.'); return 0; @@ -248,7 +251,11 @@ async sub submit_set_grade ($self, $userID, $setID, $submittedSet = undef) { } my $score = getSetPassbackScore($db, $ce, $userID, $userSet, !$self->{post_processing_mode}); - return -1 unless $score; + unless ($score) { + $self->warning("Set's critical date has not yet passed, and user has not yet met the threshold to send set's " + . 'score early. Not submitting grade.'); + return -1; + } return await $self->submit_grade($user->lis_source_did, $userSet->lis_source_did, $score->{totalRight}, $score->{total}); From a7172438e0ef5abc925c7576be5f9675c70f7862 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Mon, 25 Nov 2024 23:37:37 -0800 Subject: [PATCH 23/25] bring back scalar context output for grade_all_sets --- lib/WeBWorK/Utils/Sets.pm | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/WeBWorK/Utils/Sets.pm b/lib/WeBWorK/Utils/Sets.pm index b77ffc0f18..9d620844bd 100644 --- a/lib/WeBWorK/Utils/Sets.pm +++ b/lib/WeBWorK/Utils/Sets.pm @@ -163,7 +163,11 @@ sub grade_all_sets ( push @$includedSets, $userSet; } - return ($courseTotalRight, $courseTotal, $includedSets); + return + wantarray + ? ($courseTotalRight, $courseTotal, $includedSets) + : ($courseTotal ? $courseTotalRight / $courseTotal : 0); + } sub is_restricted ($db, $set, $studentName) { From c5fdb3888f7657f6e02357b5b6b64b528f99e545 Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Wed, 27 Nov 2024 20:39:15 -0800 Subject: [PATCH 24/25] more log messages for submit_course_grade --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 6 +++++- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index ce89b77c4a..7565239ade 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -133,7 +133,11 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return 0; } - return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); + if ($submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1)) { + $self->warning("Set's critical date has not yet passed, and user has not yet met the threshold to send set's " + . 'score early. Not submitting grade.'); + return -1; + } my ($courseTotalRight, $courseTotal, $includedSets) = grade_all_sets($db, $ce, $userID, \&getSetPassbackScore); if (@$includedSets) { diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 6509f40b24..353e8bc7f6 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -215,7 +215,11 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return 0; } - return -1 if $submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1); + if ($submittedSet && !getSetPassbackScore($db, $ce, $userID, $submittedSet, 1)) { + $self->warning("Set's critical date has not yet passed, and user has not yet met the threshold to send set's " + . 'score early. Not submitting grade.'); + return -1; + } my ($courseTotalRight, $courseTotal, $includedSets) = grade_all_sets($db, $ce, $userID, \&getSetPassbackScore); if (@$includedSets) { From f97d4c561c9bc32e58f356b2d18d5ea32548d17d Mon Sep 17 00:00:00 2001 From: Alex Jordan Date: Tue, 3 Dec 2024 11:40:52 -0800 Subject: [PATCH 25/25] return -1 (not 0) when no sets meet grade passback criteria for course grade passback --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 2 +- lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 7565239ade..b86c6d5d7b 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -148,7 +148,7 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return await $self->submit_grade($user->lis_source_did, $score); } else { $self->warning("No sets for user $userID meet criteria to be included in course grade calculation."); - return 0; + return -1; } } diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index 353e8bc7f6..5802e4a663 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -228,7 +228,7 @@ async sub submit_course_grade ($self, $userID, $submittedSet = undef) { return await $self->submit_grade($user->lis_source_did, $lineitem, $courseTotalRight, $courseTotal); } else { $self->warning("No sets for user $userID meet criteria to be included in course grade calculation."); - return 0; + return -1; } }