From 14a6aea393c06bc93c40d8fd9028d277f9333393 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sat, 9 Nov 2024 08:11:12 -0700 Subject: [PATCH 01/12] 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 d4732a0b6e..66d34b7302 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 course 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. ' From 07e3a65556cc03eb291536e2956b31a762f99e35 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sat, 9 Nov 2024 12:45:41 -0700 Subject: [PATCH 02/12] Use checkbox list instead of select for admin users. This way it is harder to unselect the admin users, to avoid accidentally unselected them if ctrl isn't being held down. --- .../CourseAdmin/add_course_form.html.ep | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 1601558a99..3f43ef36bf 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -41,12 +41,18 @@ <%= maketext( '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' =%> +
+ % for my $user ($db->listUsers) { + % my $val = $db->getPermissionLevel($user)->permission; + % my $role = maketext((grep { $ce->{userRoles}{$_} eq $val } keys %{ $ce->{userRoles} })[0]); +
+ +
+ % }
From 9b83f7b4dd7e2a65cb580154bb9976acf0485d8e Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 10 Nov 2024 17:32:08 -0700 Subject: [PATCH 03/12] CourseAdmin add new course tweaks. Use the course_admin_id instead of 'admin' when talking about the admin course name on this page and in its help. Mention setting $permissionLevels{login}='admin' in course.conf in the help as a way to control access to the admin course. Use `overflow-auto` and `max-height` in the select which admin users to add to the new course div. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 6 +++--- .../CourseAdmin/add_course_form.html.ep | 7 ++++--- templates/HelpFiles/AdminAddCourse.html.ep | 13 ++++++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index 66d34b7302..ef25af231b 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -310,14 +310,14 @@ sub do_add_course ($c) { 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, + 'User "[_1]" will not be copied from the [_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, + 'User "[_1]" will not be copied from the [_2] course as it is the initial instructor.', $userID, $ce->{admin_course_id} )); next; @@ -355,7 +355,7 @@ sub do_add_course ($c) { 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, + 'User "[_1]" will not be added to the [_2] course as it already exists.', $add_initial_userID, $ce->{admin_course_id} )); } else { diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 3f43ef36bf..be5831547c 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -39,9 +39,10 @@
<%= maketext( - 'Select admin course users to add to the new course (as the stated permission) below.') =%> + 'Select [_1] course users to add to the new course (as the stated permission) below.', + $ce->{admin_course_id}) =%>
-
+
% for my $user ($db->listUsers) { % my $val = $db->getPermissionLevel($user)->permission; % my $role = maketext((grep { $ce->{userRoles}{$_} eq $val } keys %{ $ce->{userRoles} })[0]); @@ -120,7 +121,7 @@
diff --git a/templates/HelpFiles/AdminAddCourse.html.ep b/templates/HelpFiles/AdminAddCourse.html.ep index 1c4940b51a..c01fc1e330 100644 --- a/templates/HelpFiles/AdminAddCourse.html.ep +++ b/templates/HelpFiles/AdminAddCourse.html.ep @@ -22,15 +22,18 @@ . 'on the course home page.') =%>

- <%= maketext('Select which users in the admin course to copy over to the newly created course. The WeBWorK ' + <%= maketext('Select which users in the [_1] 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.') =%> + . 'them will make it so WeBWorK administrators cannot directly login to the course.', + $ce->{admin_course_id}) =%>

<%= 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.') =%> + . 'required field is the user ID of the this user. Optionally, you can add this user to the [_1] course, ' + . 'so you can copy this user when creating future courses, or manage and email course instructors. Note, ' + . 'by default these new users will be "Dropped" and unable to login to the [_1] course. You can change ' + . 'this on the "Accounts Manager" page. Additionally you can control access to the [_1] course by setting ' + . q/$permissionLevels{login}='admin' in course.conf./, $ce->{admin_course_id}) =%>

<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. ' From beb78ea68900e71aea3c63658e670235e4c7b01a Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 10 Nov 2024 18:06:35 -0700 Subject: [PATCH 04/12] Ensure all admin users stay unchecked after error. When adding a new course, and someone unchecks all admin users to be copied to the new course, ensure they stay unchecked if an error occurs and the page is reloaded. --- .../ContentGenerator/CourseAdmin/add_course_form.html.ep | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index be5831547c..05c05b2bba 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -49,7 +49,7 @@

@@ -211,6 +211,7 @@
- <%= hidden_field add_dbLayout => 'sql_single' =%> + <%= hidden_field add_dbLayout => 'sql_single' =%> + <%= hidden_field last_page_was_add_course => 1 =%> <%= submit_button maketext('Add Course'), name => 'add_course', class => 'btn btn-primary' =%> <% end =%> From 032c3049db57f1aa95b387116516555823ea11a5 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 10 Nov 2024 18:29:22 -0700 Subject: [PATCH 05/12] Remove student ID input from add course page. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 2 -- .../ContentGenerator/CourseAdmin/add_course_form.html.ep | 7 ------- 2 files changed, 9 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index ef25af231b..a217800615 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -293,7 +293,6 @@ 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')) // ''; @@ -337,7 +336,6 @@ sub do_add_course ($c) { user_id => $add_initial_userID, first_name => $add_initial_firstName, last_name => $add_initial_lastName, - student_id => $add_initial_studentID, email_address => $add_initial_email, status => 'O', ); diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 05c05b2bba..3ce6453394 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -109,13 +109,6 @@ 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') =%> -
From 0a19eed6fa3d27ab9b5e40fcf04962d831b2efea Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Mon, 11 Nov 2024 08:09:44 -0700 Subject: [PATCH 06/12] Tweaks to add course form layout. --- .../CourseAdmin/add_course_form.html.ep | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 3ce6453394..fab6ac3c91 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -36,24 +36,26 @@
-
+
<%= maketext( 'Select [_1] course users to add to the new course (as the stated permission) below.', $ce->{admin_course_id}) =%>
-
- % for my $user ($db->listUsers) { - % my $val = $db->getPermissionLevel($user)->permission; - % my $role = maketext((grep { $ce->{userRoles}{$_} eq $val } keys %{ $ce->{userRoles} })[0]); -
- -
- % } +
+
+ % for my $user ($db->listUsers) { + % my $val = $db->getPermissionLevel($user)->permission; + % my $role = maketext((grep { $ce->{userRoles}{$_} eq $val } keys %{ $ce->{userRoles} })[0]); +
+ +
+ % } +
@@ -62,7 +64,7 @@ . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas,and underscores.' ) =%>
-
+
<%= text_field add_initial_userID => '', From c1c92bb0800554600f36e0b5d28dc97801f71ddf Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Wed, 13 Nov 2024 00:33:52 -0700 Subject: [PATCH 07/12] Allow adding multiple initial users when adding a new course. When adding a new course, there is now an 'Add Another Instructor' button which can be used to add more than one new user to the newly created course. Each time this button is hit, the form expands allowing input for a new user. It is possible to set the permission level of each of these users and to keep the form balanced, the student ID can now also be included. This also updates assigning the initial user to sets and achievements. The intention was to assign new instructors to these, so any user (either copied from the admin course or added initially) whose permission is less than admin will be assigned to the sets and achievements. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 161 ++++++++++-------- lib/WeBWorK/Utils/CourseManagement.pm | 10 +- .../CourseAdmin/add_course_form.html.ep | 138 +++++++++------ templates/HelpFiles/AdminAddCourse.html.ep | 14 +- 4 files changed, 191 insertions(+), 132 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index a217800615..f87646c2e5 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -230,17 +230,18 @@ sub pre_header_initialize ($c) { } sub add_course_form ($c) { + $c->param('number_of_additional_users', ($c->param('number_of_additional_users') // 0) + 1) + if $c->param('add_another_instructor'); + return $c->include('ContentGenerator/CourseAdmin/add_course_form'); } sub add_course_validate ($c) { my $ce = $c->ce; - my $add_courseID = trim_spaces($c->param('new_courseID')) || ''; - 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_dbLayout = trim_spaces($c->param('add_dbLayout')) || ''; + my $add_courseID = trim_spaces($c->param('new_courseID')) || ''; + my $number_of_additional_users = $c->param('number_of_additional_users') || 0; + my $add_dbLayout = trim_spaces($c->param('add_dbLayout')) || ''; my @errors; @@ -257,11 +258,24 @@ sub add_course_validate ($c) { push @errors, $c->maketext('Course ID cannot exceed [_1] characters.', $ce->{maxCourseIdLength}); } - 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.'); + for (1 .. $number_of_additional_users) { + my $userID = trim_spaces($c->param("add_initial_userID_$_")) || ''; + my $password = trim_spaces($c->param("add_initial_password_$_")) || ''; + my $confirmPassword = trim_spaces($c->param("add_initial_confirmPassword_$_")) || ''; + + if ($userID ne '') { + unless ($userID =~ /^[\w-.,]*$/) { + push @errors, + $c->maketext( + 'User ID number [_1] may only contain letters, numbers, hyphens, periods, commas, ' + . 'and underscores.', + $_ + ); + } + if ($password ne '' && $password ne $confirmPassword) { + push @errors, $c->maketext('Pasword number [_1] and its password confirmation must match.', $_); + } + } } if ($add_dbLayout eq '') { @@ -283,17 +297,10 @@ sub do_add_course ($c) { my $db = $c->db; my $authz = $c->authz; - my $add_courseID = trim_spaces($c->param('new_courseID')) // ''; - my $add_courseTitle = ($c->param('add_courseTitle') // '') =~ s/^\s*|\s*$//gr; - my $add_courseInstitution = ($c->param('add_courseInstitution') // '') =~ s/^\s*|\s\*$//gr; - - 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_initial_user = $c->param('add_initial_user') // 0; + my $add_courseID = trim_spaces($c->param('new_courseID')) // ''; + my $add_courseTitle = ($c->param('add_courseTitle') // '') =~ s/^\s*|\s*$//gr; + my $add_courseInstitution = ($c->param('add_courseInstitution') // '') =~ s/^\s*|\s\*$//gr; + my $number_of_additional_users = $c->param('number_of_additional_users') || 0; my $copy_from_course = trim_spaces($c->param('copy_from_course')) // ''; @@ -314,12 +321,17 @@ sub do_add_course ($c) { )); next; } - if ($userID eq $add_initial_userID) { - $c->addbadmessage($c->maketext( - 'User "[_1]" will not be copied from the [_2] course as it is the initial instructor.', $userID, - $ce->{admin_course_id} - )); - next; + for (1 .. $number_of_additional_users) { + my $add_initial_userID = trim_spaces($c->param("add_initial_userID_$_")) // ''; + + if ($userID eq $add_initial_userID) { + $c->addbadmessage($c->maketext( + 'User "[_1]" will not be copied from the [_2] course as it is the same as additional user ' + . 'number [_3].', + $userID, $ce->{admin_course_id}, $_ + )); + next; + } } my $PermissionLevel = $db->getPermissionLevel($userID); @@ -330,43 +342,57 @@ sub do_add_course ($c) { push @users, [ $User, $Password, $PermissionLevel ]; } - # add initial instructor if desired - if ($add_initial_userID =~ /\S/) { - my $User = $db->newUser( - user_id => $add_initial_userID, - first_name => $add_initial_firstName, - last_name => $add_initial_lastName, - email_address => $add_initial_email, - status => 'O', - ); - my $Password = $db->newPassword( - user_id => $add_initial_userID, - 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 the [_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'); + # add additional instructors if desired + for (1 .. $number_of_additional_users) { + my $userID = trim_spaces($c->param("add_initial_userID_$_")) // ''; + my $password = trim_spaces($c->param("add_initial_password_$_")) // ''; + my $confirmPassword = trim_spaces($c->param("add_initial_confirmPassword_$_")) // ''; + my $firstName = trim_spaces($c->param("add_initial_firstName_$_")) // ''; + my $lastName = trim_spaces($c->param("add_initial_lastName_$_")) // ''; + my $email = trim_spaces($c->param("add_initial_email_$_")) // ''; + my $studentID = trim_spaces($c->param("add_initial_studentID_$_")) // ''; + my $permissionLevel = trim_spaces($c->param("add_initial_permission_$_")); + my $add_user = $c->param("add_initial_user_$_") // 0; + + if ($userID =~ /\S/) { + my $User = $db->newUser( + user_id => $userID, + first_name => $firstName, + last_name => $lastName, + email_address => $email, + student_id => $studentID, + status => 'O', + ); + my $Password = $db->newPassword( + user_id => $userID, + password => $password ? cryptPassword($password) : '', + ); + my $PermissionLevel = $db->newPermissionLevel( + user_id => $userID, + permission => $permissionLevel, + ); + push @users, [ $User, $Password, $PermissionLevel ]; + + # Add initial user to admin course if asked. + if ($add_user) { + if ($db->existsUser($userID)) { + $c->addbadmessage($c->maketext( + 'User "[_1]" will not be added to the [_2] course as it already exists.', $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; + push @{ $courseOptions{PRINT_FILE_NAMES_FOR} }, + map { $_->[0]->user_id } grep { $_->[2]->permission >= $ce->{userRoles}{professor} } @users; # Include any optional arguments, including a template course and the course title and course institution. my %optional_arguments; @@ -386,11 +412,10 @@ sub do_add_course ($c) { eval { addCourse( - courseID => $add_courseID, - ce => $ce2, - courseOptions => \%courseOptions, - users => \@users, - initial_userID => $add_initial_userID, + courseID => $add_courseID, + ce => $ce2, + courseOptions => \%courseOptions, + users => \@users, %optional_arguments, ); }; @@ -419,11 +444,7 @@ sub do_add_course ($c) { "\tAdded", (defined $add_courseInstitution ? $add_courseInstitution : '(no institution specified)'), (defined $add_courseTitle ? $add_courseTitle : '(no title specified)'), - $add_courseID, - $add_initial_firstName, - $add_initial_lastName, - $add_initial_email, - ) + $add_courseID) ); push( @$output, diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index f8f6450ef9..6ea4eac5db 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -201,7 +201,7 @@ boolean options: =cut sub addCourse { - my (%options) = (initial_userID => '', @_); + my (%options) = @_; for my $key (keys(%options)) { my $value = '####UNDEF###'; @@ -217,7 +217,7 @@ sub addCourse { debug \@users; - my ($initialUser) = grep { $_->[0]{user_id} eq $options{initial_userID} } @users; + my @initialUsers = grep { $_->[2]->permission < $ce->{userRoles}{admin} } @users; # get the database layout out of the options hash my $dbLayoutName = $courseOptions{dbLayoutName}; @@ -407,7 +407,7 @@ sub addCourse { assignSetsToUsers($db, $ce, \@user_sets, [$user_id]); } } - assignSetsToUsers($db, $ce, \@set_ids, [ $initialUser->[0]{user_id} ]) if $initialUser; + assignSetsToUsers($db, $ce, \@set_ids, [ map { $_->[0]{user_id} } @initialUsers ]) if @initialUsers; } # add achievements @@ -416,9 +416,9 @@ sub addCourse { for my $achievement_id (@achievement_ids) { eval { $db->addAchievement($db0->getAchievement($achievement_id)) }; warn $@ if $@; - if ($initialUser) { + for (@initialUsers) { my $userAchievement = $db->newUserAchievement(); - $userAchievement->user_id($initialUser->[0]{user_id}); + $userAchievement->user_id($_->[0]{user_id}); $userAchievement->achievement_id($achievement_id); $db->addUserAchievement($userAchievement); } diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index fab6ac3c91..efb2ee561d 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -1,5 +1,19 @@ % use WeBWorK::Utils::CourseManagement qw(listCourses); % +% # Create an array of permission values for the permission selects. +% my $permissionLevels = []; +% for my $role (sort { $ce->{userRoles}{$a} <=> $ce->{userRoles}{$b} } keys %{ $ce->{userRoles} }) { + % next if $role eq 'nobody'; + % push( + % @$permissionLevels, + % [ + % $c->maketext($role) => $ce->{userRoles}{$role}, + % $role eq 'professor' ? (selected => undef) : () + % ] + % ); +% } +% my $number_of_additional_users = $c->param('number_of_additional_users') || 0; +%

<%= maketext('Add Course') %> <%= $c->helpMacro('AdminAddCourse') %>

% <%= form_for current_route, method => 'POST', begin =%> @@ -60,64 +74,85 @@
<%= maketext( - 'To add an additional instructor to the new course, specify user information below. ' - . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas,and underscores.' + 'To add additional instructor(s) to the new course, specify user information below. ' + . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas, and underscores.' ) =%>
-
-
-
- <%= text_field add_initial_userID => '', - id => 'add_initial_userID', - placeholder => '', - class => 'form-control' =%> - <%= label_for add_initial_userID => maketext('User ID') =%> -
-
- <%= password_field 'add_initial_password', - id => 'add_initial_password', - placeholder => '', - class => 'form-control', - autocomplete => 'new-password' =%> - <%= label_for add_initial_password => maketext('Password') =%> +
+ % for (1 .. $number_of_additional_users) { +
+
+
+ <%= text_field "add_initial_userID_$_" => '', + id => "add_initial_userID_$_", + placeholder => '', + class => 'form-control' =%> + <%= label_for "add_initial_userID_$_" => maketext('User ID') =%> +
+
+ <%= password_field "add_initial_password_$_", + id => "add_initial_password_$_", + placeholder => '', + class => 'form-control', + autocomplete => 'new-password' =%> + <%= label_for "add_initial_password_$_" => maketext('Password') =%> +
+
+ <%= password_field "add_initial_confirmPassword_$_", + id => "add_initial_confirmPassword_$_", + placeholder => '', + class => 'form-control' =%> + <%= label_for "add_initial_confirmPassword_$_" => maketext('Confirm Password') =%> +
+
+ <%= select_field "add_initial_permission_$_" => $permissionLevels, + id => "add_initial_role_$_", + class => 'form-select' =%> + <%= label_for "add_initial_permission_$_" => maketext('Permission Level') =%> +
-
- <%= password_field 'add_initial_confirmPassword', - id => 'add_initial_confirmPassword', - placeholder => '', - class => 'form-control' =%> - <%= label_for add_initial_confirmPassword => maketext('Confirm Password') =%> +
+
+ <%= text_field "add_initial_firstName_$_" => '', + id => "add_initial_firstName_$_", + placeholder => '', + class => 'form-control' =%> + <%= label_for "add_initial_firstName_$_" => maketext('First Name') =%> +
+
+ <%= text_field "add_initial_lastName_$_" => '', + id => "add_initial_lastName_$_", + placeholder => '', + class => 'form-control' =%> + <%= label_for "add_initial_lastName_$_" => maketext('Last Name') %> +
+
+ <%= text_field "add_initial_email_$_" => '', + id => "add_initial_email_$_", + placeholder => '', + 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') =%> +
-
-
- <%= text_field add_initial_firstName => '', - id => 'add_initial_firstName', - placeholder => '', - class => 'form-control' =%> - <%= label_for add_initial_firstName => maketext('First Name') =%> -
-
- <%= text_field add_initial_lastName => '', - id => 'add_initial_lastName', - placeholder => '', - class => 'form-control' =%> - <%= label_for add_initial_lastName => maketext('Last Name') %> -
-
- <%= text_field add_initial_email => '', - id => 'add_initial_email', - placeholder => '', - class => 'form-control' =%> - <%= label_for add_initial_email => maketext('Email Address') =%> -
+
+
+ % }
-
- +
+ <%= submit_button maketext('Add Another Instructor'), name => 'add_another_instructor', + class => 'btn btn-primary' =%>
<%= maketext('To copy components from an existing course, ' @@ -208,5 +243,6 @@ <%= hidden_field add_dbLayout => 'sql_single' =%> <%= hidden_field last_page_was_add_course => 1 =%> + <%= $c->hidden_fields('number_of_additional_users') =%> <%= submit_button maketext('Add Course'), name => 'add_course', class => 'btn btn-primary' =%> <% end =%> diff --git a/templates/HelpFiles/AdminAddCourse.html.ep b/templates/HelpFiles/AdminAddCourse.html.ep index c01fc1e330..734ecae554 100644 --- a/templates/HelpFiles/AdminAddCourse.html.ep +++ b/templates/HelpFiles/AdminAddCourse.html.ep @@ -28,12 +28,14 @@ $ce->{admin_course_id}) =%>

- <%= 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 [_1] course, ' - . 'so you can copy this user when creating future courses, or manage and email course instructors. Note, ' - . 'by default these new users will be "Dropped" and unable to login to the [_1] course. You can change ' - . 'this on the "Accounts Manager" page. Additionally you can control access to the [_1] course by setting ' - . q/$permissionLevels{login}='admin' in course.conf./, $ce->{admin_course_id}) =%> + <%= maketext('Optionally, to add additional instructors click the "Add Another Instructor" button, then 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 [_1] course, so you can copy ' + . 'this user when creating future courses, or manage and email course instructors. Click the "Add Another ' + . 'Instructor" button again to add multiple additional users. Note, by default these new users will be ' + . '"Dropped" and unable to login to the [_1] course. You can change this on the "Accounts Manager" page. ' + . q/Additionally you can control access to the [_1] course by setting $permissionLevels{login}='admin' in / + . 'course.conf.', $ce->{admin_course_id}) =%>

<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. ' From 1c6f9042f7eb43957e02ebb13df99fdd1069665f Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Wed, 13 Nov 2024 12:18:05 -0700 Subject: [PATCH 08/12] Only assign initial users to sets/achievements once. When copying over old sets and achievements only assign the initial users to sets or achievements once. This makes it so any initial user is always assigned to everything, and will ignore (skip over) whatever they were assigned to in the old course. Also assign all initial users (including admins) to sets/achievements. --- lib/WeBWorK/Utils/CourseManagement.pm | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/WeBWorK/Utils/CourseManagement.pm b/lib/WeBWorK/Utils/CourseManagement.pm index 6ea4eac5db..a20f797e5f 100644 --- a/lib/WeBWorK/Utils/CourseManagement.pm +++ b/lib/WeBWorK/Utils/CourseManagement.pm @@ -217,7 +217,8 @@ sub addCourse { debug \@users; - my @initialUsers = grep { $_->[2]->permission < $ce->{userRoles}{admin} } @users; + my @initialUsers = @users; + my %user_args = map { $_->[0]{user_id} => 1 } @users; # get the database layout out of the options hash my $dbLayoutName = $courseOptions{dbLayoutName}; @@ -359,7 +360,6 @@ sub addCourse { { '!=' => $options{copyConfig} ? $ce0->{userRoles}{student} : $ce->{userRoles}{student} }, user_id => { not_like => 'set_id:%' } })); - my %user_args = map { $_->[0]{user_id} => 1 } @users; for my $user_id (@non_student_ids) { next if $user_args{$user_id}; @@ -402,7 +402,8 @@ sub addCourse { } if ($options{copyNonStudents}) { foreach my $userTriple (@users) { - my $user_id = $userTriple->[0]{user_id}; + my $user_id = $userTriple->[0]{user_id}; + next if $user_args{$user_id}; # Initial users will be assigned to everything below. my @user_sets = $db0->listUserSets($user_id); assignSetsToUsers($db, $ce, \@user_sets, [$user_id]); } @@ -425,7 +426,8 @@ sub addCourse { } if ($options{copyNonStudents}) { foreach my $userTriple (@users) { - my $user_id = $userTriple->[0]{user_id}; + my $user_id = $userTriple->[0]{user_id}; + next if $user_args{$user_id}; # Initial users were assigned to all achievements above. my @user_achievements = $db0->listUserAchievements($user_id); for my $achievement_id (@user_achievements) { my $userAchievement = $db->newUserAchievement(); From 1a0169fd24e478bac195fcdecb6031480d38f04f Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Wed, 13 Nov 2024 13:12:53 -0700 Subject: [PATCH 09/12] Update langauge for copying admin users as suggsted in the gh review. --- templates/ContentGenerator/CourseAdmin/add_course_form.html.ep | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index efb2ee561d..a078ddcc41 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -53,7 +53,7 @@

<%= maketext( - 'Select [_1] course users to add to the new course (as the stated permission) below.', + 'Select users from the [_1] course to add to the new course with indicated permission.', $ce->{admin_course_id}) =%>
From b10aee46f16034ba58a591ca5d5d4ee6654fbf9b Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Sun, 17 Nov 2024 10:42:44 -0700 Subject: [PATCH 10/12] Add statement for each additional instructor on add course form. Add an "Enter information for additional instructor number X." statement for each additional instructor to clarify what inputs go with what instructor, and to break the inputs apart a little better. --- .../ContentGenerator/CourseAdmin/add_course_form.html.ep | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index a078ddcc41..95ec4ab35f 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -78,8 +78,11 @@ . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas, and underscores.' ) =%>
-
+
% for (1 .. $number_of_additional_users) { +
+ <%= maketext('Enter information for additional instructor number [_1].', $_) %> +
From 6de502782cc4f428d6c09715174e64388f88d78e Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Tue, 19 Nov 2024 14:12:02 -0700 Subject: [PATCH 11/12] Add course updates based on feedback. * Remove statements about Course ID and User ID restrictions and turn them into tooltips. * Limit Course ID to 40 characters. * Change language from "instructor" to "user" with other language tweaks. * Make "Add Additional User" button secondary. * Update add course help. --- .../CourseAdmin/add_course_form.html.ep | 38 ++++++++++--------- templates/HelpFiles/AdminAddCourse.html.ep | 12 ++---- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index 95ec4ab35f..a05430dfd8 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -20,18 +20,22 @@ <%= $c->hidden_authen_fields =%> <%= $c->hidden_fields('subDisplay') =%> % -
<%= maketext( - 'Specify an ID, title, and institution for the new course. The course ID may contain only letters, ' - . 'numbers, hyphens, and underscores, and may have at most [_1] characters.', - $ce->{maxCourseIdLength}) %> -
-
+
<%= text_field new_courseID => '', id => 'new_courseID', placeholder => '', - class => 'form-control' =%> + class => 'form-control set-id-tooltip', + maxlength => $ce->{maxCourseIdLength}, + data => { + 'bs-placement' => 'top', + 'bs-title' => maketext( + 'Course ID may contain only letters, numbers, hyphens, and underscores, ' + . 'and may have at most [_1] characters.', + $ce->{maxCourseIdLength} + ) + } =%> <%= label_for new_courseID => maketext('Course ID') =%>
@@ -72,16 +76,10 @@
-
- <%= maketext( - 'To add additional instructor(s) to the new course, specify user information below. ' - . 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas, and underscores.' - ) =%> -
% for (1 .. $number_of_additional_users) {
- <%= maketext('Enter information for additional instructor number [_1].', $_) %> + <%= maketext('Enter information for additional user number [_1].', $_) %>
@@ -89,7 +87,13 @@ <%= text_field "add_initial_userID_$_" => '', id => "add_initial_userID_$_", placeholder => '', - class => 'form-control' =%> + class => 'form-control set-id-tooltip', + data => { + 'bs-placement' => 'top', + 'bs-title' => maketext( + 'User ID may contain only numbers, letters, hyphens, periods, and underscores.' + ) + } =%> <%= label_for "add_initial_userID_$_" => maketext('User ID') =%>
@@ -154,8 +158,8 @@ % }
- <%= submit_button maketext('Add Another Instructor'), name => 'add_another_instructor', - class => 'btn btn-primary' =%> + <%= submit_button maketext('Add Additional User'), name => 'add_another_instructor', + class => 'btn btn-secondary' =%>
<%= maketext('To copy components from an existing course, ' diff --git a/templates/HelpFiles/AdminAddCourse.html.ep b/templates/HelpFiles/AdminAddCourse.html.ep index 734ecae554..17f008cfc8 100644 --- a/templates/HelpFiles/AdminAddCourse.html.ep +++ b/templates/HelpFiles/AdminAddCourse.html.ep @@ -28,14 +28,10 @@ $ce->{admin_course_id}) =%>

- <%= maketext('Optionally, to add additional instructors click the "Add Another Instructor" button, then 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 [_1] course, so you can copy ' - . 'this user when creating future courses, or manage and email course instructors. Click the "Add Another ' - . 'Instructor" button again to add multiple additional users. Note, by default these new users will be ' - . '"Dropped" and unable to login to the [_1] course. You can change this on the "Accounts Manager" page. ' - . q/Additionally you can control access to the [_1] course by setting $permissionLevels{login}='admin' in / - . 'course.conf.', $ce->{admin_course_id}) =%> + <%= maketext('Click the "Add Additional User" button to add additional users to the course. The only required ' + . 'field is the user ID. You can also add this user to the [_1] course, so you can copy this user when ' + . 'creating future courses, or manage and email course users. Note, by default these new users will be ' + . '"Dropped" and unable to login to the [_1] course.', $ce->{admin_course_id}) =%>

<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. ' From 52a38d359539f7eebf6093c7dad069f44eb9dd44 Mon Sep 17 00:00:00 2001 From: Jaimos Skriletz Date: Tue, 19 Nov 2024 21:38:26 -0700 Subject: [PATCH 12/12] Add course, make passwords text and few more tweaks. Make password fields for new users text, so they will be saved after hitting add another user or errors appear. Since they are passwords for other users no need to hide them. Remove confirm password and student ID input to make each new user take up a little less space. Also make it so new student users are current (enrolled) in the course, while all other users are observers. --- lib/WeBWorK/ContentGenerator/CourseAdmin.pm | 46 ++++++++----------- .../CourseAdmin/add_course_form.html.ep | 19 +------- 2 files changed, 21 insertions(+), 44 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm index f87646c2e5..113e52f233 100644 --- a/lib/WeBWorK/ContentGenerator/CourseAdmin.pm +++ b/lib/WeBWorK/ContentGenerator/CourseAdmin.pm @@ -259,22 +259,15 @@ sub add_course_validate ($c) { } for (1 .. $number_of_additional_users) { - my $userID = trim_spaces($c->param("add_initial_userID_$_")) || ''; - my $password = trim_spaces($c->param("add_initial_password_$_")) || ''; - my $confirmPassword = trim_spaces($c->param("add_initial_confirmPassword_$_")) || ''; + my $userID = trim_spaces($c->param("add_initial_userID_$_")) || ''; - if ($userID ne '') { - unless ($userID =~ /^[\w-.,]*$/) { - push @errors, - $c->maketext( - 'User ID number [_1] may only contain letters, numbers, hyphens, periods, commas, ' - . 'and underscores.', - $_ - ); - } - if ($password ne '' && $password ne $confirmPassword) { - push @errors, $c->maketext('Pasword number [_1] and its password confirmation must match.', $_); - } + unless ($userID =~ /^[\w-.,]*$/) { + push @errors, + $c->maketext( + 'User ID number [_1] may only contain letters, numbers, hyphens, periods, commas, ' + . 'and underscores.', + $_ + ); } } @@ -337,21 +330,21 @@ sub do_add_course ($c) { my $PermissionLevel = $db->getPermissionLevel($userID); my $User = $db->getUser($userID); my $Password = $db->getPassword($userID); - $User->status('O'); # Add admin course user as an observer. + + # Enroll student users, and make all other users observers. + $User->status($PermissionLevel->permission == $ce->{userRoles}{student} ? 'C' : 'O'); push @users, [ $User, $Password, $PermissionLevel ]; } # add additional instructors if desired for (1 .. $number_of_additional_users) { - my $userID = trim_spaces($c->param("add_initial_userID_$_")) // ''; - my $password = trim_spaces($c->param("add_initial_password_$_")) // ''; - my $confirmPassword = trim_spaces($c->param("add_initial_confirmPassword_$_")) // ''; - my $firstName = trim_spaces($c->param("add_initial_firstName_$_")) // ''; - my $lastName = trim_spaces($c->param("add_initial_lastName_$_")) // ''; - my $email = trim_spaces($c->param("add_initial_email_$_")) // ''; - my $studentID = trim_spaces($c->param("add_initial_studentID_$_")) // ''; - my $permissionLevel = trim_spaces($c->param("add_initial_permission_$_")); + my $userID = trim_spaces($c->param("add_initial_userID_$_")) // ''; + my $password = trim_spaces($c->param("add_initial_password_$_")) // ''; + my $firstName = trim_spaces($c->param("add_initial_firstName_$_")) // ''; + my $lastName = trim_spaces($c->param("add_initial_lastName_$_")) // ''; + my $email = trim_spaces($c->param("add_initial_email_$_")) // ''; + my $permissionLevel = $c->param("add_initial_permission_$_"); my $add_user = $c->param("add_initial_user_$_") // 0; if ($userID =~ /\S/) { @@ -360,8 +353,7 @@ sub do_add_course ($c) { first_name => $firstName, last_name => $lastName, email_address => $email, - student_id => $studentID, - status => 'O', + status => $permissionLevel == $ce->{userRoles}{student} ? 'C' : 'O', ); my $Password = $db->newPassword( user_id => $userID, @@ -385,7 +377,7 @@ sub do_add_course ($c) { $db->addUser($User); $db->addPassword($Password); $db->addPermissionLevel($PermissionLevel); - $User->status('O'); + $User->status($permissionLevel == $ce->{userRoles}{student} ? 'C' : 'O'); } } } diff --git a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep index a05430dfd8..a822b4b0e7 100644 --- a/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep +++ b/templates/ContentGenerator/CourseAdmin/add_course_form.html.ep @@ -97,20 +97,12 @@ <%= label_for "add_initial_userID_$_" => maketext('User ID') =%>

- <%= password_field "add_initial_password_$_", + <%= text_field "add_initial_password_$_" => '', id => "add_initial_password_$_", placeholder => '', - class => 'form-control', - autocomplete => 'new-password' =%> + class => 'form-control' =%> <%= label_for "add_initial_password_$_" => maketext('Password') =%>
-
- <%= password_field "add_initial_confirmPassword_$_", - id => "add_initial_confirmPassword_$_", - placeholder => '', - class => 'form-control' =%> - <%= label_for "add_initial_confirmPassword_$_" => maketext('Confirm Password') =%> -
<%= select_field "add_initial_permission_$_" => $permissionLevels, id => "add_initial_role_$_", @@ -140,13 +132,6 @@ 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') =%> -