[Rt-commit] rt branch, 4.4/not-apply-scrip-batch-in-nested-atomic, created. rt-4.4.4-411-g2ec8e3c200

[? sunnavy]

The branch, 4.4/not-apply-scrip-batch-in-nested-atomic has been created
        at  2ec8e3c200415b9e8fd0689162187df7e889bbb3 (commit)

- Log -----------------------------------------------------------------
commit 3528ae2bf5df8e26ff1e204dd9e1a8157cfa6a88
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Wed Dec 4 04:07:09 2019 +0800

    Call ApplyTransactionBatch only in the most outer Atomic call
    
    Batch scrips are supposed to run at last, after all the user actions.
    Previously we called ApplyTransactionBatch in every Atomic call, which
    might cause batch scrips to run earlier than they are supposed to be,
    when there are nested Atomic calls involved.
    
    E.g. on ticket display page, we have all the Process... methods wrapped
    into an Atomic call, but one of the Process methods(ProcessTicketBasics)
    calls SetOwner that makes another Atomic call, which could cause
    ApplyTransactionBatch to run before other normal changes made by the
    following Process... methods including ProcessTicketLinks,
    ProcessTicketDates, ProcessObjectCustomFieldUpdates and
    ProcessTicketReminders, which is unexpected.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index d1b83e42d8..e4a9523479 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -1761,6 +1761,8 @@ ticket changes via the UI
 sub Atomic {
     my $self = shift;
     my ($subref) = @_;
+    $self->{Atomic}++;
+
     my $has_id = defined $self->id;
     $RT::Handle->BeginTransaction;
     my $depth = $RT::Handle->TransactionDepth;
@@ -1787,8 +1789,9 @@ sub Atomic {
     }
 
     if ($RT::Handle->TransactionDepth == $depth) {
-        $self->ApplyTransactionBatch;
+        $self->ApplyTransactionBatch if $self->{Atomic} == 1;
         $RT::Handle->Commit;
+        $self->{Atomic}--;
     }
 
     return $context ? @ret : $ret[0];

commit af341c4168078223d8bbb135ee374df216d2f889
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Dec 5 02:00:06 2019 +0800

    Don't call ApplyTransactionBatch in Atomic when in DryRun mode
    
    DryRun explicitly calls ApplyTransactionBatch at last, so it's not
    needed to call it in Atomic in advance. In some sense, DryRun is kinda
    like the most outer Atomic call.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index e4a9523479..9db9bffa6a 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -1789,7 +1789,7 @@ sub Atomic {
     }
 
     if ($RT::Handle->TransactionDepth == $depth) {
-        $self->ApplyTransactionBatch if $self->{Atomic} == 1;
+        $self->ApplyTransactionBatch if $self->{Atomic} == 1 && !$self->{DryRun};
         $RT::Handle->Commit;
         $self->{Atomic}--;
     }

commit 2ec8e3c200415b9e8fd0689162187df7e889bbb3
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu May 6 03:08:43 2021 +0800

    Test ApplyTransactionBatch in nested atomic calls

diff --git a/t/ticket/scrips_batch.t b/t/ticket/scrips_batch.t
index 0a996ce453..5698d0eddf 100644
--- a/t/ticket/scrips_batch.t
+++ b/t/ticket/scrips_batch.t
@@ -2,7 +2,7 @@
 use strict;
 use warnings;
 
-use RT::Test tests => 19;
+use RT::Test tests => undef;
 use_ok('RT');
 use_ok('RT::Ticket');
 
@@ -76,6 +76,19 @@ END
     $m->click('SubmitTicket');
 
     is_deeply parse_handle($tmp_fh), ['Comment', 'Status'], 'Comment + Resolve';
+
+    $m->follow_link_ok( { text => 'Comment' } );
+    $m->form_name('TicketUpdate');
+
+    my $root = RT::User->new( RT->SystemUser );
+    $root->Load('root');
+    $m->field( Owner => $root->Id );
+
+    # Assume ticket update page is customized to add Due date input
+    $m->field( Due_Date => '2021-05-05' );
+    $m->click('SubmitTicket');
+
+    is_deeply parse_handle($tmp_fh), ['Set', 'SetWatcher', 'Set'], 'Set Owner + Set Due';
 }
 
 sub value_name {
@@ -101,3 +114,4 @@ sub parse_handle {
     return \@lines;
 }
 
+done_testing;

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