Process ticket owner updates before message updates

In an RT configured to send notifications to some users
only when they are an owner, performing a single update
that sets a new user as the owner and sends a message
results in the new owner not getting email for the
reply/comment, because the message was processed before
the owner update change was made. This can be confusing,
especially if the message contains notes specifically
addressed to the new owner, such as the reason they are
being assigned the ticket.

Add a new Process step ProcessTicketOwnerUpdate to run
before ProcessUpdateMessage so the new owner is set
before notifications are sent. This allows the new owner
to get email for associated reply/comment.

docs/UPGRADING-5.0

@@ -634,4 +634,28 @@ messages, you may need to update your system to match the new format.
 
 =back
 
+=head1 UPGRADING FROM 5.0.5 AND EARLIER
+
+=over 4
+
+=item Ticket Owner Updates and Notifications
+
+When processing ticket updates, RT previously processed the reply/comment
+before processing other ticket updates, including owner changes. For RTs
+configured to send email to some users only when they are the ticket owner,
+this processing order resulted in new owners not seeing the reply/comment associated
+with the update that made them the new owner. This could cause confusion, especially
+if the reply/comment included information specifically addressed to the new
+owner.
+
+To fix this issue, owner changes are now processed before messages, so new
+owners will now see the message associated with the change that made them
+owner.
+
+This will result in new email being sent when it wasn't previously. We believe
+this fixes the previous incorrect behavior. However, if you relied on this behavior,
+and don't want the new email, you may need to modify your scrip configurations.
+
+=back
+
 =cut

lib/RT/Interface/Web.pm

@@ -3167,14 +3167,70 @@ sub ProcessCustomFieldUpdates {
     return (@results);
 }
 
+=head2 ProcessTicketOwnerUpdate ( TicketObj => $Ticket, ARGSRef => \%ARGS );
 
+Processes just Owner updates on the provided ticket, based
+on the provided ARGS.
+
+Returns an array of results messages.
+
+=cut
+
+sub ProcessTicketOwnerUpdate {
+
+    my %args = (
+        TicketObj => undef,
+        ARGSRef   => undef,
+        @_
+    );
+
+    my $TicketObj = $args{'TicketObj'};
+    my $ARGSRef   = $args{'ARGSRef'};
+    my @results;
+
+    my $OrigOwner = $TicketObj->Owner;
+
+    # Canonicalize Owner to ID if it's not numeric
+    if ( $ARGSRef->{'Owner'} and ( $ARGSRef->{'Owner'} !~ /^(\d+)$/ ) ) {
+        my $temp = RT::User->new(RT->SystemUser);
+        $temp->Load( $ARGSRef->{'Owner'} );
+        if ( $temp->id ) {
+            $ARGSRef->{'Owner'} = $temp->Id;
+        }
+    }
+
+    # We special case owner changing, so we can use ForceOwnerChange
+    if ( $ARGSRef->{'Owner'}
+      && $ARGSRef->{'Owner'} !~ /\D/
+      && ( $OrigOwner != $ARGSRef->{'Owner'} ) ) {
+        my ($ChownType);
+        if ( $ARGSRef->{'ForceOwnerChange'} ) {
+            $ChownType = "Force";
+        }
+        else {
+            $ChownType = "Set";
+        }
+
+        my ( $val, $msg ) = $TicketObj->SetOwner( $ARGSRef->{'Owner'}, $ChownType );
+        push( @results, $msg );
+    }
+
+    return (@results);
+}
 
 =head2 ProcessTicketBasics ( TicketObj => $Ticket, ARGSRef => \%ARGS );
 
 Returns an array of results messages.
 
 =cut
 
+# ProcessTicketOwnerUpdate updates Owner only and was created to run
+# earlier in the ticket update process. Keep Owner update code here
+# also for any existing code that might call ProcessTicketBasics.
+#
+# If ProcessTicketOwnerUpdate handles the update first, it should be
+# a noop here.
+
 sub ProcessTicketBasics {
 
     my %args = (

share/html/Helpers/PreviewScrips

@@ -63,6 +63,7 @@ else {
 my @dryrun = $TicketObj->DryRun(
     sub {
         local $ARGS{UpdateContent} ||= "Content";
+        ProcessTicketOwnerUpdate(ARGSRef => \%ARGS, TicketObj => $TicketObj );
         ProcessUpdateMessage(ARGSRef  => \%ARGS, TicketObj => $TicketObj );
         ProcessTicketWatchers(ARGSRef => \%ARGS, TicketObj => $TicketObj );
         ProcessTicketBasics(  ARGSRef => \%ARGS, TicketObj => $TicketObj );

share/html/Helpers/ShowSimplifiedRecipients

@@ -64,6 +64,7 @@ else {
 my @dryrun = $TicketObj->DryRun(
     sub {
         local $ARGS{UpdateContent} ||= "Content";
+        ProcessTicketOwnerUpdate(ARGSRef => \%ARGS, TicketObj => $TicketObj );
         ProcessUpdateMessage(ARGSRef  => \%ARGS, TicketObj => $TicketObj );
         ProcessTicketWatchers(ARGSRef => \%ARGS, TicketObj => $TicketObj );
         ProcessTicketBasics(  ARGSRef => \%ARGS, TicketObj => $TicketObj );

share/html/Helpers/TicketUpdate

@@ -66,6 +66,12 @@ $m->callback(CallbackName => 'ProcessArguments',
              ARGSRef => \%ARGS,
              Actions => \@Actions);
 
+# It's common to change owner and add a reply/comment in the same
+# update. Process the owner change before the message update so the
+# new owner will see the message if they only see notifications when
+# they are the owner.
+push @Actions, ProcessTicketOwnerUpdate(ARGSRef => \%ARGS, TicketObj => $TicketObj );
+
 push @Actions, ProcessUpdateMessage(
     ARGSRef   => \%ARGS,
     Actions   => \@Actions,

share/html/Ticket/Display.html

@@ -186,6 +186,12 @@
                 ARGSRef => \%ARGS,
                 Actions => \@Actions);
 
+        # It's common to change owner and add a reply/comment in the same
+        # update. Process the owner change before the message update so the
+        # new owner will see the message if they only see notifications when
+        # they are the owner.
+        push @Actions, ProcessTicketOwnerUpdate(ARGSRef => \%ARGS, TicketObj => $TicketObj );
+
         push @Actions, ProcessUpdateMessage(
             ARGSRef   => \%ARGS,
             Actions   => \@Actions,