Skip to content

Commit 83bff86

Browse files
drgrice1drgrice1
committed
Speed up the achievement users page.
The usual thing (by now with this series of pull request) is done. That is removing database access from loops and reducing to single queries as much as possible. In addition instead of using the Mojolicious tag helpers to render the "earned" checkbox and "counter" text field, direct html is used. I discovered that this renders much faster. With the usual 5000 user test the rendering of the template alone takes 5 seconds after changing a checkbox or counter value and saving, while with the direct html it takes something like 0.2 seconds. I believe that it has something to do with their code to set the values of the inputs that is causing the slow down. I am not sure on that, but I know that the direct html is much faster. That database changes are still the biggest part of the speed improvement here in any case. This page was rendering quite slowly even on initial load before, and that part was fast with the tag helpers. After making the database changes things sped up considerably, but then timing parts of the code when saving revealed the tag helper issue.
1 parent 311ec9e commit 83bff86

2 files changed

Lines changed: 73 additions & 71 deletions

File tree

‎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)