[Rt-commit] rt branch, 4.4/dynamic-user-time, created. rt-4.4.1-347-g458f140

Jim Brandt jbrandt at bestpractical.com
Thu May 25 13:54:24 EDT 2017


The branch, 4.4/dynamic-user-time has been created
        at  458f140ab111da5ffc4cacf968a64a2b6058e502 (commit)

- Log -----------------------------------------------------------------
commit f1d99b32a8a8e306c2abd093f4360ebd67e32713
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue May 16 15:56:04 2017 -0400

    Calculate time per user dynamically rather than from an attribute
    
    Storing time per user in an attribute saved checking each transaction
    on ticket display, however it was not possible to update values
    if data ever got out of sync for some reason. In testing, the
    performance impact of generating the time dynamically was not
    significant, even for tickets with large histories.

diff --git a/share/html/Ticket/Elements/ShowBasics b/share/html/Ticket/Elements/ShowBasics
index 0a06f02..e1834e5 100644
--- a/share/html/Ticket/Elements/ShowBasics
+++ b/share/html/Ticket/Elements/ShowBasics
@@ -126,39 +126,16 @@ $UngroupedCFs => 0
 my $time_worked;
 my $show_time_worked = $Ticket->CurrentUserCanSeeTime;
 if ( $show_time_worked && $Ticket->TimeWorked ) {
-    my $time_worked_attr = $Ticket->FirstAttribute('TimeWorked');
 
-    if ($time_worked_attr) {
-        $time_worked = $time_worked_attr->Content;
-    } else {
-        $time_worked = {};
-        my $transactions = $Ticket->Transactions;
-        $transactions->Limit(
-            FIELD     => 'Type',
-            VALUE     => 'Set',
-            SUBCLAUSE => 'timeworked',
-        );
+    my $transactions = $Ticket->Transactions;
+    $transactions->Limit(
+        FIELD           => 'TimeTaken',
+        VALUE           => 0,
+        OPERATOR        => '!=',
+    );
 
-        $transactions->Limit(
-            FIELD           => 'Field',
-            VALUE           => 'TimeWorked',
-            SUBCLAUSE       => 'timeworked',
-            ENTRYAGGREGATOR => 'AND',
-        );
-
-        $transactions->Limit(
-            FIELD           => 'TimeTaken',
-            VALUE           => 0,
-            OPERATOR        => '>',
-            SUBCLAUSE       => 'timeworked',
-            ENTRYAGGREGATOR => 'OR',
-        );
-
-        while ( my $txn = $transactions->Next ) {
-            $time_worked->{ $txn->CreatorObj->Name } += $txn->TimeTaken
-              || $txn->NewValue - $txn->OldValue;
-        }
-        $Ticket->SetAttribute( Name => 'TimeWorked', Content => $time_worked );
+    while ( my $txn = $transactions->Next ) {
+        $time_worked->{ $txn->CreatorObj->Name } += $txn->TimeTaken;
     }
 }
 

commit c3ee21e860e450391a6cafb7cd8baa0950d33529
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue May 16 16:20:37 2017 -0400

    Disable user time update scrip on upgrade
    
    This scrip is no longer needed as the ticket display page now
    calculates user time on tickets dynammically.

diff --git a/etc/upgrade/4.4.2/content b/etc/upgrade/4.4.2/content
index fbf5fb5..605d2f0 100644
--- a/etc/upgrade/4.4.2/content
+++ b/etc/upgrade/4.4.2/content
@@ -56,5 +56,17 @@ our @Initial = (
             );
         }
     },
-);
 
+    # Disable scrip On TimeWorked Change Update User TimeWorked
+    sub {
+        my $scrip = RT::Scrip->new(RT->SystemUser);
+        my ($ret, $msg) = $scrip->LoadByCols( Description => 'On TimeWorked Change Update User TimeWorked' );
+
+        unless ( $ret ){
+            RT->Logger->warning("Unable to load scrip On TimeWorked Change Update User TimeWorked: $msg. If you renamed this scrip, you can manually disable it as it is no longer needed.");
+            return;
+        }
+
+        $scrip->SetDisabled(1);
+    },
+);

