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.
somiaj · Nov 12, 2024 · 6f46f8b · 6f46f8b
1 parent bba6bf2
commit 6f46f8b
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 12 deletions.
diff --git 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
@@ -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' %>

diff --git a/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep b/templates/ContentGenerator/CourseAdmin/manage_otp_secrets_form.html.ep
@@ -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') {
@@ -136,7 +147,7 @@
 									disabled                     => undef,
 									selected                     => undef,
 								],
-								@course_keys
+								sort (@course_keys, @$skipped_courses)
 							],
 							id       => 'destMultipleCourseID',
 							class    => 'form-select',
@@ -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 %>
diff --git a/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep b/templates/ContentGenerator/CourseAdmin/reset_otp_secrets_confirm.html.ep
@@ -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' %>

diff --git 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.') =%>
 </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>