diff --git a/docs/UPGRADING-5.0 b/docs/UPGRADING-5.0 index 49f7704222c..1603593eb6c 100644 --- a/docs/UPGRADING-5.0 +++ b/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 diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm index 64ce8dc643d..e63ed0e3f58 100644 --- a/lib/RT/Interface/Web.pm +++ b/lib/RT/Interface/Web.pm @@ -3167,7 +3167,56 @@ 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 ); @@ -3175,6 +3224,13 @@ 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 = ( diff --git a/share/html/Helpers/PreviewScrips b/share/html/Helpers/PreviewScrips index 9a294027b56..b2b7a42b7c4 100644 --- a/share/html/Helpers/PreviewScrips +++ b/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 ); diff --git a/share/html/Helpers/ShowSimplifiedRecipients b/share/html/Helpers/ShowSimplifiedRecipients index aef51fc34c5..a8b0c0bc9da 100644 --- a/share/html/Helpers/ShowSimplifiedRecipients +++ b/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 ); diff --git a/share/html/Helpers/TicketUpdate b/share/html/Helpers/TicketUpdate index 2e56c7e70dd..23d5e35250e 100644 --- a/share/html/Helpers/TicketUpdate +++ b/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, diff --git a/share/html/Ticket/Display.html b/share/html/Ticket/Display.html index c08d37b90a3..f831d98305e 100644 --- a/share/html/Ticket/Display.html +++ b/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,