commit a1d633e1aa02a4f60bedee959a170d283c6f18a7
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue May 23 14:29:05 2017 -0400

    Document upgrading details for time worked changes

diff --git a/docs/UPGRADING-4.4 b/docs/UPGRADING-4.4
index 184c250..652e57c 100644
--- a/docs/UPGRADING-4.4
+++ b/docs/UPGRADING-4.4
@@ -117,20 +117,22 @@ F<local/plugins/RT-Extension-ExternalStorage> from your RT installation.
 
 =item *
 
-RT now has the functionality from L<RT::Extension::ParentTimeWorked> built in.
-When a child ticket's TimeWorked field is updated, its parent ticket's
-TimeWorked field will also be incremented.
+RT now has the functionality from L<RT::Extension::ParentTimeWorked> built in,
+with some modifications. In the ParentTimeWorked extension, when a child ticket's
+TimeWorked field was updated, the parent ticket's TimeWorked field was also
+incremented. Starting with RT 4.4.2, the time from child tickets is available
+in the Total Time Worked field and the Time Worked value for the parent ticket
+is independent.
 
-It also has built-in functionality for recording time worked per user.
-When a ticket's TimeWorked field is updated, the time will be attributed
-to the currently logged in user.
+RT now also has built-in functionality to show time worked per user.
 
-Both of these functions are enabled via scrips. New installs will have these
-scrips by default. Upgrades will get them too, but set as disabled so they
-don't interfere with your existing configuration. If you would like to enable
-either of these functions, enable the 'On TimeWorked Change Update User
-TimeWorked' and/or 'On TimeWorked Change Update Parent TimeWorked' scrips. New
-deployments have these scrips enabled by default.
+For RT 4.4.0 and 4.4.1, these functions were enabled via scrips.
+New installs had these scrips by default. Upgrades got them too, but set as
+disabled so they didn't interfere with your existing configuration. These
+functions are now dynamic and no longer require these scrips.
+
+See L</"UPGRADING FROM 4.4.1 AND EARLIER"> for additional details on the changes
+introduced in 4.4.2.
 
 Users who are currently using
 L<RT::Extension::ParentTimeWorked|https://metacpan.org/pod/RT::Extension::ParentTimeWorked>
@@ -517,5 +519,80 @@ AfterUpdateCustomFieldValue. It will be removed in RT 4.6.
 
 =back
 
-=cut
+=head1 UPGRADING FROM 4.4.1 AND EARLIER
+
+=over 4
+
+=item *
+
+Changes to Time Worked Handling
+
+As noted above, in RT 4.4 we pulled in some time tracking features from
+extensions, specifically adding time worked on child tickets to the
+parent and showing time worked per user. In RT 4.4.2, we made some important
+changes to this functionality.
+
+The new features were installed but disabled for upgrades, so these changes
+are most important to RT users who installed RT starting with version 4.4.0
+or 4.4.1, or who users who upgraded from a previous RT and then manually
+enabled the time tracking scrips.
+
+=over 4
+
+=item *
+
+Parent Time Worked
+
+Based on feedback from users and our own experience, we determined that
+updating the Time Worked value on the parent ticket made it difficult
+to differentiate time recorded from children tickets from time recorded
+directly on the parent ticket. To clarify the history for these time
+entries, we created a new dynamic value called Total Time Worked that
+is a sum of Time Worked from the parent and all child tickets. You can
+activate this feature with the C<$DisplayTotalTimeWorked> configuration
+option in C<RT_SiteConfig.pm>.
 
