[Rt-commit] rt branch, 4.2/group-member-sort-order, created. rt-4.2.10-222-g0a52dfe

Alex Vandiver alexmv at bestpractical.com
Thu Mar 19 12:51:31 EDT 2015


The branch, 4.2/group-member-sort-order has been created
        at  0a52dfe8c112cd9f37c940253064a2856a5004a2 (commit)

- Log -----------------------------------------------------------------
commit f84b50de8437509c48d111f8c8c54444985ac90d
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Mar 19 12:35:48 2015 -0400

    Stop wrapping ShowUser in <a> tags
    
    This leads to nested <a> tags, only the inner of which is usable; in
    general, explicitly linking to /Admin/User/Modify.html is unnecessary,
    as the ShowUser links to the user summary, which has a tab to the Modify
    page if the user has rights to do so.

diff --git a/share/html/Admin/Groups/Members.html b/share/html/Admin/Groups/Members.html
index a4ff4be..2248a14 100644
--- a/share/html/Admin/Groups/Members.html
+++ b/share/html/Admin/Groups/Members.html
@@ -82,7 +82,7 @@ my @users = sort { lc($a->[0]) cmp lc($b->[0]) }
 % my ($rendered, $user) = @$_;
 % $UsersSeen{ $user->id } = 1 if $SkipSeenUsers;
 <li><input type="checkbox" class="checkbox" name="DeleteMember-<% $user->PrincipalObj->Id %>" value="1" />
-<a href="<% RT->Config->Get('WebPath') %>/Admin/Users/Modify.html?id=<% $user->id %>"><% $rendered |n%></a></li>
+<% $rendered |n%></li>
 % }
 </ul>
 <&|/l&>Groups</&>
diff --git a/share/html/Elements/Crypt/KeyIssues b/share/html/Elements/Crypt/KeyIssues
index 35c1264..1c3ff4a 100644
--- a/share/html/Elements/Crypt/KeyIssues
+++ b/share/html/Elements/Crypt/KeyIssues
@@ -68,7 +68,7 @@
 % foreach my $issue ( @$Issues ) {
 <li>
 % if ( $issue->{'User'} ) {
-User <a href="<% RT->Config->Get('WebPath') %>/Admin/Users/Modify.html?id=<% $issue->{'User'}->id %>"><&/Elements/ShowUser, User => $issue->{'User'} &></a> has a problem.
+User <&/Elements/ShowUser, User => $issue->{'User'} &> has a problem.
 % } else {
 There is a problem with key/certificate(s) for address <% $issue->{'EmailAddress'} %>, but there is no user in the DB for this address.
 % }
diff --git a/share/html/Ticket/Elements/EditWatchers b/share/html/Ticket/Elements/EditWatchers
index 0f613ca..db4a0b3 100644
--- a/share/html/Ticket/Elements/EditWatchers
+++ b/share/html/Ticket/Elements/EditWatchers
@@ -57,14 +57,8 @@
 <li>
 <input type="checkbox" class="checkbox" name="Ticket-DeleteWatcher-Type-<% $Watchers->Name %>-Principal-<% $watcher->MemberId %>" value="1" unchecked />
 % if ( $member->isa( 'RT::User' ) ) { 
-% if ( $session{CurrentUser}->HasRight( Right => 'AdminUsers', Object => $RT::System ) &&
-%      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
-<a href="<% RT->Config->Get('WebPath') %>/Admin/Users/Modify.html?id=<% $watcher->MemberId %>">
-<& /Elements/ShowUser, User => $member &></a> <& /Elements/ShowUserEmailFrequency, User => $member, Ticket => $TicketObj &>
-% } else {
 <& /Elements/ShowUser, User => $member &> <& /Elements/ShowUserEmailFrequency, User => $member, Ticket => $TicketObj &>
-% } }
-% else {
+% } else {
 % if ( $session{CurrentUser}->HasRight( Right => 'AdminGroup', Object => $RT::System ) &&
 %      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
 <a href="<% RT->Config->Get('WebPath') %>/Admin/Groups/Modify.html?id=<% $watcher->MemberId %>">

commit 04f9290a90ab42c722835d554d0140a8772860ee
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Mar 19 12:40:39 2015 -0400

    Sort by text-only representation of the user, not HTML
    
    /Elements/ShowUser always includes a <span data-user-id="..."> element;
    thus, sorting the result leads to sorting ASCII-betically by id -- this
    is unhelpful, to say the least.
    
    Switch to sorting by $user->Format; this is, strictly, not guaranteed to
    be the same as the ShowUser component's text, as the latter has a
    callback that might modify it.  However, it keeps compatibility with
    this callsite respecting said callback, and still wraps in data-user-id
    for metadata (currently unused), while sorting by the presumed display.
    
    The computational overhead to effectively calling ->Format twice per
    user is judged to be small; should that cease being the case, it could
    be cached on the user object.
    
    Fixes: I#30771

diff --git a/share/html/Admin/Groups/Members.html b/share/html/Admin/Groups/Members.html
index 2248a14..7a1f41a 100644
--- a/share/html/Admin/Groups/Members.html
+++ b/share/html/Admin/Groups/Members.html
@@ -74,12 +74,12 @@
 % my $Users = $Group->UserMembersObj( Recursively => 0 );
 <%perl>
 my @users = sort { lc($a->[0]) cmp lc($b->[0]) }
-            map { [$m->scomp("/Elements/ShowUser", User => $_), $_] }
+            map { [$_->Format, $m->scomp( "/Elements/ShowUser", User => $_), $_] }
             @{ $Users->ItemsArrayRef };
 </%perl>
 <ul>
 % for (@users) {
-% my ($rendered, $user) = @$_;
+% my (undef, $rendered, $user) = @{$_};
 % $UsersSeen{ $user->id } = 1 if $SkipSeenUsers;
 <li><input type="checkbox" class="checkbox" name="DeleteMember-<% $user->PrincipalObj->Id %>" value="1" />
 <% $rendered |n%></li>

commit 0a52dfe8c112cd9f37c940253064a2856a5004a2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Thu Mar 19 12:51:00 2015 -0400

    Stop pre-computing ShowUser
    
    There is no longer a reason to look up ShowUser prior to rendering; this
    turns the array into a simple Schwartzian transform.

diff --git a/share/html/Admin/Groups/Members.html b/share/html/Admin/Groups/Members.html
index 7a1f41a..91da6f1 100644
--- a/share/html/Admin/Groups/Members.html
+++ b/share/html/Admin/Groups/Members.html
@@ -73,16 +73,16 @@
 <&|/l&>Users</&>
 % my $Users = $Group->UserMembersObj( Recursively => 0 );
 <%perl>
-my @users = sort { lc($a->[0]) cmp lc($b->[0]) }
-            map { [$_->Format, $m->scomp( "/Elements/ShowUser", User => $_), $_] }
+my @users = map {$_->[1]}
+            sort { lc($a->[0]) cmp lc($b->[0]) }
+            map { [$_->Format, $_] }
             @{ $Users->ItemsArrayRef };
 </%perl>
 <ul>
-% for (@users) {
-% my (undef, $rendered, $user) = @{$_};
+% for my $user (@users) {
 % $UsersSeen{ $user->id } = 1 if $SkipSeenUsers;
 <li><input type="checkbox" class="checkbox" name="DeleteMember-<% $user->PrincipalObj->Id %>" value="1" />
-<% $rendered |n%></li>
+<& /Elements/ShowUser, User => $user &></li>
 % }
 </ul>
 <&|/l&>Groups</&>

-----------------------------------------------------------------------


More information about the rt-commit mailing list