[Rt-commit] rt branch, 5.0/allow-ticket-unmerging, created. rt-5.0.0-225-geb01bd597e
Dianne Skoll
dianne at bestpractical.com
Wed Jan 20 16:41:47 EST 2021
The branch, 5.0/allow-ticket-unmerging has been created
at eb01bd597e26d0b7e2f588af3fccbde740bd293f (commit)
- Log -----------------------------------------------------------------
commit eb01bd597e26d0b7e2f588af3fccbde740bd293f
Author: Dianne Skoll <dianne at bestpractical.com>
Date: Wed Jan 20 16:31:57 2021 -0500
Merge Ticket_Overlay.pm from RT-Extension-UnmergeTickets into Tickets.pm
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index dd340f1826..e3a65115b7 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -481,6 +481,7 @@ sub Create {
}
# Add all the custom fields
+ my @added_watchers; # roles that need to be removed when unmerging
foreach my $arg ( keys %args ) {
next unless $arg =~ /^CustomField-(\d+)$/i;
my $cfid = $1;
@@ -1931,6 +1932,9 @@ sub _MergeInto {
my $self = shift;
my $MergeInto = shift;
+ # Store original link info before any merge action, so it can be
+ # restored on unmerge if needed
+ my @saved_links= $self->_MergeLinksInfo( $MergeInto);
# We use EffectiveId here even though it duplicates information from
# the links table becasue of the massive performance hit we'd take
@@ -1987,7 +1991,7 @@ sub _MergeInto {
if (exists $old_seen{$link->Type."-".$link->Target}) {
$link->Delete;
}
- if ($link->Target eq $MergeInto->URI) {
+ elsif ($link->Target eq $MergeInto->URI) {
$link->Delete;
} else {
# First, make sure the link doesn't already exist. then move it over.
@@ -2014,6 +2018,7 @@ sub _MergeInto {
}
# add all of this ticket's watchers to that ticket.
+ my @added_watchers; # roles that would need to be removed when unmerging
for my $role ($self->Roles) {
my $group = $self->RoleGroup($role);
next unless $group->Id; # e.g. lazily-created custom role groups
@@ -2026,7 +2031,12 @@ sub _MergeInto {
PrincipalId => $watcher->MemberId,
InsideTransaction => 1,
);
- unless ($val) {
+ if( $val ) {
+ # added so it can be removed on unmerge
+ push @added_watchers, { id => $watcher->MemberId, type => $role };
+ }
+ else {
+ # watcher was already set on the ticket, so it won't have to be removed on unmerge
$RT::Logger->debug($msg);
}
}
@@ -2041,11 +2051,35 @@ sub _MergeInto {
);
# update their EffectiveId fields to the new ticket's id
+ my @previous_merges;
while ( my $ticket = $old_mergees->Next() ) {
my ( $val, $msg ) = $ticket->__Set(
Field => 'EffectiveId',
Value => $MergeInto->Id()
);
+
+ if ( $val ) {
+ push @previous_merges, $ticket->Id unless $ticket->Id == $self->Id;
+ }
+ }
+
+ my $saved_info = {
+ merged_tickets => \@previous_merges,
+ links => \@saved_links,
+ added_watchers => \@added_watchers,
+ merged => $self->Id,
+ merged_into => $MergeInto->Id,
+ };
+
+ my ( $ret, $msg ) = $MergeInto->AddAttribute(
+ Name => 'MergeInfo-' . $self->Id,
+ Description => "Merged " . $self->Id . " into " . $MergeInto->Id,
+ Content => $saved_info,
+ );
+
+ unless ( $ret ) {
+ RT->Logger->error("Unable to save merge data. Will not be able to fully unmerge ticket "
+ . $self->Id . " $msg");
}
#make a new link: this ticket is merged into that other ticket.
@@ -2056,6 +2090,311 @@ sub _MergeInto {
return ( 1, $self->loc("Merge Successful") );
}
+# nearly duplicates what's in _MergeInto, but gathers the info
+# and save it instead of updating it
+
+sub _MergeLinksInfo {
+ my $self = shift; # $self is the merged ticket
+ my $MergeInto = shift;
+
+ my @saved_links;
+
+ # get all the links that point to that old ticket
+ my $old_links_to = RT::Links->new($self->CurrentUser);
+ $old_links_to->Limit(FIELD => 'Target', VALUE => $self->URI);
+
+ my %old_seen;
+ while ( my $link = $old_links_to->Next ) {
+
+ my $direction = 'Target';
+
+ if ( exists $old_seen{$link->Base."-".$link->Type} ) {
+ next;
+ }
+ elsif ( $link->Base eq $MergeInto->URI ) {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'deleted', Direction => $direction, Link => $link);
+ }
+ else {
+ # First, make sure the link doesn't already exist. If not, save it.
+ my $tmp = RT::Link->new(RT->SystemUser);
+ $tmp->LoadByCols(Base => $link->Base, Type => $link->Type, LocalTarget => $MergeInto->id);
+ if ($tmp->id) {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'deleted', Direction => $direction, Link => $link);
+ }
+ else {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'moved', Direction => $direction, Link => $link);
+ }
+ $old_seen{$link->Base."-".$link->Type} = 1;
+ }
+
+ }
+
+ my $old_links_from = RT::Links->new($self->CurrentUser);
+ $old_links_from->Limit(FIELD => 'Base', VALUE => $self->URI);
+
+ while ( my $link = $old_links_from->Next ) {
+
+ my $direction = 'Base';
+
+ if ( exists $old_seen{$link->Type."-".$link->Target} ) {
+ next;
+ }
+ elsif ($link->Target eq $MergeInto->URI) {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'deleted', Direction => $direction, Link => $link);
+ }
+ else {
+ # First, make sure the link doesn't already exist. then move it over.
+ my $tmp = RT::Link->new(RT->SystemUser);
+ $tmp->LoadByCols(Target => $link->Target, Type => $link->Type, LocalBase => $MergeInto->id);
+ if ($tmp->id) {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'deleted', Direction => $direction, Link => $link);
+ }
+ else {
+ push @saved_links, $self->_SerializeLinkData(
+ Action => 'moved', Direction => $direction, Link => $link);
+ }
+ $old_seen{$link->Type."-".$link->Target} = 1;
+ }
+ }
+
+ return @saved_links;
+}
+
+# Note: When using UnmergeFrom in new code, mind the cache!
+#
+# ie after unmerging, if you hold an object for the unmerged ticket
+# or for tickets linked to any of the tickets, it is possible that
+# their links (and potentially other info) are cached and are not updated
+# after the unmerge. In fact the DB is updated, but some methods return
+# cached values that are stale.
+# The safest is to create a new object and reload it, like this:
+# my $id = $ticket->id;
+# $ticket = RT::Ticket->new( $CurrentUser );
+# $ticket->Load( $id);
+#
+# this occurs mostly in tests,
+
+sub UnmergeFrom {
+ my $self = shift;
+ my $merged_into = shift;
+
+ unless ( $self->CurrentUserHasRight('ModifyTicket') && $merged_into->CurrentUserHasRight('ModifyTicket')) {
+ return ( 0, $self->loc("Permission Denied") );
+ }
+
+ my $id = $self->Id;
+ my $merged_into_id = $merged_into->Id;
+
+ if( $id == $self->EffectiveId || ! $self->IsMerged) {
+ return ( 0, $self->loc( "Trying to unmerge a ticket that is not merged: ") . $id );
+ }
+
+ if( $self->EffectiveId != $merged_into_id ) {
+ return ( 0, $self->loc( "Unmerge error, tickets are not merged: ") . $id . ', ' . $merged_into_id);
+ }
+
+ # Completely clear the merge cache
+ %MERGE_CACHE = (
+ effective => {},
+ merged => {},
+ );
+
+ $RT::Handle->BeginTransaction();
+
+ my ($ok, $msg) = $self->_UnmergeFrom( $merged_into );
+
+ if( $ok) {
+ $RT::Handle->Commit();
+ } else {
+ $RT::Handle->Rollback();
+ return ( 0, $self->loc("Unmerge failed") );
+ }
+
+ return ($ok, $msg);
+}
+
+
+sub _UnmergeFrom {
+ my $self = shift;
+ my $merged_into = shift;
+
+ my $id = $self->Id;
+ my $merged_into_id = $merged_into->Id;
+
+ my( $ok, $msg);
+
+ ($ok, $msg)= $self->__Set( Field => 'IsMerged', Value => undef );
+ if( ! $ok) {
+ return (0, $self->loc("Unable to update IsMerged property") . ": $msg");
+ }
+
+ ($ok, $msg)= $self->__Set( Field => 'EffectiveId', Value => $id );
+ if( ! $ok) {
+ return (0, $self->loc("Unable to update EffectiveId for") . " $id: $msg");
+ }
+
+ ($ok, $msg)= $merged_into->DeleteLink( Type => 'MergedInto', Base => $self->URI, Silent => 1 );
+ if( ! $ok ) {
+ return ( 0, $self->loc("Unable to delete MergedInto link from [_1] to [_2]", $id, $merged_into_id) );
+ }
+
+ foreach my $type (qw(TimeEstimated TimeWorked TimeLeft)) {
+ if( $merged_into->$type() ) {
+ ($ok, $msg)= $merged_into->_Set(
+ Field => $type,
+ Value => ( $merged_into->$type() || 0 ) - ( $self->$type() || 0 ),
+ RecordTransaction => 0,
+ );
+ if( ! $ok ) {
+ RT::Logger->error( "cannot restore time field $type: $msg");
+ return ( 0, $self->loc("cannot restore time field"));
+ }
+ }
+ }
+
+ ($ok, $msg)= $self->RestoreMergeInfo( $merged_into);
+ if( ! $ok) {
+ return (0, $msg);
+ }
+
+ # record the transaction, since all others have been silenced
+ my $trans;
+ ( $ok, $msg, $trans ) = $self->_NewTransaction(
+ Type => 'DeleteLink',
+ Field => 'Unmerge',
+ Data => "Unmerged ticket #$id from ticket #$merged_into_id",
+ TimeTaken => 0,
+ );
+ if( ! $ok) {
+ return (0, $msg);
+ }
+
+ return ( 1, $self->loc("Unmerge Successful") );
+}
+
+
+sub RestoreMergeInfo {
+ my( $self, $merged_into ) = @_;
+
+ my ( $ret, $msg );
+
+ my $saved_info_attribute = $merged_into->FirstAttribute( 'MergeInfo-' . $self->Id);
+ if( ! $saved_info_attribute) {
+ RT::Logger->error( "Unable to load merge data for " . $self->Id );
+ return ( 0, $self->loc( "Unable to load merge data") );
+ }
+
+ my $saved_info = $saved_info_attribute->Content;
+ if( $saved_info->{merged_into} != $merged_into->Id) {
+ RT::Logger->error( "Incorrect ticket id found in loaded merge data. Found "
+ . $saved_info->{merged_into} . " expected " . $merged_into->Id);
+ return( 0, $self->loc( "Incorrect ticket id in merge data"));
+ }
+
+ # Restore tickets previously merged into the now un-merged ticket
+ foreach my $old_merged_id (@{$saved_info->{merged_tickets}}) {
+ my $old_merged = RT::Ticket->new( $self->CurrentUser );
+ my( $ret, $msg ) = $old_merged->LoadById( $old_merged_id );
+ if( !$ret ) {
+ return ( 0, $self->loc( "Unable to load previously merged ticket ") . $old_merged_id);
+ }
+ if( $old_merged->EffectiveId != $merged_into->Id ) {
+ RT::Logger->error( 0, "Previously merged ticket has an incorrect effective id. Found "
+ . $old_merged->EffectiveId . " expected " . $merged_into->Id );
+ return ( 0, $self->loc( "Previously merged ticket has an incorrect effective id"));
+ }
+ ( $ret, $msg ) = $old_merged->__Set(
+ Field => 'EffectiveId',
+ Value => $self->Id,
+ RecordTransaction => 0,
+ );
+ if( !$ret ) {
+ return( 0, $self->loc("Unable to restore EffectiveId ") . $self->Id);
+ }
+ }
+
+ # restore links
+ foreach my $saved_link (@{$saved_info->{links}}) {
+
+ if( $saved_link->{action} eq 'moved') {
+ my $link = RT::Link->new(RT->SystemUser);
+ my( $id, $msg)= $link->Load( $saved_link->{Id});
+ if( ! $id) {
+ # the link has been removed, recreate it
+ $self->RestoreLink( $saved_link );
+ } else {
+ $RT::Logger->debug( "moving back link $saved_link->{direction}: " . $self->URI . " [" . $self->id . "] (link id is: " . $id . ")");
+ if( $saved_link->{direction} eq 'Target') {
+ ($ret, $msg)= $link->SetTarget($self->URI);
+ if( ! $ret) {
+ $RT::Logger->debug( "could not update target: $msg");
+ }
+ $link->SetLocalTarget($self->Id);
+ } elsif( $saved_link->{direction} eq 'Base') {
+ ( $ret, $msg) = $link->SetBase($self->URI);
+ if( ! $ret) { return( 0, $msg); }
+ ( $ret, $msg) = $link->SetLocalBase($self->Id);
+ if( ! $ret) { return( 0, $msg); }
+ } else {
+ # the link points/comes from somewhere else, bail
+ return( 0, $self->loc( "cannot restore link ") . $saved_link->{Id});
+ }
+ }
+ } elsif( $saved_link->{action} eq 'deleted') {
+ # link was deleted (because it was between the 2 merged tickets, or there was already one with merged_into)
+ ( $ret, $msg )= $self->RestoreLink( $saved_link );
+ if( ! $ret) { return( 0, $msg); }
+ }
+ }
+
+ # remove added watchers
+ foreach my $added_watcher (@{$saved_info->{added_watchers}}) {
+ my ( $ret, $msg ) = $merged_into->DeleteRoleMember( PrincipalId => $added_watcher->{id}, Type => $added_watcher->{type});
+ if ( !$ret ) {
+ return( 0, $self->loc( "Unable to remove watcher ") . $added_watcher->{id})
+ }
+ }
+
+ ( $ret, $msg) = $merged_into->DeleteAttribute( 'MergeInfo-' . $self->Id);
+ if( !$ret ) { return( 0, $msg); }
+
+ return ( 1, $self->loc( "Restored saved merge info"));
+}
+
+sub RestoreLink {
+ my( $self, $link)= @_;
+
+ my $end_point = $link->{direction} eq 'Target' ? $link->{Base} : $link->{Target};
+ my( $ret, $msg ) = $self->AddLink( $link->{direction} => $end_point, Type => $link->{Type}, Silent => 1);
+ if( !$ret ) {
+ return( 0, $self->loc( "Unable to restore link: ") . $msg );
+ }
+ $RT::Logger->debug( "Restored link $link->{direction}: " . $self->URI . "/" . $self->id . " (link id is: " . $ret . ")");
+ return( $ret, "Restored link");
+}
+
+sub _SerializeLinkData {
+ my $self = shift;
+ my %args = (
+ Action => '',
+ Direction => '',
+ Link => undef,
+ @_);
+
+ return { action => $args{'Action'}, direction => $args{'Direction'},
+ map { $_ => $args{'Link'}->$_ } ( qw( Id Base Target Type)) };
+}
+
+# TODO:
+#
+# Additional testing with link scenarios, specifically when a 3rd ticket
+# is linked to both tickets, then one is merged/unmerged with the other.
+
=head2 Merged
Returns list of tickets' ids that's been merged into this ticket.
-----------------------------------------------------------------------
More information about the rt-commit
mailing list