[Rt-commit] rt branch, 4.0/preserve-ticket-basics, created. rt-4.0.22-20-g2879feb

? sunnavy sunnavy at bestpractical.com
Mon Feb 9 06:14:23 EST 2015


The branch, 4.0/preserve-ticket-basics has been created
        at  2879febc29b92c8bbb3ce2ca40696901a60ab59d (commit)

- Log -----------------------------------------------------------------
commit 2e8a49c1bf24ef0564f34f27f99a4b8647111c4e
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Mon Oct 27 20:56:40 2014 +0800

    respect InUnits arg so time input won't be converted automatically
    
    when user inputs "3 hours" worked time on ticket update page, and then clicks
    "Add More Files": old behavior is to show user "180 minutes", which is kinda
    surprising and not necessary at all.
    
    Fixes: #17985

diff --git a/share/html/Elements/EditTimeValue b/share/html/Elements/EditTimeValue
index f1126ec..141f221 100644
--- a/share/html/Elements/EditTimeValue
+++ b/share/html/Elements/EditTimeValue
@@ -46,20 +46,22 @@
 %#
 %# END BPS TAGGED BLOCK }}}
 <input name="<% $ValueName %>" value="<% $Default || '' %>" size="5" />
-<& /Elements/SelectTimeUnits, Name => $UnitName &>
+<& /Elements/SelectTimeUnits, Name => $UnitName, Default => $InUnits &>
 <%ARGS>
 $Default    => ''
 $Name       => ''
 $ValueName  => ''
 $UnitName   => ''
-$InputUnits => 'minutes'
+$InUnits => ''
 </%ARGS>
 <%INIT>
 $ValueName ||= $Name;
 $UnitName  ||= ($Name||$ValueName) . '-TimeUnits';
+$InUnits ||= $m->request_args->{ $UnitName };
+$InUnits ||= RT->Config->Get('DefaultTimeUnitsToHours', $session{'CurrentUser'}) ? 'hours' : 'minutes';
 
-if ($InputUnits eq 'minutes' && RT->Config->Get('DefaultTimeUnitsToHours', $session{'CurrentUser'})) {
-    $Default = sprintf '%.3f', $Default / 60
-        unless $Default eq '';
+if ($Default && $InUnits eq 'hours') {
+    $Default = sprintf '%.3f', $Default / 60;
+    $Default =~ s!\.000$!!;
 }
 </%INIT>
diff --git a/share/html/Elements/SelectTimeUnits b/share/html/Elements/SelectTimeUnits
index e548af4..8669ea6 100644
--- a/share/html/Elements/SelectTimeUnits
+++ b/share/html/Elements/SelectTimeUnits
@@ -46,17 +46,17 @@
 %#
 %# END BPS TAGGED BLOCK }}}
 <select class="TimeUnits" id="<% $Name %>" name="<% $Name %>">
-<option value="minutes" <% $HoursDefault ? '' : 'selected="selected"' |n%>>
+<option value="minutes" <% $Default eq 'minutes' ? 'selected="selected"' : '' |n%>>
     <% loc('Minutes') %>
 </option>
-<option value="hours"   <% $HoursDefault ? 'selected="selected"' : '' |n%>>
+<option value="hours"   <% $Default eq 'hours' ? 'selected="selected"' : '' |n%>>
     <% loc('Hours') %>
 </option>
 </select>
 <%INIT>
 $Name .= '-TimeUnits' unless $Name =~ /-TimeUnits$/io;
-my $HoursDefault = RT->Config->Get('DefaultTimeUnitsToHours', $session{'CurrentUser'});
 </%INIT>
 <%ARGS>
 $Name => ''
+$Default => RT->Config->Get('DefaultTimeUnitsToHours', $session{'CurrentUser'}) ? 'hours' : 'minutes'
 </%ARGS>
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index 69f3036..4484a41 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -215,17 +215,17 @@
 &></td></tr>
 <tr><td class="label"><&|/l&>Time Estimated</&>:</td>
 <td>
-<& /Elements/EditTimeValue, Name => 'TimeEstimated', Default => $ARGS{TimeEstimated} || '', InUnits => $ARGS{'TimeEstimated-TimeUnits'} &>
+<& /Elements/EditTimeValue, Name => 'TimeEstimated', Default => $ARGS{TimeEstimated} || '' &>
 
 </td></tr>
 <tr><td class="label"><&|/l&>Time Worked</&>:</td>
 <td>
-<& /Elements/EditTimeValue, Name => 'TimeWorked', Default => $ARGS{TimeWorked} || '', InUnits => $ARGS{'TimeWorked-TimeUnits'} &>
+<& /Elements/EditTimeValue, Name => 'TimeWorked', Default => $ARGS{TimeWorked} || '' &>
 </td></tr>
 <tr>
 <td class="label"><&|/l&>Time Left</&>:</td>
 <td>
-<& /Elements/EditTimeValue, Name => 'TimeLeft', Default => $ARGS{TimeLeft} || '', InUnits => $ARGS{'TimeLeft-TimeUnits'} &>
+<& /Elements/EditTimeValue, Name => 'TimeLeft', Default => $ARGS{TimeLeft} || '' &>
 </td></tr>
 </table>
 </&>
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index 9501b5c..9188296 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -128,7 +128,6 @@
             args => {
                 Name => 'UpdateTimeWorked',
                 Default => $ARGS{UpdateTimeWorked}||'',
-                InUnits => $ARGS{'UpdateTimeWorked-TimeUnits'}||'minutes',
             }
         },
     ]
