Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to adding users to newly created courses. #2619

Merged
merged 12 commits into from
Nov 24, 2024
Merged

Updates to adding users to newly created courses. #2619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
14a6aea
Updates to adding users to newly created courses.
somiaj Nov 9, 2024
07e3a65
Use checkbox list instead of select for admin users.
somiaj Nov 9, 2024
9b83f7b
CourseAdmin add new course tweaks.
somiaj Nov 11, 2024
beb78ea
Ensure all admin users stay unchecked after error.
somiaj Nov 11, 2024
032c304
Remove student ID input from add course page.
somiaj Nov 11, 2024
0a19eed
Tweaks to add course form layout.
somiaj Nov 11, 2024
c1c92bb
Allow adding multiple initial users when adding a new course.
somiaj Nov 13, 2024
1c6f904
Only assign initial users to sets/achievements once.
somiaj Nov 13, 2024
1a0169f
Update langauge for copying admin users as suggsted in the gh review.
somiaj Nov 13, 2024
b10aee4
Add statement for each additional instructor on add course form.
somiaj Nov 17, 2024
6de5027
Add course updates based on feedback.
somiaj Nov 19, 2024
52a38d3
Add course, make passwords text and few more tweaks.
somiaj Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 94 additions & 79 deletions lib/WeBWorK/ContentGenerator/CourseAdmin.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,20 +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_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 $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;

Expand All @@ -260,24 +258,16 @@ 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.');
for (1 .. $number_of_additional_users) {
my $userID = trim_spaces($c->param("add_initial_userID_$_")) || '';

unless ($userID =~ /^[\w-.,]*$/) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this before. This is an invalid regular expression. The - inside the braces needs to be at the end of all other characters. Otherwise it is interpreted as a range, and the range from \w to . is not valid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note the - can also be at the beginning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it can be escaped with a \.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this.

push @errors,
$c->maketext(
'User ID number [_1] may only contain letters, numbers, hyphens, periods, commas, '
. 'and underscores.',
$_
);
}
}

Expand All @@ -300,16 +290,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_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')) // '';

Expand All @@ -322,49 +306,85 @@ 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) {
for my $userID ($c->param('add-admin-users')) {
unless ($db->existsUser($userID)) {
$c->addbadmessage($c->maketext(
'User "[_1]" will not be copied from the [_2] course as it does not exist.', $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 [_2] course as it is the initial instructor.', $userID,
$ce->{admin_course_id}
'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->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');
}
}

# 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,
student_id => $add_initial_userID,
email_address => $add_initial_email,
status => 'O',
);
my $Password = $db->newPassword(
user_id => $add_initial_userID,
password => cryptPassword($add_initial_password),
);
my $PermissionLevel = $db->newPermissionLevel(
user_id => $add_initial_userID,
permission => '10',
);
my $PermissionLevel = $db->getPermissionLevel($userID);
my $User = $db->getUser($userID);
my $Password = $db->getPassword($userID);

# Enroll student users, and make all other users observers.
$User->status($PermissionLevel->permission == $ce->{userRoles}{student} ? 'C' : 'O');

push @users, [ $User, $Password, $PermissionLevel ];
}

push @{ $courseOptions{PRINT_FILE_NAMES_FOR} }, map { $_->[0]->user_id } @users;
# 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 $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/) {
my $User = $db->newUser(
user_id => $userID,
first_name => $firstName,
last_name => $lastName,
email_address => $email,
status => $permissionLevel == $ce->{userRoles}{student} ? 'C' : '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($permissionLevel == $ce->{userRoles}{student} ? 'C' : 'O');
}
}
}
}

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;
Expand All @@ -384,11 +404,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,
);
};
Expand Down Expand Up @@ -417,11 +436,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,
Expand Down
5 changes: 3 additions & 2 deletions lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm
View file
Open in desktop
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
18 changes: 10 additions & 8 deletions lib/WeBWorK/Utils/CourseManagement.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ boolean options:
=cut

sub addCourse {
my (%options) = (initial_userID => '', @_);
my (%options) = @_;

for my $key (keys(%options)) {
my $value = '####UNDEF###';
Expand All @@ -217,7 +217,8 @@ sub addCourse {

debug \@users;

my ($initialUser) = grep { $_->[0]{user_id} eq $options{initial_userID} } @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};
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -402,12 +402,13 @@ 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]);
}
}
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
Expand All @@ -416,16 +417,17 @@ 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);
}
}
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();
Expand Down
Loading