[Rt-commit] rt branch, 4.2/time-fields-on-merge, created. rt-4.1.6-361-gf25a7af

Ruslan Zakirov ruz at bestpractical.com
Fri Mar 15 18:47:33 EDT 2013


The branch, 4.2/time-fields-on-merge has been created
        at  f25a7af0001060397bf28113f8a2e04afb5af782 (commit)

- Log -----------------------------------------------------------------
commit e9a2f200d8ceb4c021bacf027dc26c79a84bb291
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Jul 26 00:10:32 2012 +0400

    don't record transactions for Time* fields on merge
    
    1) merge shouldn't generate any transactions except merge
    2) recording Time* change transaction mess numbers in history.
       As target ticket gets all transactions from both sides then
       history is already correct. Yes, numbers in the history are
       strange, for example you can have two txns saying time worked
       was changed from 0 to X and from 0 to Y, but the difference
       is what's important and is correct.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index b5eff48..2205487 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -1924,12 +1924,13 @@ sub _MergeInto {
 
     # Update time fields
     foreach my $type (qw(TimeEstimated TimeWorked TimeLeft)) {
-
-        my $mutator = "Set$type";
-        $MergeInto->$mutator(
-            ( $MergeInto->$type() || 0 ) + ( $self->$type() || 0 ) );
-
+        $MergeInto->_Set(
+            Field => $type,
+            Value => ( $MergeInto->$type() || 0 ) + ( $self->$type() || 0 ),
+            RecordTransaction => 0,
+        );
     }
+
     # add all of this ticket's watchers to that ticket.
     for my $role ($self->Roles) {
         next if $self->RoleGroup($role)->SingleMemberRoleGroup;
diff --git a/t/ticket/merge.t b/t/ticket/merge.t
index 2484b65..99c723b 100644
--- a/t/ticket/merge.t
+++ b/t/ticket/merge.t
@@ -4,7 +4,7 @@ use warnings;
 
 
 use RT;
-use RT::Test tests => '29';
+use RT::Test tests => '44';
 
 
 # validate that when merging two tickets, the comments from both tickets
@@ -134,3 +134,46 @@ ok $user && $user->id, 'loaded or created user';
     ($status,$msg) = $t->MergeInto($t2->id);
     ok($status, "Merged tickets: $msg");
 }
+
+# check Time* fields after merge
+{
+    my @tickets;
+    my @values = (
+        { Worked => 11, Estimated => 17, Left => 6 },
+        { Worked => 7, Estimated => 12, Left => 5 },
+    );
+
+    for my $i (0 .. 1) {
+        my $t = RT::Ticket->new(RT->SystemUser);
+        $t->Create( Queue => 'general');
+        ok ($t->id);
+        push @tickets, $t;
+
+        foreach my $field ( keys %{ $values[ $i ] } ) {
+            my $method = "SetTime$field";
+            my ($status, $msg) = $t->$method( $values[ $i ]{ $field } );
+            ok $status, "changed $field on the ticket"
+                or diag "error: $msg";
+        }
+    }
+
+    my ($status, $msg) = $tickets[1]->MergeInto($tickets[0]->id);
+    ok($status,$msg);
+
+    my $t = RT::Ticket->new(RT->SystemUser);
+    $t->Load( $tickets[0]->id );
+    foreach my $field ( keys %{ $values[0] } ) {
+        my $method = "Time$field";
+        my $expected = 0;
+        $expected += $_->{ $field } foreach @values;
+        is $t->$method, $expected, "correct value";
+
+        my $from_history = 0;
+        my $txns = $t->Transactions;
+        while ( my $txn = $txns->Next ) {
+            next unless $txn->Type eq 'Set' && $txn->Field eq $method;
+            $from_history += $txn->NewValue - $txn->OldValue;
+        }
+        is $from_history, $expected, "history is correct";
+    }
+}

commit 5b39b3c10ac8bea92260548add7de42b33bb1084
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Thu Jul 26 19:31:21 2012 +0400

    script that deletes bad (Set, TimeWorked) txns
    
    It try to fix history and do it as safe as possible.
    It calcs time worked using history, by the way collects
    candidates for deletion. Transactions are deleted only
    if time on ticket is equal to time from history minus
    time on candidates.
    
    It's probably possible to write similar script for TimeLeft
    and TimeEstimated, but it's much harder to do as the value
    is not recorded on Create unlike time worked which is stored
    in TimeTaken column.

