From 8cfbf828edb014f78303a1eed6df822db2cf4218 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Fri, 10 Jul 2015 15:04:55 +0100 Subject: [PATCH] Bug 8723: Update the reserves table for item-level holds If an item is moved from a biblio to another, the holds should be updated too. See discussion on the bug report for more information. Test plan: 1/ Place a item-level hold on biblio1 2/ Move the item to biblio2 3/ Confirm that the hold still exists and point to the biblio2 This patch should not change the existing behavior for bib-level holds. --- C4/Items.pm | 25 ++++++++++++++++++++----- t/db_dependent/Items/MoveItemFromBiblio.t | 27 ++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index 55bb4bd..feed7b0 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -2254,11 +2254,18 @@ Returns undef if the move failed or the biblionumber of the destination record o sub MoveItemFromBiblio { my ($itemnumber, $frombiblio, $tobiblio) = @_; my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("SELECT biblioitemnumber FROM biblioitems WHERE biblionumber = ?"); - $sth->execute( $tobiblio ); - my ( $tobiblioitem ) = $sth->fetchrow(); - $sth = $dbh->prepare("UPDATE items SET biblioitemnumber = ?, biblionumber = ? WHERE itemnumber = ? AND biblionumber = ?"); - my $return = $sth->execute($tobiblioitem, $tobiblio, $itemnumber, $frombiblio); + my ( $tobiblioitem ) = $dbh->selectrow_array(q| + SELECT biblioitemnumber + FROM biblioitems + WHERE biblionumber = ? + |, undef, $tobiblio ); + my $return = $dbh->do(q| + UPDATE items + SET biblioitemnumber = ?, + biblionumber = ? + WHERE itemnumber = ? + AND biblionumber = ? + |, undef, $tobiblioitem, $tobiblio, $itemnumber, $frombiblio ); if ($return == 1) { ModZebra( $tobiblio, "specialUpdate", "biblioserver" ); ModZebra( $frombiblio, "specialUpdate", "biblioserver" ); @@ -2270,6 +2277,14 @@ sub MoveItemFromBiblio { $order->{'biblionumber'} = $tobiblio; C4::Acquisition::ModOrder($order); } + + # Update holds + $dbh->do( q| + UPDATE reserves + SET biblionumber = ? + WHERE itemnumber = ? + |, undef, $tobiblio, $itemnumber ); + return $tobiblio; } return; diff --git a/t/db_dependent/Items/MoveItemFromBiblio.t b/t/db_dependent/Items/MoveItemFromBiblio.t index d0b10d7..1ded6f2 100644 --- a/t/db_dependent/Items/MoveItemFromBiblio.t +++ b/t/db_dependent/Items/MoveItemFromBiblio.t @@ -16,9 +16,10 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 8; use C4::Items; +use C4::Reserves; use t::lib::TestBuilder; @@ -43,6 +44,22 @@ my $item3 = $builder->build( } ); +my $bib_level_hold_not_to_move = $builder->build( + { source => 'Reserve', + value => { biblionumber => $from_biblio->{biblionumber}, }, + } +); +my $item_level_hold_not_to_move = $builder->build( + { source => 'Reserve', + value => { biblionumber => $from_biblio->{biblionumber}, itemnumber => $item1->{itemnumber} }, + } +); +my $item_level_hold_to_move = $builder->build( + { source => 'Reserve', + value => { biblionumber => $from_biblio->{biblionumber}, itemnumber => $item2->{itemnumber} }, + } +); + my $to_biblionumber_after_moved = C4::Items::MoveItemFromBiblio( $item2->{itemnumber}, $from_biblio->{biblionumber}, $to_biblio->{biblionumber} ); is( $to_biblionumber_after_moved, $to_biblio->{biblionumber}, 'MoveItemFromBiblio should return the to_biblionumber if success' ); @@ -57,3 +74,11 @@ my $get_item2 = C4::Items::GetItem( $item2->{itemnumber} ); is( $get_item2->{biblionumber}, $to_biblio->{biblionumber}, 'The item2 should have been moved' ); my $get_item3 = C4::Items::GetItem( $item3->{itemnumber} ); is( $get_item3->{biblionumber}, $to_biblio->{biblionumber}, 'The item3 should not have been moved' ); + +my $get_bib_level_hold = C4::Reserves::GetReserve( $bib_level_hold_not_to_move->{reserve_id} ); +my $get_item_level_hold_1 = C4::Reserves::GetReserve( $item_level_hold_not_to_move->{reserve_id} ); +my $get_item_level_hold_2 = C4::Reserves::GetReserve( $item_level_hold_to_move->{reserve_id} ); + +is( $get_bib_level_hold->{biblionumber}, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the biblio-level hold' ); +is( $get_item_level_hold_1->{biblionumber}, $from_biblio->{biblionumber}, 'MoveItemFromBiblio should not have moved the item-level hold placed on item 1' ); +is( $get_item_level_hold_2->{biblionumber}, $to_biblio->{biblionumber}, 'MoveItemFromBiblio should have moved the item-level hold placed on item 2' ); -- 2.1.0