[Rt-commit] rt branch, 4.2/group-member-sort-order, created. rt-4.2.10-222-ga5f62fd
Alex Vandiver
alexmv at bestpractical.com
Thu Mar 19 15:16:07 EDT 2015
The branch, 4.2/group-member-sort-order has been created
at a5f62fd949806162f8a86acbd9f811f9f5f3f8d3 (commit)
- Log -----------------------------------------------------------------
commit 77c5646484088e5fdde7a0dad6a9ccfdaf0f5a47
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 %>">
diff --git a/t/web/ticket_modify_people.t b/t/web/ticket_modify_people.t
index cefbf91..f4821a5 100644
--- a/t/web/ticket_modify_people.t
+++ b/t/web/ticket_modify_people.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Test tests => 25;
+use RT::Test tests => undef;
my $root = RT::Test->load_or_create_user( Name => 'root' );
my $group_foo = RT::Group->new($RT::SystemUser);
@@ -36,11 +36,11 @@ ok( $m->login( 'user', 'password' ), 'logged in' );
$m->get_ok( $url . "/Ticket/ModifyPeople.html?id=" . $ticket->id );
ok(
- !$m->find_link(
- text => 'Enoch Root',
- url_regex => qr!/Admin/Users/Modify\.html!,
+ $m->find_link(
+ text => 'root (Enoch Root)',
+ url_regex => qr!/User/Summary\.html!,
),
- 'no link to modify user'
+ 'contains link to user summary page'
);
$m->content_contains('Enoch Root', 'still has the user name' );
@@ -54,24 +54,6 @@ ok(
$m->content_contains('group_foo', 'still has the group name' );
-ok( RT::Test->add_rights( { Principal => $user, Right => ['AdminUsers'] }, ),
- 'added AdminUsers right' );
-$m->reload;
-ok(
- !$m->find_link(
- text => 'Enoch Root',
- url_regex => qr!/Admin/Users/Modify\.html!,
- ),
- 'still no link to modify user'
-);
-ok(
- !$m->find_link(
- text => 'group_foo',
- url_regex => qr!/Admin/Groups/Modify\.html!,
- ),
- 'still no link to modify group'
-);
-
ok(
RT::Test->add_rights( { Principal => $user, Right => ['ShowConfigTab'] }, ),
'added ShowConfigTab right',
@@ -81,9 +63,9 @@ $m->reload;
ok(
$m->find_link(
text => 'root (Enoch Root)',
- url_regex => qr!/Admin/Users/Modify\.html!,
+ url_regex => qr!/User/Summary\.html!,
),
- 'got link to modify user'
+ 'still contains link to user summary page'
);
ok(
@@ -119,5 +101,8 @@ $m->submit_form_ok({
my $foo = RT::Test->load_or_create_user( EmailAddress => 'foo at example.com' );
is $foo->RealName, "Foo Bar", "RealName matches";
+undef $m;
+done_testing;
+
# TODO test Add|Delete people
commit 7eccdee28eedff9151e2b8b6942c396683c2fe97
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 a5f62fd949806162f8a86acbd9f811f9f5f3f8d3
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