Skip to content

Commit

Permalink
Copy set level proctors when adding a course and copying sets.
Browse files Browse the repository at this point in the history
If sets are copied then everything for that set should be copied.  These
set level proctor users belong to the set.

This fixes issue openwebwork#2652.

Also add a missing `maketext` call in the related template.

Also fix some database list/get usage that I missed when these copy
options were implemented.  Never list all database records if you are
going to subsequently get all records anyway.  Listing records is the
equivalent of calling `SELECT id from table` and getting all records is
the equivalent of calling `SELECT * from table`.  Why would you
inneficiently first list to get id's and then select everything
separately?  Note that you can get always get all records by using the
webwork2 db `Where` methods with no where clause.

Note that the copying of sets that is done here is still highly
inneficient.  The current approach gets all sets, and then loops through
those sets, then gets the problems for that set and loops through those
adding one at a time, then gets set locations and loops through adding
one of those at a time, and now adds the set level proctor (of which
there can be only one).  Instead since all sets are being copyied all
sets could be fetched and added at once, then all problems for all sets
fetched and added at once, then all set level proctors for all sets
fetched and added at once.  However, adding all at once like that takes
for effort because there aren't convenience database methods for doing
that.  This is now done when assigning sets to multiple users (I helped
@taniwallach implement that) and the same could be done here.  Note that
for a course with a large number of sets and a lot of problems the
current approach is quite slow.
  • Loading branch information
drgrice1 committed Feb 14, 2025
1 parent 28b21ed commit e7713e8
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
35 changes: 25 additions & 10 deletions lib/WeBWorK/Utils/CourseManagement.pm
Original file line number Diff line number Diff line change
Expand Up @@ -383,22 +383,36 @@ sub addCourse {

# add sets
if ($db0 && $options{copySets}) {
my @set_ids = $db0->listGlobalSets;
for my $set_id (@set_ids) {
eval { $db->addGlobalSet($db0->getGlobalSet($set_id)) };
my @sets = $db0->getGlobalSetsWhere;
for my $set (@sets) {
eval { $db->addGlobalSet($set) };
warn $@ if $@;

my @Problem = $db0->getGlobalProblemsWhere({ set_id => $set_id });
my @Problem = $db0->getGlobalProblemsWhere({ set_id => $set->set_id });
for my $problem (@Problem) {
eval { $db->addGlobalProblem($problem) };
warn $@ if $@;
}

my @Location = $db0->getGlobalSetLocationsWhere({ set_id => $set_id });
my @Location = $db0->getGlobalSetLocationsWhere({ set_id => $set->set_id });
for my $location (@Location) {
eval { $db->addGlobalSetLocation($location) };
warn $@ if $@;
}

# Copy the set level proctor user for this set (despite the for loop there can only be one).
for my $setProctor ($db0->getUsersWhere({ user_id => "set_id:" . $set->set_id })) {
eval { $db->addUser($setProctor) };
warn $@ if $@;

my $password = $db0->getPassword($setProctor->user_id);
eval { $db->addPassword($password) } if $password;
warn $@ if $@;

my $permission = $db0->getPermissionLevel($setProctor->user_id);
eval { $db->addPermissionLevel($permission) } if $permission;
warn $@ if $@;
}
}
if ($options{copyNonStudents}) {
foreach my $userTriple (@users) {
Expand All @@ -408,19 +422,20 @@ sub addCourse {
assignSetsToUsers($db, $ce, \@user_sets, [$user_id]);
}
}
assignSetsToUsers($db, $ce, \@set_ids, [ map { $_->[0]{user_id} } @initialUsers ]) if @initialUsers;
assignSetsToUsers($db, $ce, [ map { $_->set_id } @sets ], [ map { $_->[0]{user_id} } @initialUsers ])
if @initialUsers;
}

# add achievements
if ($db0 && $options{copyAchievements}) {
my @achievement_ids = $db0->listAchievements;
for my $achievement_id (@achievement_ids) {
eval { $db->addAchievement($db0->getAchievement($achievement_id)) };
my @achievement = $db0->getAchievementsWhere;
for my $achievement (@achievement) {
eval { $db->addAchievement($achievement) };
warn $@ if $@;
for (@initialUsers) {
my $userAchievement = $db->newUserAchievement();
$userAchievement->user_id($_->[0]{user_id});
$userAchievement->achievement_id($achievement_id);
$userAchievement->achievement_id($achievement->achievement_id);
$db->addUserAchievement($userAchievement);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@
</div>
</div>
<fieldset class="mb-3">
<legend class="fw-bold fs-6">Copy These Components:</legend>
<legend class="fw-bold fs-6"><%= maketext('Copy These Components:') %></legend>
<div class="form-check">
<label class="form-check-label">
<%= check_box select_all => 1, class => 'select-all form-check-input',
Expand Down

0 comments on commit e7713e8

Please sign in to comment.