[Rt-commit] rt branch, 4.2/link-asstring-improvements, created. rt-4.1.6-303-g22c839a
Kevin Falcone
falcone at bestpractical.com
Tue Mar 5 20:48:25 EST 2013
The branch, 4.2/link-asstring-improvements has been created
at 22c839a3436c4c9b66b0dbfa843818fcfaa46d8e (commit)
- Log -----------------------------------------------------------------
commit cae0142162c874ec93a1cd77319079268e92e479
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 514beab9cda7be721fd462244d116bfdb7a483c1
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..823fb7e 100644
--- a/share/html/Elements/RT__Ticket/ColumnMap
+++ b/share/html/Elements/RT__Ticket/ColumnMap
@@ -69,7 +69,7 @@ my $LinkCallback = sub {
\'<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 0508cbe0006a91096a06fd81196f8204bf2e528f
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 1f8568a..d0ddb19 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' : '' %>">
@@ -71,6 +71,7 @@ $URI => undef
</%ARGS>
<%INIT>
+use Scalar::Util qw(blessed);
my $href = $URI->Resolver->HREF;
if ( $URI->IsLocal ) {
my $base = RT->Config->Get('WebBaseURL');
commit b8edfd894b66579e11f306099841c3e0a4d5f9c7
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 d0ddb19..2f04b32 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 8a4e3cb3fc032f7b25102092f698138dfa7cb7f8
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 2f04b32..e676ec4 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);
@@ -59,9 +58,6 @@
% } else {
<%$URI->Resolver->AsString%>
% }
-% } else {
-<%$URI->Resolver->AsString%>
-% }
</a>
<%ARGS>
$URI => undef
commit bfceb1011cea7ad20c8d49d3c1ce1a8937c9c271
Author: Kevin Falcone <falcone at bestpractical.com>
Date: Tue Mar 5 19:23:32 2013 -0500
Switch to using the shorter and nicer proxied methods in RT::URI
diff --git a/share/html/Elements/ShowLink b/share/html/Elements/ShowLink
index e676ec4..4995626 100644
--- a/share/html/Elements/ShowLink
+++ b/share/html/Elements/ShowLink
@@ -56,7 +56,7 @@
</span>
% } else {
-<%$URI->Resolver->AsString%>
+<%$URI->AsString%>
% }
</a>
<%ARGS>
@@ -65,7 +65,7 @@ $URI => undef
<%INIT>
use Scalar::Util qw(blessed);
-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 2e719c595f38cb2edf72e120cbb40cf296e396c0
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 "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..a85a24c 100644
--- a/lib/RT/URI/fsck_com_article.pm
+++ b/lib/RT/URI/fsck_com_article.pm
@@ -201,8 +201,12 @@ Return "Article 23"
sub AsString {
my $self = shift;
if ($self->IsLocal && $self->Object) {
- return $self->loc('Article [_1]', $self->Object->id);
-
+ 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 138a8bc4d46cdf4d933c1e813ec8851110168270
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' 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 22c839a3436c4c9b66b0dbfa843818fcfaa46d8e
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