+We generate this when displaying the ticket and we no longer update Time
+Worked on the parent ticket. This change also has the advantage that Total
+Time Worked automatically updates when a child ticket is added or removed.
+Total Time Worked is also available as a column for reports generated with
+the Query Builder.
+
+B<Important:>If you were previously using the parent time worked feature,
+the Time Worked value on parent tickets already contains time from child
+tickets. Total Time Worked will therefore be incorrect since it will add
+up all children then add the parent time (which already has the
+child time). You can choose to keep using the scrip and not show Total Time
+Worked, or update Time Worked on the parent ticket to have just parent time
+worked.
+
+Contact Best Practical at contact at bestpractical.com if you have questions
+about these changes.
+
+=item *
+
+Time Worked by User
+
+For the user time summary feature, we found some odd cases where a time
+value would not get recorded (if the scrip was mistakenly disabled, for
+example). In these cases, since the data was stored internally, it was
+difficult to update the user time values. We have made this feature
+dynamic so the user time information for the current ticket is generated
+when the ticket is displayed.
+
+If you previously used this feature, the user time data stored in an
+attribute on each ticket will still be in the database. Since it is only
+visible directly in the database, you can safely ignore it. However, if
+you want to remove it, you can locate these records with the following:
+
+    SELECT id, Name FROM Attributes WHERE ObjectType = 'RT::Ticket' AND Name = 'TimeWorked';
+
+You can delete these records from the Attributes table using an UPDATE
+and the same WHERE clause. Always make sure you have a known good backup
+of your database before making updates.
+
+=back
+
+=back
+
+=cut

commit 60a9d4055bceb2e62501c91ff71b04c720a632eb
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Tue May 23 14:35:26 2017 -0400

    Remove User TimeWorked scrip from install and upgrade
    
    Also remove the scrip action module since it operates on an
    attribute that was removed in f1d99b32a8.

diff --git a/etc/initialdata b/etc/initialdata
index e81f82d..b6c2708 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -129,10 +129,6 @@
       Description => 'Update Parent TimeWorked',     # loc
       ExecModule  => 'UpdateParentTimeWorked',
     },
-    { Name        => 'Update User TimeWorked',       # loc
-      Description => 'Update User TimeWorked',       # loc
-      ExecModule  => 'UpdateUserTimeWorked',
-    },
 );
 
 @ScripConditions = (
@@ -842,10 +838,6 @@ Hour:         { $SubscriptionObj->SubValue('Hour') }
        ScripCondition => 'On TimeWorked Change',
        ScripAction    => 'Update Parent TimeWorked',
        Template       => 'Blank' },
-    {  Description    => 'On TimeWorked Change Update User TimeWorked',
-       ScripCondition => 'On TimeWorked Change',
-       ScripAction    => 'Update User TimeWorked',
-       Template       => 'Blank' },
 );
 
 @ACL = (
diff --git a/etc/upgrade/4.3.9/content b/etc/upgrade/4.3.9/content
index 7db3bef..38f489c 100644
--- a/etc/upgrade/4.3.9/content
+++ b/etc/upgrade/4.3.9/content
@@ -7,11 +7,6 @@ our @ScripActions = (
         Description => 'Update Parent TimeWorked',    # loc
         ExecModule  => 'UpdateParentTimeWorked',
     },
-    {
-        Name        => 'Update User TimeWorked',    # loc
-        Description => 'Update User TimeWorked',    # loc
-        ExecModule  => 'UpdateUserTimeWorked',
-    },
 );
 
 
@@ -32,13 +27,6 @@ our @Scrips = (
         Template       => 'Blank',
         Disabled       => 1,
     },
-    {
-        Description    => 'On TimeWorked Change Update User TimeWorked',
-        ScripCondition => 'On TimeWorked Change',
-        ScripAction    => 'Update User TimeWorked',
-        Template       => 'Blank',
-        Disabled       => 1,
-    },
 );
 
 1;
