Skip to content

Commit abe7125

Browse files
Alex-JordanAlex-Jordan
authored
Merge pull request #2715 from drgrice1/achievement-user-editor-improvements
Speed up the achievement users page.
2 parents 68a2f5f + 74ba4de commit abe7125

File tree

2 files changed

+73
-71
lines changed

2 files changed

+73
-71
lines changed

‎lib/WeBWorK/ContentGenerator/Instructor/AchievementUserEditor.pm‎

Lines changed: 64 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -14,105 +14,112 @@ sub initialize ($c) {
1414
my $achievementID = $c->stash('achievementID');
1515
my $user = $c->param('user');
1616

17-
# Make sure this is defined for the template.
18-
$c->stash->{userRecords} = [];
17+
# Make sure these are defined for the template.
18+
$c->stash->{userRecords} = [];
19+
$c->stash->{userAchievementRecords} = [];
1920

2021
# Check permissions
2122
return unless $authz->hasPermissions($user, 'edit_achievements');
2223

23-
my @all_users = $db->listUsers;
24+
$c->stash->{userRecords} =
25+
[ $db->getUsersWhere({ user_id => { not_like => 'set_id:%' } }, [qw/section last_name first_name/]) ];
26+
$c->stash->{userAchievementRecords} =
27+
{ map { $_->user_id => $_ } $db->getUserAchievementsWhere({ achievement_id => $achievementID }) };
28+
2429
my %selectedUsers = map { $_ => 1 } $c->param('selected');
2530

2631
my $doAssignToSelected = 0;
2732

28-
#Check and see if we need to assign or unassign things
33+
# Check and see if we need to assign or unassign achievements.
2934
if (defined $c->param('assignToAll')) {
3035
$c->addgoodmessage($c->maketext('Achievement has been assigned to all users.'));
31-
%selectedUsers = map { $_ => 1 } @all_users;
36+
%selectedUsers = map { $_->user_id => 1 } @{ $c->stash->{userRecords} };
3237
$doAssignToSelected = 1;
3338
} elsif (defined $c->param('unassignFromAll')
3439
&& defined($c->param('unassignFromAllSafety'))
3540
&& $c->param('unassignFromAllSafety') == 1)
3641
{
3742
%selectedUsers = ();
38-
$c->addbadmessage($c->maketext('Achievement has been unassigned to all students.'));
43+
$c->addgoodmessage($c->maketext('Achievement has been unassigned from all users.'));
3944
$doAssignToSelected = 1;
4045
} elsif (defined $c->param('assignToSelected')) {
4146
$c->addgoodmessage($c->maketext('Achievement has been assigned to selected users.'));
4247
$doAssignToSelected = 1;
4348
} elsif (defined $c->param('unassignFromAll')) {
44-
# no action taken
4549
$c->addbadmessage($c->maketext('No action taken'));
4650
}
4751

48-
#do actual assignment and unassignment
52+
# Do the actual assignment and unassignment.
4953
if ($doAssignToSelected) {
54+
my $achievement = $db->getAchievement($achievementID);
55+
56+
my %globalUserAchievements = map { $_->user_id => $_ } $db->getGlobalUserAchievementsWhere;
5057

51-
my %achievementUsers = map { $_ => 1 } $db->listAchievementUsers($achievementID);
52-
foreach my $selectedUser (@all_users) {
53-
if (exists $selectedUsers{$selectedUser} && $achievementUsers{$selectedUser}) {
54-
# update existing user data (in case fields were changed)
55-
my $userAchievement = $db->getUserAchievement($selectedUser, $achievementID);
58+
my (
59+
@userAchievementsToInsert, @userAchievementsToUpdate, @userAchievementsToDelete,
60+
@globalUserAchievementsToInsert, @globalUserAchievementsToUpdate,
61+
);
62+
63+
for my $user (@{ $c->stash->{userRecords} }) {
64+
my $userID = $user->user_id;
65+
if ($selectedUsers{$userID} && $c->stash->{userAchievementRecords}{$userID}) {
66+
# Update existing user data (in case fields were changed).
67+
my $updatedEarned = $c->param("$userID.earned") ? 1 : 0;
68+
my $earned = $c->stash->{userAchievementRecords}{$userID}->earned ? 1 : 0;
5669

57-
my $updatedEarned = $c->param("$selectedUser.earned") ? 1 : 0;
58-
my $earned = $userAchievement->earned ? 1 : 0;
5970
if ($updatedEarned != $earned) {
71+
$c->stash->{userAchievementRecords}{$userID}->earned($updatedEarned);
6072

61-
$userAchievement->earned($updatedEarned);
62-
my $globalUserAchievement = $db->getGlobalUserAchievement($selectedUser);
63-
my $achievement = $db->getAchievement($achievementID);
73+
my $points = $achievement->points || 0;
74+
my $initialpoints = $globalUserAchievements{$userID}->achievement_points || 0;
6475

65-
my $points = $achievement->points || 0;
66-
my $initialpoints = $globalUserAchievement->achievement_points || 0;
67-
#add the correct number of points if we
68-
# are saying that the user now earned the
69-
# achievement, or remove them otherwise
76+
# Add the correct number of points if we are saying that the
77+
# user now earned the achievement, or remove them otherwise.
7078
if ($updatedEarned) {
71-
72-
$globalUserAchievement->achievement_points($initialpoints + $points);
79+
$globalUserAchievements{$userID}->achievement_points($initialpoints + $points);
7380
} else {
74-
$globalUserAchievement->achievement_points($initialpoints - $points);
81+
$globalUserAchievements{$userID}->achievement_points($initialpoints - $points);
7582
}
7683

77-
$db->putGlobalUserAchievement($globalUserAchievement);
84+
push(@globalUserAchievementsToUpdate, $globalUserAchievements{$userID});
7885
}
7986

80-
$userAchievement->counter($c->param("$selectedUser.counter"));
81-
$db->putUserAchievement($userAchievement);
82-
83-
} elsif (exists $selectedUsers{$selectedUser}) {
84-
# add users that dont exist
85-
my $userAchievement = $db->newUserAchievement();
86-
$userAchievement->user_id($selectedUser);
87-
$userAchievement->achievement_id($achievementID);
88-
$db->addUserAchievement($userAchievement);
89-
90-
#If they dont have global achievement data, then add that too
91-
if (not $db->existsGlobalUserAchievement($selectedUser)) {
92-
my $globalUserAchievement = $db->newGlobalUserAchievement();
93-
$globalUserAchievement->user_id($selectedUser);
94-
$db->addGlobalUserAchievement($globalUserAchievement);
87+
my $updatedCounter = $c->param("$userID.counter") // '';
88+
my $counter = $c->stash->{userAchievementRecords}{$userID}->counter // '';
89+
$c->stash->{userAchievementRecords}{$userID}->counter($updatedCounter)
90+
if $updatedCounter ne $counter;
91+
92+
push(@userAchievementsToUpdate, $c->stash->{userAchievementRecords}{$userID})
93+
if $updatedEarned != $earned || $updatedCounter ne $counter;
94+
} elsif ($selectedUsers{$userID}) {
95+
# Add user achievements that don't exist.
96+
$c->stash->{userAchievementRecords}{$userID} = $db->newUserAchievement;
97+
$c->stash->{userAchievementRecords}{$userID}->user_id($userID);
98+
$c->stash->{userAchievementRecords}{$userID}->achievement_id($achievementID);
99+
push(@userAchievementsToInsert, $c->stash->{userAchievementRecords}{$userID});
100+
101+
# If the user does not have global achievement data, then add that too.
102+
if (!$globalUserAchievements{$userID}) {
103+
$globalUserAchievements{$userID} = $db->newGlobalUserAchievement(user_id => $userID);
104+
push(@globalUserAchievementsToInsert, $globalUserAchievements{$userID});
95105
}
96-
97106
} else {
98-
# delete users who are not selected
99-
# but dont delete users who dont exist
100-
next unless $achievementUsers{$selectedUser};
101-
$db->deleteUserAchievement($selectedUser, $achievementID);
107+
# Delete achievements for users that are not selected, but don't delete achievements that don't exist.
108+
next unless $c->stash->{userAchievementRecords}{$userID};
109+
push(@userAchievementsToDelete, $c->stash->{userAchievementRecords}{$userID});
110+
delete $c->stash->{userAchievementRecords}{$userID};
102111
}
103112
}
104-
}
105113

106-
my @userRecords;
107-
for my $currentUser (@all_users) {
108-
my $userObj = $c->db->getUser($currentUser);
109-
die "Unable to find user object for $currentUser. " unless $userObj;
110-
push(@userRecords, $userObj);
111-
}
112-
@userRecords =
113-
sort { (lc($a->section) cmp lc($b->section)) || (lc($a->last_name) cmp lc($b->last_name)) } @userRecords;
114+
$db->GlobalUserAchievement->insert_records(\@globalUserAchievementsToInsert) if @globalUserAchievementsToInsert;
115+
$db->GlobalUserAchievement->update_records(\@globalUserAchievementsToUpdate) if @globalUserAchievementsToUpdate;
116+
$db->UserAchievement->insert_records(\@userAchievementsToInsert) if @userAchievementsToInsert;
117+
$db->UserAchievement->update_records(\@userAchievementsToUpdate) if @userAchievementsToUpdate;
114118

115-
$c->stash->{userRecords} = \@userRecords;
119+
# This is one of the rare places this can be done since user achievements don't
120+
# have any dependent rows in other tables that also need to be deleted.
121+
$db->UserAchievement->delete_records(\@userAchievementsToDelete) if @userAchievementsToDelete;
122+
}
116123

117124
return;
118125
}

‎templates/ContentGenerator/Instructor/AchievementUserEditor.html.ep‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
% last;
44
% }
55
%
6-
% my $achievementID = stash('achievementID');
7-
%
86
<%= form_for current_route, method => 'post', name => 'user-achievement-form', id => 'user-achievement-form', begin =%>
97
% # Assign to everyone message
108
<div class="my-2">
@@ -39,31 +37,28 @@
3937
<tbody class="table-group-divider">
4038
% # Output row for user
4139
% for my $userRecord (@$userRecords) {
42-
% my $statusClass = $ce->status_abbrev_to_name($userRecord->status) || '';
4340
% my $user = $userRecord->user_id;
44-
% my $userAchievement = $db->getUserAchievement($user, $achievementID);
41+
% my $userAchievement = $userAchievementRecords->{$user};
4542
% my $prettyName = $userRecord->last_name . ', ' . $userRecord->first_name;
4643
%
4744
<tr>
4845
<td class="text-center">
4946
<input type="checkbox" name="selected" id="<%= $user %>.assigned" value="<%= $user %>"
50-
class="form-check-input" <%= defined $userAchievement ? 'checked' : '' %>>
47+
class="form-check-input" <%= $userAchievement ? 'checked' : '' %>>
5148
</td>
5249
<td><%= label_for "$user.assigned" => $user %></td>
5350
<td><%= $prettyName %></td>
5451
<td class="text-center"><%= $userRecord->section %></td>
55-
% if (defined $userAchievement) {
52+
% if ($userAchievement) {
5653
<td class="text-center">
57-
<%= check_box "$user.earned" => '1',
58-
'aria-labelledby' => 'earned_header',
59-
class => 'form-check-input',
60-
(ref $userAchievement ? $userAchievement->earned : 0) ? (checked => undef) : () =%>
54+
<input type="checkbox" name="<%= $user %>.earned" value="1"
55+
aria-labelledby="earned_header" class="form-check-input"
56+
<%= $userAchievement->earned ? 'checked' : '' %>>
6157
</td>
6258
<td class="text-center">
63-
<%= text_field "$user.counter" => ref $userAchievement ? $userAchievement->counter : 0,
64-
'aria-labelledby' => 'counter_header',
65-
size => 6,
66-
class => 'form-control form-control-sm' =%>
59+
<input name="<%= $user %>.counter" value="<%= $userAchievement->counter %>"
60+
type="number" min="0" aria-labelledby="counter_header" size="6"
61+
class="form-control form-control-sm">
6762
</td>
6863
% } else {
6964
<td></td><td></td>

0 commit comments

Comments
 (0)