diff --git a/share/html/m/ticket/create b/share/html/m/ticket/create
index 7fbfc7a..b9a544d 100644
--- a/share/html/m/ticket/create
+++ b/share/html/m/ticket/create
@@ -353,21 +353,18 @@ $showrows->(
         "/Elements/EditTimeValue",
         Name    => 'TimeEstimated',
         Default => $ARGS{TimeEstimated} || '',
-        InUnits => $ARGS{'TimeEstimated-TimeUnits'}
         ).'</span>',
 
     loc("Time Worked") => '<span class="timefield">'.$m->scomp(
         "/Elements/EditTimeValue",
         Name    => 'TimeWorked',
         Default => $ARGS{TimeWorked} || '',
-        InUnits => $ARGS{'TimeWorked-TimeUnits'}
     ). '</span>',
 
     loc("Time Left") => '<span class="timefield">'.$m->scomp(
         "/Elements/EditTimeValue",
         Name    => 'TimeLeft',
         Default => $ARGS{TimeLeft} || '',
-        InUnits => $ARGS{'TimeLeft-TimeUnits'}
     ).'</span>',
 );
 
diff --git a/share/html/m/ticket/reply b/share/html/m/ticket/reply
index 4db5401..b2b5d65 100644
--- a/share/html/m/ticket/reply
+++ b/share/html/m/ticket/reply
@@ -78,7 +78,6 @@
 <& /Elements/EditTimeValue,
     Name => 'UpdateTimeWorked',
     Default => $ARGS{UpdateTimeWorked}||'',
-    InUnits => $ARGS{'UpdateTimeWorked-TimeUnits'}||'minutes',
 &>
 </span></div>
 % $m->callback( %ARGS, CallbackName => 'AfterWorked', Ticket => $t );

commit a045e1232d368165ab0c6e928036c928a1cf9173
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Mon Feb 9 18:19:26 2015 +0800

    preserve ticket basics in Jumbo
    
    so when we are clicking "Add More Files" or "Go"es to search people or groups,
    the changes we made could be preserved.
    
    note that I moved the handling "2 Owners" code out of the condition block so
    it can be handled always. without this, the owner input can't be successfully
    preserved.  (i.e. you could get an empty default value no matter which owner
    you chose)

diff --git a/share/html/Ticket/ModifyAll.html b/share/html/Ticket/ModifyAll.html
index e890c80..67a25d5 100644
--- a/share/html/Ticket/ModifyAll.html
+++ b/share/html/Ticket/ModifyAll.html
@@ -56,7 +56,7 @@
 <input type="hidden" class="hidden" name="id" value="<%$Ticket->Id%>" />
 
 <&| /Widgets/TitleBox, title => loc('Modify ticket # [_1]', $Ticket->Id), class=>'ticket-info-basics' &>
-<& Elements/EditBasics, TicketObj => $Ticket &>
+<& Elements/EditBasics, TicketObj => $Ticket, defaults => \%ARGS &>
 <& Elements/EditCustomFields, TicketObj => $Ticket &>
 </&>
 
@@ -157,21 +157,22 @@ ProcessAttachments(ARGSRef => \%ARGS);
 $m->callback( TicketObj => $Ticket, ARGSRef => \%ARGS );
 my @results;
 
