From e36c5612339916a04ea32e04ae6acb2b0bc08518 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 11 Nov 2026 09:30:17 -0600 Subject: [PATCH 1/2] Use the user record full_name method everywhere a user's full name is shown. Several places built a user's full name by joining the first and last names manually instead of using the `full_name` method of the user database record. Those places now all use that method. Co-Authored-By: Claude Opus 5.5 --- lib/Caliper/Actor.pm | 2 +- lib/WeBWorK/ContentGenerator/Grades.pm | 2 +- lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm | 8 +++----- .../ContentGenerator/Instructor/StudentProgress.pm | 2 +- lib/WeBWorK/ContentGenerator/Options.pm | 5 ++--- lib/WeBWorK/Utils/Rendering.pm | 2 +- templates/ContentGenerator/Hardcopy/form.html.ep | 2 +- .../Instructor/StudentProgress/set_progress.html.ep | 2 +- templates/ContentGenerator/Instructor/UserDetail.html.ep | 2 +- templates/ContentGenerator/LoginProctor.html.ep | 2 +- templates/ContentGenerator/Options.html.ep | 2 +- 11 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/Caliper/Actor.pm b/lib/Caliper/Actor.pm index 6c1e4ea30c..f3babde578 100644 --- a/lib/Caliper/Actor.pm +++ b/lib/Caliper/Actor.pm @@ -7,7 +7,7 @@ sub generate_default_actor ($c, $user) { return { id => Caliper::ResourceIri->new($c->ce)->actor_homepage($user->user_id), type => 'Person', - name => $user->first_name . ' ' . $user->last_name + name => $user->full_name }; } diff --git a/lib/WeBWorK/ContentGenerator/Grades.pm b/lib/WeBWorK/ContentGenerator/Grades.pm index 64fb0f1c68..e0befbc0cb 100644 --- a/lib/WeBWorK/ContentGenerator/Grades.pm +++ b/lib/WeBWorK/ContentGenerator/Grades.pm @@ -366,7 +366,7 @@ sub displayStudentGrades ($c) { return $c->include( 'ContentGenerator/Grades/student_grades', effectiveUser => $effectiveUser, - fullName => join(' ', $studentRecord->first_name, $studentRecord->last_name), + fullName => $studentRecord->full_name, notOpen => \@notOpen, open => \@open, reduced => \@reduced, diff --git a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm index 81250c9e62..9ca540b75e 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/SendMail.pm @@ -343,11 +343,9 @@ sub print_preview ($c) { if ($merge_file && !defined $rh_merge_data->{ $c->{preview_user}->student_id }) { $c->addbadmessage($c->maketext( - 'No merge data for student id: [_1]; name: [_2] [_3]; login: [_4]', - map { xml_escape($_) } ( - $c->{preview_user}->student_id, $c->{preview_user}->first_name, - $c->{preview_user}->last_name, $c->{preview_user}->user_id - ) + 'No merge data for student id: [_1]; name: [_2]; login: [_3]', + map { xml_escape($_) } + ($c->{preview_user}->student_id, $c->{preview_user}->full_name, $c->{preview_user}->user_id) )); } diff --git a/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm b/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm index 4e6c346d60..3e5a06f8c1 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/StudentProgress.pm @@ -328,7 +328,7 @@ sub displayStudentStats ($c) { } } - my $fullName = join(' ', $studentRecord->first_name, $studentRecord->last_name); + my $fullName = $studentRecord->full_name; my $effectiveUser = $studentRecord->user_id(); my $max_problems = 0; diff --git a/lib/WeBWorK/ContentGenerator/Options.pm b/lib/WeBWorK/ContentGenerator/Options.pm index bad811cdc6..794208adff 100644 --- a/lib/WeBWorK/ContentGenerator/Options.pm +++ b/lib/WeBWorK/ContentGenerator/Options.pm @@ -41,7 +41,7 @@ sub initialize ($c) { # Check that either password is not defined or if it is defined then we have the right one. if (!defined $password || utf8Crypt($currP // '', $password->password) eq $password->password) { - my $e_user_name = $c->{effectiveUser}->first_name . ' ' . $c->{effectiveUser}->last_name; + my $e_user_name = $c->{effectiveUser}->full_name; if ($newP eq $confirmP) { if (!defined $effectiveUserPassword) { $effectiveUserPassword = $db->newPassword(); @@ -85,8 +85,7 @@ sub initialize ($c) { )); } } else { - my $fieldText = - $c->maketext("[_1]'s Current Password", $c->{user}->first_name . ' ' . $c->{user}->last_name); + my $fieldText = $c->maketext("[_1]'s Current Password", $c->{user}->full_name); $c->addbadmessage($c->maketext( 'The password you entered in the [_1] field does not match your current password. ' . 'Please retype your current password and try again.', diff --git a/lib/WeBWorK/Utils/Rendering.pm b/lib/WeBWorK/Utils/Rendering.pm index 3ef3eb9547..f4d64d2fd5 100644 --- a/lib/WeBWorK/Utils/Rendering.pm +++ b/lib/WeBWorK/Utils/Rendering.pm @@ -132,7 +132,7 @@ sub constructPGOptions ($ce, $user, $set, $problem, $psvn, $formFields, $transla $options{setDescription} = $set->description; $options{setNumber} = $set->set_id; $options{studentLogin} = $user->user_id; - $options{studentName} = $user->first_name . ' ' . $user->last_name; + $options{studentName} = $user->full_name; $options{studentID} = $user->student_id; # Permission level of actual user (deprecated) diff --git a/templates/ContentGenerator/Hardcopy/form.html.ep b/templates/ContentGenerator/Hardcopy/form.html.ep index e1f3fe5ec1..c17f5a894d 100644 --- a/templates/ContentGenerator/Hardcopy/form.html.ep +++ b/templates/ContentGenerator/Hardcopy/form.html.ep @@ -60,7 +60,7 @@ dir => 'ltr', format_set_name_display($selected_set_id =~ s/,v(\d+)$/ (version $1)/r) ), - xml_escape(join(' ', $user->first_name, $user->last_name)) + xml_escape($user->full_name) ) =%>

% } diff --git a/templates/ContentGenerator/Instructor/StudentProgress/set_progress.html.ep b/templates/ContentGenerator/Instructor/StudentProgress/set_progress.html.ep index 44cc1e5e91..6ac4a6cfb5 100644 --- a/templates/ContentGenerator/Instructor/StudentProgress/set_progress.html.ep +++ b/templates/ContentGenerator/Instructor/StudentProgress/set_progress.html.ep @@ -241,7 +241,7 @@ % my $prevUserID = ''; % % for my $rec (@$user_set_list) { - % my $fullName = join('', $rec->{record}{first_name}, " ", $rec->{record}{last_name}); + % my $fullName = $rec->{record}->full_name; % my $email = $rec->{record}{email_address}; % % if ($setIsVersioned) { diff --git a/templates/ContentGenerator/Instructor/UserDetail.html.ep b/templates/ContentGenerator/Instructor/UserDetail.html.ep index b31b545a66..4e6aed4707 100644 --- a/templates/ContentGenerator/Instructor/UserDetail.html.ep +++ b/templates/ContentGenerator/Instructor/UserDetail.html.ep @@ -42,7 +42,7 @@ % % my @editForSets = param('editForSets'); % -% my $userName = $c->{userRecord}->first_name . ' ' . $c->{userRecord}->last_name; +% my $userName = $c->{userRecord}->full_name; % % # Display a message about how many sets have been assigned to this user. % my $numberSets = scalar(@{ $c->{setRecords} }); diff --git a/templates/ContentGenerator/LoginProctor.html.ep b/templates/ContentGenerator/LoginProctor.html.ep index fe4af28282..d371957303 100644 --- a/templates/ContentGenerator/LoginProctor.html.ep +++ b/templates/ContentGenerator/LoginProctor.html.ep @@ -56,7 +56,7 @@
<%= maketext(q{User's username is:}) %> <%= param('effectiveUser') // '' %>
<%= maketext(q{User's name is:}) %> - <%= $c->{effectiveUser}->first_name %> <%= $c->{effectiveUser}->last_name %> + <%= $c->{effectiveUser}->full_name %>
diff --git a/templates/ContentGenerator/Options.html.ep b/templates/ContentGenerator/Options.html.ep index ea3130eb0b..fa224d0394 100644 --- a/templates/ContentGenerator/Options.html.ep +++ b/templates/ContentGenerator/Options.html.ep @@ -1,7 +1,7 @@ % require WeBWorK::PG; % % my $userID = $c->{user}->user_id; -% my $eUserName = $c->{effectiveUser}->first_name . ' ' . $c->{effectiveUser}->last_name; +% my $eUserName = $c->{effectiveUser}->full_name; % % $c->stash->{footerWidthClass} = 'col-lg-8 col-md-10'; % From 0f57fd6280945f6542748f95a6c98b249aa6570a Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Wed, 11 Nov 2026 10:15:48 -0600 Subject: [PATCH 2/2] Add a sortable_name method to the user record and use it for "last, first" names. Places that displayed a user's name in "last, first" format built it manually, and handled missing first or last names inconsistently (or not at all, which resulted in a stray comma). The new `sortable_name` method of the user database record returns "last, first", or just the name that is set, or an empty string if neither is set. Places that show just the name fall back to the user id when the name is empty. Places that also show the user id in parentheses show "(user_id)" alone when the name is empty. Also clean up the style of the other methods in the user record. Co-Authored-By: Claude Opus 5.5 --- lib/WeBWorK/ContentGenerator/GatewayQuiz.pm | 5 ++- .../Instructor/ProblemGrader.pm | 3 +- lib/WeBWorK/DB/Record/User.pm | 31 ++++++++++++------- lib/WeBWorK/HTML/StudentNav.pm | 2 +- .../Instructor/AchievementUserEditor.html.ep | 2 +- .../AddUsers/student_entry_report.html.ep | 4 ++- .../Instructor/ProblemSetDetail.html.ep | 2 +- .../Instructor/SendMail/main_form.html.ep | 12 +++---- .../Instructor/StudentProgress/index.html.ep | 6 ++-- .../StudentProgress/siblings.html.ep | 2 +- .../Instructor/UsersAssignedToSet.html.ep | 10 +++--- 11 files changed, 42 insertions(+), 37 deletions(-) diff --git a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm index 6099a44e32..5f18fcfd01 100644 --- a/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm +++ b/lib/WeBWorK/ContentGenerator/GatewayQuiz.pm @@ -1365,9 +1365,8 @@ sub nav ($c, $args) { push @userRecords, $user; - $user->{displayName} = - ($user->last_name || $user->first_name ? $user->last_name . ', ' . $user->first_name : $user->user_id); - $user->{setVersion} = $_->[2]; + $user->{displayName} = $user->sortable_name || $user->user_id; + $user->{setVersion} = $_->[2]; } # Sort by last name, then first name, then user_id, then set version. diff --git a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm index b5571a90dc..ea59280313 100644 --- a/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm +++ b/lib/WeBWorK/ContentGenerator/Instructor/ProblemGrader.pm @@ -67,8 +67,7 @@ async sub initialize ($c) { # First process student problems and answers and cache relevant data used both for # saving grades and displaying the grader table. for my $user (@{ $c->stash->{users} }) { - $user->{displayName} = - $user->last_name || $user->first_name ? $user->last_name . ', ' . $user->first_name : $user->user_id; + $user->{displayName} = $user->sortable_name || $user->user_id; $c->stash->{haveSections} = 1 if $user->section; if ($c->stash->{set}->assignment_type =~ /gateway/) { diff --git a/lib/WeBWorK/DB/Record/User.pm b/lib/WeBWorK/DB/Record/User.pm index afac971983..feebfd98cf 100644 --- a/lib/WeBWorK/DB/Record/User.pm +++ b/lib/WeBWorK/DB/Record/User.pm @@ -35,15 +35,22 @@ sub full_name { my $first = $self->first_name; my $last = $self->last_name; - if (defined $first and $first ne "" and defined $last and $last ne "") { - return "$first $last"; - } elsif (defined $first and $first ne "") { - return $first; - } elsif (defined $last and $last ne "") { - return $last; - } else { - return ""; - } + return "$first $last" if defined $first && $first =~ /\S/ && defined $last && $last =~ /\S/; + return $first if defined $first && $first =~ /\S/; + return $last if defined $last && $last =~ /\S/; + return ''; +} + +sub sortable_name { + my ($self) = @_; + + my $first = $self->first_name; + my $last = $self->last_name; + + return "$last, $first" if defined $first && $first =~ /\S/ && defined $last && $last =~ /\S/; + return $last if defined $last && $last =~ /\S/; + return $first if defined $first && $first =~ /\S/; + return ''; } # Do not base64 encode the names in the address. @@ -54,14 +61,14 @@ sub rfc822_mailbox { my $full_name = $self->full_name; my $address = $self->email_address; - if (defined $address and $address ne "") { - if (defined $full_name and $full_name ne "") { + if (defined $address && $address ne '') { + if (defined $full_name && $full_name ne '') { return "$full_name <$address>"; } else { return $address; } } else { - return ""; + return ''; } } diff --git a/lib/WeBWorK/HTML/StudentNav.pm b/lib/WeBWorK/HTML/StudentNav.pm index be8caf88af..939bb751f0 100644 --- a/lib/WeBWorK/HTML/StudentNav.pm +++ b/lib/WeBWorK/HTML/StudentNav.pm @@ -57,7 +57,7 @@ sub studentNav ($c, $setID) { push @userRecords, $_; # Construct a display name. - $_->{displayName} = ($_->last_name || $_->first_name ? $_->last_name . ', ' . $_->first_name : $_->user_id); + $_->{displayName} = $_->sortable_name || $_->user_id; } my $prevUser = $currentUserIndex > 0 ? $userRecords[ $currentUserIndex - 1 ] : 0; my $nextUser = $currentUserIndex < $#userRecords ? $userRecords[ $currentUserIndex + 1 ] : 0; diff --git a/templates/ContentGenerator/Instructor/AchievementUserEditor.html.ep b/templates/ContentGenerator/Instructor/AchievementUserEditor.html.ep index 44719849d8..3af512e69c 100644 --- a/templates/ContentGenerator/Instructor/AchievementUserEditor.html.ep +++ b/templates/ContentGenerator/Instructor/AchievementUserEditor.html.ep @@ -39,7 +39,7 @@ % for my $userRecord (@$userRecords) { % my $user = $userRecord->user_id; % my $userAchievement = $userAchievementRecords->{$user}; - % my $prettyName = $userRecord->last_name . ', ' . $userRecord->first_name; + % my $prettyName = $userRecord->sortable_name || $user; % diff --git a/templates/ContentGenerator/Instructor/AddUsers/student_entry_report.html.ep b/templates/ContentGenerator/Instructor/AddUsers/student_entry_report.html.ep index e70ffdc8e0..766dca1da6 100644 --- a/templates/ContentGenerator/Instructor/AddUsers/student_entry_report.html.ep +++ b/templates/ContentGenerator/Instructor/AddUsers/student_entry_report.html.ep @@ -2,7 +2,9 @@ <%= $addError ? maketext('Failed to add user:') : maketext('Added user:') %> - <%= $newUser->last_name %>, <%= $newUser->first_name %> + % if ($newUser->sortable_name) { + <%= $newUser->sortable_name %> + % } <%= maketext('Login:') %> <%= $newUser->user_id %> % if ($newUser->student_id) { diff --git a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep index 81d5824dc6..d1d5e28aef 100644 --- a/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep +++ b/templates/ContentGenerator/Instructor/ProblemSetDetail.html.ep @@ -88,7 +88,7 @@ % my $user = $db->getUser($userID); % my $line = begin - <%= $user->last_name %>, <%= $user->first_name %> + <%= $user->sortable_name %> (<%= link_to $user->user_id => 'mailto:' . $user->email_address %>). % if (!$editingSetVersion) { diff --git a/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep b/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep index 8204445628..4bbf596d68 100644 --- a/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep +++ b/templates/ContentGenerator/Instructor/SendMail/main_form.html.ep @@ -82,13 +82,13 @@ <%= maketext('for') %>  - <%= $c->{preview_user}->last_name . ', ' - . $c->{preview_user}->first_name . ' (' - . $c->{preview_user}->user_id . ')' =%> + <%= join(' ', + $c->{preview_user}->sortable_name || (), + '(' . $c->{preview_user}->user_id . ')') =%>
diff --git a/templates/ContentGenerator/Instructor/StudentProgress/index.html.ep b/templates/ContentGenerator/Instructor/StudentProgress/index.html.ep index d11ca242e8..4e502c4243 100644 --- a/templates/ContentGenerator/Instructor/StudentProgress/index.html.ep +++ b/templates/ContentGenerator/Instructor/StudentProgress/index.html.ep @@ -25,10 +25,8 @@ diff --git a/templates/ContentGenerator/Instructor/StudentProgress/siblings.html.ep b/templates/ContentGenerator/Instructor/StudentProgress/siblings.html.ep index 4d2af346ec..a5c86c4aad 100644 --- a/templates/ContentGenerator/Instructor/StudentProgress/siblings.html.ep +++ b/templates/ContentGenerator/Instructor/StudentProgress/siblings.html.ep @@ -18,7 +18,7 @@ class => 'nav-link' ), begin =%> - <%= $_->last_name =%>, <%= $_->first_name %> (<%= $_->user_id %>) + <%= join(' ', $_->sortable_name || (), '(' . $_->user_id . ')') %> <% end =%> % } diff --git a/templates/ContentGenerator/Instructor/UsersAssignedToSet.html.ep b/templates/ContentGenerator/Instructor/UsersAssignedToSet.html.ep index 0644f70427..8017007b47 100644 --- a/templates/ContentGenerator/Instructor/UsersAssignedToSet.html.ep +++ b/templates/ContentGenerator/Instructor/UsersAssignedToSet.html.ep @@ -77,9 +77,9 @@ % param('selected', []); % for my $user (@$user_records) { - % my $userID = $user->user_id; - % my $userSet = $set_records->{$userID}; - % my $username = $user->last_name . ', ' . $user->first_name; + % my $userID = $user->user_id; + % my $userSet = $set_records->{$userID}; + % my $name = $user->sortable_name; % @@ -88,7 +88,7 @@ id => "selected_$userID", class => 'form-check-input', data => { - username => "$username ($userID)", + username => join(' ', $name || (), "($userID)"), defined $userSet ? (is_assigned => 1) : (), $current_users->{$userID} ? (is_current => 1) : () }, @@ -100,7 +100,7 @@ - <%= $username =%> + <%= $name || $userID =%> <%= $user->section %> % if (defined $userSet) {