From 6f46f8b7bfc08f58da5ded5fb74e6f2e8c5bd664 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Mon, 11 Nov 2024 18:04:18 -0700 Subject: [PATCH] Only show small courses initially when managing OTP secrets. Large courses can cause a lot of CPU load when the javascript generates the menus to select users of the course. Also large courses will be hard to navigate with using the menus. By default courses with 200 or more users will not be listed in the drop down menus. If any course is not included, a warning will state which courses have been ignored and a button to show all courses will appear. The ignored courses are still listed in the list of destination courses when copying multiple secrets. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 23 +++++++++++++++---- .../copy_otp_secrets_confirm.html.ep | 2 +- .../manage_otp_secrets_form.html.ep | 17 +++++++++++--- .../reset_otp_secrets_confirm.html.ep | 3 +-- .../HelpFiles/AdminManageOTPSecrets.html.ep | 7 +++++- 5 files changed, 40 insertions(+), 12 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 66a0ed95f5..635e52fec3 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -214,7 +214,7 @@ sub pre_header_initialize ($c) { $method_to_call = 'manage_lti_course_map_form'; } } elsif ($subDisplay eq 'manage_otp_secrets') { - if (defined $c->param('action')) { + if (defined $c->param('take_action')) { if ($c->param('action') eq 'reset') { $method_to_call = 'reset_otp_secrets_confirm'; } else { @@ -2361,14 +2361,23 @@ sub do_save_lti_course_map ($c) { # Form to copy or reset OTP secrets. sub manage_otp_secrets_form ($c) { - my $courses = {}; - my $dbs = {}; + my $courses = {}; + my $dbs = {}; + my $skipped_courses = []; + my $show_all_courses = $c->param('show_all_courses') || 0; # Create course data first, since it is used in all cases and initializes course db references. for my $courseID (listCourses($c->ce)) { my $ce = WeBWorK::CourseEnvironment->new({ courseName => $courseID }); $dbs->{$courseID} = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} }); - $courses->{$courseID} = [ $dbs->{$courseID}->listUsers ]; + + # By default ignore courses larger than 200 users, as this can cause a large load building menus. + my @users = $dbs->{$courseID}->listUsers; + if ($show_all_courses || scalar @users < 200) { + $courses->{$courseID} = \@users; + } else { + push(@$skipped_courses, $courseID); + } } # Process the confirmed rest or copy actions here. @@ -2410,7 +2419,11 @@ sub manage_otp_secrets_form ($c) { } } - return $c->include('ContentGenerator/CourseAdmin/manage_otp_secrets_form', courses => $courses); + return $c->include( + 'ContentGenerator/CourseAdmin/manage_otp_secrets_form', + courses => $courses, + skipped_courses => $skipped_courses + ); } # Deals with both single and multiple copy confirmation. diff --git a/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep index 4b72b3ee3b..81c8a3d3d9 100644 --- a/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/copy_otp_secrets_confirm.html.ep @@ -40,7 +40,7 @@ - <%= $c->hidden_fields('subDisplay') =%> + <%= $c->hidden_fields('subDisplay', 'show_all_courses') =%> % if ($total > 0) { % my $skipped = @$action_rows - $total; <%= content 'hidden-rows' %> diff --git a/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep b/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep index a5d65ac48e..6d10a86335 100644 --- a/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep @@ -8,6 +8,17 @@ %

<%= maketext('Manage OTP Secrets') %> <%= $c->helpMacro('AdminManageOTPSecrets') %>

<%= form_for current_route, method => 'POST', begin =%> + % if (@$skipped_courses) { +
+
+ <%= maketext('The following large courses are not shown: [_1]', join(', ', @$skipped_courses)); =%> +
+

+ <%= submit_button maketext('Show All Courses'), + name => 'show_all_courses', class => 'btn btn-primary' =%> +

+
+ % }
- <%= $c->hidden_fields('subDisplay') =%> + <%= $c->hidden_fields('subDisplay', 'show_all_courses') =%> <%= hidden_field action => $active_tab, id => 'current_action' =%>
<%= submit_button $active_tab eq 'single' @@ -192,6 +203,6 @@ : $active_tab eq 'multiple' ? maketext('Copy Multiple Secrets') : maketext('Reset Secrets'), - id => 'take_action', class => 'btn btn-primary' =%> + id => 'take_action', name => 'take_action', class => 'btn btn-primary' =%>
<%= end %> diff --git a/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep index 18564b1607..d804518ee5 100644 --- a/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep +++ b/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep @@ -28,8 +28,7 @@ - <%= $c->hidden_fields('subDisplay') =%> - <%= $c->hidden_fields('sourceResetCourseID') =%> + <%= $c->hidden_fields('subDisplay', 'show_all_courses', 'sourceResetCourseID') =%> % if ($total > 0) { % my $skipped = @$action_rows - $total; <%= content 'hidden-rows' %> diff --git a/templates/HelpFiles/AdminManageOTPSecrets.html.ep b/templates/HelpFiles/AdminManageOTPSecrets.html.ep index 1fbe9e1485..080dbbdc8c 100644 --- a/templates/HelpFiles/AdminManageOTPSecrets.html.ep +++ b/templates/HelpFiles/AdminManageOTPSecrets.html.ep @@ -30,7 +30,12 @@ . 'source users to copy to one (or more) destination courses. This can be used to copy multiple secrets ' . 'at once, but the user IDs must be the same in the source and destination courses.') =%>

-

+

<%= maketext('If resetting secrets, choose a source course ID, then choose one (or more) users whose secrets ' . 'will be reset. Note, the user(s) will have to setup two factor authentication afterwards.') =%>

+

+ <%= maketext('By default, courses larger than 200 users will not be shown, as this can cause extra CPU usage ' + . 'when generating the user menus. If any large course is skipped, a message will inform you of which ' + . 'courses were skipped, and a button to show all courses can be used to include those courses.') =%> +