Skip to content

Commit d9e3038

Browse files
committed
Copy set level proctors when adding a course and copying sets.
If sets are copied then everything for that set should be copied. These set level proctor users belong to the set. This fixes issue #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.
1 parent 28b21ed commit d9e3038

File tree

2 files changed

+26
-11
lines changed

2 files changed

+26
-11
lines changed

lib/WeBWorK/Utils/CourseManagement.pm

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -383,22 +383,36 @@ sub addCourse {
383383

384384
# add sets
385385
if ($db0 && $options{copySets}) {
386-
my @set_ids = $db0->listGlobalSets;
387-
for my $set_id (@set_ids) {
388-
eval { $db->addGlobalSet($db0->getGlobalSet($set_id)) };
386+
my @sets = $db0->getGlobalSetsWhere;
387+
for my $set (@sets) {
388+
eval { $db->addGlobalSet($set) };
389389
warn $@ if $@;
390390

391-
my @Problem = $db0->getGlobalProblemsWhere({ set_id => $set_id });
391+
my @Problem = $db0->getGlobalProblemsWhere({ set_id => $set->set_id });
392392
for my $problem (@Problem) {
393393
eval { $db->addGlobalProblem($problem) };
394394
warn $@ if $@;
395395
}
396396

397-
my @Location = $db0->getGlobalSetLocationsWhere({ set_id => $set_id });
397+
my @Location = $db0->getGlobalSetLocationsWhere({ set_id => $set->set_id });
398398
for my $location (@Location) {
399399
eval { $db->addGlobalSetLocation($location) };
400400
warn $@ if $@;
401401
}
402+
403+
# Copy the set level proctor user for this set (despite the for loop there can only be one).
404+
for my $setProctor ($db0->getUsersWhere({ user_id => 'set_id:' . $set->set_id })) {
405+
eval { $db->addUser($setProctor) };
406+
warn $@ if $@;
407+
408+
my $password = $db0->getPassword($setProctor->user_id);
409+
eval { $db->addPassword($password) } if $password;
410+
warn $@ if $@;
411+
412+
my $permission = $db0->getPermissionLevel($setProctor->user_id);
413+
eval { $db->addPermissionLevel($permission) } if $permission;
414+
warn $@ if $@;
415+
}
402416
}
403417
if ($options{copyNonStudents}) {
404418
foreach my $userTriple (@users) {
@@ -408,19 +422,20 @@ sub addCourse {
408422
assignSetsToUsers($db, $ce, \@user_sets, [$user_id]);
409423
}
410424
}
411-
assignSetsToUsers($db, $ce, \@set_ids, [ map { $_->[0]{user_id} } @initialUsers ]) if @initialUsers;
425+
assignSetsToUsers($db, $ce, [ map { $_->set_id } @sets ], [ map { $_->[0]{user_id} } @initialUsers ])
426+
if @initialUsers;
412427
}
413428

414429
# add achievements
415430
if ($db0 && $options{copyAchievements}) {
416-
my @achievement_ids = $db0->listAchievements;
417-
for my $achievement_id (@achievement_ids) {
418-
eval { $db->addAchievement($db0->getAchievement($achievement_id)) };
431+
my @achievement = $db0->getAchievementsWhere;
432+
for my $achievement (@achievement) {
433+
eval { $db->addAchievement($achievement) };
419434
warn $@ if $@;
420435
for (@initialUsers) {
421436
my $userAchievement = $db->newUserAchievement();
422437
$userAchievement->user_id($_->[0]{user_id});
423-
$userAchievement->achievement_id($achievement_id);
438+
$userAchievement->achievement_id($achievement->achievement_id);
424439
$db->addUserAchievement($userAchievement);
425440
}
426441
}

templates/ContentGenerator/CourseAdmin/add_course_form.html.ep

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@
166166
</div>
167167
</div>
168168
<fieldset class="mb-3">
169-
<legend class="fw-bold fs-6">Copy These Components:</legend>
169+
<legend class="fw-bold fs-6"><%= maketext('Copy These Components:') %></legend>
170170
<div class="form-check">
171171
<label class="form-check-label">
172172
<%= check_box select_all => 1, class => 'select-all form-check-input',

0 commit comments

Comments
 (0)