PATCH: 'Bounce' (re-send) action for data transactions

Hi,

This is my first contribution (as well as my first semi-serious

foray into both RT innards and Mason), so be please gentle…

Summary:
In our RT installation, we have often wished to be able to

forward a message from RT to an arbitrary address, most often for the
purpose of e-mailing attachments that came with the message (downloading
them by hand and then mailing them is a pain). This seems impossible in
current RT; so I implemented this capability. I have added a third
action tab (‘[Bounce]’) to share/html/Ticket/Elements/ShowTransaction,
and I have created share/html/Ticket/Bounce.html

The code allows you to specify an arbitrary list of To: and Cc:

recipients, as well as the subject, the latter being initially set from
the subject of the first attachment of transaction, or the ticket
subject if the former doesn’t exist. It will insert the ticket tag into
subject if necessary. Once it receives the destination information, it
will construct a MIME::Entity object, populate it with all the
attachments for a given transaction, and send it on its way via the
Action::Mail::SendEmail facility.

Two questions:
  1. is the patch stylistically OK? is this patch submission? Should I
    supply any additional info, such as the snapshot of the modified
    interface?
  2. Is it actually useful to anyone besides my group?

| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein |

Bounce.patch (4.65 KB)

I can’t make any meaningful comments about the code, but the functionality will be useful to me. It’s a fairly common occurence to have an attachment in RT that has to be sent back out again. Easier from inside, and the return address means any responses get back to the right spot.

I’ll take a look at the patch and probably try it this weekend.

Victor Danilchenko wrote:

Hi,

This is my first contribution (as well as my first semi-serious
foray into both RT innards and Mason), so be please gentle…

Summary:
In our RT installation, we have often wished to be able to
forward a message from RT to an arbitrary address, most often for the
purpose of e-mailing attachments that came with the message (downloading
them by hand and then mailing them is a pain). This seems impossible in
current RT; so I implemented this capability. I have added a third
action tab (‘[Bounce]’) to share/html/Ticket/Elements/ShowTransaction,
and I have created share/html/Ticket/Bounce.html

The code allows you to specify an arbitrary list of To: and Cc:
recipients, as well as the subject, the latter being initially set from
the subject of the first attachment of transaction, or the ticket
subject if the former doesn’t exist. It will insert the ticket tag into
subject if necessary. Once it receives the destination information, it
will construct a MIME::Entity object, populate it with all the
attachments for a given transaction, and send it on its way via the
Action::Mail::SendEmail facility.

Two questions:

  1. is the patch stylistically OK? is this patch submission? Should I
    supply any additional info, such as the snapshot of the modified
    interface?
    See below.
  2. Is it actually useful to anyone besides my group?
    Yes. Not often but yes.

— share/html/Ticket/Elements/ShowTransaction.orig 2003-10-23 14:19:03.000000000 -0400
+++ share/html/Ticket/Elements/ShowTransaction 2003-10-23 14:21:02.000000000 -0400
@@ -164,8 +164,16 @@
$titlebar_commands .=
“[<a href="Update.html?id=”.$Transaction->Ticket.
“&QuoteTransaction=”.$Transaction->Id.

  •  "&Action=Comment\">". loc('Comment') ."</a>]";
    
  •  "&Action=Comment\">". loc('Comment') ."</a>]&nbsp;";
    
    }
  • if ($Transaction->TicketObj->CurrentUserHasRight(‘ShowTicket’)) {
    ShowTransaction element called on each transaction. Don’t you think that
    it’s very expensive to check this Ticket ↔ User right? Also this
    check must be done before call to this Element and RT do it now. So
    skip it.
  • $titlebar_commands .= 
    
  •   "[<a href=\"Bounce.html?id=".
    
  •   $Transaction->Ticket . "&QuoteTransaction=".$Transaction->Id.
    
  •   "\">". loc('Bounce') ."</a>]";
    
  • }

}

</%INIT>
— share/html/Ticket/Bounce.html 2003-10-23 14:41:55.000000000 -0400
+++ share/html/Ticket/Bounce.html 2003-10-23 14:33:37.000000000 -0400
@@ -0,0 +1,107 @@
+%# BEGIN LICENSE BLOCK
+%#