diff --git a/etc/upgrade/time-worked-history.pl b/etc/upgrade/time-worked-history.pl
new file mode 100644
index 0000000..7ed5535
--- /dev/null
+++ b/etc/upgrade/time-worked-history.pl
@@ -0,0 +1,63 @@
+#!/usr/bin/env perl
+use 5.8.3;
+use strict;
+use warnings;
+
+use RT;
+RT::LoadConfig();
+RT->Config->Set('LogToScreen' => 'info');
+RT::Init();
+
+my $dbh = $RT::Handle->dbh;
+my $ids = $dbh->selectcol_arrayref(
+    "SELECT t1.id FROM Tickets t1, Tickets t2 WHERE t1.id = t2.EffectiveId"
+    ." AND t2.id != t2.EffectiveId AND t2.EffectiveId = t1.id"
+);
+foreach my $id ( @$ids ) {
+    my $t = RT::Ticket->new( RT->SystemUser );
+    $t->Load( $id );
+    unless ( $t->id ) {
+        $RT::Logger->error("Couldn't load ticket #$id");
+        next;
+    }
+
+    fix_time_worked_history($t);
+}
+
+sub fix_time_worked_history {
+    my ($t) = (@_);
+
+    my $history = 0;
+    my $candidate = undef;
+    my @delete = ();
+    my $delete_time = 0;
+
+    my $txns = $t->Transactions;
+    while ( my $txn = $txns->Next ) {
+        if ( $txn->Type =~ /^(Create|Correspond|Comment)$/ ) {
+            $history += $txn->TimeTaken || 0;
+        } elsif ( $txn->Type eq 'Set' && $txn->Field eq 'TimeWorked' ) {
+            $history += $txn->NewValue - $txn->OldValue;
+            $candidate = $txn;
+        } elsif ( $candidate && $txn->Field eq 'MergedInto' ) {
+            if ($candidate->Creator eq $txn->Creator ) {
+                push @delete, $candidate;
+                $delete_time += $candidate->NewValue - $candidate->OldValue;
+            }
+
+            $candidate = undef;
+        }
+    }
+
+    if ( $history == $t->TimeWorked ) {
+        $RT::Logger->info("Ticket #". $t->id . " has TimeWorked matching history. Skipping");
+    } elsif ( $history - $delete_time == $t->TimeWorked ) {
+        $RT::Logger->warn( "Ticket #". $t->id ." has TimeWorked mismatch. Deleting transactions" );
+        foreach my $dtxn ( @delete ) {
+            my ($status, $msg) = $dtxn->Delete;
+            $RT::Logger->error("Couldn't delete transaction: $msg") unless $status;
+        }
+    } else {
+        $RT::Logger->error( "Ticket #". $t->id ." has TimeWorked mismatch, but we couldn't find correct transactions to delete. Skipping" );
+    }
+}

commit f25a7af0001060397bf28113f8a2e04afb5af782
Author: Ruslan Zakirov <ruz at bestpractical.com>
Date:   Sat Mar 16 02:46:53 2013 +0400

    update upgrade docs with manual step

diff --git a/docs/UPGRADING-4.2 b/docs/UPGRADING-4.2
index 9257929..7d5c0cb 100644
--- a/docs/UPGRADING-4.2
+++ b/docs/UPGRADING-4.2
@@ -76,3 +76,15 @@ UPGRADING FROM RT 4.0.0 and greater
   using this in an rt-crontool cronjob or had used a
   Googleish_Local.pm to add features, you will need to convert to
   using RT::Search::Simple instead.
+
+* RT was recording additional time change transactions during merge, so
+  difference in ticket's history doesn't add up value on ticket. This has
+  been fixed, time is adjusted during merge, but now transactions are recorded.
+
+  In order to fix history records of old ticket you can run the following
+  command:
+
+    perl -I /opt/rt4/local/lib/ -I /opt/rt4/lib/ etc/upgrade/time-worked-history.pl
+
+  This command deletes records from Transactions table. This script can only fix
+  TimeWorked mismatch, but not TimeLeft or TimeEstimated.

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


More information about the Rt-commit mailing list