[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