[Rt-commit] rt branch, 4.2/links-in-history, created. rt-4.1.5-15-gec01660

Thomas Sibley trs at bestpractical.com
Thu Dec 6 21:20:48 EST 2012


The branch, 4.2/links-in-history has been created
        at  ec01660cb2e70781f3716b762f5e3b829cd967d4 (commit)

- Log -----------------------------------------------------------------
commit 4164ad8ebfc14cc5d0d8b10d9074c0e071290fc5
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Dec 5 15:35:28 2012 -0800

    Consolidate all BriefDescription generation for transactions
    
    Most handlers were in the %_BriefDescriptions hash, but some remained
    outside in the method for no apparent reason.  Consistent handling is
    easier with them all in one place.
    
    The few regex comparisons are now string equality, but the regexes
    aren't necessary for any core RT transaction types.  Any extension which
    created transaction types matching
    
        /(Status|SystemError|Forward (Ticket|Transaction))/
    
    but not matching those unrolled fixed strings will now need to add
    handlers to %RT::Transaction::_BriefDescriptions.

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 3ffb9a7..d1f59b9 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -612,27 +612,47 @@ sub BriefDescription {
         return ( $self->loc("Permission Denied") );
     }
 
-    my $type = $self->Type;    #cache this, rather than calling it 30 times
+    my $type = $self->Type;
 
     unless ( defined $type ) {
         return $self->loc("No transaction type specified");
     }
 
