[Rt-commit] rt branch, 4.2/html-transaction-descriptions, created. rt-4.1.5-25-g00d18ef
Thomas Sibley
trs at bestpractical.com
Wed Dec 12 16:37:41 EST 2012
The branch, 4.2/html-transaction-descriptions has been created
at 00d18effe1a3d4882a032bd9a14a951d9398ebc4 (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 84255cda0be647082d5fdb554df45a51288a7540
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..6906038 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -820,17 +820,21 @@ sub _ProcessReturnValues {
my $URI = RT::URI->new( $self->CurrentUser );
$URI->FromURI( $self->NewValue );
if ( $URI->Resolver ) {
- $value = $URI->Resolver->AsString;
+ $value = [
+ \'<a href="', $URI->AsHREF, \'">',
+ $URI->Resolver->AsString,
+ \'</a>'
+ ];
}
else {
$value = $self->NewValue;
}
+
if ( $self->Field eq 'DependsOn' ) {
return ( "Dependency on [_1] added", $value ); #loc
}
elsif ( $self->Field eq 'DependedOnBy' ) {
return ( "Dependency by [_1] added", $value ); #loc
-
}
elsif ( $self->Field eq 'RefersTo' ) {
return ( "Reference to [_1] added", $value ); #loc
@@ -859,7 +863,11 @@ sub _ProcessReturnValues {
my $URI = RT::URI->new( $self->CurrentUser );
$URI->FromURI( $self->OldValue );
if ( $URI->Resolver ) {
- $value = $URI->Resolver->AsString;
+ $value = [
+ \'<a href="', $URI->AsHREF, \'">',
+ $URI->Resolver->AsString,
+ \'</a>'
+ ];
}
else {
$value = $self->OldValue;
commit 2ea98e29d4840ea0756092576509553e4a7dc4fb
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 6906038..b1a5309 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 cc30ca08f3433d5b81b0c749bb815748924b94af
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 b1a5309..d113c08 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -981,19 +981,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 $subject = $self->ObjectType eq 'RT::Ticket'
+ ? [\'<a href="', RT->Config->Get('WebPath'),
+ "/Ticket/Reminders.html?id=", $self->ObjectId,
+ "#reminder-", $ticket->id, \'">', $ticket->Subject, \'</a>']
+ : $ticket->Subject;
+ return ("Reminder '[_1]' added", $subject); #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 $subject = $self->ObjectType eq 'RT::Ticket'
+ ? [\'<a href="', RT->Config->Get('WebPath'),
+ "/Ticket/Reminders.html?id=", $self->ObjectId,
+ "#reminder-", $ticket->id, \'">', $ticket->Subject, \'</a>']
+ : $ticket->Subject;
+ return ("Reminder '[_1]' reopened", $subject); #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 $subject = $self->ObjectType eq 'RT::Ticket'
+ ? [\'<a href="', RT->Config->Get('WebPath'),
+ "/Ticket/Reminders.html?id=", $self->ObjectId,
+ "#reminder-", $ticket->id, \'">', $ticket->Subject, \'</a>']
+ : $ticket->Subject;
+ return ("Reminder '[_1]' completed", $subject); #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') ) {
commit d3d7e9cc01cacec1ac8237144bc5fc3ce14634e3
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Dec 7 14:57:22 2012 -0800
Use the per-reminder anchors for links in "My Reminders"
diff --git a/share/html/Elements/ShowReminders b/share/html/Elements/ShowReminders
index 34cde46..d1a8881 100644
--- a/share/html/Elements/ShowReminders
+++ b/share/html/Elements/ShowReminders
@@ -66,7 +66,7 @@ if ( my $ticket= $targets->First ) {
</%PERL>
<tr class="<% $i%2 ? 'oddline' : 'evenline' %>">
<td class="collection-as-table">
-<a href="<% RT->Config->Get('WebPath') %>/Ticket/Reminders.html?id=<% $ticket->id %>"><% $reminder->Subject %></a>
+<a href="<% RT->Config->Get('WebPath') %>/Ticket/Reminders.html?id=<% $ticket->id %>#reminder-<% $reminder->id %>"><% $reminder->Subject %></a>
</td>
<td class="collection-as-table">
<% $overdue ? '<span class="overdue">' : '' |n %><% $dueobj->AgeAsString || loc('Not set') %><% $overdue ? '</span>' : '' |n %>
commit 25f0819141a8f0506c7c0e3aff0a4a6125a7632d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Dec 7 12:12:26 2012 -0800
Remove extraneous license lines from the ShowUser components
diff --git a/share/html/Elements/ShowUser b/share/html/Elements/ShowUser
index 3654977..4e17473 100644
--- a/share/html/Elements/ShowUser
+++ b/share/html/Elements/ShowUser
@@ -45,7 +45,6 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-%# Released under the terms of version 2 of the GNU Public License
<%INIT>
# $User is an RT::User object
# $Address is Email::Address object
diff --git a/share/html/Elements/ShowUserConcise b/share/html/Elements/ShowUserConcise
index b56ba83..09b4319 100644
--- a/share/html/Elements/ShowUserConcise
+++ b/share/html/Elements/ShowUserConcise
@@ -45,7 +45,6 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-%# Released under the terms of version 2 of the GNU Public License
<% $display |n %>\
<%ARGS>
$User => undef
diff --git a/share/html/Elements/ShowUserVerbose b/share/html/Elements/ShowUserVerbose
index 9ff8cbb..6c3c509 100644
--- a/share/html/Elements/ShowUserVerbose
+++ b/share/html/Elements/ShowUserVerbose
@@ -45,7 +45,6 @@
%# those contributions and any derivatives thereof.
%#
%# END BPS TAGGED BLOCK }}}
-%# Released under the terms of version 2 of the GNU Public License
<% $display |n %>\
<%INIT>
my $phrase = '';
commit 615bc335d51e0bdae6450ba60c6ed927ad378e9f
Author: Thomas Sibley <trs at bestpractical.com>
Date: Fri Dec 7 13:06:30 2012 -0800
Attempt to load a user from the supplied address for all user formats
Refactors the logic of ShowUserConcise into ShowUser to apply to all
cases. ShowUserVerbose is able to display more information if only an
address is passed and but it corresponds to an existing RT::User. An
RT::User object is always better than just an email address.
diff --git a/share/html/Elements/ShowUser b/share/html/Elements/ShowUser
index 4e17473..386bb2a 100644
--- a/share/html/Elements/ShowUser
+++ b/share/html/Elements/ShowUser
@@ -57,9 +57,13 @@ unless ( RT::Interface::Web->ComponentPathIsSafe($comp) and $m->comp_exists( $co
. ' picked UsernameFormat '. $style
. ', but '. $comp . "doesn't exist"
);
- return $m->comp('/Elements/ShowUserConcise',
- User => $User, Address => $Address, NoEscape => $NoEscape
- );
+ $comp = '/Elements/ShowUserConcise';
+}
+
+if (not $User and $Address) {
+ $User = RT::User->new( $session{'CurrentUser'} );
+ $User->LoadByEmail( $Address->address );
+ undef $User unless $User->id;
}
return $m->comp( $comp, User => $User, Address => $Address, NoEscape => $NoEscape );
</%INIT>
diff --git a/share/html/Elements/ShowUserConcise b/share/html/Elements/ShowUserConcise
index 09b4319..8002fa0 100644
--- a/share/html/Elements/ShowUserConcise
+++ b/share/html/Elements/ShowUserConcise
@@ -51,16 +51,7 @@ $User => undef
$Address => undef
</%ARGS>
<%INIT>
-if ( !$User && $Address ) {
- $User = RT::User->new( $session{'CurrentUser'} );
- $User->LoadByEmail( $Address->address );
- if ( $User->Id ) {
- $Address = '';
- } else {
- $Address = $Address->address;
- }
-}
-my $display = $Address || $User->RealName || $User->Name;
+my $display = $User ? ($User->RealName || $User->Name) : $Address->address;
$display = $m->interp->apply_escapes( $display, 'h' )
unless $ARGS{'NoEscape'};
</%INIT>
commit ee5c58250d7d4783687fd0f17f9beae7edc2ce7d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Dec 10 17:16:17 2012 -0800
Move ShowUser* components into RT::User
These formatters generally deal in just text, not HTML, and they can't
be used by other library code when written in Mason.
diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 43a9305..637f96b 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -22,3 +22,14 @@ UPGRADING FROM RT 4.0.0 and greater
* MakeClicky handlers added via a callback are now passed an "object" key in
the parameter hash instead of "ticket". The object may be any RT::Record
subclass.
+
+* ShowUser handlers (/Elements/ShowUser*) have moved out of Mason components
+ and into RT::User methods. Any custom username formats will need to be
+ reimplemented as RT::User methods. Renaming should follow that of the core
+ components:
+
+ /Elements/ShowUserConcise => RT::User->_FormatUserConcise
+ /Elements/ShowUserVerbose => RT::User->_FormatUserVerbose
+
+ The _FormatUser* methods are passed a hash containing the keys User and
+ Address, which have the same properties as before.
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 176d30c..cf31492 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -66,6 +66,7 @@ package RT::User;
use strict;
use warnings;
+use Scalar::Util qw(blessed);
use base 'RT::Record';
@@ -1538,9 +1539,120 @@ Return the friendly name
sub FriendlyName {
my $self = shift;
- return $self->RealName if defined($self->RealName);
- return $self->Name if defined($self->Name);
- return "";
+ return $self->RealName if defined $self->RealName and length $self->RealName;
+ return $self->Name;
+}
+
+=head2 Format
+
+Class or object method.
+
+Returns a string describing a user in the current user's preferred format.
+
+May be invoked in three ways:
+
+ $UserObj->Format;
+ RT::User->Format( User => $UserObj ); # same as above
+ RT::User->Format( Address => $AddressObj, CurrentUser => $CurrentUserObj );
+
+Possible arguments are:
+
+=over
+
+=item User
+
+An L<RT::User> object representing the user to format. Preferred to Address.
+
+=item Address
+
+An L<Email::Address> object representing the user address to format. Address
+will be used to lookup an L<RT::User> if possible.
+
+=item CurrentUser
+
+Required when Format is called as a class method with an Address argument.
+Otherwise, this argument is ignored in preference to the CurrentUser of the
+involved L<RT::User> object.
+
+=item Format
+
+Specifies the format to use, overriding any set from the config or current
+user's preferences.
+
+=back
+
+=cut
+
+sub Format {
+ my $self = shift;
+ my %args = (
+ User => undef,
+ Address => undef,
+ CurrentUser => undef,
+ Format => undef,
+ @_
+ );
+
+ if (blessed($self) and $self->id) {
+ @args{"User", "CurrentUser"} = ($self, $self->CurrentUser);
+ }
+ elsif ($args{User} and $args{User}->id) {
+ $args{CurrentUser} = $args{User}->CurrentUser;
+ }
+ elsif ($args{Address} and $args{CurrentUser}) {
+ $args{User} = RT::User->new( $args{CurrentUser} );
+ $args{User}->LoadByEmail( $args{Address}->address );
+ if ($args{User}->id) {
+ delete $args{Address};
+ } else {
+ delete $args{User};
+ }
+ }
+ else {
+ RT->Logger->warning("Invalid arguments to RT::User->Format at @{[join '/', caller]}");
+ return "";
+ }
+
+ $args{Format} ||= RT->Config->Get("UsernameFormat", $args{CurrentUser});
+ $args{Format} =~ s/[^A-Za-z0-9_]+//g;
+
+ my $method = "_FormatUser" . ucfirst lc $args{Format};
+ my $formatter = $self->can($method);
+
+ unless ($formatter) {
+ RT->Logger->error(
+ "Either system config or user #" . $args{CurrentUser}->id .
+ " picked UsernameFormat $args{Format}, but RT::User->$method doesn't exist"
+ );
+ $formatter = $self->can("_FormatUserConcise");
+ }
+ return $formatter->( $self, map { $_ => $args{$_} } qw(User Address) );
+}
+
+sub _FormatUserConcise {
+ my $self = shift;
+ my %args = @_;
+ return $args{User} ? $args{User}->FriendlyName : $args{Address}->address;
+}
+
+sub _FormatUserVerbose {
+ my $self = shift;
+ my %args = @_;
+ my ($user, $address) = @args{"User", "Address"};
+
+ my $email = '';
+ my $phrase = '';
+ my $comment = '';
+
+ if ($user) {
+ $email = $user->EmailAddress || '';
+ $phrase = $user->RealName if $user->RealName and lc $user->RealName ne lc $email;
+ $comment = $user->Name if lc $user->Name ne lc $email;
+ } else {
+ ($email, $phrase, $comment) = (map { $address->$_ } "address", "phrase", "comment");
+ }
+
+ return join " ", grep { $_ } ($phrase || $comment || ''), ($email ? "<$email>" : "");
}
=head2 PreferredKey
diff --git a/share/html/Elements/ShowUser b/share/html/Elements/ShowUser
index 386bb2a..f456af6 100644
--- a/share/html/Elements/ShowUser
+++ b/share/html/Elements/ShowUser
@@ -49,27 +49,20 @@
# $User is an RT::User object
# $Address is Email::Address object
-my $comp = '/Elements/ShowUser'. ucfirst lc $style;
-unless ( RT::Interface::Web->ComponentPathIsSafe($comp) and $m->comp_exists( $comp ) ) {
- $RT::Logger->error(
- 'Either system config or user #'
- . $session{'CurrentUser'}->id
- . ' picked UsernameFormat '. $style
- . ', but '. $comp . "doesn't exist"
- );
- $comp = '/Elements/ShowUserConcise';
-}
+my $display = RT::User->Format(
+ User => $User,
+ Address => $Address,
+ CurrentUser => $session{CurrentUser},
+ Format => $style,
+);
-if (not $User and $Address) {
- $User = RT::User->new( $session{'CurrentUser'} );
- $User->LoadByEmail( $Address->address );
- undef $User unless $User->id;
-}
-return $m->comp( $comp, User => $User, Address => $Address, NoEscape => $NoEscape );
+$display = $m->interp->apply_escapes($display, 'h')
+ unless $NoEscape;
</%INIT>
<%ARGS>
$User => undef
$Address => undef
$NoEscape => 0
-$style => RT->Config->Get('UsernameFormat', $session{'CurrentUser'})
+$style => undef
</%ARGS>
+<% $display |n %>\
diff --git a/share/html/Elements/ShowUserConcise b/share/html/Elements/ShowUserConcise
deleted file mode 100644
index 8002fa0..0000000
--- a/share/html/Elements/ShowUserConcise
+++ /dev/null
@@ -1,57 +0,0 @@
-%# BEGIN BPS TAGGED BLOCK {{{
-%#
-%# COPYRIGHT:
-%#
-%# This software is Copyright (c) 1996-2012 Best Practical Solutions, LLC
-%# <sales at bestpractical.com>
-%#
-%# (Except where explicitly superseded by other copyright notices)
-%#
-%#
-%# LICENSE:
-%#
-%# This work is made available to you under the terms of Version 2 of
-%# the GNU General Public License. A copy of that license should have
-%# been provided with this software, but in any event can be snarfed
-%# from www.gnu.org.
-%#
-%# This work is distributed in the hope that it will be useful, but
-%# WITHOUT ANY WARRANTY; without even the implied warranty of
-%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-%# General Public License for more details.
-%#
-%# You should have received a copy of the GNU General Public License
-%# along with this program; if not, write to the Free Software
-%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
-%# 02110-1301 or visit their web page on the internet at
-%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
-%#
-%#
-%# CONTRIBUTION SUBMISSION POLICY:
-%#
-%# (The following paragraph is not intended to limit the rights granted
-%# to you to modify and distribute this software under the terms of
-%# the GNU General Public License and is only of importance to you if
-%# you choose to contribute your changes and enhancements to the
-%# community by submitting them to Best Practical Solutions, LLC.)
-%#
-%# By intentionally submitting any modifications, corrections or
-%# derivatives to this work, or any other work intended for use with
-%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
-%# you are the copyright holder for those contributions and you grant
-%# Best Practical Solutions, LLC a nonexclusive, worldwide, irrevocable,
-%# royalty-free, perpetual, license to use, copy, create derivative
-%# works based on those contributions, and sublicense and distribute
-%# those contributions and any derivatives thereof.
-%#
-%# END BPS TAGGED BLOCK }}}
-<% $display |n %>\
-<%ARGS>
-$User => undef
-$Address => undef
-</%ARGS>
-<%INIT>
-my $display = $User ? ($User->RealName || $User->Name) : $Address->address;
- $display = $m->interp->apply_escapes( $display, 'h' )
- unless $ARGS{'NoEscape'};
-</%INIT>
diff --git a/share/html/Elements/ShowUserVerbose b/share/html/Elements/ShowUserVerbose
deleted file mode 100644
index 6c3c509..0000000
--- a/share/html/Elements/ShowUserVerbose
+++ /dev/null
@@ -1,70 +0,0 @@
-%# BEGIN BPS TAGGED BLOCK {{{
-%#
-%# COPYRIGHT:
-%#
-%# This software is Copyright (c) 1996-2012 Best Practical Solutions, LLC
-%# <sales at bestpractical.com>
-%#
-%# (Except where explicitly superseded by other copyright notices)
-%#
-%#
-%# LICENSE:
-%#
-%# This work is made available to you under the terms of Version 2 of
-%# the GNU General Public License. A copy of that license should have
-%# been provided with this software, but in any event can be snarfed
-%# from www.gnu.org.
-%#
-%# This work is distributed in the hope that it will be useful, but
-%# WITHOUT ANY WARRANTY; without even the implied warranty of
-%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
-%# General Public License for more details.
-%#
-%# You should have received a copy of the GNU General Public License
-%# along with this program; if not, write to the Free Software
-%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
-%# 02110-1301 or visit their web page on the internet at
-%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
-%#
-%#
-%# CONTRIBUTION SUBMISSION POLICY:
-%#
-%# (The following paragraph is not intended to limit the rights granted
-%# to you to modify and distribute this software under the terms of
-%# the GNU General Public License and is only of importance to you if
-%# you choose to contribute your changes and enhancements to the
-%# community by submitting them to Best Practical Solutions, LLC.)
-%#
-%# By intentionally submitting any modifications, corrections or
-%# derivatives to this work, or any other work intended for use with
-%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
-%# you are the copyright holder for those contributions and you grant
-%# Best Practical Solutions, LLC a nonexclusive, worldwide, irrevocable,
-%# royalty-free, perpetual, license to use, copy, create derivative
-%# works based on those contributions, and sublicense and distribute
-%# those contributions and any derivatives thereof.
-%#
-%# END BPS TAGGED BLOCK }}}
-<% $display |n %>\
-<%INIT>
-my $phrase = '';
-my $address = '';
-my $comment = '';
-
-if ($User) {
- $address = $User->EmailAddress;
- $phrase = $User->RealName if $User->RealName && lc $User->RealName ne lc $address;
- $comment = $User->Name if lc $User->Name ne lc $address;
-} else {
- $address = $Address;
-}
-
-my $display = ($phrase || $comment || '' ) . ($address ? ' <'.$address.'>' : '');
-
-$display = $m->interp->apply_escapes( $display, 'h' )
- unless $ARGS{'NoEscape'};
-</%INIT>
-<%ARGS>
-$User => undef
-$Address => undef
-</%ARGS>
commit cd158ffeccac3f82af8139da092f72887f46ff16
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Dec 10 17:21:31 2012 -0800
Add a callback to ShowUser to allow for HTML injection into the format
diff --git a/share/html/Elements/ShowUser b/share/html/Elements/ShowUser
index f456af6..84a1810 100644
--- a/share/html/Elements/ShowUser
+++ b/share/html/Elements/ShowUser
@@ -56,6 +56,14 @@ my $display = RT::User->Format(
Format => $style,
);
+$m->callback(
+ ARGSRef => \%ARGS,
+ User => $User,
+ Address => $Address,
+ NoEscape => \$NoEscape,
+ CallbackName => 'Modify',
+);
+
$display = $m->interp->apply_escapes($display, 'h')
unless $NoEscape;
</%INIT>
commit 778101f6dda33fa78ae59c475abd4101f5772ceb
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Dec 10 17:36:15 2012 -0800
Replace some calls to /Elements/ShowUser with Format in RT::User
Calls which aren't in full HTML contexts should call the method to avoid
any HTML. This includes most loc strings, <option> contents, RSS
elements, and autocomplete results. Almost any call to ShowUser which
sets NoEscape to a true value should probably be using $User->Format
instead.
diff --git a/share/html/Dashboards/Elements/SelectPrivacy b/share/html/Dashboards/Elements/SelectPrivacy
index 523790b..d428d22 100644
--- a/share/html/Dashboards/Elements/SelectPrivacy
+++ b/share/html/Dashboards/Elements/SelectPrivacy
@@ -60,11 +60,11 @@ foreach my $object (@Objects) {
if (ref($object) eq 'RT::User') {
$label = $object->id == $session{'CurrentUser'}->Id
? loc("My dashboards")
- : loc("[_1]'s dashboards", $m->scomp('/Elements/ShowUser', User => $object));
+ : loc("[_1]'s dashboards", $object->Format);
} else {
- $label = loc("[_1]'s dashboards", $m->interp->apply_escapes($object->Name, 'h'));
+ $label = loc("[_1]'s dashboards", $object->Name);
}
</%perl>
-<option <%$selected|n%> value="<%$privacy%>"><% $label |n %></option>
+<option <%$selected|n%> value="<%$privacy%>"><% $label %></option>
% }
</select>
diff --git a/share/html/Elements/SelectOwnerDropdown b/share/html/Elements/SelectOwnerDropdown
index 5dd6429..96a2c63 100644
--- a/share/html/Elements/SelectOwnerDropdown
+++ b/share/html/Elements/SelectOwnerDropdown
@@ -50,8 +50,8 @@
<option value=""<% !$Default ? qq[ selected="selected"] : '' |n %>><%$DefaultLabel |n%></option>
%}
% $Default = 0 unless defined $Default && $Default =~ /^\d+$/;
-% my @formatednames = sort {lc $a->[1] cmp lc $b->[1]} map {[$_, $m->scomp('/Elements/ShowUser', User => $_)]} grep { $_->id != RT->Nobody->id } @users;
-% my $nobody = [RT->Nobody, $m->scomp('/Elements/ShowUser', User => RT->Nobody)];
+% my @formatednames = sort {lc $a->[1] cmp lc $b->[1]} map {[$_, $_->Format]} grep { $_->id != RT->Nobody->id } @users;
+% my $nobody = [RT->Nobody, RT->Nobody->Format(CurrentUser => $session{CurrentUser})];
% unshift @formatednames, $nobody;
%foreach my $UserRef ( @formatednames) {
%my $User = $UserRef->[0];
@@ -61,7 +61,7 @@
%} elsif ($ValueAttribute eq 'Name') {
value="<%$User->Name%>"
%}
-><% $UserRef->[1] |n %></option>
+><% $UserRef->[1] %></option>
%}
</select>
<%INIT>
diff --git a/share/html/Helpers/Autocomplete/Owners b/share/html/Helpers/Autocomplete/Owners
index 1d065f2..a9b53e4 100644
--- a/share/html/Helpers/Autocomplete/Owners
+++ b/share/html/Helpers/Autocomplete/Owners
@@ -117,7 +117,7 @@ foreach my $spec (map { [split /\-/, $_, 2] } split /\|/, $limit) {
next if $user_uniq_hash{ $User->Id };
$user_uniq_hash{ $User->Id() } = [
$User,
- $m->scomp('/Elements/ShowUser', User => $User, NoEscape => 1)
+ $User->Format,
];
}
}
@@ -127,7 +127,7 @@ my $nobody = qr/^n(?:o(?:b(?:o(?:d(?:y)?)?)?)?)?$/i;
if ( not $user_uniq_hash{RT->Nobody->id} and $term =~ $nobody ) {
$user_uniq_hash{RT->Nobody->id} = [
RT->Nobody,
- $m->scomp('/Elements/ShowUser', User => RT->Nobody, NoEscape => 1)
+ RT->Nobody->Format,
];
}
diff --git a/share/html/Helpers/Autocomplete/Users b/share/html/Helpers/Autocomplete/Users
index c2b92c1..799d5d9 100644
--- a/share/html/Helpers/Autocomplete/Users
+++ b/share/html/Helpers/Autocomplete/Users
@@ -123,9 +123,7 @@ while ( my $user = $users->Next ) {
next if $user->id == RT->SystemUser->id
or $user->id == RT->Nobody->id;
- my $formatted = $m->scomp('/Elements/ShowUser', User => $user, NoEscape => 1);
- $formatted =~ s/\n//g;
- my $suggestion = { label => $formatted, value => $user->$return };
+ my $suggestion = { label => $user->Format, value => $user->$return };
$m->callback( CallbackName => "ModifySuggestion", suggestion => $suggestion, user => $user );
push @suggestions, $suggestion;
}
diff --git a/share/html/Search/Elements/ResultsRSSView b/share/html/Search/Elements/ResultsRSSView
index 5033c8c..bb74f58 100644
--- a/share/html/Search/Elements/ResultsRSSView
+++ b/share/html/Search/Elements/ResultsRSSView
@@ -119,7 +119,7 @@ $r->content_type('application/rss+xml');
while ( my $Ticket = $Tickets->Next()) {
- my $creator_str = $m->scomp('/Elements/ShowUser', User => $Ticket->CreatorObj);
+ my $creator_str = $Ticket->CreatorObj->Format;
$creator_str =~ s/[\r\n]//g;
$rss->add_item(
title => $Ticket->Subject || loc('No Subject'),
diff --git a/share/html/Search/Elements/SearchPrivacy b/share/html/Search/Elements/SearchPrivacy
index f36c1c8..f897e2e 100644
--- a/share/html/Search/Elements/SearchPrivacy
+++ b/share/html/Search/Elements/SearchPrivacy
@@ -53,9 +53,9 @@ my $label;
if (ref($Object) eq 'RT::User') {
$label = $Object->id == $session{'CurrentUser'}->Id
? loc("My saved searches")
- : loc("[_1]'s saved searches", $m->scomp('/Elements/ShowUser', User => $Object));
+ : loc("[_1]'s saved searches", $Object->Format);
} else {
- $label = loc("[_1]'s saved searches", $m->interp->apply_escapes($Object->Name, 'h'));
+ $label = loc("[_1]'s saved searches", $Object->Name);
}
</%init>
-<% $label |n %>\
+<% $label %>\
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index 5734f95..f5cf78e 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -119,7 +119,7 @@
Name => "Owner",
TicketObj => $TicketObj,
QueueObj => $TicketObj->QueueObj,
- DefaultLabel => loc("[_1] (Unchanged)", $m->scomp('/Elements/ShowUser', User => $TicketObj->OwnerObj)),
+ DefaultLabel => loc("[_1] (Unchanged)", $TicketObj->OwnerObj->Format),
Default => $ARGS{'Owner'}
}
},
diff --git a/share/html/m/ticket/reply b/share/html/m/ticket/reply
index 18587bc..2b4cd72 100644
--- a/share/html/m/ticket/reply
+++ b/share/html/m/ticket/reply
@@ -70,7 +70,7 @@
Name => "Owner",
TicketObj => $t,
QueueObj => $t->QueueObj,
- DefaultLabel => loc("[_1] (Unchanged)", $m->scomp('/Elements/ShowUser', User => $t->OwnerObj)),
+ DefaultLabel => loc("[_1] (Unchanged)", $t->OwnerObj->Format),
Default => $ARGS{'Owner'}
&>
</div></div>
commit 69fb56927b99ebf828fb85250cc7ea0ad926840a
Author: Thomas Sibley <trs at bestpractical.com>
Date: Mon Dec 10 17:38:03 2012 -0800
Show all users in history using the current user's preferred format
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index d113c08..0e4c051 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -720,13 +720,21 @@ sub _ProcessReturnValues {
},
"Forward Transaction" => sub {
my $self = shift;
+ my $recipients = join ", ", map {
+ RT::User->Format( Address => $_, CurrentUser => $self->CurrentUser )
+ } RT::EmailParser->ParseEmailAddress($self->Data);
+
return ( "Forwarded [_3]Transaction #[_1][_4] to [_2]", #loc
- $self->Field, $self->Data,
+ $self->Field, $recipients,
[\'<a href="#txn-', $self->Field, \'">'], \'</a>');
},
"Forward Ticket" => sub {
my $self = shift;
- return ( "Forwarded Ticket to [_1]", $self->Data ); #loc
+ my $recipients = join ", ", map {
+ RT::User->Format( Address => $_, CurrentUser => $self->CurrentUser )
+ } RT::EmailParser->ParseEmailAddress($self->Data);
+
+ return ( "Forwarded Ticket to [_1]", $recipients ); #loc
},
CommentEmailRecord => sub {
my $self = shift;
@@ -784,31 +792,31 @@ sub _ProcessReturnValues {
my $New = RT::User->new( $self->CurrentUser );
$New->Load( $self->NewValue );
- return ("Owner forcibly changed from [_1] to [_2]" , $Old->Name , $New->Name); #loc
+ return ("Owner forcibly changed from [_1] to [_2]", $Old->Format, $New->Format); #loc
},
Steal => sub {
my $self = shift;
my $Old = RT::User->new( $self->CurrentUser );
$Old->Load( $self->OldValue );
- return ("Stolen from [_1]", $Old->Name); #loc
+ return ("Stolen from [_1]", $Old->Format); #loc
},
Give => sub {
my $self = shift;
my $New = RT::User->new( $self->CurrentUser );
$New->Load( $self->NewValue );
- return ( "Given to [_1]", $New->Name ); #loc
+ return ( "Given to [_1]", $New->Format ); #loc
},
AddWatcher => sub {
my $self = shift;
my $principal = RT::Principal->new($self->CurrentUser);
$principal->Load($self->NewValue);
- return ( "[_1] [_2] added", $self->loc($self->Field), $principal->Object->Name); #loc
+ return ( "[_1] [_2] added", $self->loc($self->Field), $principal->Object->Format); #loc
},
DelWatcher => sub {
my $self = shift;
my $principal = RT::Principal->new($self->CurrentUser);
$principal->Load($self->OldValue);
- return ( "[_1] [_2] deleted", $self->loc($self->Field), $principal->Object->Name); #loc
+ return ( "[_1] [_2] deleted", $self->loc($self->Field), $principal->Object->Format); #loc
},
Subject => sub {
my $self = shift;
@@ -945,25 +953,26 @@ sub _ProcessReturnValues {
return ("Taken"); #loc
}
else {
- return ( "Given to [_1]", $New->Name ); #loc
+ return ( "Given to [_1]", $New->Format ); #loc
}
}
else {
if ( $New->id == $self->Creator ) {
- return ("Stolen from [_1]", $Old->Name); #loc
+ return ("Stolen from [_1]", $Old->Format ); #loc
}
elsif ( $Old->id == $self->Creator ) {
if ( $New->id == RT->Nobody->id ) {
return ("Untaken"); #loc
}
else {
- return ( "Given to [_1]", $New->Name ); #loc
+ return ( "Given to [_1]", $New->Format ); #loc
}
}
else {
return (
"Owner forcibly changed from [_1] to [_2]", #loc
- $Old->Name, $New->Name );
+ $Old->Format, $New->Format
+ );
}
}
}
commit 373a7d8fc738627f481a3ebf8ad9213db769aabe
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 11 17:06:26 2012 -0800
Wrap mentioned users in history with an identifying <span>
This helps future extensions customize the history display of users.
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 0e4c051..1b5ac06 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -677,6 +677,16 @@ sub _ProcessReturnValues {
} @values;
}
+sub _FormatUser {
+ my $self = shift;
+ my $user = shift;
+ return [
+ \'<span class="user" data-user-id="', $user->id, \'">',
+ $user->Format,
+ \'</span>'
+ ];
+}
+
%_BriefDescriptions = (
Create => sub {
my $self = shift;
@@ -792,31 +802,32 @@ sub _ProcessReturnValues {
my $New = RT::User->new( $self->CurrentUser );
$New->Load( $self->NewValue );
- return ("Owner forcibly changed from [_1] to [_2]", $Old->Format, $New->Format); #loc
+ return ("Owner forcibly changed from [_1] to [_2]", #loc
+ map { $self->_FormatUser($_) } $Old, $New);
},
Steal => sub {
my $self = shift;
my $Old = RT::User->new( $self->CurrentUser );
$Old->Load( $self->OldValue );
- return ("Stolen from [_1]", $Old->Format); #loc
+ return ("Stolen from [_1]", $self->_FormatUser($Old)); #loc
},
Give => sub {
my $self = shift;
my $New = RT::User->new( $self->CurrentUser );
$New->Load( $self->NewValue );
- return ( "Given to [_1]", $New->Format ); #loc
+ return ( "Given to [_1]", $self->_FormatUser($New)); #loc
},
AddWatcher => sub {
my $self = shift;
my $principal = RT::Principal->new($self->CurrentUser);
$principal->Load($self->NewValue);
- return ( "[_1] [_2] added", $self->loc($self->Field), $principal->Object->Format); #loc
+ return ( "[_1] [_2] added", $self->loc($self->Field), $self->_FormatUser($principal->Object)); #loc
},
DelWatcher => sub {
my $self = shift;
my $principal = RT::Principal->new($self->CurrentUser);
$principal->Load($self->OldValue);
- return ( "[_1] [_2] deleted", $self->loc($self->Field), $principal->Object->Format); #loc
+ return ( "[_1] [_2] deleted", $self->loc($self->Field), $self->_FormatUser($principal->Object)); #loc
},
Subject => sub {
my $self = shift;
@@ -953,25 +964,25 @@ sub _ProcessReturnValues {
return ("Taken"); #loc
}
else {
- return ( "Given to [_1]", $New->Format ); #loc
+ return ( "Given to [_1]", $self->_FormatUser($New) ); #loc
}
}
else {
if ( $New->id == $self->Creator ) {
- return ("Stolen from [_1]", $Old->Format ); #loc
+ return ("Stolen from [_1]", $self->_FormatUser($Old) ); #loc
}
elsif ( $Old->id == $self->Creator ) {
if ( $New->id == RT->Nobody->id ) {
return ("Untaken"); #loc
}
else {
- return ( "Given to [_1]", $New->Format ); #loc
+ return ( "Given to [_1]", $self->_FormatUser($New) ); #loc
}
}
else {
return (
"Owner forcibly changed from [_1] to [_2]", #loc
- $Old->Format, $New->Format
+ map { $self->_FormatUser($_) } $Old, $New
);
}
}
commit a9ed8c149a9b06c16ae564a4ca6bd5b27196ca4d
Author: Thomas Sibley <trs at bestpractical.com>
Date: Tue Dec 11 18:59:10 2012 -0800
Normalize forward recipients before sending and adjust tests to match reality
None of the usual parsing and normalization of email recipients was done
when forwarding tickets or transactions. Use RT::EmailParser's
ParseEmailAddress method to convert to Email::Address objects
(effectively a split), then reformat and join back together. This
ensures domain-less usernames are supported in forwards just like
elsewhere. Importantly, this change also ensures the transaction
recording the forward action has a Data column which is guaranteed
parseable.
The forwarding tests previously tested a domain-less user in the same
field as a full email address, which is not supported by RT's email
parsing and would likely never work as expected with a real MTA. Switch
to testing domain-less users are parsed correctly when in their own
recipient field.
These bugs were uncovered by RT::Transaction's new parsing of $txn->Data
for generating history descriptions.
diff --git a/lib/RT/Interface/Email.pm b/lib/RT/Interface/Email.pm
index 1c7f9b7..b10322e 100644
--- a/lib/RT/Interface/Email.pm
+++ b/lib/RT/Interface/Email.pm
@@ -638,6 +638,9 @@ sub ForwardTransaction {
my $txn = shift;
my %args = ( To => '', Cc => '', Bcc => '', @_ );
+ $args{$_} = join ", ", map { $_->format } RT::EmailParser->ParseEmailAddress($args{$_} || '')
+ for qw(To Cc Bcc);
+
my $entity = $txn->ContentAsMIME;
my ( $ret, $msg ) = SendForward( %args, Entity => $entity, Transaction => $txn );
@@ -665,6 +668,9 @@ sub ForwardTicket {
my $ticket = shift;
my %args = ( To => '', Cc => '', Bcc => '', @_ );
+ $args{$_} = join ", ", map { $_->format } RT::EmailParser->ParseEmailAddress($args{$_} || '')
+ for qw(To Cc Bcc);
+
my $txns = $ticket->Transactions;
$txns->Limit(
FIELD => 'Type',
diff --git a/t/web/ticket_forward.t b/t/web/ticket_forward.t
index 3aceaa4..7a7d8d1 100644
--- a/t/web/ticket_forward.t
+++ b/t/web/ticket_forward.t
@@ -36,19 +36,22 @@ diag "Forward Ticket" if $ENV{TEST_VERBOSE};
$m->submit_form(
form_name => 'ForwardMessage',
fields => {
- To => 'rt-test, rt-to at example.com',
- Cc => 'rt-cc at example.com',
+ To => 'rt-to at example.com, rt-too at example.com',
+ Cc => 'rt-cc at example.com',
+ Bcc => 'root',
},
button => 'ForwardAndReturn'
);
$m->content_contains( 'Sent email successfully', 'sent mail msg' );
$m->content_contains(
- 'Forwarded Ticket to rt-test, rt-to at example.com, rt-cc at example.com',
+ 'Forwarded Ticket to rt-to at example.com, rt-too at example.com, rt-cc at example.com, Enoch Root',
'txn msg' );
my ($mail) = RT::Test->fetch_caught_mails;
like( $mail, qr!Subject: test forward!, 'Subject field' );
- like( $mail, qr!To: rt-test, rt-to\@example.com!, 'To field' );
+ like( $mail, qr!To: .*?rt-to\@example.com!i, 'To field' );
+ like( $mail, qr!To: .*?rt-too\@example.com!i, 'To field' );
like( $mail, qr!Cc: rt-cc\@example.com!i, 'Cc field' );
+ like( $mail, qr!Bcc: "root" <root\@localhost>!i, 'Bcc field' );
like( $mail, qr!This is a forward of ticket!, 'content' );
like( $mail, qr!this is an attachment!, 'att content' );
like( $mail, qr!$att_name!, 'att file name' );
@@ -60,22 +63,23 @@ diag "Forward Transaction" if $ENV{TEST_VERBOSE};
$m->submit_form(
form_name => 'ForwardMessage',
fields => {
- To => 'rt-test, rt-to at example.com',
+ To => 'rt-to at example.com, rt-too at example.com',
Cc => 'rt-cc at example.com',
- Bcc => 'rt-bcc at example.com'
+ Bcc => 'root'
},
button => 'ForwardAndReturn'
);
$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-to\@example.com, rt-too\@example.com, rt-cc\@example.com, Enoch Root/,
'txn msg'
);
my ($mail) = RT::Test->fetch_caught_mails;
like( $mail, qr!Subject: test forward!, 'Subject field' );
- like( $mail, qr!To: rt-test, rt-to\@example.com!, 'To field' );
+ like( $mail, qr!To: .*rt-to\@example.com!i, 'To field' );
+ like( $mail, qr!To: .*rt-too\@example.com!i, 'To field' );
like( $mail, qr!Cc: rt-cc\@example.com!i, 'Cc field' );
- like( $mail, qr!Bcc: rt-bcc\@example.com!i, 'Bcc field' );
+ like( $mail, qr!Bcc: "root" <root\@localhost>!i, 'Bcc field' );
like( $mail, qr!This is a forward of transaction!, 'content' );
like( $mail, qr!$att_name!, 'att file name' );
like( $mail, qr!this is an attachment!, 'att content' );
commit 00d18effe1a3d4882a032bd9a14a951d9398ebc4
Author: Thomas Sibley <trs at bestpractical.com>
Date: Wed Dec 12 13:37:00 2012 -0800
Don't distinguish between visited and unvisited links in transaction descriptions
diff --git a/share/html/NoAuth/css/base/history.css b/share/html/NoAuth/css/base/history.css
index acb8771..9a2a430 100644
--- a/share/html/NoAuth/css/base/history.css
+++ b/share/html/NoAuth/css/base/history.css
@@ -114,6 +114,10 @@ div.history-container {
font-weight: bold;
}
+.transaction .description a:visited {
+ color: inherit;
+}
+
.transaction span.time-taken {
margin-left: 1em;
}
-----------------------------------------------------------------------
More information about the Rt-commit
mailing list