[Rt-commit] rt branch, 4.2/link-asstring-improvements, created. rt-4.1.6-310-gc676925
Kevin Falcone
falcone at bestpractical.com
Wed Mar 6 18:36:00 EST 2013
The branch, 4.2/link-asstring-improvements has been created
at c6769254c4389c3a7f99636d112447f5bcf50230 (commit)
- Log -----------------------------------------------------------------
commit 3ae7e31bab4f1f5451e20db94b5c6389fe1a04ed
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 17:10:14 2013 -0500
Convert from convoluted code to using AsString
All core objects already handle printing "Ticket 42" or
http://bestpractical.com by checking locality. AsString is an immediate
win over the old behavior of printing (42).
diff --git a/share/html/Elements/RT__Ticket/ColumnMap b/share/html/Elements/RT__Ticket/ColumnMap
index dc6e769..66b5023 100644
--- a/share/html/Elements/RT__Ticket/ColumnMap
+++ b/share/html/Elements/RT__Ticket/ColumnMap
@@ -62,14 +62,13 @@ my $LinkCallback = sub {
my $type = $RT::Link::TYPEMAP{$method}{Type};
my $other_mode = ($mode eq "Target" ? "Base" : "Target");
my $mode_uri = $mode.'URI';
- my $local_type = 'Local'.$mode;
return sub {
map {
\'<a href="',
$_->$mode_uri->Resolver->HREF,
\'">',
- ( $_->$mode_uri->IsLocal ? $_->$local_type : $_->$mode ),
+ ( $_->$mode_uri->Resolver->AsString ),
\'</a><br />',
} @{ $_[0]->Links($other_mode,$type)->ItemsArrayRef }
}
commit e72d6793aa7f2bfd87f75ef4ec436b43f6577cdf
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 17:09:46 2013 -0500
Add some AsString sugar
We have a number of proxy methods in URI that just pass to Resolver, add
AsString to the list.
diff --git a/lib/RT/URI.pm b/lib/RT/URI.pm
index e36826f..32b1927 100644
--- a/lib/RT/URI.pm
+++ b/lib/RT/URI.pm
@@ -285,6 +285,17 @@ sub Resolver {
return ($self->{'resolver'});
}
+=head2 AsString
+
+Returns a friendly display form of the object if Local, or the full URI
+
+=cut
+
+sub AsString {
+ my $self = shift;
+ return $self->Resolver->AsString;
+}
+
RT::Base->_ImportOverlays();
1;
commit 6efaeb6357467c5f78a85d61ce0909f4c845b132
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Wed Mar 6 18:04:44 2013 -0500
Switch to using the shorter and nicer proxied methods in RT::URI
Now that I added AsString, clean up a bunch of ->Resolver
There are possibly more candidates
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index e73a312..13635ec 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -847,7 +847,7 @@ sub _FormatUser {
if ( $URI->FromURI( $self->NewValue ) ) {
$value = [
\'<a href="', $URI->AsHREF, \'">',
- $URI->Resolver->AsString,
+ $URI->AsString,
\'</a>'
];
}
@@ -889,7 +889,7 @@ sub _FormatUser {
if ( $URI->FromURI( $self->OldValue ) ) {
$value = [
\'<a href="', $URI->AsHREF, \'">',
- $URI->Resolver->AsString,
+ $URI->AsString,
\'</a>'
];
}
diff --git a/share/html/Articles/Article/Elements/EditLinks b/share/html/Articles/Article/Elements/EditLinks
index cf9a27b..24c3e1a 100644
--- a/share/html/Articles/Article/Elements/EditLinks
+++ b/share/html/Articles/Article/Elements/EditLinks
@@ -65,7 +65,7 @@
<li>
<input type="checkbox" name="DeleteLink--<%$link->Type%>-<%$link->Target%>" />
% if ($link->TargetURI->IsLocal) {
-<a href="<%$member->Resolver->HREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%></a>:
+<a href="<%$member->AsHREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%></a>:
% if (UNIVERSAL::isa($member->Object, "RT::Article") or UNIVERSAL::can($member->Object, 'Name')) {
<%$member->Object->Name%>
% } elsif (UNIVERSAL::isa($member->Object, "RT::Ticket") or UNIVERSAL::can($member->Object, 'Subject')) {
@@ -73,7 +73,7 @@
% }
</a>
% } else {
-<a href="<%$member->Resolver->HREF%>"><%$link->Target%></a>
+<a href="<%$member->AsHREF%>"><%$link->Target%></a>
% }
% }
% }
@@ -92,7 +92,7 @@
<li>
<input type="checkbox" name="DeleteLink-<%$link->Base%>-<%$link->Type%>-" />
% if ($link->BaseURI->IsLocal) {
-<a href="<%$member->Resolver->HREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
+<a href="<%$member->AsHREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
% if (UNIVERSAL::isa($member->Object, "RT::Article") or UNIVERSAL::can($member->Object, 'Name')) {
<%$member->Object->Name%>
% } elsif (UNIVERSAL::isa($member->Object, "RT::Ticket") or UNIVERSAL::can($member->Object, 'Subject')) {
@@ -100,7 +100,7 @@
% }
</a>
% } else {
-<a href="<%$member->Resolver->HREF%>"><%$link->Base%></a>
+<a href="<%$member->AsHREF%>"><%$link->Base%></a>
% }
% }
% }
diff --git a/share/html/Articles/Article/Elements/ShowLinks b/share/html/Articles/Article/Elements/ShowLinks
index 12b0b2f..4d5aa30 100644
--- a/share/html/Articles/Article/Elements/ShowLinks
+++ b/share/html/Articles/Article/Elements/ShowLinks
@@ -53,7 +53,7 @@
% my $member = $link->TargetURI;
<li>
% if ($link->TargetURI->IsLocal) {
-<a href="<%$member->Resolver->HREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
+<a href="<%$member->AsHREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
% if (UNIVERSAL::isa($member->Object, "RT::Article") or UNIVERSAL::can($member->Object, 'Name')) {
<%$member->Object->Name%>
% } elsif (UNIVERSAL::isa($member->Object, "RT::Ticket") or UNIVERSAL::can($member->Object, 'Subject')) {
@@ -61,7 +61,7 @@
% }
</a>
% } else {
-<a href="<%$member->Resolver->HREF%>"><%$link->Target%></a>
+<a href="<%$member->AsHREF%>"><%$link->Target%></a>
% }
</li>
% }
@@ -74,7 +74,7 @@
% my $member = $link->BaseURI;
<li>
% if ($member->IsLocal) {
-<a href="<%$member->Resolver->HREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
+<a href="<%$member->AsHREF%>"><% loc($member->Object->RecordType) %> <%$member->Object->Id%>:
% if (UNIVERSAL::isa($member->Object, "RT::Article") or UNIVERSAL::can($member->Object, 'Name')) {
<%$member->Object->Name%>
% } elsif (UNIVERSAL::isa($member->Object, "RT::Ticket") or UNIVERSAL::can($member->Object, 'Subject')) {
@@ -82,7 +82,7 @@
% }
</a>
% } else {
-<a href="<%$member->Resolver->HREF%>"><%$link->Base%></a>
+<a href="<%$member->AsHREF%>"><%$link->Base%></a>
% }
</li>
% }
diff --git a/share/html/Elements/RT__Ticket/ColumnMap b/share/html/Elements/RT__Ticket/ColumnMap
index 66b5023..31d4d0c 100644
--- a/share/html/Elements/RT__Ticket/ColumnMap
+++ b/share/html/Elements/RT__Ticket/ColumnMap
@@ -66,9 +66,9 @@ my $LinkCallback = sub {
return sub {
map {
\'<a href="',
- $_->$mode_uri->Resolver->HREF,
+ $_->$mode_uri->AsHREF,
\'">',
- ( $_->$mode_uri->Resolver->AsString ),
+ ( $_->$mode_uri->AsString ),
\'</a><br />',
} @{ $_[0]->Links($other_mode,$type)->ItemsArrayRef }
}
diff --git a/share/html/Elements/ShowLink b/share/html/Elements/ShowLink
index 1f8568a..d8be261 100644
--- a/share/html/Elements/ShowLink
+++ b/share/html/Elements/ShowLink
@@ -63,7 +63,7 @@
<%$URI->Resolver->AsString%>
% }
% } else {
-<%$URI->Resolver->AsString%>
+<%$URI->AsString%>
% }
</a>
<%ARGS>
@@ -71,7 +71,7 @@ $URI => undef
</%ARGS>
<%INIT>
-my $href = $URI->Resolver->HREF;
+my $href = $URI->AsHREF;
if ( $URI->IsLocal ) {
my $base = RT->Config->Get('WebBaseURL');
# URI->rel doesn't contain the leading '/'
commit 4d4d2df98edbeeef309ee82f453649a2989ba6d3
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Wed Mar 6 17:20:17 2013 -0500
Support __Link.{Object}__
This means that __RefersTo.{Article}__ now only prints Articles
This works for any sort of Local object type, and will implicitly skip
non local links.
diff --git a/share/html/Elements/RT__Ticket/ColumnMap b/share/html/Elements/RT__Ticket/ColumnMap
index 31d4d0c..cd3d236 100644
--- a/share/html/Elements/RT__Ticket/ColumnMap
+++ b/share/html/Elements/RT__Ticket/ColumnMap
@@ -64,13 +64,21 @@ my $LinkCallback = sub {
my $mode_uri = $mode.'URI';
return sub {
+ my $ObjectType = $_[2]||'';
map {
\'<a href="',
$_->$mode_uri->AsHREF,
\'">',
( $_->$mode_uri->AsString ),
\'</a><br />',
- } @{ $_[0]->Links($other_mode,$type)->ItemsArrayRef }
+ } # if someone says __RefersTo.{Ticket}__ filter for only local links that are tickets
+ grep { $ObjectType ?
+ ( $_->$mode_uri->IsLocal ?
+ ( $_->$mode_uri->Object->RecordType eq $ObjectType )
+ : 0 )
+ : 1
+ }
+ @{ $_[0]->Links($other_mode,$type)->ItemsArrayRef }
}
};
commit e3b1b248db8b128c0e4e1e7f19ea65a3daa77a4d
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Wed Mar 6 18:16:47 2013 -0500
Import Scalar::Util::blessed globally
Since mason is all one namespace, we were doing this implicitly in all
the various places we said use Scalar::Util qw(blessed). Make it an
explicit thing by shoving it into lib.
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index b2fb18b..fd6e839 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1572,6 +1572,8 @@ package HTML::Mason::Commands;
use vars qw/$r $m %session/;
+use Scalar::Util qw(blessed);
+
sub Menu {
return $HTML::Mason::Commands::m->notes('menu');
}
diff --git a/share/html/Admin/Elements/EditRights b/share/html/Admin/Elements/EditRights
index 76e9013..b66309b 100644
--- a/share/html/Admin/Elements/EditRights
+++ b/share/html/Admin/Elements/EditRights
@@ -51,8 +51,6 @@ $Principals
$AddPrincipal => undef
</%args>
<%init>
-use Scalar::Util qw(blessed);
-
# Let callbacks get at principals and context before we do anything with them
$m->callback( Principals => $Principals, Context => $Context );
diff --git a/share/html/Dashboards/Elements/ShowPortlet/dashboard b/share/html/Dashboards/Elements/ShowPortlet/dashboard
index f0af23a..b2eb0a3 100644
--- a/share/html/Dashboards/Elements/ShowPortlet/dashboard
+++ b/share/html/Dashboards/Elements/ShowPortlet/dashboard
@@ -56,7 +56,6 @@ $Depth => 0
<%init>
my $current_dashboard;
-use Scalar::Util 'blessed';
if (blessed($Portlet) && $Portlet->isa('RT::Dashboard')) {
$current_dashboard = $Portlet;
}
diff --git a/share/html/Elements/Header b/share/html/Elements/Header
index 4589fe6..481b8d1 100644
--- a/share/html/Elements/Header
+++ b/share/html/Elements/Header
@@ -107,8 +107,6 @@
% }
<div id="header"><h1><% $Title %></h1></div>
<%INIT>
-use Scalar::Util qw(blessed);
-
$r->headers_out->{'Pragma'} = 'no-cache';
$r->headers_out->{'Cache-control'} = 'no-cache';
diff --git a/share/html/Elements/Logo b/share/html/Elements/Logo
index a00f6bd..95dd8c1 100644
--- a/share/html/Elements/Logo
+++ b/share/html/Elements/Logo
@@ -67,7 +67,6 @@ if ( exists $ARGS{'show_name'} ) {
$ShowName = delete $ARGS{'show_name'};
}
-use Scalar::Util qw(blessed);
my $user_logo = blessed $RT::System ? $RT::System->FirstAttribute('UserLogo') : undef;
# If we have the attribute, but no content, we don't really have a user logo
commit 02bf4d780d7f8723df8deded7ad7223162c73004
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 19:21:22 2013 -0500
Stop using UNIVERSAL::isa
diff --git a/share/html/Elements/ShowLink b/share/html/Elements/ShowLink
index d8be261..39f1b30 100644
--- a/share/html/Elements/ShowLink
+++ b/share/html/Elements/ShowLink
@@ -49,7 +49,7 @@
% if ($URI->IsLocal) {
% my $member = $URI->Object;
% my $has_name = UNIVERSAL::can($member, 'Name') || (UNIVERSAL::can($member, '_Accessible') && $member->_Accessible('Name', 'read'));
-% if (UNIVERSAL::isa($member, "RT::Ticket")) {
+% if (blessed($member) && $member->isa("RT::Ticket")) {
% my $inactive = $member->QueueObj->IsInactiveStatus($member->Status);
<span class="<% $inactive ? 'ticket-inactive' : '' %>">
commit af5a86fb8011caecf771793432c280035975c16d
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 19:22:06 2013 -0500
Stop checking for Name
We can just make AsString smarter and include the Name, rather than
going through contortions here.
diff --git a/share/html/Elements/ShowLink b/share/html/Elements/ShowLink
index 39f1b30..e32efd2 100644
--- a/share/html/Elements/ShowLink
+++ b/share/html/Elements/ShowLink
@@ -48,7 +48,6 @@
<a href="<% $href %>">
% if ($URI->IsLocal) {
% my $member = $URI->Object;
-% my $has_name = UNIVERSAL::can($member, 'Name') || (UNIVERSAL::can($member, '_Accessible') && $member->_Accessible('Name', 'read'));
% if (blessed($member) && $member->isa("RT::Ticket")) {
% my $inactive = $member->QueueObj->IsInactiveStatus($member->Status);
@@ -57,8 +56,6 @@
<%$member->Id%>: (<& /Elements/ShowUser, User => $member->OwnerObj &>) <%$member->Subject || ''%> [<% loc($member->Status) %>]
</span>
-% } elsif ($has_name) {
-<%$URI->Resolver->AsString%>: <%$member->Name%>
% } else {
<%$URI->Resolver->AsString%>
% }
commit df330a7d11b6b556baa710e281c6c691118cfd0b
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 19:22:44 2013 -0500
We don't really care if this is local or not
All the core AsString methods already handle Local vs non-local so just
let that code make the decision about whether to return "Foo #3" or
http://example.com.
diff --git a/share/html/Elements/ShowLink b/share/html/Elements/ShowLink
index e32efd2..9593230 100644
--- a/share/html/Elements/ShowLink
+++ b/share/html/Elements/ShowLink
@@ -46,7 +46,6 @@
%#
%# END BPS TAGGED BLOCK }}}
<a href="<% $href %>">
-% if ($URI->IsLocal) {
% my $member = $URI->Object;
% if (blessed($member) && $member->isa("RT::Ticket")) {
% my $inactive = $member->QueueObj->IsInactiveStatus($member->Status);
@@ -57,9 +56,6 @@
</span>
% } else {
-<%$URI->Resolver->AsString%>
-% }
-% } else {
<%$URI->AsString%>
% }
</a>
commit 453f0286e3bdf8e43c0b977eb595e82441e4e2f3
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 19:40:13 2013 -0500
AsString on Tickets and Articles now includes the Name
This allows us to remove the ugly code in ShowLink and make it ugly in
here instead.
This means that Formats which say __RefersTo__ or __HasMember__ will
now say "ticket #42: Ticket Subject" rather than the current state of
the world where that renders as "ticket #42" which is not helpful.
The code in rt.pm's AsString is ugly because it assumes that you might
use this on some non-RT::Ticket object. That never really happens, but
all of this code pretends it might happen so I kept up the illusion.
diff --git a/lib/RT/URI/fsck_com_article.pm b/lib/RT/URI/fsck_com_article.pm
index 8d12a5d..19e913b 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -200,9 +200,12 @@ Return "Article 23"
sub AsString {
my $self = shift;
- if ($self->IsLocal && $self->Object) {
- return $self->loc('Article [_1]', $self->Object->id);
-
+ if ($self->IsLocal && ( my $object = $self->Object )) {
+ if ( $object->Name ) {
+ return $self->loc('Article #[_1]: [_2]', $object->id, $object->Name);
+ } else {
+ return $self->loc('Article #[_1]', $object->id);
+ }
} else {
return $self->SUPER::AsString(@_);
}
diff --git a/lib/RT/URI/fsck_com_rt.pm b/lib/RT/URI/fsck_com_rt.pm
index 34249d0..4772959 100644
--- a/lib/RT/URI/fsck_com_rt.pm
+++ b/lib/RT/URI/fsck_com_rt.pm
@@ -215,8 +215,18 @@ Returns either a localized string 'ticket #23' or the full URI if the object is
sub AsString {
my $self = shift;
- if ($self->IsLocal && $self->Object) {
- return $self->loc("[_1] #[_2]", $self->ObjectType, $self->Object->Id);
+ if ($self->IsLocal && ( my $object = $self->Object )) {
+ my $name = "";
+ if ($object->isa('RT::Ticket')) {
+ $name = $object->Subject||'';
+ } elsif ( $object->_Accessible('Name', 'read') ) {
+ $name = $object->Name||'';
+ }
+ if ( $name ) {
+ return $self->loc("[_1] #[_2]: [_3]", $self->ObjectType, $object->Id, $name);
+ } else {
+ return $self->loc("[_1] #[_2]", $self->ObjectType, $object->Id);
+ }
}
else {
return $self->URI;
commit f9eee73db51f1b8d35e20118c707195ab64c6aff
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 20:21:34 2013 -0500
Render as Ticket #42 instead of ticket #42
It looks really odd that we say 'ticket' but every other link type is
ucfirsted. However, we rely on that hardcoded lowercase 'ticket'
(hardcoded in ObjectType) in HREF and other link building code.
diff --git a/lib/RT/URI/fsck_com_rt.pm b/lib/RT/URI/fsck_com_rt.pm
index 4772959..c1df39f 100644
--- a/lib/RT/URI/fsck_com_rt.pm
+++ b/lib/RT/URI/fsck_com_rt.pm
@@ -222,10 +222,14 @@ sub AsString {
} elsif ( $object->_Accessible('Name', 'read') ) {
$name = $object->Name||'';
}
+
+ # awful historical baggage, we rely on the lowercase ticket in a number of places
+ my $object_type = ($self->ObjectType eq 'ticket' ? 'Ticket' : $self->ObjectType);
+
if ( $name ) {
- return $self->loc("[_1] #[_2]: [_3]", $self->ObjectType, $object->Id, $name);
+ return $self->loc("[_1] #[_2]: [_3]", $object_type, $object->Id, $name);
} else {
- return $self->loc("[_1] #[_2]", $self->ObjectType, $object->Id);
+ return $self->loc("[_1] #[_2]", $object_type, $object->Id);
}
}
else {
commit c6769254c4389c3a7f99636d112447f5bcf50230
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 20:10:37 2013 -0500
Fix up failing tests
Article links now contain #
Merge and other links in ticket history contain subjects
ticket 33 is now Ticket 33
diff --git a/t/web/articles-links.t b/t/web/articles-links.t
index 713dc05..40996e3 100644
--- a/t/web/articles-links.t
+++ b/t/web/articles-links.t
@@ -44,7 +44,7 @@ $m->click('SubmitTicket');
$m->follow_link_ok({text => 'Links'});
-$m->text_contains('Article ' . $article->id . ': instance of ticket #17421', 'Article appears with its name in the links table');
+$m->text_contains('Article #' . $article->id . ': instance of ticket #17421', 'Article appears with its name in the links table');
my $refers_to = $ticket->RefersTo;
is($refers_to->Count, 1, 'the ticket has a refers-to link');
diff --git a/t/web/command_line.t b/t/web/command_line.t
index c49f6be..4ffe311 100644
--- a/t/web/command_line.t
+++ b/t/web/command_line.t
@@ -401,9 +401,9 @@ expect_send("merge $merge_ticket_B $merge_ticket_A", 'Merging the tickets...');
expect_like(qr/Merge completed/, 'Merged the tickets');
expect_send("show ticket/$merge_ticket_A/history", 'Checking merge on first ticket');
-expect_like(qr/Merged into ticket #$merge_ticket_A by root/, 'Merge recorded in first ticket');
+expect_like(qr/Merged into Ticket #$merge_ticket_A: CLIMergeTest1-$$ by root/, 'Merge recorded in first ticket');
expect_send("show ticket/$merge_ticket_B/history", 'Checking merge on second ticket');
-expect_like(qr/Merged into ticket #$merge_ticket_A by root/, 'Merge recorded in second ticket');
+expect_like(qr/Merged into Ticket #$merge_ticket_A: CLIMergeTest1-$$ by root/, 'Merge recorded in second ticket');
{
# create a user; give them privileges to take and steal
diff --git a/t/web/ticket_links.t b/t/web/ticket_links.t
index d739a85..994630e 100644
--- a/t/web/ticket_links.t
+++ b/t/web/ticket_links.t
@@ -165,7 +165,7 @@ for my $type ( "DependsOn", "MemberOf", "RefersTo" ) {
$m->content_lacks('hello test reminder subject');
if ($type eq 'RefersTo') {
$m->text_contains("$baseurl/test_ticket_reference");
- $m->text_contains("Article " . $article->Id . ': test article');
+ $m->text_contains("Article #" . $article->Id . ': test article');
}
}
}
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list