From ec81ac097b6da3ae5a14285764d0933242b15b8f Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sat, 9 Nov 2024 08:11:12 -0700 Subject: [PATCH] Updates to adding users to newly created courses. When adding a course on the course admin page, list all users in the admin course and let the admin select which users to copy over to the new course (by default all admin users are selected). This way an admin can select an existing user to copy over which can include any password or OTP secrets setup for that use (currently the admin will have to copy that over to the admin course manually). This also allows only copying a subset of admins to a newly created course instead of all. When creating a new user to add to a course, be consistent with adding a user on the accounts management page, which doesn't require a password (useful when using external auth), email, etc. Now the only required field is the new user ID. Flag the new user password input as 'new-password', so webbrowsers don't try to auto fill it in. Add the student ID as a field they can fill out instead of making it equal to the user ID. Since users can be copied from the admin course, add an option to add the new user to the admin course (as a dropped user so they can't login) so admins can select the same user to add to future courses as needed. Remove the empty list of sets to assign users to when adding new users on the accounts management page in the admin course. Make it so all users are shown on the Email page in the admin course, since most instructors are "Dropped" to still allow sending them emails. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 88 ++++++++++--------- .../ContentGenerator/Instructor/SendMail.pm | 5 +- .../CourseAdmin/add_course_form.html.ep | 35 +++++--- .../Instructor/AddUsers.html.ep | 10 ++- templates/HelpFiles/AdminAddCourse.html.ep | 12 +-- 5 files changed, 87 insertions(+), 63 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index b8274bbf95..2205d09f69 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -240,9 +240,6 @@ sub add_course_validate ($c) { my $add_initial_userID = trim_spaces($c->param('add_initial_userID')) || ''; my $add_initial_password = trim_spaces($c->param('add_initial_password')) || ''; my $add_initial_confirmPassword = trim_spaces($c->param('add_initial_confirmPassword')) || ''; - my $add_initial_firstName = trim_spaces($c->param('add_initial_firstName')) || ''; - my $add_initial_lastName = trim_spaces($c->param('add_initial_lastName')) || ''; - my $add_initial_email = trim_spaces($c->param('add_initial_email')) || ''; my $add_dbLayout = trim_spaces($c->param('add_dbLayout')) || ''; my @errors; @@ -260,25 +257,11 @@ sub add_course_validate ($c) { push @errors, $c->maketext('Course ID cannot exceed [_1] characters.', $ce->{maxCourseIdLength}); } - if ($add_initial_userID ne '') { - if ($add_initial_password eq '') { - push @errors, $c->maketext('You must specify a password for the initial instructor.'); - } - if ($add_initial_confirmPassword eq '') { - push @errors, $c->maketext('You must confirm the password for the initial instructor.'); - } - if ($add_initial_password ne $add_initial_confirmPassword) { - push @errors, $c->maketext('The password and password confirmation for the instructor must match.'); - } - if ($add_initial_firstName eq '') { - push @errors, $c->maketext('You must specify a first name for the initial instructor.'); - } - if ($add_initial_lastName eq '') { - push @errors, $c->maketext('You must specify a last name for the initial instructor.'); - } - if ($add_initial_email eq '') { - push @errors, $c->maketext('You must specify an email address for the initial instructor.'); - } + if ($add_initial_userID ne '' + && $add_initial_password ne '' + && $add_initial_password ne $add_initial_confirmPassword) + { + push @errors, $c->maketext('The password and password confirmation for the instructor must match.'); } if ($add_dbLayout eq '') { @@ -310,6 +293,8 @@ sub do_add_course ($c) { my $add_initial_firstName = trim_spaces($c->param('add_initial_firstName')) // ''; my $add_initial_lastName = trim_spaces($c->param('add_initial_lastName')) // ''; my $add_initial_email = trim_spaces($c->param('add_initial_email')) // ''; + my $add_initial_studentID = trim_spaces($c->param('add_initial_studentID')) // ''; + my $add_initial_user = $c->param('add_initial_user') // 0; my $copy_from_course = trim_spaces($c->param('copy_from_course')) // ''; @@ -322,25 +307,28 @@ sub do_add_course ($c) { my @users; # copy users from current (admin) course if desired - if ($c->param('add_admin_users')) { - for my $userID ($db->listUsers) { - if ($userID eq $add_initial_userID) { - $c->addbadmessage($c->maketext( - 'User "[_1]" will not be copied from [_2] course as it is the initial instructor.', $userID, - $ce->{admin_course_id} - )); - next; - } - my $PermissionLevel = $db->newPermissionLevel(); - $PermissionLevel->user_id($userID); - $PermissionLevel->permission($ce->{userRoles}{admin}); - my $User = $db->getUser($userID); - my $Password = $db->getPassword($userID); - $User->status('O'); # Add admin user as an observer. - - push @users, [ $User, $Password, $PermissionLevel ] - if $authz->hasPermissions($userID, 'create_and_delete_courses'); + for my $userID ($c->param('add-admin-users')) { + unless ($db->existsUser($userID)) { + $c->addbadmessage($c->maketext( + 'User "[_1]" will not be copied from [_2] course as it does not exist.', $userID, + $ce->{admin_course_id} + )); + next; } + if ($userID eq $add_initial_userID) { + $c->addbadmessage($c->maketext( + 'User "[_1]" will not be copied from [_2] course as it is the initial instructor.', $userID, + $ce->{admin_course_id} + )); + next; + } + + my $PermissionLevel = $db->getPermissionLevel($userID); + my $User = $db->getUser($userID); + my $Password = $db->getPassword($userID); + $User->status('O'); # Add admin user as an observer. + + push @users, [ $User, $Password, $PermissionLevel ]; } # add initial instructor if desired @@ -349,19 +337,35 @@ sub do_add_course ($c) { user_id => $add_initial_userID, first_name => $add_initial_firstName, last_name => $add_initial_lastName, - student_id => $add_initial_userID, + student_id => $add_initial_studentID, email_address => $add_initial_email, status => 'O', ); my $Password = $db->newPassword( user_id => $add_initial_userID, - password => cryptPassword($add_initial_password), + password => $add_initial_password ? cryptPassword($add_initial_password) : '', ); my $PermissionLevel = $db->newPermissionLevel( user_id => $add_initial_userID, permission => '10', ); push @users, [ $User, $Password, $PermissionLevel ]; + + # Add initial user to admin course if asked. + if ($add_initial_user) { + if ($db->existsUser($add_initial_userID)) { + $c->addbadmessage($c->maketext( + 'User "[_1]" will not be added to [_2] course as it already exists.', $add_initial_userID, + $ce->{admin_course_id} + )); + } else { + $User->status('D'); # By default don't allow user to login. + $db->addUser($User); + $db->addPassword($Password); + $db->addPermissionLevel($PermissionLevel); + $User->status('O'); + } + } } push @{ $courseOptions{PRINT_FILE_NAMES_FOR} }, map { $_->[0]->user_id } @users; diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm index 723e922758..1b0c86a2d1 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm @@ -113,8 +113,9 @@ sub initialize ($c) { 'user_id' ); - # Filter out users who don't get included in email - @Users = grep { $ce->status_abbrev_has_behavior($_->status, "include_in_email") } @Users; + # Filter out users who don't get included in email unless in the admin course. + @Users = grep { $ce->status_abbrev_has_behavior($_->status, "include_in_email") } @Users + unless $ce->{courseName} eq $ce->{admin_course_id}; # Cache the user records for later use. $c->{ra_user_records} = \@Users; diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 5d258f7ab9..1601558a99 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -39,13 +39,14 @@
<%= maketext( - 'To add the WeBWorK administrators to the new course (as administrators) check the box below.') =%> + 'Select admin course users to add to the new course (as the stated permission) below.') =%>
-
- +
+ <%= select_field 'add-admin-users' => [ map { + my $val = $db->getPermissionLevel($_)->permission; + my $level = maketext((grep { $ce->{userRoles}{$_} eq $val } keys %{ $ce->{userRoles} })[0]); + [ "$_ ($level)" => $_, $val == 20 ? (selected => undef) : () ] } $db->listUsers ], + size => 5, multiple => undef, class =>'form-select mb-1' =%>
@@ -54,7 +55,7 @@ . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas,and underscores.' ) =%>
-
+
<%= text_field add_initial_userID => '', @@ -65,9 +66,10 @@
<%= password_field 'add_initial_password', - id => 'add_initial_password', - placeholder => '', - class => 'form-control' =%> + id => 'add_initial_password', + placeholder => '', + class => 'form-control', + autocomplete => 'new-password' =%> <%= label_for add_initial_password => maketext('Password') =%>
@@ -100,8 +102,21 @@ class => 'form-control' =%> <%= label_for add_initial_email => maketext('Email Address') =%>
+
+ <%= text_field add_initial_studentID => '', + id => 'add_initial_studentID', + placeholder => '', + class => 'form-control' =%> + <%= label_for add_initial_studentID => maketext('Student ID') =%> +
+
+ +
<%= maketext('To copy components from an existing course, ' . 'select the course and check which components to copy.') =%> diff --git a/templates/ContentGenerator/Instructor/AddUsers.html.ep b/templates/ContentGenerator/Instructor/AddUsers.html.ep index 0cd6147035..a23ce58a5d 100644 --- a/templates/ContentGenerator/Instructor/AddUsers.html.ep +++ b/templates/ContentGenerator/Instructor/AddUsers.html.ep @@ -148,9 +148,11 @@
-

