From 0aca89ef233595b0483412dbe541ae2763c64946 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 16 Oct 2023 05:18:58 -0500 Subject: [PATCH 1/6] Add a page to manage jobs in the job queue. All jobs for a course are listed on this page. The table displays the job id (this is used to reference specific jobs in action messages, otherwise it would not be shown), task name, created time, started time, finished time, and state. Also a button that opens a popover containing the job result is in the state column if the job has completed. Note that the Minion job queue automatically removes jobs from the job queue after two days (that is the default at least which we don't change). So the real importance of this page is to allow the instructor to see the status of recently completed or in progress jobs. At this point the actions available on the page are filter, sort, and delete. Jobs can be filtered by id, task name, or state. Jobs can be sorted by clicking on the headers, or by using the sort form. Jobs that are not active can be deleted. Minion does not allow deletion of active jobs. Note that an active job means a job that is currently running. As such they can not be selected on this page. Perhaps an option to stop running jobs could be added at if there is a problem with jobs hanging, but active jobs can not be directly stopped. The Minion worker is in a different process so the Mojolicious app needs to broadcast a signal to the Minion worker to do so. An inactive job (i.e., a job that has been queued but has not started running yet) can be selected and deleted. However, it is possible that the inactive job could start before the form is submitted. In that case the job can not be deleted, and so an alert will show that. In order to reliably associate a course with a job there is a new rule for tasks. The job must pass the course id via the "notes" option of the Minion enqueue method. The existing tasks have been updated to do this. There is also a backwards compatibility check to find jobs that passed it one of the ways the two jobs did it before in the job arguments. Since the job fail/finish messages are now displayed in the UI, those messages are now translated. That is all except the first few messages in each task before the course environment is established, since a course environment is required to obtain the language of the course. The send_instructor_email task no longer sends an email to the instructor after sending the emails to the students. Instead the job result contains all of the information that would have been in that email. This is a far more reliable way of getting that information to the instructor sending the email. The instructor just needs to go to the "Job Manager" page to see the result. The message on the "Email" page tells the instructor this. This page is also available for the admin course. In the admin course all jobs for all courses are shown. There is an additional column in the jobs table that shows the course id for the course the job was enqueued by. The errors that are reported when sending emails are made less verbose by calling the `message` method of the Mojo::Exception which does not include the traceback. --- htdocs/js/JobManager/jobmanager.js | 47 ++++ htdocs/js/JobManager/jobmanager.scss | 8 + lib/Mojolicious/WeBWorK.pm | 5 +- .../WeBWorK/Tasks/LTIMassUpdate.pm | 40 ++-- .../WeBWorK/Tasks/SendInstructorEmail.pm | 94 ++++---- lib/WeBWorK/Authen/LTI/MassUpdate.pm | 2 +- lib/WeBWorK/ContentGenerator/Feedback.pm | 2 +- .../ContentGenerator/Instructor/JobManager.pm | 205 ++++++++++++++++++ .../ContentGenerator/Instructor/SendMail.pm | 8 +- lib/WeBWorK/Utils/ProblemProcessing.pm | 2 +- lib/WeBWorK/Utils/Routes.pm | 8 + .../ContentGenerator/Base/admin_links.html.ep | 20 +- templates/ContentGenerator/Base/links.html.ep | 2 + .../Instructor/JobManager.html.ep | 184 ++++++++++++++++ .../Instructor/JobManager/delete_form.html.ep | 13 ++ .../Instructor/JobManager/filter_form.html.ep | 38 ++++ .../Instructor/JobManager/sort_form.html.ep | 41 ++++ .../Instructor/SendMail.html.ep | 5 +- .../HelpFiles/InstructorJobManager.html.ep | 80 +++++++ 19 files changed, 715 insertions(+), 89 deletions(-) create mode 100644 htdocs/js/JobManager/jobmanager.js create mode 100644 htdocs/js/JobManager/jobmanager.scss create mode 100644 lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm create mode 100644 templates/ContentGenerator/Instructor/JobManager.html.ep create mode 100644 templates/ContentGenerator/Instructor/JobManager/delete_form.html.ep create mode 100644 templates/ContentGenerator/Instructor/JobManager/filter_form.html.ep create mode 100644 templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep create mode 100644 templates/HelpFiles/InstructorJobManager.html.ep diff --git a/htdocs/js/JobManager/jobmanager.js b/htdocs/js/JobManager/jobmanager.js new file mode 100644 index 0000000000..c64344835c --- /dev/null +++ b/htdocs/js/JobManager/jobmanager.js @@ -0,0 +1,47 @@ +(() => { + // Show/hide the filter elements depending on if the field matching option is selected. + const filter_select = document.getElementById('filter_select'); + const filter_elements = document.getElementById('filter_elements'); + if (filter_select && filter_elements) { + const toggle_filter_elements = () => { + if (filter_select.value === 'match_regex') filter_elements.style.display = 'block'; + else filter_elements.style.display = 'none'; + }; + filter_select.addEventListener('change', toggle_filter_elements); + toggle_filter_elements(); + } + + // Submit the job list form when a sort header is clicked or enter or space is pressed when it has focus. + const currentAction = document.getElementById('current_action'); + if (currentAction) { + for (const header of document.querySelectorAll('.sort-header')) { + const submitSortMethod = (e) => { + e.preventDefault(); + + currentAction.value = 'sort'; + + const sortInput = document.createElement('input'); + sortInput.name = 'labelSortMethod'; + sortInput.value = header.dataset.sortField; + sortInput.type = 'hidden'; + currentAction.form.append(sortInput); + + currentAction.form.submit(); + }; + + header.addEventListener('click', submitSortMethod); + header.addEventListener('keydown', (e) => { + if (e.key === ' ' || e.key === 'Enter') submitSortMethod(e); + }); + } + } + + // Activate the results popovers. + document.querySelectorAll('.result-popover-btn').forEach((popoverBtn) => { + new bootstrap.Popover(popoverBtn, { + trigger: 'hover focus', + customClass: 'job-queue-result-popover', + html: true + }); + }); +})(); diff --git a/htdocs/js/JobManager/jobmanager.scss b/htdocs/js/JobManager/jobmanager.scss new file mode 100644 index 0000000000..e5ed920f3a --- /dev/null +++ b/htdocs/js/JobManager/jobmanager.scss @@ -0,0 +1,8 @@ +.job-queue-result-popover { + --bs-popover-max-width: 500px; + + .popover-body { + overflow-y: auto; + max-height: 25vh; + } +} diff --git a/lib/Mojolicious/WeBWorK.pm b/lib/Mojolicious/WeBWorK.pm index 5eff7a429d..1d64d32866 100644 --- a/lib/Mojolicious/WeBWorK.pm +++ b/lib/Mojolicious/WeBWorK.pm @@ -26,7 +26,7 @@ use Env qw(WEBWORK_SERVER_ADMIN); use WeBWorK; use WeBWorK::CourseEnvironment; -use WeBWorK::Utils qw(x writeTimingLogEntry); +use WeBWorK::Utils qw(writeTimingLogEntry); use WeBWorK::Utils::Routes qw(setup_content_generator_routes); sub startup ($app) { @@ -84,7 +84,8 @@ sub startup ($app) { # Add the themes directory to the template search paths. push(@{ $app->renderer->paths }, $ce->{webworkDirs}{themes}); - # Setup the Minion job queue. + # Setup the Minion job queue. Make sure that any task added here is represented in the TASK_NAMES hash in + # WeBWorK::ContentGenerator::Instructor::JobManager. $app->plugin(Minion => { $ce->{job_queue}{backend} => $ce->{job_queue}{database_dsn} }); $app->minion->add_task(lti_mass_update => 'Mojolicious::WeBWorK::Tasks::LTIMassUpdate'); $app->minion->add_task(send_instructor_email => 'Mojolicious::WeBWorK::Tasks::SendInstructorEmail'); diff --git a/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm b/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm index 163f2eedd1..e112f5231e 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm @@ -22,27 +22,22 @@ use WeBWorK::CourseEnvironment; use WeBWorK::DB; # Perform a mass update of grades via LTI. -sub run ($job, $courseID, $userID = '', $setID = '') { - # Establish a lock guard that only allow 1 job at a time (technichally more than one could run at a time if a job +sub run ($job, $userID = '', $setID = '') { + # Establish a lock guard that only allows 1 job at a time (technically more than one could run at a time if a job # takes more than an hour to complete). As soon as a job completes (or fails) the lock is released and a new job - # can start. New jobs retry every minute until they can aquire their own lock. + # can start. New jobs retry every minute until they can acquire their own lock. return $job->retry({ delay => 60 }) unless my $guard = $job->minion->guard('lti_mass_update', 3600); + my $courseID = $job->info->{notes}{courseID}; + return $job->fail('The course id was not passed when this job was enqueued.') unless $courseID; + my $ce = eval { WeBWorK::CourseEnvironment->new({ courseName => $courseID }) }; - return $job->fail("Could not construct course environment for $courseID.") unless $ce; + return $job->fail('Could not construct course environment.') unless $ce; - my $db = WeBWorK::DB->new($ce->{dbLayout}); - return $job->fail("Could not obtain database connection for $courseID.") unless $db; + $job->{language_handle} = WeBWorK::Localize::getLoc($ce->{language} || 'en'); - if ($setID && $userID && $ce->{LTIGradeMode} eq 'homework') { - $job->app->log->info("LTI Mass Update: Starting grade update for user $userID and set $setID."); - } elsif ($setID && $ce->{LTIGradeMode} eq 'homework') { - $job->app->log->info("LTI Mass Update: Starting grade update for all users assigned to set $setID."); - } elsif ($userID) { - $job->app->log->info("LTI Mass Update: Starting grade update of all sets assigned to user $userID."); - } else { - $job->app->log->info('LTI Mass Update: Starting grade update for all sets and users.'); - } + my $db = WeBWorK::DB->new($ce->{dbLayout}); + return $job->fail($job->maketext('Could not obtain database connection.')) unless $db; # Pass a fake controller object that will work for the grader. my $grader = @@ -76,8 +71,19 @@ sub run ($job, $courseID, $userID = '', $setID = '') { } } - $job->app->log->info("Updated grades via LTI for course $courseID."); - return $job->finish("Updated grades via LTI for course $courseID."); + if ($setID && $userID && $ce->{LTIGradeMode} eq 'homework') { + return $job->finish($job->maketext('Updated grades via LTI for user [_1] and set [_2].', $userID, $setID)); + } elsif ($setID && $ce->{LTIGradeMode} eq 'homework') { + return $job->finish($job->maketext('Updated grades via LTI all users assigned to set [_1].', $setID)); + } elsif ($userID) { + return $job->finish($job->maketext('Updated grades via LTI of all sets assigned to user [_1].', $userID)); + } else { + return $job->finish($job->maketext('Updated grades via LTI for all sets and users.')); + } +} + +sub maketext ($job, @args) { + return &{ $job->{language_handle} }(@args); } 1; diff --git a/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm b/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm index c3f65d91e4..9ae99c08eb 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm @@ -28,46 +28,46 @@ use WeBWorK::Utils qw/processEmailMessage createEmailSenderTransportSMTP/; # Send instructor email messages to students. # FIXME: This job currently allows multiple jobs to run at once. Should it be limited? sub run ($job, $mail_data) { - my $ce = eval { WeBWorK::CourseEnvironment->new({ courseName => $mail_data->{courseName} }) }; - return $job->fail("Could not construct course environment for $mail_data->{courseName}.") unless $ce; + my $courseID = $job->info->{notes}{courseID}; + return $job->fail('The course id was not passed when this job was enqueued.') unless $courseID; - my $db = WeBWorK::DB->new($ce->{dbLayout}); - return $job->fail("Could not obtain database connection for $mail_data->{courseName}.") unless $db; + my $ce = eval { WeBWorK::CourseEnvironment->new({ courseName => $courseID }) }; + return $job->fail('Could not construct course environment.') unless $ce; $job->{language_handle} = WeBWorK::Localize::getLoc($ce->{language} || 'en'); - my $result_message = eval { $job->mail_message_to_recipients($ce, $db, $mail_data) }; - if ($@) { - $result_message .= "An error occurred while trying to send email.\n" . "The error message is:\n\n$@\n\n"; - $job->app->log->error("An error occurred while trying to send email: $@\n"); - } + my $db = WeBWorK::DB->new($ce->{dbLayout}); + return $job->fail($job->maketext('Could not obtain database connection.')) unless $db; - eval { $job->email_notification($ce, $mail_data, $result_message) }; + my @result_messages = eval { $job->mail_message_to_recipients($ce, $db, $mail_data) }; if ($@) { - $job->app->log->error("An error occurred while trying to send the email notification: $@\n"); - return $job->fail("FAILURE: Unable to send email notifation to instructor."); + push(@result_messages, + $job->maketext('An error occurred while trying to send email.'), + $job->maketext('The error message is:'), + ref($@) ? $@->message : $@); + return $job->fail(\@result_messages); } - return $job->finish("SUCCESS: Email messages sent."); + return $job->finish(\@result_messages); } sub mail_message_to_recipients ($job, $ce, $db, $mail_data) { - my $result_message = ''; + my @result_messages; my $failed_messages = 0; - my $error_messages = ''; + my @error_messages; my @recipients = @{ $mail_data->{recipients} }; for my $recipient (@recipients) { - $error_messages = ''; + @error_messages = (); my $user_record = $db->getUser($recipient); unless ($user_record) { - $error_messages .= "Record for user $recipient not found\n"; + push(@error_messages, $job->maketext('Record for user [_1] not found.', $recipient)); next; } unless ($user_record->email_address =~ /\S/) { - $error_messages .= "User $recipient does not have an email address -- skipping\n"; + push(@error_messages, $job->maketext('User [_1] does not have an email address.', $recipient)); next; } @@ -86,52 +86,44 @@ sub mail_message_to_recipients ($job, $ce, $db, $mail_data) { transport => createEmailSenderTransportSMTP($ce), $ce->{mail}{set_return_path} ? (from => $ce->{mail}{set_return_path}) : () }); - debug 'email sent successfully to ' . $user_record->email_address; + debug 'Email successfully sent to ' . $user_record->email_address; }; if ($@) { - debug "Error sending email: $@"; - $error_messages .= "Error sending email: $@"; + my $exception_message = ref($@) ? $@->message : $@; + debug 'Error sending email to ' . $user_record->email_address . ": $exception_message"; + push( + @error_messages, + $job->maketext( + 'Error sending email to [_1]: [_2]', $user_record->email_address, $exception_message + ) + ); next; } - $result_message .= - $job->maketext('Message sent to [_1] at [_2].', $recipient, $user_record->email_address) . "\n" - unless $error_messages; + push(@result_messages, $job->maketext('Message sent to [_1] at [_2].', $recipient, $user_record->email_address)) + unless @error_messages; } continue { # Update failed messages before continuing loop. - if ($error_messages) { + if (@error_messages) { $failed_messages++; - $result_message .= $error_messages; + push(@result_messages, @error_messages); } } my $number_of_recipients = @recipients - $failed_messages; - return $job->maketext( - 'A message with the subject line "[_1]" has been sent to [quant,_2,recipient] in the class [_3]. ' - . 'There were [_4] message(s) that could not be sent.', - $mail_data->{subject}, $number_of_recipients, $mail_data->{courseName}, + return ( + $job->maketext( + 'A message with the subject line "[_1]" has been sent to [quant,_2,recipient].', + $mail_data->{subject}, $number_of_recipients + ), $failed_messages - ) - . "\n\n" - . $result_message; -} - -sub email_notification ($job, $ce, $mail_data, $result_message) { - my $email = - Email::Stuffer->to($mail_data->{defaultFrom})->from($mail_data->{defaultFrom})->subject('WeBWorK email sent') - ->text_body($result_message)->header('X-Remote-Host' => $mail_data->{remote_host}); - - eval { - $email->send_or_die({ - transport => createEmailSenderTransportSMTP($ce), - $ce->{mail}{set_return_path} ? (from => $ce->{mail}{set_return_path}) : () - }); - }; - $job->app->log->error("Error sending email: $@") if $@; - - $job->app->log->info("WeBWorK::Tasks::SendInstructorEmail: Instructor message sent from $mail_data->{defaultFrom}"); - - return; + ? ($job->maketext( + 'There [plural,_1,was,were] [quant,_1,message] that could not be sent.', + $failed_messages + )) + : (), + @result_messages + ); } sub maketext ($job, @args) { diff --git a/lib/WeBWorK/Authen/LTI/MassUpdate.pm b/lib/WeBWorK/Authen/LTI/MassUpdate.pm index cd4758b296..967b879e4a 100644 --- a/lib/WeBWorK/Authen/LTI/MassUpdate.pm +++ b/lib/WeBWorK/Authen/LTI/MassUpdate.pm @@ -63,7 +63,7 @@ sub mass_update ($c, $manual_update = 0, $userID = undef, $setID = undef) { } } - $c->minion->enqueue(lti_mass_update => [ $ce->{courseName}, $userID, $setID ]); + $c->minion->enqueue(lti_mass_update => [ $userID, $setID ], { notes => { courseID => $ce->{courseName} } }); return; } diff --git a/lib/WeBWorK/ContentGenerator/Feedback.pm b/lib/WeBWorK/ContentGenerator/Feedback.pm index efb622e65b..b558bf53be 100644 --- a/lib/WeBWorK/ContentGenerator/Feedback.pm +++ b/lib/WeBWorK/ContentGenerator/Feedback.pm @@ -250,7 +250,7 @@ $emailableURL $ce->{mail}{set_return_path} ? (from => $ce->{mail}{set_return_path}) : () }); } catch { - $c->stash->{send_error} = $c->maketext('Failed to send message: [_1]', $_); + $c->stash->{send_error} = $c->maketext('Failed to send message: [_1]', ref($_) ? $_->message : $_); }; } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm new file mode 100644 index 0000000000..cb8190c5c5 --- /dev/null +++ b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm @@ -0,0 +1,205 @@ +################################################################################ +# WeBWorK Online Homework Delivery System +# Copyright © 2000-2021 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::ContentGenerator::Instructor::JobManager; +use Mojo::Base 'WeBWorK::ContentGenerator', -signatures; + +=head1 NAME + +WeBWorK::ContentGenerator::Instructor::JobManager - Minion job queue job management + +=cut + +use WeBWorK::Utils qw(x); + +use constant ACTION_FORMS => [ [ filter => x('Filter') ], [ sort => x('Sort') ], [ delete => x('Delete') ] ]; + +# All tasks added in the Mojolicious::WeBWorK module need to be listed here. +use constant TASK_NAMES => { + lti_mass_update => x('LTI Mass Update'), + send_instructor_email => x('Send Instructor Email') +}; + +# This constant is not used. It is here so that gettext adds these strings to the translation files. +use constant JOB_STATES => [ x('inactive'), x('active'), x('finished'), x('failed') ]; + +use constant FIELDS => [ + [ id => x('Id') ], + [ courseID => x('Course Id') ], + [ task => x('Task') ], + [ created => x('Created') ], + [ started => x('Started') ], + [ finished => x('Finished') ], + [ state => x('State') ], +]; + +use constant SORT_SUBS => { + id => \&byJobID, + courseID => \&byCourseID, + task => \&byTask, + created => \&byCreatedTime, + started => \&byStartedTime, + finished => \&byFinishedTime, + state => \&byState +}; + +sub initialize ($c) { + $c->stash->{taskNames} = TASK_NAMES(); + $c->stash->{actionForms} = ACTION_FORMS(); + $c->stash->{fields} = $c->stash->{courseID} eq 'admin' ? FIELDS() : [ grep { $_ ne 'courseID' } @{ FIELDS() } ]; + $c->stash->{jobs} = {}; + $c->stash->{visibleJobs} = {}; + $c->stash->{selectedJobs} = {}; + $c->stash->{sortedJobs} = []; + $c->stash->{primarySortField} = $c->param('primarySortField') || 'created'; + $c->stash->{secondarySortField} = $c->param('secondarySortField') || 'task'; + $c->stash->{ternarySortField} = $c->param('ternarySortField') || 'state'; + + return unless $c->authz->hasPermissions($c->param('user'), 'access_instructor_tools'); + + # Get a list of all jobs. If this is not the admin course, then restrict to the jobs for this course. + my $jobs = $c->minion->jobs; + while (my $job = $jobs->next) { + # Get the course id from the job arguments for backwards compatibility with jobs before the job manager was + # added and the course id was moved to the notes. + unless (defined $job->{notes}{courseID}) { + if (ref($job->{args}[0]) eq 'HASH' && defined $job->{args}[0]{courseName}) { + $job->{notes}{courseID} = $job->{args}[0]{courseName}; + } else { + $job->{notes}{courseID} = $job->{args}[0]; + } + } + + # Copy the courseID from the notes hash directly to the job for convenience of access. Particularly, so that + # that the filter_handler method can access it the same as for other fields. + $job->{courseID} = $job->{notes}{courseID}; + + $c->stash->{jobs}{ $job->{id} } = $job + if $c->stash->{courseID} eq 'admin' || $job->{courseID} eq $c->stash->{courseID}; + } + + if (defined $c->param('visible_jobs')) { + $c->stash->{visibleJobs} = { map { $_ => 1 } @{ $c->every_param('visible_jobs') } }; + } elsif (defined $c->param('no_visible_jobs')) { + $c->stash->{visibleJobs} = {}; + } else { + $c->stash->{visibleJobs} = { map { $_ => 1 } keys %{ $c->stash->{jobs} } }; + } + + $c->stash->{selectedJobs} = { map { $_ => 1 } @{ $c->every_param('selected_jobs') // [] } }; + + my $actionID = $c->param('action'); + if ($actionID) { + my $actionHandler = "${actionID}_handler"; + die $c->maketext('Action [_1] not found', $actionID) unless $c->can($actionHandler); + $c->addgoodmessage($c->$actionHandler); + } + + # Sort jobs + my $primarySortSub = SORT_SUBS()->{ $c->stash->{primarySortField} }; + my $secondarySortSub = SORT_SUBS()->{ $c->stash->{secondarySortField} }; + my $ternarySortSub = SORT_SUBS()->{ $c->stash->{ternarySortField} }; + + # byJobID is included to ensure a definite sort order in case the + # first three sorts do not determine a proper order. + $c->stash->{sortedJobs} = [ + map { $_->{id} } + sort { &$primarySortSub || &$secondarySortSub || &$ternarySortSub || byJobID } + grep { $c->stash->{visibleJobs}{ $_->{id} } } (values %{ $c->stash->{jobs} }) + ]; + + return; +} + +sub filter_handler ($c) { + my $ce = $c->ce; + + my $scope = $c->param('action.filter.scope'); + if ($scope eq 'all') { + $c->stash->{visibleJobs} = { map { $_ => 1 } keys %{ $c->stash->{jobs} } }; + return $c->maketext('Showing all jobs.'); + } elsif ($scope eq 'selected') { + $c->stash->{visibleJobs} = $c->stash->{selectedJobs}; + return $c->maketext('Showing selected jobs.'); + } elsif ($scope eq 'match_regex') { + my $regex = $c->param('action.filter.text'); + my $field = $c->param('action.filter.field'); + $c->stash->{visibleJobs} = {}; + for my $jobID (keys %{ $c->stash->{jobs} }) { + $c->stash->{visibleJobs}{$jobID} = 1 if $c->stash->{jobs}{$jobID}{$field} =~ /^$regex/i; + } + return $c->maketext('Showing matching jobs.'); + } + + # This should never happen. As such it is not translated. + return 'Not filtering. Unknown filter given.'; +} + +sub sort_handler ($c) { + if (defined $c->param('labelSortMethod')) { + $c->stash->{ternarySortField} = $c->stash->{secondarySortField}; + $c->stash->{secondarySortField} = $c->stash->{primarySortField}; + $c->stash->{primarySortField} = $c->param('labelSortMethod'); + $c->param('action.sort.primary', $c->stash->{primarySortField}); + $c->param('action.sort.secondary', $c->stash->{secondarySortField}); + $c->param('action.sort.ternary', $c->stash->{ternarySortField}); + } else { + $c->stash->{primarySortField} = $c->param('action.sort.primary'); + $c->stash->{secondarySortField} = $c->param('action.sort.secondary'); + $c->stash->{ternarySortField} = $c->param('action.sort.ternary'); + } + + return $c->maketext( + 'Users sorted by [_1], then by [_2], then by [_3]', + $c->maketext((grep { $_->[0] eq $c->stash->{primarySortField} } @{ FIELDS() })[0][1]), + $c->maketext((grep { $_->[0] eq $c->stash->{secondarySortField} } @{ FIELDS() })[0][1]), + $c->maketext((grep { $_->[0] eq $c->stash->{ternarySortField} } @{ FIELDS() })[0][1]) + ); + +} + +sub delete_handler ($c) { + my $num = 0; + return $c->maketext('Deleted [quant,_1,job].', $num) if $c->param('action.delete.scope') eq 'none'; + + for my $jobID (keys %{ $c->stash->{selectedJobs} }) { + # If a job was inactive (not yet started) when the page was previously loaded, then it may be selected to be + # deleted. By the time the delete form is submitted the job may have started and may now be active. In that + # case it can not be deleted. + if ($c->stash->{jobs}{$jobID}{state} eq 'active') { + $c->addbadmessage( + $c->maketext('Unable to delete job [_1] as it has transitioned to an active state.', $jobID)); + next; + } + delete $c->stash->{jobs}{$jobID}; + delete $c->stash->{visibleJobs}{$jobID}; + delete $c->stash->{selectedJobs}{$jobID}; + $c->minion->job($jobID)->remove; + ++$num; + } + + return $c->maketext('Deleted [quant,_1,job].', $num); +} + +# Sort methods +sub byJobID { return $a->{id} <=> $b->{id} } +sub byCourseID { return lc $a->{courseID} cmp lc $b->{courseID} } +sub byTask { return $a->{task} cmp $b->{task} } +sub byCreatedTime { return $a->{created} <=> $b->{created} } +sub byStartedTime { return ($a->{started} || 0) <=> ($b->{started} || 0) } +sub byFinishedTime { return ($a->{finished} || 0) <=> ($b->{finished} || 0) } +sub byState { return $a->{state} cmp $b->{state} } + +1; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm index 0c9272ef1c..c0eb5bf9ac 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm @@ -348,11 +348,10 @@ sub initialize ($c) { # we don't set the response until we're sure that email can be sent $c->{response} = 'send_email'; - # Do actual mailing in the after the response is sent, since it could take a long time - # FIXME we need to do a better job providing status notifications for long-running email jobs + # The emails are actually sent in the job queue, since it could take a long time. + # Note that the instructor can check the job manager page to see the status of the job. $c->minion->enqueue( send_instructor_email => [ { - courseName => $c->stash('courseID'), recipients => $c->{ra_send_to}, subject => $c->{subject}, text => ${ $c->{r_text} // \'' }, @@ -360,7 +359,8 @@ sub initialize ($c) { from => $c->{from}, defaultFrom => $c->{defaultFrom}, remote_host => $c->{remote_host}, - } ] + } ], + { notes => { courseID => $c->stash('courseID') } } ); } else { $c->addbadmessage($c->maketext(q{Didn't recognize action})); diff --git a/lib/WeBWorK/Utils/ProblemProcessing.pm b/lib/WeBWorK/Utils/ProblemProcessing.pm index 3267b7229a..4915e49837 100644 --- a/lib/WeBWorK/Utils/ProblemProcessing.pm +++ b/lib/WeBWorK/Utils/ProblemProcessing.pm @@ -458,7 +458,7 @@ Comment: $comment }); debug('Successfully sent JITAR alert message'); } catch { - $c->log->error("Failed to send JITAR alert message: $_"); + $c->log->error('Failed to send JITAR alert message: ' . (ref($_) ? $_->message : $_)); }; return ''; diff --git a/lib/WeBWorK/Utils/Routes.pm b/lib/WeBWorK/Utils/Routes.pm index acddc7608f..97f3f323ea 100644 --- a/lib/WeBWorK/Utils/Routes.pm +++ b/lib/WeBWorK/Utils/Routes.pm @@ -104,6 +104,8 @@ PLEASE FOR THE LOVE OF GOD UPDATE THIS IF YOU CHANGE THE ROUTES BELOW!!! instructor_lti_update /$courseID/instructor/lti_update + instructor_job_manager /$courseID/instructor/job_manager + problem_list /$courseID/$setID problem_detail /$courseID/$setID/$problemID show_me_another /$courseID/$setID/$problemID/show_me_another @@ -328,6 +330,7 @@ my %routeParameters = ( instructor_progress instructor_problem_grader instructor_lti_update + instructor_job_manager ) ], module => 'Instructor::Index', path => '/instructor' @@ -487,6 +490,11 @@ my %routeParameters = ( module => 'Instructor::LTIUpdate', path => '/lti_update' }, + instructor_job_manager => { + title => x('Job Manager'), + module => 'Instructor::JobManager', + path => '/job_manager' + }, problem_list => { title => '[_2]', diff --git a/templates/ContentGenerator/Base/admin_links.html.ep b/templates/ContentGenerator/Base/admin_links.html.ep index 3379a2a0d0..6e256e2f92 100644 --- a/templates/ContentGenerator/Base/admin_links.html.ep +++ b/templates/ContentGenerator/Base/admin_links.html.ep @@ -11,14 +11,14 @@ % for ( % [ - % 'add_course', - % maketext('Add Courses'), - % { - % add_admin_users => 1, - % add_config_file => 1, - % add_dbLayout => 'sql_single', - % add_templates_course => $ce->{siteDefaults}{default_templates_course} || '' - % } + % 'add_course', + % maketext('Add Courses'), + % { + % add_admin_users => 1, + % add_config_file => 1, + % add_dbLayout => 'sql_single', + % add_templates_course => $ce->{siteDefaults}{default_templates_course} || '' + % } % ], % [ 'rename_course', maketext('Rename Courses') ], % [ 'delete_course', maketext('Delete Courses') ], @@ -27,8 +27,7 @@ % [ 'upgrade_course', maketext('Upgrade Courses') ], % [ 'hide_inactive_course', maketext('Hide Courses') ], % [ 'manage_locations', maketext('Manage Locations') ], - % ) - % { + % ) {
  • <%= $makelink->( 'set_list', @@ -42,6 +41,7 @@ % if ($authz->hasPermissions($userID, 'send_mail')) {
  • <%= $makelink->('instructor_mail_merge') %>
    • % } +
  • <%= $makelink->('instructor_job_manager') %>
  • <%= $makelink->( 'instructor_file_manager', diff --git a/templates/ContentGenerator/Base/links.html.ep b/templates/ContentGenerator/Base/links.html.ep index e4127feda4..3a615792d0 100644 --- a/templates/ContentGenerator/Base/links.html.ep +++ b/templates/ContentGenerator/Base/links.html.ep @@ -352,6 +352,8 @@ % if ($authz->hasPermissions($userID, 'send_mail')) {
  • <%= $makelink->('instructor_mail_merge') %>
    • % } + % # Job Manager +
  • <%= $makelink->('instructor_job_manager') %>
    • % # File Manager % if ($authz->hasPermissions($userID, 'manage_course_files')) {
  • <%= $makelink->('instructor_file_manager') %>
    • diff --git a/templates/ContentGenerator/Instructor/JobManager.html.ep b/templates/ContentGenerator/Instructor/JobManager.html.ep new file mode 100644 index 0000000000..3dcad80a6f --- /dev/null +++ b/templates/ContentGenerator/Instructor/JobManager.html.ep @@ -0,0 +1,184 @@ +% use WeBWorK::Utils qw(getAssetURL); +% +% content_for css => begin + <%= stylesheet getAssetURL($ce, 'js/JobManager/jobmanager.css') =%> +% end +% +% content_for js => begin + <%= javascript getAssetURL($ce, 'js/ActionTabs/actiontabs.js'), defer => undef =%> + <%= javascript getAssetURL($ce, 'js/SelectAll/selectall.js'), defer => undef =%> + <%= javascript getAssetURL($ce, 'js/JobManager/jobmanager.js'), defer => undef =%> +% end +% +% unless ($authz->hasPermissions(param('user'), 'access_instructor_tools')) { +
    <%= maketext('You are not authorized to access instructor tools.') =%>
    + % last; +% } +% +% unless (keys %$jobs) { +
    <%= maketext('No jobs in queue.') %>
    + % last; +% } +% +<%= form_for current_route, method => 'POST', name => 'joblist', begin =%> + <%= $c->hidden_authen_fields =%> + % + % if (keys %$visibleJobs) { + % for (keys %$visibleJobs) { + <%= hidden_field visible_jobs => $_ =%> + % } + % } else { + <%= hidden_field no_visible_jobs => '1' =%> + % } + % + <%= hidden_field primarySortField => $primarySortField =%> + <%= hidden_field secondarySortField => $secondarySortField =%> + <%= hidden_field ternarySortField => $ternarySortField =%> + % + % # Output action forms + % for my $form (@$actionForms) { + % my $active = $form->[0] eq 'filter' ? ' active' : ''; + % + % content_for 'tab-list' => begin + + % end + % + % content_for 'tab-content' => begin +
    " id="<%= $form->[0] %>" + role="tabpanel" aria-labelledby="<%= $form->[0] %>-tab"> + <%= include "ContentGenerator/Instructor/JobManager/$form->[0]_form" =%> +
    + % end + % } + % + <%= hidden_field action => $actionForms->[0][0], id => 'current_action' =%> +
    + +
    <%= content 'tab-content' %>
    +
    + % +
    + <%= submit_button maketext($actionForms->[0][1]), id => 'take_action', class => 'btn btn-primary' =%> +
    + % + % # Show the jobs table +
    + + + + + + % if ($courseID eq 'admin') { + + % } + + + + + + + + + % for my $jobID (@$sortedJobs) { + + % if ($jobs->{$jobID}{state} eq 'active') { + % # Active jobs can not be deleted, and so a checkbox is not provided to select them. + + + % } else { + + + % } + % if ($courseID eq 'admin') { + + % } + + + + + + + % } + +
    + <%= check_box 'select-all' => 'on', id => 'select-all', + 'aria-label' => maketext('Select all jobs'), + data => { select_group => 'selected_jobs' }, + class => 'select-all form-check-input' =%> + + <%= label_for 'select-all' => + link_to maketext('Id') => '#', class => 'sort-header', data => { sort_field => 'id' } =%> + + <%= link_to maketext('Course Id') => '#', class => 'sort-header', + data => { sort_field => 'courseID' } =%> + + <%= link_to maketext('Task') => '#', class => 'sort-header', + data => { sort_field => 'task' } =%> + + <%= link_to maketext('Created') => '#', class => 'sort-header', + data => { sort_field => 'created' } =%> + + <%= link_to maketext('Started') => '#', class => 'sort-header', + data => { sort_field => 'started' } =%> + + <%= link_to maketext('Finished') => '#', class => 'sort-header', + data => { sort_field => 'finished' } =%> + + <%= link_to maketext('State') => '#', class => 'sort-header', + data => { sort_field => 'state' } =%> +
    <%= $jobID =%> + <%= check_box selected_jobs => $jobID, id => "job_${jobID}_checkbox", + class => 'form-check-input', $selectedJobs->{$jobID} ? (checked => undef) : () =%> + <%= label_for "job_${jobID}_checkbox" => $jobID =%><%= $jobs->{$jobID}{courseID} =~ s/_/ /gr =%> + <%= $taskNames->{ $jobs->{$jobID}{task} } + ? maketext($taskNames->{ $jobs->{$jobID}{task} }) + : $jobs->{$jobID}{task} =%> + + <%= $c->formatDateTime( + $jobs->{$jobID}{created}, '', 'datetime_format_medium', $ce->{language}) =%> + + % if ($jobs->{$jobID}{started}) { + <%= $c->formatDateTime( + $jobs->{$jobID}{started}, '', 'datetime_format_medium', $ce->{language}) =%> + % } + + % if ($jobs->{$jobID}{finished}) { + <%= $c->formatDateTime( + $jobs->{$jobID}{finished}, '', 'datetime_format_medium', $ce->{language}) =%> + % } + +
    + <%= maketext($jobs->{$jobID}{state}) =%> + % if (defined $jobs->{$jobID}{result}) { + % content_for "result_$jobID", begin + % if (ref($jobs->{$jobID}{result}) eq 'ARRAY') { +
      + % for (@{ $jobs->{$jobID}{result} } ) { +
    • <%= $_ %>
      • + % } +
    + % } else { + <%= $jobs->{$jobID}{result} =%> + % } + % end + xml_escape %>"> + + + + % } +
    +
    +
    +% end diff --git a/templates/ContentGenerator/Instructor/JobManager/delete_form.html.ep b/templates/ContentGenerator/Instructor/JobManager/delete_form.html.ep new file mode 100644 index 0000000000..944a84c44e --- /dev/null +++ b/templates/ContentGenerator/Instructor/JobManager/delete_form.html.ep @@ -0,0 +1,13 @@ +
    +
    + <%= label_for delete_select => maketext('Delete which jobs?'), + class => 'col-form-label col-form-label-sm col-auto' =%> +
    + <%= select_field 'action.delete.scope' => [ + [ maketext('no jobs') => 'none', selected => undef ], + [ maketext('selected jobs') => 'selected' ] + ], + id => 'delete_select', class => 'form-select form-select-sm' =%> +
    +
    +
    diff --git a/templates/ContentGenerator/Instructor/JobManager/filter_form.html.ep b/templates/ContentGenerator/Instructor/JobManager/filter_form.html.ep new file mode 100644 index 0000000000..eec3845e44 --- /dev/null +++ b/templates/ContentGenerator/Instructor/JobManager/filter_form.html.ep @@ -0,0 +1,38 @@ +
    +
    + <%= label_for filter_select => maketext('Show which jobs?'), + class => 'col-form-label col-form-label-sm col-sm-auto' =%> +
    + <%= select_field 'action.filter.scope' => [ + [ maketext('all jobs') => 'all' ], + [ maketext('selected jobs') => 'selected' ], + [ maketext('jobs that match on selected field') => 'match_regex', selected => undef ] + ], + id => 'filter_select', class => 'form-select form-select-sm' =%> +
    +
    +
    +
    + <%= label_for 'filter_type_select' => maketext('What field should filtered jobs match on?'), + class => 'col-form-label col-form-label-sm col-sm-auto' =%> +
    + <%= select_field 'action.filter.field' => [ + [ maketext('Id') => 'id', selected => undef ], + $courseID eq 'admin' ? [ maketext('Course Id') => 'courseID' ] : (), + [ maketext('Task') => 'task' ], + [ maketext('State') => 'state' ] + ], + id => 'filter_type_select', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for 'filter_text', class => 'col-form-label col-form-label-sm col-sm-auto', begin =%> + <%= maketext('Filter by what text?') %>* + <% end =%> +
    + <%= text_field 'action.filter.text' => '', id => 'filter_text', 'aria-required' => 'true', + class => 'form-control form-control-sm' =%> +
    +
    +
    +
    diff --git a/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep b/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep new file mode 100644 index 0000000000..8f21fc6df5 --- /dev/null +++ b/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep @@ -0,0 +1,41 @@ +
    +
    + <%= label_for sort_select_1 => maketext('Sort by') . ':', class => 'col-form-label col-form-label-sm', + style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.primary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'created' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_1', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for sort_select_2 => maketext('Then by') . ':', class => 'col-form-label col-form-label-sm', + style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.secondary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'task' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_2', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for sort_select_3 => maketext('Then by') . ':', class => 'col-form-label col-form-label-sm', + style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.ternary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'state' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_3', class => 'form-select form-select-sm' =%> +
    +
    +
    diff --git a/templates/ContentGenerator/Instructor/SendMail.html.ep b/templates/ContentGenerator/Instructor/SendMail.html.ep index 55f8819c74..66739fb6b0 100644 --- a/templates/ContentGenerator/Instructor/SendMail.html.ep +++ b/templates/ContentGenerator/Instructor/SendMail.html.ep @@ -23,8 +23,9 @@ % my $message = begin <%= maketext( - 'Email is being sent to [quant,_1,recipient]. You will be notified by email ' - . 'when the task is completed. This may take several minutes if the class is large.', + 'Email is being sent to [quant,_1,recipient]. ' + . 'This job may take several minutes to complete if the class is large. ' + . 'Go to the "Job Manager" to see the status of this job.', scalar(@{ $c->{ra_send_to} }) ) =%> diff --git a/templates/HelpFiles/InstructorJobManager.html.ep b/templates/HelpFiles/InstructorJobManager.html.ep new file mode 100644 index 0000000000..3ddf3325a0 --- /dev/null +++ b/templates/HelpFiles/InstructorJobManager.html.ep @@ -0,0 +1,80 @@ +%################################################################################ +%# WeBWorK Online Homework Delivery System +%# Copyright © 2000-2023 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. +%################################################################################ +% +% layout 'help_macro'; +% title maketext('Job Manager Help'); +% +

    + <%= maketext('This page allows one to view and manage jobs in job queue. Note that completed jobs are ' + . 'automatically removed from the job queue after two days. So there is no real need to delete jobs. ' + . 'The importance of this page is to see the status of recently completed or in progress jobs.') =%> +

    +

    <%= maketext('Job Table Column Descriptions:') %>

    +
    +
    <%= maketext('Id') %>
    +
    + <%= maketext('The job id is an automatically incremented integer. It is used internally to uniquely identify ' + . 'jobs. It is also used to reference jobs in messages on this page.') =%> +
    +
    <%= maketext('Task') %>
    +
    + <%= maketext('The name of the task.') =%> +
    +
    <%= maketext('Created') %>
    +
    + <%= maketext('The time that the job was added to the queue.') =%> +
    +
    <%= maketext('Started') %>
    +
    + <%= maketext('The time that execution of the job begins. If execution of the job has not started, ' + . 'then this will be empty.') =%> +
    +
    <%= maketext('Finished') %>
    +
    + <%= maketext('The time that execution of the job completes. If execution of the job has not yet completed, ' + . 'then this will be empty.') =%> +
    +
    <%= maketext('State') %>
    +
    + <%= maketext('The current state of the job. This will be one of "inactive", "active", "finished", or ' + . '"failed". If a job is "inactive" it means that the job has been added to the queue, but execution ' + . 'of the job has not yet started. If a job is "active" it means that the job is currently being ' + . 'executed. If a job is "finished" it means that the execution of the job has successfully ' + . 'completed. If a job is "failed" it means that the execution of job has completed, but there were ' + . 'errors in the execution of the job. If the job is in the "finished" or "failed" state, then there ' + . 'will also be a popover containing information about what happened when the job was executed.') =%> +
    +
    +% +

    <%= maketext('Actions:') %>

    +
    +
    <%= maketext('Filter') %>
    +
    + <%= maketext('Filter jobs that are shown in the job table. Jobs can be filtered by Id, Task, or State, ' + . 'or by selection.') =%> +
    +
    <%= maketext('Sort') %>
    +
    + <%= maketext('Sort jobs in the table by fields. The jobs in the table can also be sorted by clicking on ' + . 'column headers in the table.') =%> +
    +
    <%= maketext('Delete') %>
    +
    + <%= maketext('Delete selected jobs. Note that jobs that are in the "active" state can not be deleted. ' + . 'Jobs that are in the "inactive" state can be deleted, but it is possible that by the time the request ' + . 'to delete the job occurs the job may have transitioned into the "active" state. In that case the job ' + . 'will not be deleted. Jobs that are in the "finished" or "failed" states can always be deleted.') =%> +
    +
    From 0f3288d9c0375ace8d1990ee5234dd076daaf57a Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Sat, 21 Oct 2023 10:08:25 -0500 Subject: [PATCH 2/6] Add reverse sorting. --- htdocs/js/JobManager/jobmanager.js | 25 ---- .../ContentGenerator/Instructor/JobManager.pm | 72 ++++++++--- .../Instructor/JobManager.html.ep | 71 ++++++++--- .../Instructor/JobManager/sort_button.html.ep | 37 ++++++ .../Instructor/JobManager/sort_form.html.ep | 117 +++++++++++++----- 5 files changed, 225 insertions(+), 97 deletions(-) create mode 100644 templates/ContentGenerator/Instructor/JobManager/sort_button.html.ep diff --git a/htdocs/js/JobManager/jobmanager.js b/htdocs/js/JobManager/jobmanager.js index c64344835c..ef28c00b1f 100644 --- a/htdocs/js/JobManager/jobmanager.js +++ b/htdocs/js/JobManager/jobmanager.js @@ -11,31 +11,6 @@ toggle_filter_elements(); } - // Submit the job list form when a sort header is clicked or enter or space is pressed when it has focus. - const currentAction = document.getElementById('current_action'); - if (currentAction) { - for (const header of document.querySelectorAll('.sort-header')) { - const submitSortMethod = (e) => { - e.preventDefault(); - - currentAction.value = 'sort'; - - const sortInput = document.createElement('input'); - sortInput.name = 'labelSortMethod'; - sortInput.value = header.dataset.sortField; - sortInput.type = 'hidden'; - currentAction.form.append(sortInput); - - currentAction.form.submit(); - }; - - header.addEventListener('click', submitSortMethod); - header.addEventListener('keydown', (e) => { - if (e.key === ' ' || e.key === 'Enter') submitSortMethod(e); - }); - } - } - // Activate the results popovers. document.querySelectorAll('.result-popover-btn').forEach((popoverBtn) => { new bootstrap.Popover(popoverBtn, { diff --git a/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm index cb8190c5c5..2b7ee3b0e9 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm @@ -46,13 +46,13 @@ use constant FIELDS => [ ]; use constant SORT_SUBS => { - id => \&byJobID, - courseID => \&byCourseID, - task => \&byTask, - created => \&byCreatedTime, - started => \&byStartedTime, - finished => \&byFinishedTime, - state => \&byState + id => { ASC => \&byJobID, DESC => \&byDescJobID }, + courseID => { ASC => \&byCourseID, DESC => \&byDescCourseID }, + task => { ASC => \&byTask, DESC => \&byDescTask }, + created => { ASC => \&byCreatedTime, DESC => \&byDescCreatedTime }, + started => { ASC => \&byStartedTime, DESC => \&byDescStartedTime }, + finished => { ASC => \&byFinishedTime, DESC => \&byDescFinishedTime }, + state => { ASC => \&byState, DESC => \&byDescState } }; sub initialize ($c) { @@ -64,8 +64,11 @@ sub initialize ($c) { $c->stash->{selectedJobs} = {}; $c->stash->{sortedJobs} = []; $c->stash->{primarySortField} = $c->param('primarySortField') || 'created'; + $c->stash->{primarySortOrder} = $c->param('primarySortOrder') || 'ASC'; $c->stash->{secondarySortField} = $c->param('secondarySortField') || 'task'; + $c->stash->{secondarySortOrder} = $c->param('secondarySortOrder') || 'ASC'; $c->stash->{ternarySortField} = $c->param('ternarySortField') || 'state'; + $c->stash->{ternarySortOrder} = $c->param('ternarySortOrder') || 'ASC'; return unless $c->authz->hasPermissions($c->param('user'), 'access_instructor_tools'); @@ -108,9 +111,9 @@ sub initialize ($c) { } # Sort jobs - my $primarySortSub = SORT_SUBS()->{ $c->stash->{primarySortField} }; - my $secondarySortSub = SORT_SUBS()->{ $c->stash->{secondarySortField} }; - my $ternarySortSub = SORT_SUBS()->{ $c->stash->{ternarySortField} }; + my $primarySortSub = SORT_SUBS()->{ $c->stash->{primarySortField} }{ $c->stash->{primarySortOrder} }; + my $secondarySortSub = SORT_SUBS()->{ $c->stash->{secondarySortField} }{ $c->stash->{secondarySortOrder} }; + my $ternarySortSub = SORT_SUBS()->{ $c->stash->{ternarySortField} }{ $c->stash->{ternarySortOrder} }; # byJobID is included to ensure a definite sort order in case the # first three sorts do not determine a proper order. @@ -148,26 +151,47 @@ sub filter_handler ($c) { } sub sort_handler ($c) { - if (defined $c->param('labelSortMethod')) { - $c->stash->{ternarySortField} = $c->stash->{secondarySortField}; - $c->stash->{secondarySortField} = $c->stash->{primarySortField}; - $c->stash->{primarySortField} = $c->param('labelSortMethod'); - $c->param('action.sort.primary', $c->stash->{primarySortField}); - $c->param('action.sort.secondary', $c->stash->{secondarySortField}); - $c->param('action.sort.ternary', $c->stash->{ternarySortField}); + if (defined $c->param('labelSortMethod') || defined $c->param('labelSortOrder')) { + if (defined $c->param('labelSortOrder')) { + $c->stash->{ $c->param('labelSortOrder') . 'SortOrder' } = + $c->stash->{ $c->param('labelSortOrder') . 'SortOrder' } eq 'ASC' ? 'DESC' : 'ASC'; + } elsif ($c->param('labelSortMethod') eq $c->stash->{primarySortField}) { + $c->stash->{primarySortOrder} = $c->stash->{primarySortOrder} eq 'ASC' ? 'DESC' : 'ASC'; + } else { + $c->stash->{ternarySortField} = $c->stash->{secondarySortField}; + $c->stash->{ternarySortOrder} = $c->stash->{secondarySortOrder}; + $c->stash->{secondarySortField} = $c->stash->{primarySortField}; + $c->stash->{secondarySortOrder} = $c->stash->{primarySortOrder}; + $c->stash->{primarySortField} = $c->param('labelSortMethod'); + $c->stash->{primarySortOrder} = 'ASC'; + } + + $c->param('action.sort.primary', $c->stash->{primarySortField}); + $c->param('action.sort.primary.order', $c->stash->{primarySortOrder}); + $c->param('action.sort.secondary', $c->stash->{secondarySortField}); + $c->param('action.sort.secondary.order', $c->stash->{secondarySortOrder}); + $c->param('action.sort.ternary', $c->stash->{ternarySortField}); + $c->param('action.sort.ternary.order', $c->stash->{ternarySortOrder}); } else { $c->stash->{primarySortField} = $c->param('action.sort.primary'); + $c->stash->{primarySortOrder} = $c->param('action.sort.primary.order'); $c->stash->{secondarySortField} = $c->param('action.sort.secondary'); + $c->stash->{secondarySortOrder} = $c->param('action.sort.secondary.order'); $c->stash->{ternarySortField} = $c->param('action.sort.ternary'); + $c->stash->{ternarySortOrder} = $c->param('action.sort.ternary.order'); } return $c->maketext( - 'Users sorted by [_1], then by [_2], then by [_3]', + 'Jobs sorted by [_1] in [plural,_2,ascending,descending] order, ' + . 'then by [_3] in [plural,_4,ascending,descending] order,' + . 'and then by [_5] in [plural,_6,ascending,descending] order.', $c->maketext((grep { $_->[0] eq $c->stash->{primarySortField} } @{ FIELDS() })[0][1]), + $c->stash->{primarySortOrder} eq 'ASC' ? 1 : 2, $c->maketext((grep { $_->[0] eq $c->stash->{secondarySortField} } @{ FIELDS() })[0][1]), - $c->maketext((grep { $_->[0] eq $c->stash->{ternarySortField} } @{ FIELDS() })[0][1]) + $c->stash->{secondarySortOrder} eq 'ASC' ? 1 : 2, + $c->maketext((grep { $_->[0] eq $c->stash->{ternarySortField} } @{ FIELDS() })[0][1]), + $c->stash->{ternarySortOrder} eq 'ASC' ? 1 : 2 ); - } sub delete_handler ($c) { @@ -202,4 +226,12 @@ sub byStartedTime { return ($a->{started} || 0) <=> ($b->{started} || 0) } sub byFinishedTime { return ($a->{finished} || 0) <=> ($b->{finished} || 0) } sub byState { return $a->{state} cmp $b->{state} } +sub byDescJobID { local ($b, $a) = ($a, $b); return byJobID(); } +sub byDescCourseID { local ($b, $a) = ($a, $b); return byCourseID(); } +sub byDescTask { local ($b, $a) = ($a, $b); return byTask(); } +sub byDescCreatedTime { local ($b, $a) = ($a, $b); return byCreatedTime(); } +sub byDescStartedTime { local ($b, $a) = ($a, $b); return byStartedTime(); } +sub byDescFinishedTime { local ($b, $a) = ($a, $b); return byFinishedTime(); } +sub byDescState { local ($b, $a) = ($a, $b); return byState(); } + 1; diff --git a/templates/ContentGenerator/Instructor/JobManager.html.ep b/templates/ContentGenerator/Instructor/JobManager.html.ep index 3dcad80a6f..3429042708 100644 --- a/templates/ContentGenerator/Instructor/JobManager.html.ep +++ b/templates/ContentGenerator/Instructor/JobManager.html.ep @@ -32,8 +32,11 @@ % } % <%= hidden_field primarySortField => $primarySortField =%> + <%= hidden_field primarySortOrder => $primarySortOrder =%> <%= hidden_field secondarySortField => $secondarySortField =%> + <%= hidden_field secondarySortOrder => $secondarySortOrder =%> <%= hidden_field ternarySortField => $ternarySortField =%> + <%= hidden_field ternarySortOrder => $ternarySortOrder =%> % % # Output action forms % for my $form (@$actionForms) { @@ -74,41 +77,71 @@ - % if ($courseID eq 'admin') { % } diff --git a/templates/ContentGenerator/Instructor/JobManager/sort_button.html.ep b/templates/ContentGenerator/Instructor/JobManager/sort_button.html.ep new file mode 100644 index 0000000000..aa6707afac --- /dev/null +++ b/templates/ContentGenerator/Instructor/JobManager/sort_button.html.ep @@ -0,0 +1,37 @@ +% if ($primarySortField eq $field) { + +% } elsif ($secondarySortField eq $field) { + +% } elsif ($ternarySortField eq $field) { + +% } diff --git a/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep b/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep index 8f21fc6df5..5505d718d4 100644 --- a/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep +++ b/templates/ContentGenerator/Instructor/JobManager/sort_form.html.ep @@ -1,41 +1,92 @@
    -
    - <%= label_for sort_select_1 => maketext('Sort by') . ':', class => 'col-form-label col-form-label-sm', - style => 'width:4.5rem' =%> -
    - <%= select_field 'action.sort.primary' => [ - map { [ - maketext($_->[1]) => $_->[0], - $_->[0] eq 'created' ? (selected => undef) : () - ] } @$fields - ], - id => 'sort_select_1', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for sort_select_1 => maketext('Sort by') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.primary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'created' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_1', class => 'form-select form-select-sm' =%> +
    +
    +
    +
    +
    + <%= label_for sort_order_select_1 => maketext('Ordered') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.primary.order' => [ + [ maketext('Ascending') => 'ASC' ], + [ maketext('Descending') => 'DESC' ], + ], + id => 'sort_order_select_1', class => 'form-select form-select-sm' =%> +
    +
    -
    - <%= label_for sort_select_2 => maketext('Then by') . ':', class => 'col-form-label col-form-label-sm', - style => 'width:4.5rem' =%> -
    - <%= select_field 'action.sort.secondary' => [ - map { [ - maketext($_->[1]) => $_->[0], - $_->[0] eq 'task' ? (selected => undef) : () - ] } @$fields - ], - id => 'sort_select_2', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for sort_select_2 => maketext('Then by') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.secondary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'task' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_2', class => 'form-select form-select-sm' =%> +
    +
    +
    +
    +
    + <%= label_for sort_order_select_2 => maketext('Ordered') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.secondary.order' => [ + [ maketext('Ascending') => 'ASC' ], + [ maketext('Descending') => 'DESC' ], + ], + id => 'sort_order_select_2', class => 'form-select form-select-sm' =%> +
    +
    -
    - <%= label_for sort_select_3 => maketext('Then by') . ':', class => 'col-form-label col-form-label-sm', - style => 'width:4.5rem' =%> -
    - <%= select_field 'action.sort.ternary' => [ - map { [ - maketext($_->[1]) => $_->[0], - $_->[0] eq 'state' ? (selected => undef) : () - ] } @$fields - ], - id => 'sort_select_3', class => 'form-select form-select-sm' =%> +
    +
    +
    + <%= label_for sort_select_3 => maketext('Then by') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.ternary' => [ + map { [ + maketext($_->[1]) => $_->[0], + $_->[0] eq 'state' ? (selected => undef) : () + ] } @$fields + ], + id => 'sort_select_3', class => 'form-select form-select-sm' =%> +
    +
    +
    +
    +
    + <%= label_for sort_order_select_3 => maketext('Ordered') . ':', + class => 'col-form-label col-form-label-sm', style => 'width:4.5rem' =%> +
    + <%= select_field 'action.sort.ternary.order' => [ + [ maketext('Ascending') => 'ASC' ], + [ maketext('Descending') => 'DESC' ], + ], + id => 'sort_order_select_3', class => 'form-select form-select-sm' =%> +
    +
    From a05631c1c396fd6f828023a11c1a0cd1c6dce04e Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 23 Oct 2023 07:56:28 -0500 Subject: [PATCH 3/6] Capture logs for the LTIMassUpdate task and return them for the job result. Note that the job will still succeed but now some failures/messages will appear in the job result. If `debug_lti_grade_passback` or `debug_lti_parameters` are enabled then the job result will now be rather extensive. Everything that is sent to the log will be in result. Also add logs back that were removed initially. Since Minion removes jobs after two days, the messages in the result are lost with them. So also log them so that they are more permanently stored. --- .../WeBWorK/Tasks/LTIMassUpdate.pm | 20 +++++++++++++++---- .../WeBWorK/Tasks/SendInstructorEmail.pm | 6 ++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm b/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm index e112f5231e..8e07facb4b 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/LTIMassUpdate.pm @@ -39,6 +39,13 @@ sub run ($job, $userID = '', $setID = '') { my $db = WeBWorK::DB->new($ce->{dbLayout}); return $job->fail($job->maketext('Could not obtain database connection.')) unless $db; + my @messages; + my $job_logger = sub { + my ($log, $level, @lines) = @_; + push @messages, $lines[-1]; + }; + $job->app->log->on(message => $job_logger); + # Pass a fake controller object that will work for the grader. my $grader = $ce->{LTIVersion} eq 'v1p1' @@ -72,14 +79,19 @@ sub run ($job, $userID = '', $setID = '') { } if ($setID && $userID && $ce->{LTIGradeMode} eq 'homework') { - return $job->finish($job->maketext('Updated grades via LTI for user [_1] and set [_2].', $userID, $setID)); + unshift(@messages, $job->maketext('Updated grades via LTI for user [_1] and set [_2].', $userID, $setID)); } elsif ($setID && $ce->{LTIGradeMode} eq 'homework') { - return $job->finish($job->maketext('Updated grades via LTI all users assigned to set [_1].', $setID)); + unshift(@messages, $job->maketext('Updated grades via LTI all users assigned to set [_1].', $setID)); } elsif ($userID) { - return $job->finish($job->maketext('Updated grades via LTI of all sets assigned to user [_1].', $userID)); + unshift(@messages, $job->maketext('Updated grades via LTI of all sets assigned to user [_1].', $userID)); } else { - return $job->finish($job->maketext('Updated grades via LTI for all sets and users.')); + unshift(@messages, $job->maketext('Updated grades via LTI for all sets and users.')); } + + $job->app->log->unsubscribe(message => $job_logger); + + $job->app->log->info($messages[0]); + return $job->finish(@messages > 1 ? \@messages : $messages[0]); } sub maketext ($job, @args) { diff --git a/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm b/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm index 9ae99c08eb..299911e1c5 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/SendInstructorEmail.pm @@ -43,11 +43,13 @@ sub run ($job, $mail_data) { if ($@) { push(@result_messages, $job->maketext('An error occurred while trying to send email.'), - $job->maketext('The error message is:'), - ref($@) ? $@->message : $@); + $job->maketext('The error message is: [_1]', ref($@) ? $@->message : $@), + ); + $job->app->log->error($_) for @result_messages; return $job->fail(\@result_messages); } + $job->app->log->error($_) for @result_messages; return $job->finish(\@result_messages); } From 7586fb0fdd895a974d4cb8264a5c235a308ce8f1 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 27 Oct 2023 18:18:12 -0400 Subject: [PATCH 4/6] Switch to a modal dialog for showing the job results. Also use a list group instead of a native list for the lines of the job result. --- htdocs/js/JobManager/jobmanager.js | 9 ---- htdocs/js/JobManager/jobmanager.scss | 8 ---- .../Instructor/JobManager.html.ep | 46 +++++++++++-------- .../HelpFiles/InstructorJobManager.html.ep | 3 +- 4 files changed, 30 insertions(+), 36 deletions(-) delete mode 100644 htdocs/js/JobManager/jobmanager.scss diff --git a/htdocs/js/JobManager/jobmanager.js b/htdocs/js/JobManager/jobmanager.js index ef28c00b1f..fe57f3949a 100644 --- a/htdocs/js/JobManager/jobmanager.js +++ b/htdocs/js/JobManager/jobmanager.js @@ -10,13 +10,4 @@ filter_select.addEventListener('change', toggle_filter_elements); toggle_filter_elements(); } - - // Activate the results popovers. - document.querySelectorAll('.result-popover-btn').forEach((popoverBtn) => { - new bootstrap.Popover(popoverBtn, { - trigger: 'hover focus', - customClass: 'job-queue-result-popover', - html: true - }); - }); })(); diff --git a/htdocs/js/JobManager/jobmanager.scss b/htdocs/js/JobManager/jobmanager.scss deleted file mode 100644 index e5ed920f3a..0000000000 --- a/htdocs/js/JobManager/jobmanager.scss +++ /dev/null @@ -1,8 +0,0 @@ -.job-queue-result-popover { - --bs-popover-max-width: 500px; - - .popover-body { - overflow-y: auto; - max-height: 25vh; - } -} diff --git a/templates/ContentGenerator/Instructor/JobManager.html.ep b/templates/ContentGenerator/Instructor/JobManager.html.ep index 3429042708..0d2c1277bc 100644 --- a/templates/ContentGenerator/Instructor/JobManager.html.ep +++ b/templates/ContentGenerator/Instructor/JobManager.html.ep @@ -1,9 +1,5 @@ % use WeBWorK::Utils qw(getAssetURL); % -% content_for css => begin - <%= stylesheet getAssetURL($ce, 'js/JobManager/jobmanager.css') =%> -% end -% % content_for js => begin <%= javascript getAssetURL($ce, 'js/ActionTabs/actiontabs.js'), defer => undef =%> <%= javascript getAssetURL($ce, 'js/SelectAll/selectall.js'), defer => undef =%> @@ -187,25 +183,39 @@
    <%= maketext($jobs->{$jobID}{state}) =%> % if (defined $jobs->{$jobID}{result}) { - % content_for "result_$jobID", begin - % if (ref($jobs->{$jobID}{result}) eq 'ARRAY') { -
      - % for (@{ $jobs->{$jobID}{result} } ) { -
    • <%= $_ %>
      • - % } -
    - % } else { - <%= $jobs->{$jobID}{result} =%> - % } - % end - xml_escape %>"> + "> +
    " tabindex="-1" + aria-labelledby="<%= "result-$jobID-label" %>" aria-hidden="true"> +
    +
    +
    +

    "> + <%= maketext('Result for job [_1]', $jobID) %> +

    + +
    +
    + % if (ref($jobs->{$jobID}{result}) eq 'ARRAY') { +
      + % for (@{ $jobs->{$jobID}{result} } ) { +
    • <%= $_ %>
      • + % } +
    + % } else { + <%= $jobs->{$jobID}{result} =%> + % } +
    +
    +
    +
    % }
    diff --git a/templates/HelpFiles/InstructorJobManager.html.ep b/templates/HelpFiles/InstructorJobManager.html.ep index 3ddf3325a0..fafb9acf8d 100644 --- a/templates/HelpFiles/InstructorJobManager.html.ep +++ b/templates/HelpFiles/InstructorJobManager.html.ep @@ -54,7 +54,8 @@ . 'executed. If a job is "finished" it means that the execution of the job has successfully ' . 'completed. If a job is "failed" it means that the execution of job has completed, but there were ' . 'errors in the execution of the job. If the job is in the "finished" or "failed" state, then there ' - . 'will also be a popover containing information about what happened when the job was executed.') =%> + . 'will also be a button which when clicked will show a dialog containing information about what happened ' + . 'when the job was executed.') =%> % From c208b257d94a161e086c4160e36bc447d734e0bd Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Fri, 27 Oct 2023 05:38:57 -0500 Subject: [PATCH 5/6] Send some of the grade passback messages when post processing is in effect so that the job queue result is more thorough. --- lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm | 25 +++++++++++++------ .../Authen/LTIAdvantage/SubmitGrade.pm | 2 +- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm index 8c010dec49..27bef22b35 100644 --- a/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvanced/SubmitGrade.pm @@ -122,9 +122,10 @@ async sub submit_course_grade ($self, $userID) { my $user = $db->getUser($userID); return 0 unless $user; - $self->warning("submitting all grades for user: $userID") if $ce->{debug_lti_grade_passback}; + $self->warning("submitting all grades 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}; + 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))); } @@ -140,9 +141,10 @@ async sub submit_set_grade ($self, $userID, $setID) { my $userSet = $db->getMergedSet($userID, $setID); - $self->warning("Submitting grade for user $userID and set $setID.") if $ce->{debug_lti_grade_passback}; + $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}; + if !$userSet->lis_source_did && ($ce->{debug_lti_grade_passback} || $self->{post_processing_mode}); return await $self->submit_grade( $userSet->lis_source_did, @@ -229,7 +231,8 @@ EOS $bodyhash .= '='; } - $self->warning("Retrieving prior grade using sourcedid: $sourcedid") if $ce->{debug_lti_parameters}; + $self->warning("Retrieving prior grade using sourcedid: $sourcedid") + if $ce->{debug_lti_parameters} || $self->{post_processing_mode}; my $requestGen = Net::OAuth->request('consumer'); @@ -303,7 +306,7 @@ EOS if $ce->{debug_lti_grade_passback}; $self->warning('LMS grade will NOT be updated - grade unchanged. ' . "Old score: $oldScore; New score: $score") - if ($ce->{debug_lti_grade_passback}); + 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") @@ -314,7 +317,7 @@ EOS $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) - if ($ce->{debug_lti_grade_passback}); + 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); @@ -412,17 +415,23 @@ EOS my $message = $1; $self->warning("result is: $message") if $ce->{debug_lti_grade_passback}; if ($message ne 'success') { - debug("Unable to update LMS grade $sourcedid . LMS responded with message: $message"); + $self->warning("Unable to update LMS grade $sourcedid. LMS responded with message: $message") + if $self->{post_processing_mode}; + debug("Unable to update LMS grade $sourcedid. LMS responded with message: $message"); return 0; } else { # If we got here, we got successes from both the post and the lms. debug("Successfully updated LMS grade $sourcedid. LMS responded with message: $message"); } } else { + $self->warning("Unable to update LMS grade $sourcedid. Error: " . $response->message) + if $self->{post_processing_mode}; debug("Unable to update LMS grade $sourcedid. Error: " . $response->message); debug($response->body); return 0; } + $self->warning("Success submitting grade using sourcedid: $sourcedid and score: $score") + if $self->{post_processing_mode}; debug("Success submitting grade using sourcedid: $sourcedid and score: $score"); return 1; diff --git a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm index b4eb475b35..b5f89e5a90 100644 --- a/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm +++ b/lib/WeBWorK/Authen/LTIAdvantage/SubmitGrade.pm @@ -50,7 +50,7 @@ sub new ($invocant, $c, $post_processing_mode = 0) { # is set, but these warnings are always sent to the debug log if debugging is enabled. sub warning ($self, $warning) { debug($warning); - return unless $self->{c}{ce}{debug_lti_grade_passback}; + return unless $self->{c}{ce}{debug_lti_grade_passback} || $self->{post_processing_mode}; if ($self->{post_processing_mode}) { $self->{c}{app}->log->info($warning); From 4580b98ed5e4c9d1a09728851f8fc0f53ff6e7be Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Mon, 4 Dec 2023 18:14:24 -0600 Subject: [PATCH 6/6] Add a task name for the AchievementNotification.pm task and other minor cleanup. Also use the notes to pass the course id as in the other tasks. --- .../WeBWorK/Tasks/AchievementNotification.pm | 21 ++++++++++--------- lib/WeBWorK/AchievementEvaluator.pm | 4 ++-- .../AchievementNotificationEditor.pm | 4 +++- .../ContentGenerator/Instructor/JobManager.pm | 5 +++-- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm b/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm index 810a59d15f..a030e7ee1c 100644 --- a/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm +++ b/lib/Mojolicious/WeBWorK/Tasks/AchievementNotification.pm @@ -31,30 +31,31 @@ use WeBWorK::SafeTemplate; # send student notification that they have earned an achievement sub run ($job, $mail_data) { - my $ce = eval { WeBWorK::CourseEnvironment->new({ courseName => $mail_data->{courseName} }); }; - return $job->fail("Could not construct course environment for $mail_data->{courseName}.") + my $courseID = $job->info->{notes}{courseID}; + + my $ce = eval { WeBWorK::CourseEnvironment->new({ courseName => $courseID }); }; + return $job->fail("Could not construct course environment for $courseID.") unless $ce; - $job->{language_handle} = - WeBWorK::Localize::getLoc($ce->{language} || 'en'); + $job->{language_handle} = WeBWorK::Localize::getLoc($ce->{language} || 'en'); my $db = WeBWorK::DB->new($ce->{dbLayout}); - return $job->fail($job->maketext("Could not obtain database connection for [_1].", $mail_data->{courseName})) + return $job->fail($job->maketext('Could not obtain database connection for [_1].', $courseID)) unless $db; - return $job->fail($job->maketext("Cannot notify student without an achievement.")) + return $job->fail($job->maketext('Cannot notify student without an achievement.')) unless $mail_data->{achievementID}; $mail_data->{achievement} = $db->getAchievement($mail_data->{achievementID}); - return $job->fail($job->maketext("Could not find achievement [_1].", $mail_data->{achievementID})) + return $job->fail($job->maketext('Could not find achievement [_1].', $mail_data->{achievementID})) unless $mail_data->{achievement}; my $result_message = eval { $job->send_achievement_notification($ce, $db, $mail_data) }; if ($@) { - $job->app->log->error($job->maketext("An error occurred while trying to send email: $@")); - return $job->fail($job->maketext("An error occurred while trying to send email: [_1]", $@)); + $job->app->log->error("An error occurred while trying to send email: $@"); + return $job->fail($job->maketext('An error occurred while trying to send email: [_1]', $@)); } $job->app->log->info("Message sent to $mail_data->{recipient}"); - return $job->finish($job->maketext("Message sent to [_1]", $mail_data->{recipient})); + return $job->finish($result_message); } sub send_achievement_notification ($job, $ce, $db, $mail_data) { diff --git a/lib/WeBWorK/AchievementEvaluator.pm b/lib/WeBWorK/AchievementEvaluator.pm index bd72708f12..812e161a81 100644 --- a/lib/WeBWorK/AchievementEvaluator.pm +++ b/lib/WeBWorK/AchievementEvaluator.pm @@ -257,13 +257,13 @@ sub checkForAchievements ($problem_in, $pg, $c, %options) { send_achievement_email => [ { recipient => $user_id, subject => 'Congratulations on earning a new achievement!', - courseName => $ce->{courseName}, achievementID => $achievement_id, setID => $set_id, nextLevelPoints => $nextLevelPoints || 0, pointsEarned => $achievementPoints, remote_host => $c->tx->remote_address || "UNKNOWN", - } ] + } ], + { notes => { courseID => $ce->{courseName} } } ) if ($ce->{mail}{achievementEmailFrom} && $achievement->email_template); } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm b/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm index 9f26f87011..01abffb81f 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/AchievementNotificationEditor.pm @@ -131,7 +131,7 @@ sub saveFileChanges ($c, $outputFilePath) { print $OUTPUTFILE $c->stash('achievementNotification'); close $OUTPUTFILE; }; - my $writeFileErrors = $@ if $@; + my $writeFileErrors = $@; # Catch errors in saving files, if ($writeFileErrors) { @@ -290,6 +290,8 @@ sub disable_handler ($c) { $c->stash('achievementID') )); } + + return; } 1; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm index 2b7ee3b0e9..e128e70cf6 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/JobManager.pm @@ -28,8 +28,9 @@ use constant ACTION_FORMS => [ [ filter => x('Filter') ], [ sort => x('Sort') ], # All tasks added in the Mojolicious::WeBWorK module need to be listed here. use constant TASK_NAMES => { - lti_mass_update => x('LTI Mass Update'), - send_instructor_email => x('Send Instructor Email') + lti_mass_update => x('LTI Mass Update'), + send_instructor_email => x('Send Instructor Email'), + send_achievement_email => x('Send Achiement Email') }; # This constant is not used. It is here so that gettext adds these strings to the translation files.
    - <%= check_box 'select-all' => 'on', id => 'select-all', - 'aria-label' => maketext('Select all jobs'), - data => { select_group => 'selected_jobs' }, - class => 'select-all form-check-input' =%> + + <%= label_for 'select-all', begin =%> + <%= check_box 'select-all' => 'on', id => 'select-all', + class => 'select-all form-check-input set-id-tooltip', + 'aria-label' => maketext('Select all jobs'), + data => { + select_group => 'selected_jobs', + bs_toggle => 'tooltip', + bs_placement => 'right', + bs_title => maketext('Select all jobs') + } =%> + + <% end =%> - <%= label_for 'select-all' => - link_to maketext('Id') => '#', class => 'sort-header', data => { sort_field => 'id' } =%> +
    + <%= link_to maketext('Id') => '#', class => 'sort-header', + data => { sort_field => 'id' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'id' =%> +
    		- <%= link_to maketext('Course Id') => '#', class => 'sort-header', - data => { sort_field => 'courseID' } =%> +
    + <%= link_to maketext('Course Id') => '#', class => 'sort-header', + data => { sort_field => 'courseID' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', + field => 'course_id' =%> +
    		- <%= link_to maketext('Task') => '#', class => 'sort-header', - data => { sort_field => 'task' } =%> +
    + <%= link_to maketext('Task') => '#', class => 'sort-header', + data => { sort_field => 'task' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'task' =%> +
    		- <%= link_to maketext('Created') => '#', class => 'sort-header', - data => { sort_field => 'created' } =%> +
    + <%= link_to maketext('Created') => '#', class => 'sort-header', + data => { sort_field => 'created' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'created' =%> +
    		- <%= link_to maketext('Started') => '#', class => 'sort-header', - data => { sort_field => 'started' } =%> +
    + <%= link_to maketext('Started') => '#', class => 'sort-header', + data => { sort_field => 'started' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'started' =%> +
    		- <%= link_to maketext('Finished') => '#', class => 'sort-header', - data => { sort_field => 'finished' } =%> +
    + <%= link_to maketext('Finished') => '#', class => 'sort-header', + data => { sort_field => 'finished' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'finished' =%> +
    		- <%= link_to maketext('State') => '#', class => 'sort-header', - data => { sort_field => 'state' } =%> +
    + <%= link_to maketext('State') => '#', class => 'sort-header', + data => { sort_field => 'state' } =%> + <%= include 'ContentGenerator/Instructor/JobManager/sort_button', field => 'state' =%> +