-    my $obj_type = $self->FriendlyObjectType;
-
-    if ( $type eq 'Create' ) {
-        return ( $self->loc( "[_1] created", $obj_type ) );
-    }
-    elsif ( $type eq 'Enabled' ) {
-        return ( $self->loc( "[_1] enabled", $obj_type ) );
-    }
-    elsif ( $type eq 'Disabled' ) {
-        return ( $self->loc( "[_1] disabled", $obj_type ) );
+    if ( my $code = $_BriefDescriptions{$type} ) {
+        return $code->($self);
     }
-    elsif ( $type =~ /Status/ ) {
+
+    return $self->loc(
+        "Default: [_1]/[_2] changed from [_3] to [_4]",
+        $type,
+        $self->Field,
+        (
+            $self->OldValue
+            ? "'" . $self->OldValue . "'"
+            : $self->loc("(no value)")
+        ),
+        "'" . $self->NewValue . "'"
+    );
+}
+
+%_BriefDescriptions = (
+    Create => sub {
+        my $self = shift;
+        return ( $self->loc( "[_1] created", $self->FriendlyObjectType ) );
+    },
+    Enabled => sub {
+        my $self = shift;
+        return ( $self->loc( "[_1] enabled", $self->FriendlyObjectType ) );
+    },
+    Disabled => sub {
+        my $self = shift;
+        return ( $self->loc( "[_1] disabled", $self->FriendlyObjectType ) );
+    },
+    Status => sub {
+        my $self = shift;
         if ( $self->Field eq 'Status' ) {
             if ( $self->NewValue eq 'deleted' ) {
-                return ( $self->loc( "[_1] deleted", $obj_type ) );
+                return ( $self->loc( "[_1] deleted", $self->FriendlyObjectType ) );
             }
             else {
                 return (
@@ -656,36 +676,20 @@ sub BriefDescription {
                 "'" . $self->NewValue . "'"
             )
         );
-    }
-    elsif ( $type =~ /SystemError/ ) {
+    },
+    SystemError => sub {
+        my $self = shift;
         return $self->loc("System error");
-    }
-    elsif ( $type =~ /Forward Transaction/ ) {
+    },
+    "Forward Transaction" => sub {
+        my $self = shift;
         return $self->loc( "Forwarded Transaction #[_1] to [_2]",
             $self->Field, $self->Data );
-    }
-    elsif ( $type =~ /Forward Ticket/ ) {
+    },
+    "Forward Ticket" => sub {
+        my $self = shift;
         return $self->loc( "Forwarded Ticket to [_1]", $self->Data );
-    }
-
-    if ( my $code = $_BriefDescriptions{$type} ) {
-        return $code->($self);
-    }
-
-    return $self->loc(
-        "Default: [_1]/[_2] changed from [_3] to [_4]",
-        $type,
-        $self->Field,
-        (
-            $self->OldValue
-            ? "'" . $self->OldValue . "'"
-            : $self->loc("(no value)")
-        ),
-        "'" . $self->NewValue . "'"
-    );
-}
-
-%_BriefDescriptions = (
+    },
     CommentEmailRecord => sub {
         my $self = shift;
         return $self->loc("Outgoing email about a comment recorded");

commit 007eb26b8f8414614f2f5445ab49de33e86662ac
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Wed Dec 5 15:43:14 2012 -0800

    Provide transaction brief descriptions as HTML
    
    None of the descriptions are actually HTML yet, but the method is
    available and waiting for descriptions to be HTML-ified.
    
    Plain text brief descriptions are now scrubbed versions of the HTML for
    consistency and single point of maintenance.
    
    Instead of calling loc() themselves, the description handlers now return
    a list which is passed to loc() after being properly escaped.  The
    handling is similar to column map return values: scalar refs are passed
    through unescaped, array refs are recursively processed and joined into
    a single value, and everything else is HTML escaped.  This makes it easy
    to start providing HTML descriptions without the hassle of escaping and
    concatenation everywhere.
    
    One negative side-effect of this is that all the localizable strings
    will lose their example parameters when extracted into the message
    catalog.  We can remedy this in the future by providing a comment mark
    like #loc that also captures following parameters.

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index d1f59b9..652f399 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -84,7 +84,11 @@ use RT::Ruleset;
 
 use HTML::FormatText;
 use HTML::TreeBuilder;
+use HTML::Scrubber;
 
+# For EscapeHTML() and decode_entities()
+require RT::Interface::Web;
+require HTML::Entities;
 
 sub Table {'Transactions'}
 
@@ -605,7 +609,25 @@ Returns a text string which briefly describes this transaction
 
 =cut
 
-sub BriefDescription {
+{
+    my $scrubber = HTML::Scrubber->new(default => 0); # deny everything
+
+    sub BriefDescription {
+        my $self = shift;
+        my $desc = $self->BriefDescriptionAsHTML;
+           $desc = $scrubber->scrub($desc);
+           $desc = HTML::Entities::decode_entities($desc);
+        return $desc;
+    }
+}
+
+=head2 BriefDescriptionAsHTML
+
+Returns an HTML string which briefly describes this transaction.
+
+=cut
+
+sub BriefDescriptionAsHTML {
     my $self = shift;
 
     unless ( $self->CurrentUserCanSee ) {
@@ -618,12 +640,8 @@ sub BriefDescription {
         return $self->loc("No transaction type specified");
     }
 
-    if ( my $code = $_BriefDescriptions{$type} ) {
-        return $code->($self);
-    }
-
-    return $self->loc(
-        "Default: [_1]/[_2] changed from [_3] to [_4]",
+    my ($template, @params) = (
+        "Default: [_1]/[_2] changed from [_3] to [_4]", #loc
         $type,
         $self->Field,
         (
@@ -631,80 +649,99 @@ sub BriefDescription {
             ? "'" . $self->OldValue . "'"
             : $self->loc("(no value)")
         ),
-        "'" . $self->NewValue . "'"
+        (
+            $self->NewValue
+            ? "'" . $self->NewValue . "'"
+            : $self->loc("(no value)")
+        ),
     );
+
+    if ( my $code = $_BriefDescriptions{$type} ) {
+        ($template, @params) = $code->($self);
+    }
+
+    unless ($template) {
+        ($template, @params) = ("(No description)"); #loc
+    }
+    return $self->loc($template, $self->_ProcessReturnValues(@params));
+}
+
+sub _ProcessReturnValues {
+    my $self   = shift;
+    my @values = @_;
+    return map {
+        if    (ref eq 'ARRAY')  { $_ = join "", $self->_ProcessReturnValues(@$_) }
+        elsif (ref eq 'SCALAR') { $_ = $$_ }
+        else                    { RT::Interface::Web::EscapeHTML(\$_) }
+        $_
+    } @values;
 }
 
 %_BriefDescriptions = (
     Create => sub {
         my $self = shift;
-        return ( $self->loc( "[_1] created", $self->FriendlyObjectType ) );
+        return ( "[_1] created", $self->FriendlyObjectType );   #loc
     },
     Enabled => sub {
         my $self = shift;
-        return ( $self->loc( "[_1] enabled", $self->FriendlyObjectType ) );
+        return ( "[_1] enabled", $self->FriendlyObjectType );   #loc
     },
     Disabled => sub {
         my $self = shift;
-        return ( $self->loc( "[_1] disabled", $self->FriendlyObjectType ) );
+        return ( "[_1] disabled", $self->FriendlyObjectType );  #loc
     },
     Status => sub {
         my $self = shift;
         if ( $self->Field eq 'Status' ) {
             if ( $self->NewValue eq 'deleted' ) {
-                return ( $self->loc( "[_1] deleted", $self->FriendlyObjectType ) );
+                return ( "[_1] deleted", $self->FriendlyObjectType );   #loc
             }
             else {
                 return (
-                    $self->loc(
-                        "Status changed from [_1] to [_2]",
-                        "'" . $self->loc( $self->OldValue ) . "'",
-                        "'" . $self->loc( $self->NewValue ) . "'"
-                    )
+                    "Status changed from [_1] to [_2]",                 #loc
+                    "'" . $self->loc( $self->OldValue ) . "'",
+                    "'" . $self->loc( $self->NewValue ) . "'"
                 );
-
             }
         }
 
         # Generic:
         my $no_value = $self->loc("(no value)");
         return (
-            $self->loc(
-                "[_1] changed from [_2] to [_3]",
-                $self->Field,
-                ( $self->OldValue ? "'" . $self->OldValue . "'" : $no_value ),
-                "'" . $self->NewValue . "'"
-            )
+            "[_1] changed from [_2] to [_3]", #loc
+            $self->Field,
+            ( $self->OldValue ? "'" . $self->OldValue . "'" : $no_value ),
+            "'" . $self->NewValue . "'"
         );
     },
     SystemError => sub {
         my $self = shift;
-        return $self->loc("System error");
+        return ("System error"); #loc
     },
     "Forward Transaction" => sub {
         my $self = shift;
-        return $self->loc( "Forwarded Transaction #[_1] to [_2]",
+        return ( "Forwarded Transaction #[_1] to [_2]", #loc
             $self->Field, $self->Data );
     },
     "Forward Ticket" => sub {
         my $self = shift;
-        return $self->loc( "Forwarded Ticket to [_1]", $self->Data );
+        return ( "Forwarded Ticket to [_1]", $self->Data ); #loc
     },
     CommentEmailRecord => sub {
         my $self = shift;
-        return $self->loc("Outgoing email about a comment recorded");
+        return ("Outgoing email about a comment recorded"); #loc
     },
     EmailRecord => sub {
         my $self = shift;
-        return $self->loc("Outgoing email recorded");
+        return ("Outgoing email recorded"); #loc
     },
     Correspond => sub {
         my $self = shift;
-        return $self->loc("Correspondence added");
+        return ("Correspondence added");    #loc
     },
     Comment => sub {
         my $self = shift;
-        return $self->loc("Comments added");
+        return ("Comments added");          #loc
     },
     CustomField => sub {
         my $self = shift;
@@ -722,22 +759,22 @@ sub BriefDescription {
         my $old = $self->OldValue;
 
         if ( !defined($old) || $old eq '' ) {
-            return $self->loc("[_1] [_2] added", $field, $new);
+            return ("[_1] [_2] added", $field, $new);   #loc
         }
         elsif ( !defined($new) || $new eq '' ) {
-            return $self->loc("[_1] [_2] deleted", $field, $old);
+            return ("[_1] [_2] deleted", $field, $old); #loc
         }
         else {
-            return $self->loc("[_1] [_2] changed to [_3]", $field, $old, $new);
+            return ("[_1] [_2] changed to [_3]", $field, $old, $new);   #loc
         }
     },
     Untake => sub {
         my $self = shift;
-        return $self->loc("Untaken");
+        return ("Untaken"); #loc
     },
     Take => sub {
         my $self = shift;
-        return $self->loc("Taken");
+        return ("Taken"); #loc
     },
     Force => sub {
         my $self = shift;
@@ -746,35 +783,35 @@ sub BriefDescription {
         my $New = RT::User->new( $self->CurrentUser );
         $New->Load( $self->NewValue );
 
-        return $self->loc("Owner forcibly changed from [_1] to [_2]" , $Old->Name , $New->Name);
+        return ("Owner forcibly changed from [_1] to [_2]" , $Old->Name , $New->Name); #loc
     },
     Steal => sub {
         my $self = shift;
         my $Old = RT::User->new( $self->CurrentUser );
         $Old->Load( $self->OldValue );
-        return $self->loc("Stolen from [_1]",  $Old->Name);
+        return ("Stolen from [_1]",  $Old->Name);   #loc
     },
     Give => sub {
         my $self = shift;
         my $New = RT::User->new( $self->CurrentUser );
         $New->Load( $self->NewValue );
-        return $self->loc( "Given to [_1]",  $New->Name );
+        return ( "Given to [_1]",  $New->Name );    #loc
     },
     AddWatcher => sub {
         my $self = shift;
         my $principal = RT::Principal->new($self->CurrentUser);
         $principal->Load($self->NewValue);
-        return $self->loc( "[_1] [_2] added", $self->loc($self->Field), $principal->Object->Name);
+        return ( "[_1] [_2] added", $self->loc($self->Field), $principal->Object->Name);    #loc
     },
     DelWatcher => sub {
         my $self = shift;
         my $principal = RT::Principal->new($self->CurrentUser);
         $principal->Load($self->OldValue);
-        return $self->loc( "[_1] [_2] deleted", $self->loc($self->Field), $principal->Object->Name);
+        return ( "[_1] [_2] deleted", $self->loc($self->Field), $principal->Object->Name);  #loc
     },
     Subject => sub {
         my $self = shift;
-        return $self->loc( "Subject changed to [_1]", $self->Data );
+        return ( "Subject changed to [_1]", $self->Data );  #loc
     },
     AddLink => sub {
         my $self = shift;
@@ -789,30 +826,30 @@ sub BriefDescription {
                 $value = $self->NewValue;
             }
             if ( $self->Field eq 'DependsOn' ) {
-                return $self->loc( "Dependency on [_1] added", $value );
+                return ( "Dependency on [_1] added", $value );  #loc
             }
             elsif ( $self->Field eq 'DependedOnBy' ) {
-                return $self->loc( "Dependency by [_1] added", $value );
+                return ( "Dependency by [_1] added", $value );  #loc
 
             }
             elsif ( $self->Field eq 'RefersTo' ) {
-                return $self->loc( "Reference to [_1] added", $value );
+                return ( "Reference to [_1] added", $value );   #loc
             }
             elsif ( $self->Field eq 'ReferredToBy' ) {
-                return $self->loc( "Reference by [_1] added", $value );
+                return ( "Reference by [_1] added", $value );   #loc
             }
             elsif ( $self->Field eq 'MemberOf' ) {
-                return $self->loc( "Membership in [_1] added", $value );
+                return ( "Membership in [_1] added", $value );  #loc
             }
             elsif ( $self->Field eq 'HasMember' ) {
-                return $self->loc( "Member [_1] added", $value );
+                return ( "Member [_1] added", $value );         #loc
             }
             elsif ( $self->Field eq 'MergedInto' ) {
-                return $self->loc( "Merged into [_1]", $value );
+                return ( "Merged into [_1]", $value );          #loc
             }
         }
         else {
-            return ( $self->Data );
+            return ( "[_1]", $self->Data ); #loc
         }
     },
     DeleteLink => sub {
@@ -829,27 +866,26 @@ sub BriefDescription {
             }
 
             if ( $self->Field eq 'DependsOn' ) {
-                return $self->loc( "Dependency on [_1] deleted", $value );
+                return ( "Dependency on [_1] deleted", $value );    #loc
             }
             elsif ( $self->Field eq 'DependedOnBy' ) {
-                return $self->loc( "Dependency by [_1] deleted", $value );
-
+                return ( "Dependency by [_1] deleted", $value );    #loc
             }
             elsif ( $self->Field eq 'RefersTo' ) {
-                return $self->loc( "Reference to [_1] deleted", $value );
+                return ( "Reference to [_1] deleted", $value );     #loc
             }
             elsif ( $self->Field eq 'ReferredToBy' ) {
-                return $self->loc( "Reference by [_1] deleted", $value );
+                return ( "Reference by [_1] deleted", $value );     #loc
             }
             elsif ( $self->Field eq 'MemberOf' ) {
-                return $self->loc( "Membership in [_1] deleted", $value );
+                return ( "Membership in [_1] deleted", $value );    #loc
             }
             elsif ( $self->Field eq 'HasMember' ) {
-                return $self->loc( "Member [_1] deleted", $value );
+                return ( "Member [_1] deleted", $value );           #loc
             }
         }
         else {
-            return ( $self->Data );
+            return ( "[_1]", $self->Data ); #loc
         }
     },
     Told => sub {
@@ -859,26 +895,26 @@ sub BriefDescription {
             $t1->Set(Format => 'ISO', Value => $self->NewValue);
             my $t2 = RT::Date->new($self->CurrentUser);
             $t2->Set(Format => 'ISO', Value => $self->OldValue);
-            return $self->loc( "[_1] changed from [_2] to [_3]", $self->loc($self->Field), $t2->AsString, $t1->AsString );
+            return ( "[_1] changed from [_2] to [_3]", $self->loc($self->Field), $t2->AsString, $t1->AsString );    #loc
         }
         else {
-            return $self->loc( "[_1] changed from [_2] to [_3]",
-                               $self->loc($self->Field),
-                               ($self->OldValue? "'".$self->OldValue ."'" : $self->loc("(no value)")) , "'". $self->NewValue."'" );
+            return ( "[_1] changed from [_2] to [_3]",  #loc
+                    $self->loc($self->Field),
+                    ($self->OldValue? "'".$self->OldValue ."'" : $self->loc("(no value)")) , "'". $self->NewValue."'" );
         }
     },
     Set => sub {
         my $self = shift;
         if ( $self->Field eq 'Password' ) {
-            return $self->loc('Password changed');
+            return ('Password changed');    #loc
         }
         elsif ( $self->Field eq 'Queue' ) {
             my $q1 = RT::Queue->new( $self->CurrentUser );
             $q1->Load( $self->OldValue );
             my $q2 = RT::Queue->new( $self->CurrentUser );
             $q2->Load( $self->NewValue );
-            return $self->loc("[_1] changed from [_2] to [_3]",
-                              $self->loc($self->Field) , $q1->Name , $q2->Name);
+            return ("[_1] changed from [_2] to [_3]",   #loc
+                    $self->loc($self->Field) , $q1->Name , $q2->Name);
         }
 
         # Write the date/time change at local time:
@@ -887,7 +923,7 @@ sub BriefDescription {
             $t1->Set(Format => 'ISO', Value => $self->NewValue);
             my $t2 = RT::Date->new($self->CurrentUser);
             $t2->Set(Format => 'ISO', Value => $self->OldValue);
-            return $self->loc( "[_1] changed from [_2] to [_3]", $self->loc($self->Field), $t2->AsString, $t1->AsString );
+            return ( "[_1] changed from [_2] to [_3]", $self->loc($self->Field), $t2->AsString, $t1->AsString );    #loc
         }
         elsif ( $self->Field eq 'Owner' ) {
             my $Old = RT::User->new( $self->CurrentUser );
@@ -897,61 +933,58 @@ sub BriefDescription {
 
             if ( $Old->id == RT->Nobody->id ) {
                 if ( $New->id == $self->Creator ) {
-                    return $self->loc("Taken");
+                    return ("Taken");   #loc
                 }
                 else {
-                    return $self->loc( "Given to [_1]",  $New->Name );
+                    return ( "Given to [_1]",  $New->Name );    #loc
                 }
             }
             else {
                 if ( $New->id == $self->Creator ) {
-                    return $self->loc("Stolen from [_1]",  $Old->Name);
+                    return ("Stolen from [_1]",  $Old->Name);   #loc
                 }
                 elsif ( $Old->id == $self->Creator ) {
                     if ( $New->id == RT->Nobody->id ) {
-                        return $self->loc("Untaken");
+                        return ("Untaken"); #loc
                     }
                     else {
-                        return $self->loc( "Given to [_1]", $New->Name );
+                        return ( "Given to [_1]", $New->Name ); #loc
                     }
                 }
                 else {
-                    return $self->loc(
-                        "Owner forcibly changed from [_1] to [_2]",
+                    return (
+                        "Owner forcibly changed from [_1] to [_2]", #loc
                         $Old->Name, $New->Name );
                 }
             }
         }
         else {
-            return $self->loc( "[_1] changed from [_2] to [_3]",
-                               $self->loc($self->Field),
-                               ($self->OldValue? "'".$self->OldValue ."'" : $self->loc("(no value)")) , "'". $self->NewValue."'" );
+            return ( "[_1] changed from [_2] to [_3]",  #loc
+                    $self->loc($self->Field),
+                    ($self->OldValue? "'".$self->OldValue ."'" : $self->loc("(no value)")) , "'". $self->NewValue."'" );
         }
     },
     PurgeTransaction => sub {
         my $self = shift;
-        return $self->loc("Transaction [_1] purged", $self->Data);
+        return ("Transaction [_1] purged", $self->Data);    #loc
     },
     AddReminder => sub {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return $self->loc("Reminder '[_1]' added", $ticket->Subject);
+        return ("Reminder '[_1]' added", $ticket->Subject); #loc
     },
     OpenReminder => sub {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return $self->loc("Reminder '[_1]' reopened", $ticket->Subject);
-    
+        return ("Reminder '[_1]' reopened", $ticket->Subject);  #loc
     },
     ResolveReminder => sub {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return $self->loc("Reminder '[_1]' completed", $ticket->Subject);
-    
-    
+        return ("Reminder '[_1]' completed", $ticket->Subject); #loc
     }
 );
 
diff --git a/sbin/rt-test-dependencies.in b/sbin/rt-test-dependencies.in
index 54efc23..8e84002 100644
--- a/sbin/rt-test-dependencies.in
+++ b/sbin/rt-test-dependencies.in
@@ -229,6 +229,7 @@ Net::CIDR
 Regexp::Common::net::CIDR
 Regexp::IPv6
 Symbol::Global::Name
+HTML::Entities
 .
 
 $deps{'MASON'} = [ text_to_hash( << '.') ];

commit 4a471ac0a4bfc75c34af83b4c9da20014f86ea50
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 6 15:52:56 2012 -0800

    Use the HTML transaction descriptions in history

diff --git a/share/html/Elements/ShowTransaction b/share/html/Elements/ShowTransaction
index 271d608..eb4256f 100644
--- a/share/html/Elements/ShowTransaction
+++ b/share/html/Elements/ShowTransaction
@@ -57,7 +57,7 @@
 % $m->callback( %ARGS, Transaction => $Transaction, CallbackName => 'AfterAnchor' );
     <span class="date"><% $date |n %></span>
     <span class="description">
-      <& /Elements/ShowUser, User => $Transaction->CreatorObj &> - <% $desc %>
+      <& /Elements/ShowUser, User => $Transaction->CreatorObj &> - <% $desc |n %>
 % $m->callback( %ARGS, Transaction => $Transaction, CallbackName => 'AfterDescription' );
     </span>
     <span class="time-taken"><% $time %></span>
@@ -121,11 +121,13 @@ my @classes = (
     ($RowNum % 2 ? 'odd' : 'even')
 );
 
-my $desc = $Transaction->BriefDescription;
+my $desc = $Transaction->BriefDescriptionAsHTML;
 if ( $Object->id != $Transaction->ObjectId ) {
     # merged objects
-    $desc = loc("[_1] #[_1]:", loc($record_type), $Transaction->ObjectId)
-        .' - '. $desc;
+    $desc = join " - ",
+        $m->interp->apply_escapes(
+            loc("[_1] #[_1]:", loc($record_type), $Transaction->ObjectId), 'h'),
+        $desc;
 }
 
 my $date = $Transaction->CreatedAsString;

commit 5e1ed9f4c9adc1e0eb6acb30f3493ae8b1d35ce4
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 6 16:52:28 2012 -0800

    Link to the objects or URIs referenced by AddLink and DeleteLink transactions
    
    The Links are now links!  And clickable!

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 652f399..d60f2bc 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -817,32 +817,34 @@ sub _ProcessReturnValues {
         my $self = shift;
         my $value;
         if ( $self->NewValue ) {
+            my @html;
             my $URI = RT::URI->new( $self->CurrentUser );
             $URI->FromURI( $self->NewValue );
             if ( $URI->Resolver ) {
                 $value = $URI->Resolver->AsString;
+                @html  = ([\'<a href="', $URI->AsHREF, \'">'], \'</a>');
             }
             else {
                 $value = $self->NewValue;
+                @html  = ("", "");
             }
             if ( $self->Field eq 'DependsOn' ) {
-                return ( "Dependency on [_1] added", $value );  #loc
+                return ( "Dependency on [_2][_1][_3] added", $value, @html );   #loc
             }
             elsif ( $self->Field eq 'DependedOnBy' ) {
-                return ( "Dependency by [_1] added", $value );  #loc
-
+                return ( "Dependency by [_2][_1][_3] added", $value, @html );   #loc
             }
             elsif ( $self->Field eq 'RefersTo' ) {
-                return ( "Reference to [_1] added", $value );   #loc
+                return ( "Reference to [_2][_1][_3] added", $value, @html );    #loc
             }
             elsif ( $self->Field eq 'ReferredToBy' ) {
-                return ( "Reference by [_1] added", $value );   #loc
+                return ( "Reference by [_2][_1][_3] added", $value, @html );    #loc
             }
             elsif ( $self->Field eq 'MemberOf' ) {
-                return ( "Membership in [_1] added", $value );  #loc
+                return ( "Membership in [_2][_1][_3] added", $value, @html );   #loc
             }
             elsif ( $self->Field eq 'HasMember' ) {
-                return ( "Member [_1] added", $value );         #loc
+                return ( "Member [_2][_1][_3] added", $value, @html );          #loc
             }
             elsif ( $self->Field eq 'MergedInto' ) {
                 return ( "Merged into [_1]", $value );          #loc
@@ -856,32 +858,35 @@ sub _ProcessReturnValues {
         my $self = shift;
         my $value;
         if ( $self->OldValue ) {
+            my @html;
             my $URI = RT::URI->new( $self->CurrentUser );
             $URI->FromURI( $self->OldValue );
             if ( $URI->Resolver ) {
                 $value = $URI->Resolver->AsString;
+                @html  = ([\'<a href="', $URI->AsHREF, \'">'], \'</a>');
             }
             else {
                 $value = $self->OldValue;
+                @html  = ("", "");
             }
 
             if ( $self->Field eq 'DependsOn' ) {
-                return ( "Dependency on [_1] deleted", $value );    #loc
+                return ( "Dependency on [_2][_1][_3] deleted", $value, @html );     #loc
             }
             elsif ( $self->Field eq 'DependedOnBy' ) {
-                return ( "Dependency by [_1] deleted", $value );    #loc
+                return ( "Dependency by [_2][_1][_3] deleted", $value, @html );     #loc
             }
             elsif ( $self->Field eq 'RefersTo' ) {
-                return ( "Reference to [_1] deleted", $value );     #loc
+                return ( "Reference to [_2][_1][_3] deleted", $value, @html );      #loc
             }
             elsif ( $self->Field eq 'ReferredToBy' ) {
-                return ( "Reference by [_1] deleted", $value );     #loc
+                return ( "Reference by [_2][_1][_3] deleted", $value, @html );      #loc
             }
             elsif ( $self->Field eq 'MemberOf' ) {
-                return ( "Membership in [_1] deleted", $value );    #loc
+                return ( "Membership in [_2][_1][_3] deleted", $value, @html );     #loc
             }
             elsif ( $self->Field eq 'HasMember' ) {
-                return ( "Member [_1] deleted", $value );           #loc
+                return ( "Member [_2][_1][_3] deleted", $value, @html );            #loc
             }
         }
         else {

commit d02fe78f74abb0e9ce0d5c52a665cd6984cc1f86
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 6 16:54:16 2012 -0800

    Link to the transaction that was forwarded
    
    Now people don't need to poke through the HTML or hover over dozens of
    anchors to figure out which transaction was forwarded.  Unlike outgoing
    email record transactions, which occur immediately after the relevant
    comment or correspond, forwards often occur much later in history.

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index d60f2bc..7a09a01 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -720,8 +720,9 @@ sub _ProcessReturnValues {
     },
     "Forward Transaction" => sub {
         my $self = shift;
-        return ( "Forwarded Transaction #[_1] to [_2]", #loc
-            $self->Field, $self->Data );
+        return ( "Forwarded [_3]Transaction #[_1][_4] to [_2]", #loc
+            $self->Field, $self->Data,
+            [\'<a href="#txn-', $self->Field, \'">'], \'</a>');
     },
     "Forward Ticket" => sub {
         my $self = shift;
diff --git a/t/web/ticket_forward.t b/t/web/ticket_forward.t
index adf4d6f..3aceaa4 100644
--- a/t/web/ticket_forward.t
+++ b/t/web/ticket_forward.t
@@ -68,7 +68,7 @@ diag "Forward Transaction" if $ENV{TEST_VERBOSE};
     );
     $m->content_contains( 'Sent email successfully', 'sent mail msg' );
     $m->content_like(
-qr/Forwarded Transaction #\d+ to rt-test, rt-to\@example.com, rt-cc\@example.com, rt-bcc\@example.com/,
+qr/Forwarded .*?Transaction #\d+.*? to rt-test, rt-to\@example.com, rt-cc\@example.com, rt-bcc\@example.com/,
         'txn msg'
     );
     my ($mail) = RT::Test->fetch_caught_mails;
@@ -138,7 +138,7 @@ diag "Forward Transaction with attachments but empty content" if $ENV{TEST_VERBO
         button => 'ForwardAndReturn'
     );
     $m->content_contains( 'Sent email successfully', 'sent mail msg' );
-    $m->content_like( qr/Forwarded Transaction #\d+ to rt-test\@example\.com/, 'txn msg' );
+    $m->content_like( qr/Forwarded .*?Transaction #\d+.*? to rt-test\@example\.com/, 'txn msg' );
     my ($mail) = RT::Test->fetch_caught_mails;
     like( $mail, qr/Subject: test forward, empty content but attachments/, 'Subject field' );
     like( $mail, qr/To: rt-test\@example.com/,         'To field' );
@@ -196,7 +196,7 @@ diag "Forward Transaction with attachments but no 'content' part" if $ENV{TEST_V
         button => 'ForwardAndReturn'
     );
     $m->content_contains( 'Sent email successfully', 'sent mail msg' );
-    $m->content_like( qr/Forwarded Transaction #\d+ to rt-test\@example\.com/, 'txn msg' );
+    $m->content_like( qr/Forwarded .*?Transaction #\d+.*? to rt-test\@example\.com/, 'txn msg' );
     
     # Forward ticket
     $m->follow_link_ok( { text => 'Forward', n => 1 }, 'follow 1st Forward' );

commit ec01660cb2e70781f3716b762f5e3b829cd967d4
Author: Thomas Sibley <trs at bestpractical.com>
Date:   Thu Dec 6 16:56:35 2012 -0800

    Link to added/started/completed reminders from their history entries
    
    We don't expose reminders through the normal ticket display framework,
    so link only to the per-ticket "all reminders" page, scrolled to the
    specific reminder in question via an anchor.

diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 7a09a01..9fb8aad 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -978,19 +978,34 @@ sub _ProcessReturnValues {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return ("Reminder '[_1]' added", $ticket->Subject); #loc
+        my @html = $self->ObjectType eq 'RT::Ticket'
+            ? ([\'<a href="', RT->Config->Get('WebPath'),
+                "/Ticket/Reminders.html?id=", $self->ObjectId,
+                "#reminder-", $ticket->id, \'">'], \'</a>')
+            : ("", "");
+        return ("Reminder '[_2][_1][_3]' added", $ticket->Subject, @html); #loc
     },
     OpenReminder => sub {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return ("Reminder '[_1]' reopened", $ticket->Subject);  #loc
+        my @html = $self->ObjectType eq 'RT::Ticket'
+            ? ([\'<a href="', RT->Config->Get('WebPath'),
+                "/Ticket/Reminders.html?id=", $self->ObjectId,
+                "#reminder-", $ticket->id, \'">'], \'</a>')
+            : ("", "");
+        return ("Reminder '[_2][_1][_3]' reopened", $ticket->Subject, @html);  #loc
     },
     ResolveReminder => sub {
         my $self = shift;
         my $ticket = RT::Ticket->new($self->CurrentUser);
         $ticket->Load($self->NewValue);
-        return ("Reminder '[_1]' completed", $ticket->Subject); #loc
+        my @html = $self->ObjectType eq 'RT::Ticket'
+            ? ([\'<a href="', RT->Config->Get('WebPath'),
+                "/Ticket/Reminders.html?id=", $self->ObjectId,
+                "#reminder-", $ticket->id, \'">'], \'</a>')
+            : ("", "");
+        return ("Reminder '[_2][_1][_3]' completed", $ticket->Subject, @html); #loc
     }
 );
 
diff --git a/share/html/Ticket/Elements/Reminders b/share/html/Ticket/Elements/Reminders
index c12159e..1f2d881 100644
--- a/share/html/Ticket/Elements/Reminders
+++ b/share/html/Ticket/Elements/Reminders
@@ -150,7 +150,7 @@ $Reminder
 $Ticket
 $Index
 </%args>
-<tr class="<% $Index%2 ? 'oddline' : 'evenline' %>">
+<tr class="<% $Index%2 ? 'oddline' : 'evenline' %>" id="reminder-<% $Reminder->id %>">
 <td class="entry">
 % unless ( $Reminder->CurrentUserHasRight('ModifyTicket') ) {
 <input name="Complete-Reminder-<% $Reminder->id %>" type="hidden" 
@@ -192,7 +192,7 @@ $Index
 </%args>
 % my $dueobj = $Reminder->DueObj;
 % my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0;
-<tr class="<% $Index%2 ? 'oddline' : 'evenline' %>">
+<tr class="<% $Index%2 ? 'oddline' : 'evenline' %>" id="reminder-<% $Reminder->id %>">
 
 <td class="collection-as-table">
 % unless ( $Reminder->CurrentUserHasRight('ModifyTicket') ) {

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


More information about the Rt-commit mailing list