Skip to content

Commit

Permalink
feedback from PR#2617
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alex-Jordan
Alex-Jordan committed Nov 18, 2024
1 parent f4373dd commit f251b04
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 42 deletions.
8 changes: 4 additions & 4 deletions lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand Down
24 changes: 13 additions & 11 deletions lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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."
Expand Down
2 changes: 1 addition & 1 deletion lib/WeBWorK/ContentGenerator/GatewayQuiz.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 6 additions & 4 deletions lib/WeBWorK/Utils/ProblemProcessing.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
43 changes: 21 additions & 22 deletions lib/WeBWorK/Utils/Sets.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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);
Expand All @@ -185,35 +184,37 @@ 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} // '';

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
Expand Down Expand Up @@ -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 ];
}
Expand All @@ -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});
}

Expand Down

0 comments on commit f251b04

Please sign in to comment.