Skip to content

Commit

Permalink
Updates to adding users to newly created courses.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@somiaj
somiaj committed Nov 9, 2024
1 parent c033f33 commit 3e1aa50
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 63 deletions.
88 changes: 46 additions & 42 deletions lib/WeBWorK/ContentGenerator/CourseAdmin.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 '') {
Expand Down Expand Up @@ -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')) // '';

Expand All @@ -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
Expand All @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 25 additions & 10 deletions templates/ContentGenerator/CourseAdmin/add_course_form.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,14 @@
<div class="mb-3">
<div class="mb-1">
<%= 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.') =%>
</div>
<div class="form-check">
<label class="form-check-label">
<%= check_box 'add_admin_users' => 1, class => 'form-check-input' =%>
<%= maketext('Add WeBWorK administrators to new course') =%>
</label>
<div class="col-lg-4 col-md-5 col-sm-6">
<%= 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' =%>
</div>
</div>
<div class="mb-2">
Expand All @@ -54,7 +55,7 @@
. 'The user ID may contain only numbers, letters, hyphens, periods (dots), commas,and underscores.'
) =%>
</div>
<div class="row mb-3">
<div class="row">
<div class="col-lg-4 col-md-5 col-sm-6">
<div class="form-floating mb-1">
<%= text_field add_initial_userID => '',
Expand All @@ -65,9 +66,10 @@
</div>
<div class="form-floating mb-1">
<%= 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') =%>
</div>
<div class="form-floating mb-1">
Expand Down Expand Up @@ -100,8 +102,21 @@
class => 'form-control' =%>
<%= label_for add_initial_email => maketext('Email Address') =%>
</div>
<div class="form-floating mb-1">
<%= text_field add_initial_studentID => '',
id => 'add_initial_studentID',
placeholder => '',
class => 'form-control' =%>
<%= label_for add_initial_studentID => maketext('Student ID') =%>
</div>
</div>
</div>
<div class="form-check mb-3">
<label class="form-check-label">
<%= check_box 'add_initial_user' => 1, class => 'form-check-input' =%>
<%= maketext('Add new user to admin course.') =%>
</label>
</div>
<div class="mb-1">
<%= maketext('To copy components from an existing course, '
. 'select the course and check which components to copy.') =%>
Expand Down
10 changes: 6 additions & 4 deletions templates/ContentGenerator/Instructor/AddUsers.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,11 @@
</tbody>
</table>
</div>
<p class="my-2"><%= maketext('Select sets below to assign them to the newly-created users.') %></p>
% 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}) {
<p class="my-2"><%= maketext('Select sets below to assign them to the newly-created users.') %></p>
% param('assignSets', undef);
<%= select_field assignSets => [ map { [ format_set_name_display($_) => $_ ] } $db->listGlobalSets ],
size => 10, multiple => undef, class => 'form-select w-auto mb-2' =%>
% }
<p><%= submit_button maketext('Add Users'), name => 'addStudents', class => 'btn btn-primary' =%></p>
<% end =%>
12 changes: 7 additions & 5 deletions templates/HelpFiles/AdminAddCourse.html.ep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
. 'on the course home page.') =%>
</p>
<p>
<%= 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.') =%>
</p>
<p>
<%= 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.') =%>
</p>
<p class="mb-0">
<%= maketext('You may choose a course to copy components from. Select the course and which components to copy. '
Expand Down

0 comments on commit 3e1aa50

Please sign in to comment.