diff --git a/lib/RT/Action/UpdateUserTimeWorked.pm b/lib/RT/Action/UpdateUserTimeWorked.pm
deleted file mode 100644
index 76fbd82..0000000
--- a/lib/RT/Action/UpdateUserTimeWorked.pm
+++ /dev/null
@@ -1,95 +0,0 @@
-# BEGIN BPS TAGGED BLOCK {{{
-#
-# COPYRIGHT:
-#
-# This software is Copyright (c) 1996-2017 Best Practical Solutions, LLC
-#                                          <sales at bestpractical.com>
-#
-# (Except where explicitly superseded by other copyright notices)
-#
-#
-# LICENSE:
-#
-# This work is made available to you under the terms of Version 2 of
-# the GNU General Public License. A copy of that license should have
-# been provided with this software, but in any event can be snarfed
-# from www.gnu.org.
-#
-# This work is distributed in the hope that it will be useful, but
-# WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-# General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
-# 02110-1301 or visit their web page on the internet at
-# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
-#
-#
-# CONTRIBUTION SUBMISSION POLICY:
-#
-# (The following paragraph is not intended to limit the rights granted
-# to you to modify and distribute this software under the terms of
-# the GNU General Public License and is only of importance to you if
-# you choose to contribute your changes and enhancements to the
-# community by submitting them to Best Practical Solutions, LLC.)
-#
-# By intentionally submitting any modifications, corrections or
-# derivatives to this work, or any other work intended for use with
-# Request Tracker, to Best Practical Solutions, LLC, you confirm that
-# you are the copyright holder for those contributions and you grant
-# Best Practical Solutions,  LLC a nonexclusive, worldwide, irrevocable,
-# royalty-free, perpetual, license to use, copy, create derivative
-# works based on those contributions, and sublicense and distribute
-# those contributions and any derivatives thereof.
-#
-# END BPS TAGGED BLOCK }}}
-
-use strict;
-use warnings;
-
-package RT::Action::UpdateUserTimeWorked;
-use base 'RT::Action';
-
-=head1 NAME
-
-RT::Action::UpdateUserTimeWorked - RT's scrip action to set/update the time
-worked for a user each time they log time worked on a ticket
-
-=head1 DESCRIPTION
-
-This action is used as an action for the 'On TimeWorked Change' condition.
-
-When it fires, a ticket attribute stores the amount of time the user updating
-the ticket worked on it.
-
-=cut
-
-sub Prepare {
-    return 1;
-}
-
-sub Commit {
-    my $self   = shift;
-    my $ticket = $self->TicketObj;
-    my $txn    = $self->TransactionObj;
-
-    my $time_worked_attr = $ticket->FirstAttribute('TimeWorked');
-    # if the attribute is not defined, we will initialize it in the callback,
-    # so no need to handle it here
-    if ( $time_worked_attr ) {
-        my $time_worked = $time_worked_attr->Content;
-        $time_worked->{ $txn->CreatorObj->Name } += $txn->TimeTaken
-          || $txn->NewValue - $txn->OldValue;
-        $time_worked_attr->SetContent( $time_worked );
-    }
-}
-
-=head1 AUTHOR
-
-Best Practical Solutions, LLC E<lt>modules at bestpractical.comE<gt>
-
-=cut
-
-1;

commit 4e690a67b3ed94c3c82de0769c08bb64286f8164
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu May 25 13:44:53 2017 -0400

    Note details on operation of parent time worked action

diff --git a/lib/RT/Action/UpdateParentTimeWorked.pm b/lib/RT/Action/UpdateParentTimeWorked.pm
index 29d4c8f..ddf27fb 100644
--- a/lib/RT/Action/UpdateParentTimeWorked.pm
+++ b/lib/RT/Action/UpdateParentTimeWorked.pm
@@ -61,9 +61,42 @@ worked on a parent ticket when a child ticket's TimeWorked is added to.
 
 This action is used as an action for the 'On TimeWorked Change' condition.
 