-unless ($OnlySearchForPeople or $OnlySearchForGroup or $ARGS{'AddMoreAttach'} ) {
-    # There might be two owners. 
-    if ( ref ($ARGS{'Owner'} )) {
-        my @owners =@{$ARGS{'Owner'}};
-        delete $ARGS{'Owner'};
-        foreach my $owner(@owners){
-            if (defined($owner) && $owner =~ /\D/) {
-                $ARGS{'Owner'} = $owner unless ($Ticket->OwnerObj->Name eq $owner);
-            }
-            elsif (length $owner) {
-                $ARGS{'Owner'} = $owner unless ($Ticket->OwnerObj->id == $owner);
-            }
+# There might be two owners. 
+if ( ref ($ARGS{'Owner'} )) {
+    my @owners =@{$ARGS{'Owner'}};
+    delete $ARGS{'Owner'};
+    foreach my $owner(@owners){
+        if (defined($owner) && $owner =~ /\D/) {
+            $ARGS{'Owner'} = $owner unless ($Ticket->OwnerObj->Name eq $owner);
+        }
+        elsif (length $owner) {
+            $ARGS{'Owner'} = $owner unless ($Ticket->OwnerObj->id == $owner);
         }
 
     }
+}
+
+unless ($OnlySearchForPeople or $OnlySearchForGroup or $ARGS{'AddMoreAttach'} ) {
 
     push @results, ProcessTicketWatchers( TicketObj => $Ticket, ARGSRef => \%ARGS);
     push @results, ProcessObjectCustomFieldUpdates( Object => $Ticket, ARGSRef => \%ARGS);

commit 2879febc29b92c8bbb3ce2ca40696901a60ab59d
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Mon Oct 27 21:34:50 2014 +0800

    test preserved tickets basic fields

diff --git a/t/web/ticket_preserve_basics.t b/t/web/ticket_preserve_basics.t
new file mode 100644
index 0000000..1459414
--- /dev/null
+++ b/t/web/ticket_preserve_basics.t
@@ -0,0 +1,110 @@
+use strict;
+use warnings;
+
+use RT::Test tests => undef;
+
+my $ticket = RT::Test->create_ticket(
+    Subject => 'test ticket basics',
+    Queue   => 1,
+);
+
+my ( $url, $m ) = RT::Test->started_ok;
+ok( $m->login, 'logged in' );
+
+my $root = RT::Test->load_or_create_user( Name => 'root' );
+
+# Failing test where the time units are not preserved when you
+# click 'Add more files' on Display
+my @form_tries = (
+    {Subject => "hello rt"},
+    {Status  => "open"},
+    {Owner   => $root->id},
+
+    (
+        map +{
+            "Time$_"           => undef,
+            "Time$_-TimeUnits" => 'hours',
+        }, qw/Estimated Worked Left/
+    ),
+    (
+        map +{
+            "Time$_"           => '1',
+            "Time$_-TimeUnits" => 'hours',
+        }, qw/Estimated Worked Left/
+    ),
+
+    {InitialPriority      => "10"},
+    {FinalPriority => "10"},
+);
+
+for my $try (@form_tries) {
+    $m->goto_create_ticket(1);
+    $m->form_name('TicketCreate');
+    $m->set_fields(%$try);
+    $m->click('AddMoreAttach');
+    $m->form_name('TicketCreate');
+    for my $field (keys %$try) {
+        is(
+            $m->value($field),
+            defined($try->{$field}) ? $try->{$field} : '',
+            "field $field is the same after the form was submitted"
+        );
+    }
+}
+
+# Test for time unit preservation in Jumbo
+for my $try (@form_tries) {
+    my $jumbo_ticket = RT::Test->create_ticket(
+        Subject => 'test jumbo ticket basics',
+        Queue   => 1,
+    );
+
+    local($try->{Priority}) = delete local($try->{InitialPriority})
+        if exists $try->{InitialPriority};
+
+    $m->get( $url . "/Ticket/ModifyAll.html?id=" . $jumbo_ticket->id );
+    $m->form_name('TicketModifyAll');
+    $m->set_fields(%$try);
+    $m->click('AddMoreAttach');
+    $m->form_name('TicketModifyAll');
+    for my $field (keys %$try) {
+        is(
+            $m->value($field),
+            defined($try->{$field}) ? $try->{$field} : '',
+            "field $field is the same after the Jumbo form was submitted"
+        );
+    }
+}
+
+my $cf = RT::Test->load_or_create_custom_field(
+    Name       => 'CF1',
+    Type       => 'Freeform',
+    Pattern    => '.', # mandatory
+    Queue      => 'General',
+);
+
+# More time unit testing by a failing CF validation
+$m->get_ok($url.'/Admin/CustomFields/Objects.html?id='.$cf->id);
+$m->form_with_fields('UpdateObjs');
+$m->tick('AddCustomField-'.$cf->id => '0'); # Make CF global
+$m->click('UpdateObjs');
+$m->text_contains('Object created', 'CF applied globally');
+
+# Test for preservation when a ticket is submitted and CF validation fails
+for my $try (@form_tries) {
+    $m->goto_create_ticket(1);
+    $m->form_name('TicketCreate');
+    $m->set_fields(%$try);
+    $m->submit();
+    $m->form_name('TicketCreate');
+    for my $field (keys %$try) {
+        is(
+            $m->value($field),
+            defined($try->{$field}) ? $try->{$field} : '',
+            "field $field is the same after the form was submitted"
+        );
+    }
+}
+
+undef $m;
+done_testing();

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


More information about the rt-commit mailing list