[rt-devel] PATCH: 'Bounce' (re-send) action for data transactions

[Victor Danilchenko]

On Fri, 24 Oct 2003, Ruslan U. Zakirov wrote:

>> +	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 at cs.umass.edu | run in circles, scream, and shout. |
|   CSCF   |   5-4231   |                    Robert Heinlein |