Bugzilla – Attachment 56920 Details for
Bug 17512
Improve handling dates in C4::Items
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17512: Improve handling dates in C4::Items
Bug-17512-Improve-handling-dates-in-C4Items.patch (text/plain), 5.69 KB, created by
Marcel de Rooy
on 2016-10-27 13:18:47 UTC
(
hide
)
Description:
Bug 17512: Improve handling dates in C4::Items
Filename:
MIME Type:
Creator:
Marcel de Rooy
Created:
2016-10-27 13:18:47 UTC
Size:
5.69 KB
patch
obsolete
>From 7c5b9f00f62bb9f07173f491bc296c553c4eeca9 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Thu, 27 Oct 2016 15:07:29 +0200 >Subject: [PATCH] Bug 17512: Improve handling dates in C4::Items >Content-Type: text/plain; charset=utf-8 > >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 >--- > 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 7c5dfe6..527ac4f 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -2085,6 +2085,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 = ?, >@@ -2348,6 +2349,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=?,"; >@@ -2365,6 +2367,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 3e886e2..00dba52 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; > >@@ -666,6 +666,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 ) = @_; >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 17512
:
56920
|
59759
|
59789