From 137cbd8d09be549c9ae97dfad746a9c52d27b9a6 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 27 Oct 2016 15:07:29 +0200 Subject: [PATCH] Bug 17512: Improve handling dates in C4::Items This is a follow-up on the internal server error on 0000-00-00 in the items column onloan. This patch deals with preventing to have such dates at all in the date fields of items. It is accomplished by: [1] Adding a (private) subroutine _mod_item_dates. It takes an item hash and replaces date values if needed. [2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these routines a call to the new _mod_item_dates is inserted. [3] Although the routine is actually private, I have added some unit tests to Items.t. Test plan: [1] Add a new item. Fill a correct date in dateaccessioned and an invalid date in Price effective from (=replacementpricedate). [2] Verify that dateaccessioned is saved correctly and replacementpricedate is still null (does not contain 0000-00-00). [3] Edit the item again. Fill some text in dateaccessioned and put a correct date in replacementpricedate. Verify the results. [4] Run t/db_dependent/Items.t Signed-off-by: Josef Moravec Signed-off-by: Jonathan Druart Signed-off-by: Kyle M Hall --- C4/Items.pm | 30 ++++++++++++++++++++++++ t/db_dependent/Items.t | 52 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/C4/Items.pm b/C4/Items.pm index 2188ca492b..0280a18941 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -2071,6 +2071,7 @@ sub _koha_new_item { my $dbh=C4::Context->dbh; my $error; $item->{permanent_location} //= $item->{location}; + _mod_item_dates( $item ); my $query = "INSERT INTO items SET biblionumber = ?, @@ -2334,6 +2335,7 @@ sub _koha_modify_item { my $query = "UPDATE items SET "; my @bind; + _mod_item_dates( $item ); for my $key ( keys %$item ) { next if ( $key eq 'itemnumber' ); $query.="$key=?,"; @@ -2351,6 +2353,34 @@ sub _koha_modify_item { return ($item->{'itemnumber'},$error); } +sub _mod_item_dates { # date formatting for date fields in item hash + my ( $item ) = @_; + return if !$item || ref($item) ne 'HASH'; + + my @keys = grep + { $_ =~ /^onloan$|^date|date$|datetime$/ } + keys %$item; + # Incl. dateaccessioned,replacementpricedate,datelastborrowed,datelastseen + # NOTE: We do not (yet) have items fields ending with datetime + # Fields with _on$ have been handled already + + foreach my $key ( @keys ) { + next if !defined $item->{$key}; # skip undefs + my $dt = eval { dt_from_string( $item->{$key} ) }; + # eval: dt_from_string will die on us if we pass illegal dates + + my $newstr; + if( defined $dt && ref($dt) eq 'DateTime' ) { + if( $key =~ /datetime/ ) { + $newstr = DateTime::Format::MySQL->format_datetime($dt); + } else { + $newstr = DateTime::Format::MySQL->format_date($dt); + } + } + $item->{$key} = $newstr; # might be undef to clear garbage + } +} + =head2 _koha_delete_item _koha_delete_item( $itemnum ); diff --git a/t/db_dependent/Items.t b/t/db_dependent/Items.t index 9e25704535..f85d48aebe 100755 --- a/t/db_dependent/Items.t +++ b/t/db_dependent/Items.t @@ -26,7 +26,7 @@ use Koha::Library; use t::lib::Mocks; use t::lib::TestBuilder; -use Test::More tests => 10; +use Test::More tests => 11; use Test::Warn; @@ -732,6 +732,56 @@ subtest 'C4::Items::_build_default_values_for_mod_marc' => sub { $schema->storage->txn_rollback; }; +subtest '_mod_item_dates' => sub { + plan tests => 11; + + is( C4::Items::_mod_item_dates(), undef, 'Call without parameters' ); + is( C4::Items::_mod_item_dates(1), undef, 'Call without hashref' ); + + my $orgitem; + my $item = { + itemcallnumber => 'V II 149 1963', + barcode => '109304', + }; + $orgitem = { %$item }; + C4::Items::_mod_item_dates($item); + is_deeply( $item, $orgitem, 'No dates passed to _mod_item_dates' ); + + # add two correct dates + t::lib::Mocks::mock_preference('dateformat', 'us'); + $item->{dateaccessioned} = '01/31/2016'; + $item->{onloan} = $item->{dateaccessioned}; + $orgitem = { %$item }; + C4::Items::_mod_item_dates($item); + is( $item->{dateaccessioned}, '2016-01-31', 'dateaccessioned is fine' ); + is( $item->{onloan}, '2016-01-31', 'onloan is fine too' ); + + + # add some invalid dates + $item->{notexistingcolumndate} = '13/1/2015'; # wrong format + $item->{anotherdate} = 'tralala'; # even worse + $item->{myzerodate} = '0000-00-00'; # wrong too + C4::Items::_mod_item_dates($item); + is( $item->{notexistingcolumndate}, undef, 'Invalid date became NULL' ); + is( $item->{anotherdate}, undef, 'Second invalid date became NULL too' ); + is( $item->{myzerodate}, undef, '0000-00-00 became NULL too' ); + + # check if itemlost_on was not touched + $item->{itemlost_on} = '12345678'; + $item->{withdrawn_on} = '12/31/2015 23:59:00'; + $orgitem = { %$item }; + C4::Items::_mod_item_dates($item); + is_deeply( $item, $orgitem, 'Colums with _on are not touched' ); + + t::lib::Mocks::mock_preference('dateformat', 'metric'); + $item->{dateaccessioned} = '01/31/2016'; #wrong + $item->{yetanotherdatetime} = '20/01/2016 13:58:00'; #okay + C4::Items::_mod_item_dates($item); + is( $item->{dateaccessioned}, undef, 'dateaccessioned wrong format' ); + is( $item->{yetanotherdatetime}, '2016-01-20 13:58:00', + 'yetanotherdatetime is ok' ); +}; + # Helper method to set up a Biblio. sub get_biblio { my ( $frameworkcode ) = @_; -- 2.39.5