[Rt-commit] rt branch, 4.0/transaction-batch-twice, updated. rt-4.0.2-140-gf96ca0c

[Kevin Falcone]

The branch, 4.0/transaction-batch-twice has been updated
       via  f96ca0c5be883c96e313cca0871faf24d7481f62 (commit)
      from  dee4d21c7bbd8aead13f52000c675907fb261071 (commit)

Summary of changes:
 lib/RT/Scrips.pm |   10 ++++++----
 lib/RT/Ticket.pm |   10 ++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

- Log -----------------------------------------------------------------
commit f96ca0c5be883c96e313cca0871faf24d7481f62
Author: Kevin Falcone <falcone at bestpractical.com>
Date:   Tue Oct 11 17:10:07 2011 -0400

    Stop TransactionBatch scrips from running twice.
    
    Also prevent the privilege leak originally fixed in
    2338cd19ed7a7f4c1e94f639ab2789d6586d01f3.
    
    Unfortunately, cloning the ticket passed in to ApplyTransactionBatch
    into $scrips->{TicketObj} means that we run TransactionBatch Scrips once
    for the ticket that called ApplyTransactionBatch and once for
    $self->{TicketObj}.
    
    Trying to solve this with a Load that makes $self->{TicketObj}
    completely separate from $args{TicketObj} means that $self->{TicketObj}
    doesn't get the _TransactionBatch cache and when we get into
    RT::Scrip->IsApplicable we won't have the Transactions handy to check
    our Condition against.
    
    Copying the _TransactionBatch cache into $self->{TicketObj} and deleting
    it from $args{TicketObj} results in TransactionBatch scrips running
    twice again because $self->{TicketObj} will be DESTROYed, triggering
    ApplyTransactionBatch.
    
    The only non-bandaid and smallish solution to this is having a flag for
    'run transaction batch' which is separate from the cache of Transactions
    which are batched.
    
    The correct long term solution is most likely a separate table to track
    which transactions are batched together.

diff --git a/lib/RT/Scrips.pm b/lib/RT/Scrips.pm
index 077dbfe..b241460 100644
--- a/lib/RT/Scrips.pm
+++ b/lib/RT/Scrips.pm
@@ -280,10 +280,12 @@ sub _SetupSourceObjects {
             @_ );
 
 
-    if ( $args{'TicketObj'} ) {
-        # clone the ticket here as we need to change CurrentUser
-        $self->{'TicketObj'} = bless { %{$args{'TicketObj'} } }, 'RT::Ticket';
-        $self->{'TicketObj'}->CurrentUser( $self->CurrentUser );
+    if ( $self->{'TicketObj'} = $args{'TicketObj'} ) {
+        # this clobbers a passed in TicketObj by turning it into a RT_System
+        # object.  Anywhere in the Web UI currently calling in to this which
+        # run into the privilege leak is wrapped in ApplyTransactionBatch
+        # which bandaids over the top of this.
+        $self->{'TicketObj'}->Load( $args{'TicketObj'}->Id );
     }
     else {
         $self->{'TicketObj'} = RT::Ticket->new( $self->CurrentUser );
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index f299c81..eeb6021 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -3276,9 +3276,19 @@ sub ApplyTransactionBatch {
     my $batch = $self->TransactionBatch;
     return unless $batch && @$batch;
 
+    # this is a giant bandaid.
+    # RT::Scrips->_SetupSourceObjects will clobber
+    # the CurrentUser, but we need to keep this ticket
+    # so that the _TransactionBatch cache is maintained
+    # and doesn't run twice.  sigh.
+    my $old_current_user = $self->CurrentUser;
+
     $self->_ApplyTransactionBatch;
 
     $self->{_TransactionBatch} = [];
+
+    # Apply the bandaid.
+    $self->CurrentUser($old_current_user);
 }
 
 sub _ApplyTransactionBatch {

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