From 5c9f8b78c350ebc1c2529491fadaf8915546dfe6 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Wed, 1 Apr 2020 16:48:22 -0300 Subject: [PATCH] [OPTIONAL] Bug 25020: (QA follow-up) Add some tests This patch adds a new method Koha::Checkout->shift_due_date that accepts the same parameters we provide in the form. It catches bad scenarios (type errors, passing both parameters when only one is accepted, and so on). Date manipulation is tested so time is kept and resulting dates are correct. The controller script is cleaned a bit to use the introduced method. I do this because: - We really need tests for this and doing it with selenium is no-end - I see a use for this new method for encapsulating behaviours, for example we might want to add Calendar support for the 'days' use case, and having the method here assures we will have tests, etc. To test: 1. Apply this patches 2. Repeat the original test plan => SUCCESS: Everything works as expected 3. Run: $ kshell k$ prove t/db_dependent/Koha/Checkout.t => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- Koha/Checkout.pm | 48 +++++++++++++++++- t/db_dependent/Koha/Checkout.t | 88 +++++++++++++++++++++++++++++++++ tools/batch_extend_due_dates.pl | 55 +++++++-------------- 3 files changed, 153 insertions(+), 38 deletions(-) create mode 100644 t/db_dependent/Koha/Checkout.t diff --git a/Koha/Checkout.pm b/Koha/Checkout.pm index e256b6c051..f5870fb4da 100644 --- a/Koha/Checkout.pm +++ b/Koha/Checkout.pm @@ -22,11 +22,13 @@ use Modern::Perl; use Carp; use DateTime; +use Scalar::Util qw( looks_like_number ); use Try::Tiny; use Koha::Checkouts::ReturnClaims; use Koha::Database; -use Koha::DateUtils; +use Koha::DateUtils qw(dt_from_string); +use Koha::Exceptions; use Koha::Items; use base qw(Koha::Object); @@ -161,6 +163,50 @@ sub claim_returned { }; } +=head3 shift_due_date + + $checkout->shift_due_date({ hard_due_date => $dt }); + $checkout->shift_due_date({ days => $days_count }); + +=cut + +sub shift_due_date { + my ($self, $params) = @_; + + my $hard_due_date = $params->{hard_due_date}; + my $days = $params->{days}; + + if ( $hard_due_date and $days ) { + Koha::Exceptions::BadParameter->throw( "Called with both 'days' and 'hard_due_date'. Only one can be used." ); + } + + if ( !( $days or $hard_due_date ) ) { + Koha::Exceptions::BadParameter->throw( "At least one parameter needs to be passed" ); + } + + my $due_date = dt_from_string($self->date_due); + if ( $hard_due_date ) { + # We require a DateTime + Koha::Exceptions::WrongParameter->throw( + "'hard_due_date' must be a DateTime object" + ) unless $hard_due_date->isa('DateTime'); + + $due_date = $hard_due_date->clone->set( + hour => $due_date->hour, + minute => $due_date->minute, + second => $due_date->second + ); + } + else { + Koha::Exceptions::WrongParameter->throw( + "'days' must be an integer" + ) unless looks_like_number($days); + $due_date->add( days => $days ); + } + + return $self->date_due( $due_date ); +} + =head2 Internal methods =head3 _type diff --git a/t/db_dependent/Koha/Checkout.t b/t/db_dependent/Koha/Checkout.t new file mode 100644 index 0000000000..62f532cc95 --- /dev/null +++ b/t/db_dependent/Koha/Checkout.t @@ -0,0 +1,88 @@ +#!/usr/bin/perl + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha 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 Koha; if not, see . + +use Modern::Perl; + +use Test::More tests => 1; +use Test::Exception; + +use t::lib::TestBuilder; + +use Koha::DateUtils qw(dt_from_string output_pref); + +my $schema = Koha::Database->new->schema; +my $builder = t::lib::TestBuilder->new; + +subtest 'shift_due_date() tests' => sub { + + plan tests => 10; + + $schema->storage->txn_begin; + + my $checkout = $builder->build_object( + { + class => 'Koha::Checkouts', + value => { + date_due => '2020-04-01 13:06:16' + } + } + ); + + throws_ok + { $checkout->shift_due_date({ hard_due_date => 'cat', days => 'dog' }) } + 'Koha::Exceptions::BadParameter', + 'Passing both params makes it raise an exception'; + + is( "$@", "Called with both 'days' and 'hard_due_date'. Only one can be used.", 'Exception stringified correctly' ); + + throws_ok + { $checkout->shift_due_date({ }) } + 'Koha::Exceptions::BadParameter', + 'Passing no params makes it raise an exception'; + + is( "$@", "At least one parameter needs to be passed", 'Exception stringified correctly' ); + + throws_ok + { $checkout->shift_due_date({ hard_due_date => 'cat' }) } + 'Koha::Exceptions::WrongParameter', + 'Passing an invalid param type makes it raise an exception'; + + is( "$@", "'hard_due_date' must be a DateTime object", 'Exception stringified correctly' ); + + throws_ok + { $checkout->shift_due_date({ days => 'dog' }) } + 'Koha::Exceptions::WrongParameter', + 'Passing an invalid param type makes it raise an exception'; + + is( "$@", "'days' must be an integer", 'Exception stringified correctly' ); + + my $hard_due_date = dt_from_string( '2020-04-13' ); + $checkout->shift_due_date({ hard_due_date => $hard_due_date }); + + is( + output_pref({ dt => dt_from_string($checkout->date_due) }), + output_pref({ dt => dt_from_string('2020-04-13 13:06:16') }) + ); + + $checkout->shift_due_date({ days => 3 }); + is( + output_pref({ dt => dt_from_string($checkout->date_due) }), + output_pref({ dt => dt_from_string('2020-04-16 13:06:16') }) + ); + + $schema->storage->txn_rollback; +}; diff --git a/tools/batch_extend_due_dates.pl b/tools/batch_extend_due_dates.pl index 572e57bdc9..015bf6ee26 100755 --- a/tools/batch_extend_due_dates.pl +++ b/tools/batch_extend_due_dates.pl @@ -101,19 +101,21 @@ elsif ( $op eq 'list' ) { ); my @new_due_dates; + my @checkouts; while ( my $checkout = $checkouts->next ) { - push @new_due_dates, - output_pref({ dt => calc_new_due_date( - { - due_date => dt_from_string($checkout->date_due), - new_hard_due_date => $new_hard_due_date, - add_days => $due_date_days - } - ), dateformat => 'iso' }); + + # Update checkout's due date + if ( $new_hard_due_date ) { + $checkout->shift_due_date({ hard_due_date => $new_hard_due_date }); + } + else { + $checkout->shift_due_date({ days => $due_date_days }); + } + push @checkouts, $checkout; } $template->param( - checkouts => $checkouts, + checkouts => \@checkouts, new_hard_due_date => $new_hard_due_date ? dt_from_string($new_hard_due_date) : undef, @@ -133,19 +135,17 @@ elsif ( $op eq 'modify' ) { my $checkouts = Koha::Checkouts->search( { issue_id => { -in => \@issue_ids } } ); while ( my $checkout = $checkouts->next ) { - my $new_due_date = calc_new_due_date( - { - due_date => dt_from_string($checkout->date_due), - new_hard_due_date => $new_hard_due_date, - add_days => $due_date_days - } - ); # Update checkout's due date - $checkout->date_due($new_due_date)->store; + if ( $new_hard_due_date ) { + $checkout->shift_due_date({ hard_due_date => $new_hard_due_date })->store; + } + else { + $checkout->shift_due_date({ days => $due_date_days })->store; + } # Update items.onloan - $checkout->item->onloan($new_due_date)->store; + $checkout->item->onloan( $checkout->date_due )->store; } $template->param( @@ -154,23 +154,4 @@ elsif ( $op eq 'modify' ) { ); } -sub calc_new_due_date { - my ($params) = @_; - my $due_date = $params->{due_date}; - my $new_hard_due_date = $params->{new_hard_due_date}; - my $add_days = $params->{add_days}; - - my $new; - if ( $new_hard_due_date ) { - $new = $new_hard_due_date->clone->set( - hour => $due_date->hour, - minute => $due_date->minute, - second => $due_date->second, - ) - } else { - $new = $due_date->clone->add( days => $add_days ); - } - return $new; -} - output_html_with_http_headers $input, $cookie, $template->output; -- 2.26.0