[Rt-commit] rt branch, 4.4/validate-time-input, created. rt-4.4.3-59-g820df69ba
? sunnavy
sunnavy at bestpractical.com
Wed Oct 10 08:27:11 EDT 2018
The branch, 4.4/validate-time-input has been created
at 820df69baa211e0733e3865a579dedf6f6d31037 (commit)
- Log -----------------------------------------------------------------
commit 86055935f3ac8e951317f6afd041eca4404a1a44
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed May 30 20:53:34 2018 +0800
Normalize and validate time inputs
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index e8a4545de..d35bc8f9f 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -73,6 +73,7 @@ use List::MoreUtils qw();
use JSON qw();
use Plack::Util;
use HTTP::Status qw();
+use Regexp::Common;
=head2 SquishedCSS $style
@@ -1242,18 +1243,35 @@ sub DecodeARGS {
sub PreprocessTimeUpdates {
my $ARGS = shift;
- # This code canonicalizes time inputs in hours into minutes
+ my @msg;
+
+ # This code validates and canonicalizes time inputs(including hours into minutes)
foreach my $field ( keys %$ARGS ) {
next unless $field =~ /^(.*)-TimeUnits$/i && $ARGS->{$1};
my $local = $1;
$ARGS->{$local} =~ s{\b (?: (\d+) \s+ )? (\d+)/(\d+) \b}
{($1 || 0) + $3 ? $2 / $3 : 0}xe;
+
+ $ARGS->{$local} =~ s!^\s+!!;
+ $ARGS->{$local} =~ s!\s+$!!;
+ $ARGS->{$local} =~ s!,!!g;
+
+ if ( $ARGS->{$local} && $ARGS->{$local} !~ /^$RE{num}{real}$/ ) {
+ push @msg, HTML::Mason::Commands::loc( 'Invalid [_1]: it should be a number', HTML::Mason::Commands::loc( $local ) );
+ next;
+ }
if ( $ARGS->{$field} && $ARGS->{$field} =~ /hours/i ) {
$ARGS->{$local} *= 60;
}
+
+ # keep decimal part as the column in db is int
+ $ARGS->{$local} = sprintf( '%.0f', $ARGS->{$local} );
+
delete $ARGS->{$field};
}
+ return 1 unless @msg;
+ return wantarray ? ( 0, @msg ) : 0;
}
sub MaybeEnableSQLStatementLog {
@@ -4530,6 +4548,10 @@ sub GetCustomFieldInputNamePrefix {
RT::Interface::Web::GetCustomFieldInputNamePrefix(@_);
}
+sub PreprocessTimeUpdates {
+ RT::Interface::Web::PreprocessTimeUpdates(@_);
+}
+
package RT::Interface::Web;
RT::Base->_ImportOverlays();
diff --git a/share/html/SelfService/Create.html b/share/html/SelfService/Create.html
index 6bf192dc1..086435401 100644
--- a/share/html/SelfService/Create.html
+++ b/share/html/SelfService/Create.html
@@ -137,6 +137,12 @@ my $skip_create = 0;
push @results, @msg;
$skip_create = 1;
}
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $skip_create = 1;
+ }
}
$m->callback( CallbackName => 'BeforeCreate', ARGSRef => \%ARGS, skip_create => \$skip_create, results => \@results );
diff --git a/share/html/Ticket/Create.html b/share/html/Ticket/Create.html
index c8b73f4e1..9ca9f61f4 100644
--- a/share/html/Ticket/Create.html
+++ b/share/html/Ticket/Create.html
@@ -464,6 +464,12 @@ my $checks_failure = 0;
$checks_failure = 1;
push @results, @msg;
}
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $checks_failure = 1;
+ }
}
my $gnupg_widget = $m->comp('/Elements/Crypt/SignEncryptWidget:new', Arguments => \%ARGS );
diff --git a/share/html/Ticket/Modify.html b/share/html/Ticket/Modify.html
index d8eecbb6f..6c6bbab67 100644
--- a/share/html/Ticket/Modify.html
+++ b/share/html/Ticket/Modify.html
@@ -91,6 +91,12 @@ $m->callback( TicketObj => $TicketObj, CustomFields => $CustomFields, ARGSRef =>
push @results, @msg;
$skip_update = 1;
}
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $skip_update = 1;
+ }
}
unless ($skip_update) {
diff --git a/share/html/Ticket/ModifyAll.html b/share/html/Ticket/ModifyAll.html
index a4b504a0d..0d4784c1f 100644
--- a/share/html/Ticket/ModifyAll.html
+++ b/share/html/Ticket/ModifyAll.html
@@ -174,6 +174,12 @@ $m->callback( TicketObj => $Ticket, ARGSRef => \%ARGS, skip_update => \$skip_upd
push @results, @msg;
$skip_update = 1;
}
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $skip_update = 1;
+ }
}
# There might be two owners.
diff --git a/share/html/Ticket/Update.html b/share/html/Ticket/Update.html
index 6e73b50a8..cf7a4e5d7 100644
--- a/share/html/Ticket/Update.html
+++ b/share/html/Ticket/Update.html
@@ -331,6 +331,12 @@ if ( $ARGS{'SubmitTicket'} ) {
TicketObj => $TicketObj,
);
$checks_failure = 1 unless $status;
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $checks_failure = 1;
+ }
}
# check email addresses for RT's
diff --git a/share/html/m/ticket/create b/share/html/m/ticket/create
index 5fbfecff3..09ebb41ff 100644
--- a/share/html/m/ticket/create
+++ b/share/html/m/ticket/create
@@ -160,6 +160,12 @@ my $checks_failure = 0;
$checks_failure = 1;
push @results, @msg;
}
+
+ ( $status, @msg ) = PreprocessTimeUpdates( \%ARGS );
+ unless ( $status ) {
+ push @results, @msg;
+ $checks_failure = 1;
+ }
}
my $gnupg_widget = $m->comp('/Elements/Crypt/SignEncryptWidget:new', Arguments => \%ARGS );
commit 820df69baa211e0733e3865a579dedf6f6d31037
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed May 30 21:05:04 2018 +0800
Test time values on ticket create/update/modify pages
diff --git a/t/web/ticket_time.t b/t/web/ticket_time.t
new file mode 100644
index 000000000..52ac777ab
--- /dev/null
+++ b/t/web/ticket_time.t
@@ -0,0 +1,147 @@
+use strict;
+use warnings;
+use RT::Test tests => undef;
+
+my ( $baseurl, $m ) = RT::Test->started_ok;
+ok( $m->login, "Logged in" );
+
+my $queue = RT::Test->load_or_create_queue( Name => 'General' );
+ok( $queue->id, "loaded the General queue" );
+
+my %valid = (
+ '0' => 0,
+ '-8' => -8,
+ '2' => 2,
+ '5.' => 5,
+ '5.5' => 6,
+ ' 15 ' => 15,
+ '1,000' => 1000,
+ '.5h' => 30,
+ '1.5h' => 90,
+ '2h' => 120,
+);
+my @invalid = ( 'a', '3;4', '3+4' );
+
+$m->goto_create_ticket( $queue );
+for my $time ( @invalid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketCreate',
+ fields => { TimeEstimated => $number, $hour ? ( 'TimeEstimated-TimeUnits' => 'hours' ) : (), },
+ },
+ "Submit time $time",
+ );
+ $m->text_contains( 'Invalid TimeEstimated: it should be a number' );
+ $m->text_unlike( qr/Ticket \d+ created in queue/ );
+}
+
+for my $time ( sort keys %valid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->goto_create_ticket( $queue );
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketCreate',
+ fields => { TimeEstimated => $number, $hour ? ( 'TimeEstimated-TimeUnits' => 'hours' ) : (), },
+ },
+ "Submit time $time",
+ );
+ $m->text_lacks( 'Invalid TimeEstimated: it should be a number' );
+ $m->text_like( qr/Ticket \d+ created in queue/ );
+ my $ticket = RT::Test->last_ticket;
+ is( $ticket->TimeEstimated, $valid{$time}, 'TimeEstimated is set' );
+}
+
+my $ticket = RT::Test->last_ticket;
+for my $page ( qw/Modify ModifyAll/ ) {
+ $m->goto_ticket( $ticket->id, $page );
+
+ for my $time ( @invalid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->submit_form_ok(
+ {
+ form_name => "Ticket$page",
+ fields => { map { $_ => $number, $hour ? ( "$_-TimeUnits" => 'hours' ) : () } qw/TimeLeft TimeWorked/ },
+ },
+ "Submit time $time",
+ );
+
+ $ticket->Load( $ticket->id );
+ for my $field ( qw/TimeLeft TimeWorked/ ) {
+ $m->text_contains( "Invalid $field: it should be a number" );
+ ok( !$ticket->$field, "$field is not updated" );
+ }
+ }
+
+ for my $time ( sort keys %valid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->submit_form_ok(
+ {
+ form_name => "Ticket$page",
+ fields => { map { $_ => $number, $hour ? ( "$_-TimeUnits" => 'hours' ) : () } qw/TimeLeft TimeWorked/ },
+ },
+ "Submit time $time",
+ );
+ $ticket->Load( $ticket->id );
+
+ for my $field ( qw/TimeLeft TimeWorked/ ) {
+ $m->text_lacks( "Invalid $field: it should be a number" );
+ if ( $field eq 'TimeLeft' ) {
+ $m->text_like( qr/$field changed/ );
+ }
+ else {
+ $m->text_like( qr/worked -?[\d.]+ (?:minute|hour)|adjusted time worked/i );
+ }
+ is( $ticket->$field, $valid{$time}, "$field is updated" );
+ }
+ }
+
+ for my $field ( qw/TimeLeft TimeWorked/ ) {
+ my $set_method = "Set$field";
+ my ( $ret, $msg ) = $ticket->$set_method( 0 );
+ ok( $ret, 'Reset $field to 0' );
+ }
+}
+
+$m->goto_ticket( $ticket->id, 'Update' );
+
+for my $time ( @invalid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketUpdate',
+ fields => { UpdateTimeWorked => $number, $hour ? ( 'UpdateTimeWorked-TimeUnits' => 'hours' ) : (), },
+ button => 'SubmitTicket',
+ },
+ "Submit time $time",
+ );
+ $m->text_contains( 'Invalid UpdateTimeWorked: it should be a number' );
+ $ticket->Load( $ticket->id );
+ ok( !$ticket->TimeWorked, 'TimeWorked is not updated' );
+}
+
+my $time_worked = $ticket->TimeWorked;
+for my $time ( sort keys %valid ) {
+ my ( $number, $hour ) = $time =~ /^(.+?)(h?)$/;
+
+ $m->goto_ticket( $ticket->id, 'Update' );
+ $m->submit_form_ok(
+ {
+ form_name => 'TicketUpdate',
+ fields => { UpdateTimeWorked => $number, $hour ? ( 'UpdateTimeWorked-TimeUnits' => 'hours' ) : (), },
+ button => 'SubmitTicket',
+ },
+ "Submit time $time",
+ );
+ $m->text_lacks( 'Invalid UpdateTimeWorked: it should be a number' );
+ $ticket->Load( $ticket->id );
+ is( $ticket->TimeWorked, $time_worked + $valid{$time}, 'TimeWorked is updated' );
+ $time_worked = $ticket->TimeWorked;
+}
+
+done_testing;
-----------------------------------------------------------------------
More information about the rt-commit
mailing list