+%#
+%# (Except where explictly superceded by other copyright notices)
+%#
+%# 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.
+%#
+%# Unless otherwise specified, all modifications, corrections or
+%# extensions to this work which alter its source code become the
+%# property of Best Practical Solutions, LLC when submitted for
+%# inclusion in the work.
+%#
+%#
+%# END LICENSE BLOCK
+<& /Elements/Header, Title => loc(“Bounce Transaction # [_1] [_2]”, $Ticket->Id, $Ticket->Subject) &>
+<& /Ticket/Elements/Tabs,

  • Ticket => $Ticket, current_tab => ‘Ticket/Bounce.html?id=’.$Ticket->id,
  • Title => loc(“Bounce Transaction # [_1] [_2]”, $Ticket->Id, $Ticket->Subject) &>

+<% $To && “

Message sent!

” | n %>
+
+% if ($To) {
+% use RT::Action::SendEmail;
+% use MIME::Entity;
+% my $sub = $Subject;
+% my $tag = “[$RT::rtname #” . $Ticket->id . “]”;
+% $sub = “$tag $sub” unless $sub =~ /\Q$tag\E/;
+% my $attachments = $Transaction->Attachments;
+% my $MIMEObj = MIME::Entity->build (Type => “multipart/mixed”,
+% From => “$RT::CorrespondAddress”,
+% To => “$To”,
+% Cc => “$Cc”,
+% Subject => “$sub”);
+% for (my $attch = $attachments->First; $attch; $attch = $attachments->Next) {
How about something like this?
while (my $attch = $attachment->Next) {
+% $MIMEObj->attach(Type => $attch->ContentType,
+% Data => $attch->Content);
+% }
+%
+% my $mailer = new RT::Action::SendEmail;
+% $mailer->SendMessage($MIMEObj);
+% }
For such blocks of code use <%perl> </%perl> section instead of comment
style.
+
+

+<FORM ACTION=“Bounce.html” NAME=“TicketBounce”

  • METHOD=GET enctype=“multipart/form-data”>
    +<input type=“hidden” name=“id” value=“<% “$id” %>”>
    +

+


+

+

+

+

+

Subject: ">
To: ">
Cc:

+<& /Elements/Submit, Name => ‘SubmitTicket’ &>
+


+
+

  •  <& /Ticket/Elements/ShowTransaction, Ticket => $Ticket, Transaction => $Transaction, ShowHeaders => $ARGS{'ShowHeaders'}, ShowTitleBarCommands => $ShowTitleBarCommands, Collapsed => $Collapsed &>
    

+


+
+
+<%ARGS>
+$id => undef
+$ShowTitleBarCommands => 0
+$Collapsed => 0
+$To => undef
+$Cc => undef
+$Subject => undef
+</%ARGS>
+
+<%INIT>
+
+
+
+my $Ticket = LoadTicket ($id);
+my $Transaction;
+for (my $trs = $Ticket->Transactions;

  • $Transaction = $trs->Next;) {
    
  • last if $Transaction->id == $ARGS{‘QuoteTransaction’};
    What about pasting only TransactionId?
    my $Transaction = RT::Transaction->new($RT::CurrentUser);
    $Transaction->Load($ARGS{‘QuoteTransaction’});
    my $Ticket = $Transaction->TicketObj;

+my $header = (grep (/^Subject/i, split (/\n/, $Transaction->Message->First->Headers)))[0];
Wrong way to get Subject it could be multiline. But also it’s wrong we
have allready func for this: $Transaction->Subject();
+$Subject = $1 if ($header && $header =~ /^subject:\s+(.*)/i);
+$Subject ||= $Ticket->Subject;
+
+unless ($Ticket->CurrentUserHasRight(‘ShowTicket’)) {

  • Abort(“No permission to view ticket”);
    +}
    Do such test just after you’ve got $Ticket object in such case you
    economy one select to DB if user don’t have rights.

For some reason I had to add the following to Bounce.html

    $MIMEObj->attach(Type => $attch->ContentType,
                      Data => $attch->Content,
                      Filename => $attch->Filename);

Without the filename the attachement came in as xxxxx.dat

I’m also wondering if this would be a good place to insert a callback to allow modification of $titlebar_commands. This seems more in keeping with the RT ‘style’

I’ve written a callback using the existing ‘ModifyDisplay’ hook which allow $desc to be changed. I also changed ShowTransaction not to escape $desc.

local/html/Callbacks/EWS/Ticket/Elements/ShowTransaction//ModifyDisplay

<%init>
$$text .= ’ '.
"[<a href=“Bounce.html?id=”.
$Transaction->Ticket . “&QuoteTransaction=”.$Transaction->Id.
"">". loc(‘Bounce’) ."]";
</%init>
<%args>
$text => undef;
$Transaction => undef;
</%args>

and added the following to ShowTransaction

From this…

<%$Transaction->CreatorObj->Name%> - <%$TicketString%> <%$desc%>

To this…

<%$Transaction->CreatorObj->Name%> - <%$TicketString%> <%$desc|n%>

Excuse the lack of expertise

Bruce

Thanks for all your suggestions, folks. My response may be
delayed, unfortunately – my wife was due to go into labor yesterday,
but she hasn’t started yet, so Any Moment Now I will be yanked away from
here for at least two weeks. I will get to improving this patch ASAP.On Thu, 23 Oct 2003, Victor Danilchenko wrote:

Hi,

This is my first contribution (as well as my first semi-serious
foray into both RT innards and Mason), so be please gentle…

| Victor Danilchenko ±---------------------------------------------+
| danilche@cs.umass.edu | Does Emacs have the buddha nature? |
| CSCF | 5-4231 | Why not, it has bloody well everything else! |

For some reason I had to add the following to Bounce.html

   $MIMEObj->attach(Type => $attch->ContentType,
                     Data => $attch->Content,
                     Filename => $attch->Filename);

Without the filename the attachement came in as xxxxx.dat

Ah, good idea! I didn't pay attention to the filename.

I’m also wondering if this would be a good place to insert a callback
to allow modification of $titlebar_commands. This seems more in keeping
with the RT ‘style’

I’ve written a callback using the existing ‘ModifyDisplay’ hook which
allow $desc to be changed. I also changed ShowTransaction not to
escape $desc.

local/html/Callbacks/EWS/Ticket/Elements/ShowTransaction//ModifyDisplay

<%init>
$$text .= ’ '.
“[<a href="Bounce.html?id=”.
$Transaction->Ticket . “&QuoteTransaction=”.$Transaction->Id.
“">”. loc(‘Bounce’) .“]”;
</%init>
<%args>
$text => undef;
$Transaction => undef;
</%args>

and added the following to ShowTransaction

From this…

<%$Transaction->CreatorObj->Name%> - <%$TicketString%> <%$desc%>

To this…

<%$Transaction->CreatorObj->Name%> - <%$TicketString%> <%$desc|n%>

Excuse the lack of expertise

Since this is my first submission, I figured i would keep my

changes minimal, localized, and try to ‘blend in’ my code as much as
possible (so I basically copied relevant extant code whenever I could).
Still, your suggestion should be possible to put into a patch completely
orthogonal to mine – why don’t you go ahead and submit it?

| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein |

  • if ($Transaction->TicketObj->CurrentUserHasRight(‘ShowTicket’)) {
    ShowTransaction element called on each transaction. Don’t you think that
    it’s very expensive to check this Ticket ↔ User right? Also this
    check must be done before call to this Element and RT do it now. So
    skip it.
The same check is performed for the other two transaction tabs

– ‘Reply’ and ‘Comment’. If it’s too expensive to perform on each
transaction display, then these checks should be ripped out of
ShowTransaction altogether, and performed once per ticket rather than
once per transaction. it’s a good idea, but I don’t want to mess with
that – I’d rather keep my patch minimally disruptive.

+% for (my $attch = $attachments->First; $attch; $attch = $attachments->Next) {
How about something like this?
while (my $attch = $attachment->Next) {

Yeah, that was stupid <blushes>. The code underwent a few

back-and-forth modifications, and I hadn’t gone back and scrubbed the
syntax thoroughly enough at the end.

+% $MIMEObj->attach(Type => $attch->ContentType,
+% Data => $attch->Content);
+% }
+%
+% my $mailer = new RT::Action::SendEmail;
+% $mailer->SendMessage($MIMEObj);
+% }
For such blocks of code use <%perl> </%perl> section instead of comment
style.

Good point. Done.

+my $Ticket = LoadTicket ($id);
+my $Transaction;
+for (my $trs = $Ticket->Transactions;

  • $Transaction = $trs->Next;) {
    
  • last if $Transaction->id == $ARGS{‘QuoteTransaction’};
    What about pasting only TransactionId?
    my $Transaction = RT::Transaction->new($RT::CurrentUser);
    $Transaction->Load($ARGS{‘QuoteTransaction’});
    my $Ticket = $Transaction->TicketObj;
that looks like a really good idea, but when I do it, there is

some weird problem with the resulting $Transaction object:

error: Can’t call method “HasRight” without a package or object
reference at /nfs/wos/data0/rt/lib/RT/Transaction_Overlay.pm line 828.
context:

824: sub CurrentUserHasRight {
825: my $self = shift;
826: my $right = shift;
827: return (
828: $self->CurrentUser->HasRight(
829: Right => “$right”,
830: Object => $self->TicketObj
831: )
832: );

code stack: /nfs/wos/data0/rt/lib/RT/Transaction_Overlay.pm:828
g /nfs/wos/data0/rt/lib/RT/Attachment_Overlay.pm:535
g /usr/lib/perl5/site_perl/5.8.0/DBIx/SearchBuilder/Record.pm:409
g /nfs/wos/data0/rt/lib/RT/Attachment_Overlay.pm:459
g /nfs/wos/data0/rt/share/html/Ticket/Bounce.html:112
g /nfs/wos/data0/rt/share/html/autohandler:182

I left my code in for now. it's ugly and kludgey, but it works...

Do you have any idea what the above error is caused by?

+my $header = (grep (/^Subject/i, split (/\n/, $Transaction->Message->First->Headers)))[0];
Wrong way to get Subject it could be multiline. But also it’s wrong we
have allready func for this: $Transaction->Subject();

Thanks.

+$Subject = $1 if ($header && $header =~ /^subject:\s+(.*)/i);
+$Subject ||= $Ticket->Subject;
+
+unless ($Ticket->CurrentUserHasRight(‘ShowTicket’)) {

  • Abort(“No permission to view ticket”);
    +}
    Do such test just after you’ve got $Ticket object in such case you
    economy one select to DB if user don’t have rights.
Done.

The modified patch is to follow momentarily...

| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein |

Updated patch, incorporating most of the suggested
modifications, and some new ones I thought of.

| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein |

Bounce.patch (4.41 KB)

Updated patch, incorporating most of the suggested
modifications, and some new ones I thought of.

Oops, that patch contained one typo that messed up the default

subject value. Here’s the correct one.

| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein |

Bounce.patch (4.39 KB)

OK, things have quited down a bit with my newborn…

I have been using this patch, and it seems to work fine. I would be 

interested in seeing if it can be included into the RT trunk. What is
the process for that? Should I contact Jesse directly? or is his
silence an implicit response in itself?

More generally, I am thinking about some more work to do on RT 

(specifically e-mail alias lists, to allow multiple correspondent
e-mail addresses to be mapped to the same RT account), but I would not
wish to invest time and effort into it, only to end up being the only
user of the modification; I can brew up a much simpler one-shot
solution in such a case. Is there a process for
submitting/pre-screening/etc. code modifications and feature additions?On Oct 28, 2003, at 3:37 PM, Victor Danilchenko wrote:

On Tue, 28 Oct 2003, Victor Danilchenko wrote:

Updated patch, incorporating most of the suggested
modifications, and some new ones I thought of.

Oops, that patch contained one typo that messed up the default
subject value. Here’s the correct one.


| Victor Danilchenko | When in danger or in doubt, |
| danilche@cs.umass.edu | run in circles, scream, and shout. |
| CSCF | 5-4231 | Robert Heinlein
|<Bounce.patch>