Skip to content

Commit

Permalink
Only show small courses initially when managing OTP secrets.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
somiaj committed Nov 23, 2024
1 parent f3e0453 commit 3d7ab27
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 13 deletions.
25 changes: 19 additions & 6 deletions lib/WeBWorK/ContentGenerator/CourseAdmin.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -2388,14 +2388,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 ];
$dbs->{$courseID} = WeBWorK::DB->new($ce->{dbLayouts}{ $ce->{dbLayoutName} });

# 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.
Expand Down Expand Up @@ -2437,7 +2446,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
</tbody>
</table>
</div>
<%= $c->hidden_fields('subDisplay') =%>
<%= $c->hidden_fields('subDisplay', 'show_all_courses') =%>
% if ($total > 0) {
% my $skipped = @$action_rows - $total;
<%= content 'hidden-rows' %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,17 @@
%
<h2><%= maketext('Manage OTP Secrets') %> <%= $c->helpMacro('AdminManageOTPSecrets') %></h2>
<%= form_for current_route, method => 'POST', begin =%>
% if (@$skipped_courses) {
<div class="mb-3">
<div class="alert alert-warning mb-2">
<%= maketext('The following large courses are not shown: [_1]', join(', ', @$skipped_courses)); =%>
</div>
<p>
<%= submit_button maketext('Show All Courses'),
name => 'show_all_courses', class => 'btn btn-primary' =%>
</p>
</div>
% }
<div>
<ul class="nav nav-tabs mb-2" role="tablist">
% for my $tab ('single', 'multiple', 'reset') {
Expand Down Expand Up @@ -136,7 +147,7 @@
disabled => undef,
selected => undef,
],
@course_keys
sort (@course_keys, @$skipped_courses)
],
id => 'destMultipleCourseID',
class => 'form-select',
Expand Down Expand Up @@ -184,14 +195,14 @@
</div>
</div>
</div>
<%= $c->hidden_fields('subDisplay') =%>
<%= $c->hidden_fields('subDisplay', 'show_all_courses') =%>
<%= hidden_field action => $active_tab, id => 'current_action' =%>
<div class="mb-3">
<%= submit_button $active_tab eq 'single'
? maketext('Copy Single Secret')
: $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' =%>
</div>
<%= end %>
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@
</tbody>
</table>
</div>
<%= $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' %>
Expand Down
7 changes: 6 additions & 1 deletion templates/HelpFiles/AdminManageOTPSecrets.html.ep
Original file line number Diff line number Diff line change
Expand Up @@ -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.') =%>
</p>
<p class="mb-0">
<p>
<%= 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.') =%>
</p>
<p class="mb-0">
<%= 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.') =%>
</p>

0 comments on commit 3d7ab27

Please sign in to comment.