-When it fires it finds a ticket's parent tickets and increments the time on
-those tickets along with the built in behavior of incrementing the TimeWorked
-on the current ticket.
+When it fires it finds a ticket's parent tickets and increments the Time Worked
+value on those tickets along with the built-in behavior of incrementing Time
+Worked on the current ticket.
+
+=head2 Important Notes on Operation
+
+There are some important details related to the use of this scrip for time
+tracking that you should take into account when using it.
+
+=over
+
+=item * Parent and child time entries are combined on the parent
+
+This is the intended function of the scrip, but since the parent ticket has
+only one Time Worked value, it is difficult to differentiate time recorded
+directly on the parent ticket from time added from child tickets. If you record
+time only on child tickets, this is typically not an issue. If you record time
+on the parent ticket in addition to child tickets, it can make it difficult to
+pull out the time specifically logged on the parent.
+
+=item * A ticket must be linked as a child when the time is recorded
+
+For this scrip to work properly, a ticket must be linked to the parent as
+a child before time is entered. If you link a child ticket to a parent
+and it already has time recorded, that time will not be added to the parent.
+Similarly, if you remove a child ticket link from a parent, any previously
+recorded time will remain in the parent's Time Worked value. If you want the
+time removed, you must subtract it manually.
+
+=back
+
+RT has a feature called C<$TotalTimeWorked> that you can activate in your
+C<RT_SiteConfig.pm> file. This feature dynamically calculates time worked
+on child tickets and shows it on the parent. This solves the issues above
+because it doesn't modify the parent's Time Worked value and it will
+update if children tickets are added or removed.
 
 =cut
 

commit 458f140ab111da5ffc4cacf968a64a2b6058e502
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Thu May 25 13:47:03 2017 -0400

    Remove ParentTimeWorked from upgrades and new installs
    
    ParentTimeWorked functionality was cored from an extension
    in 4.4.0, but after more use several drawbacks to this
    approach to tracking time from child tickets were observed.
    4e690a67b documented these notes on the action module.
    
    Remove the scrip and action from new installs and upgrades
    in favor of the new TotalTimeWorked feature.

diff --git a/etc/initialdata b/etc/initialdata
index b6c2708..e650661 100644
--- a/etc/initialdata
+++ b/etc/initialdata
@@ -125,10 +125,6 @@
        Description => 'Set the due date according to an agreement' , # loc
        ExecModule  => 'SLA_SetDue',
     },
-    { Name        => 'Update Parent TimeWorked',     # loc
-      Description => 'Update Parent TimeWorked',     # loc
-      ExecModule  => 'UpdateParentTimeWorked',
-    },
 );
 
 @ScripConditions = (
@@ -834,10 +830,6 @@ Hour:         { $SubscriptionObj->SubValue('Hour') }
        ScripCondition    => 'Require due set according to SLA',
        ScripAction       => 'Set due date according to SLA',
        Template          => 'Blank' },
-    {  Description    => 'On TimeWorked Change Update Parent TimeWorked',
-       ScripCondition => 'On TimeWorked Change',
-       ScripAction    => 'Update Parent TimeWorked',
-       Template       => 'Blank' },
 );
 
 @ACL = (
diff --git a/etc/upgrade/4.3.9/content b/etc/upgrade/4.3.9/content
index 38f489c..bcf6d69 100644
--- a/etc/upgrade/4.3.9/content
+++ b/etc/upgrade/4.3.9/content
@@ -1,15 +1,6 @@
 use warnings;
 use strict;
 
-our @ScripActions = (
-    {
-        Name        => 'Update Parent TimeWorked',    # loc
-        Description => 'Update Parent TimeWorked',    # loc
-        ExecModule  => 'UpdateParentTimeWorked',
-    },
-);
-
-
 our @ScripConditions = (
     {
         Name                 => 'On TimeWorked Change',      # loc
@@ -19,14 +10,5 @@ our @ScripConditions = (
     },
 );
 
-our @Scrips = (
-    {
-        Description    => 'On TimeWorked Change Update Parent TimeWorked',
-        ScripCondition => 'On TimeWorked Change',
-        ScripAction    => 'Update Parent TimeWorked',
-        Template       => 'Blank',
-        Disabled       => 1,
-    },
-);
 
 1;

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


More information about the rt-commit mailing list