<%= maketext('Select sets below to assign them to the newly-created users.') %>

- % param('assignSets', undef); - <%= select_field assignSets => [ map { [ format_set_name_display($_) => $_ ] } $db->listGlobalSets ], - size => 10, multiple => undef, class => 'form-select w-auto mb-2' =%> + % unless ($ce->{courseName} eq $ce->{admin_course_id}) { +

<%= maketext('Select sets below to assign them to the newly-created users.') %>

+ % param('assignSets', undef); + <%= select_field assignSets => [ map { [ format_set_name_display($_) => $_ ] } $db->listGlobalSets ], + size => 10, multiple => undef, class => 'form-select w-auto mb-2' =%> + % }

<%= submit_button maketext('Add Users'), name => 'addStudents', class => 'btn btn-primary' =%>

<% end =%> diff --git a/templates/HelpFiles/AdminAddCourse.html.ep b/templates/HelpFiles/AdminAddCourse.html.ep index 90414aa727..1c4940b51a 100644 --- a/templates/HelpFiles/AdminAddCourse.html.ep +++ b/templates/HelpFiles/AdminAddCourse.html.ep @@ -22,13 +22,15 @@ . 'on the course home page.') =%>

- <%= maketext('The WeBWorK administrators need to be added to the course in order for them to access the course. ' - . 'Unchecking this option will make it so WeBWorK administrators cannot directly login to the course.') =%> + <%= maketext('Select which users in the admin course to copy over to the newly created course. The WeBWorK ' + . 'administrators need to be added to the course in order for them to access the course. Unselecting ' + . 'them will make it so WeBWorK administrators cannot directly login to the course.') =%>

- <%= maketext('Enter the details of the course instructor to be added when the course is created. This user ' - . 'will also be added to the admin course with the username "userID_courseID" so you can manage and ' - . 'email the instructors of the courses on the server.') =%> + <%= maketext('Enter the details of a new course instructor to be added when the course is created. The only ' + . 'required field is the user ID of the this user. Optionally, you can add this user to the administration ' + . 'course, so you can copy this user when creating future courses, or manage and email course instructors. ' + . 'The instructor will be added as a "Dropped" user so they cannot login to the administration course.') =%>

<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. '