From 5ffc0d86713dcf1eb0118b02bf079832830cd783 Mon Sep 17 00:00:00 2001 From: Barton Chittenden Date: Thu, 22 Oct 2015 12:24:33 -0700 Subject: [PATCH] bug 14504: split logic from DelItemCheck() into ItemSafeToDelete() Signed-off-by: Tomas Cohen Arazi --- C4/Items.pm | 97 +++++++++++++++++------- t/db_dependent/Items_DelItemCheck.t | 145 ++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 28 deletions(-) create mode 100644 t/db_dependent/Items_DelItemCheck.t diff --git a/C4/Items.pm b/C4/Items.pm index 81a8263..6b518c6 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -80,6 +80,7 @@ BEGIN { GetItemnumberFromBarcode GetBarcodeFromItemnumber GetHiddenItemnumbers + ItemSafeToDelete DelItemCheck MoveItemFromBiblio GetLatestAcquisitions @@ -2217,68 +2218,108 @@ sub MoveItemFromBiblio { return; } -=head2 DelItemCheck +=head2 ItemSafeToDelete - DelItemCheck($dbh, $biblionumber, $itemnumber); + ItemSafeToDelete($dbh, $biblionumber, $itemnumber); -Exported function (core API) for deleting an item record in Koha if there no current issue. +Exported function (core API) for checking whether an item record is safe to delete. + +returns 1 if the item is safe to delete, + +"book_on_loan" if the item is checked out, + +"not_same_branch" if the item is blocked by independent branches, + +"book_reserved" if the there are holds aganst the item, or + +"linked_analytics" if the item has linked analytic records. =cut -sub DelItemCheck { +sub ItemSafeToDelete { my ( $dbh, $biblionumber, $itemnumber ) = @_; - + my $status; $dbh ||= C4::Context->dbh; my $error; - my $countanalytics=GetAnalyticsCount($itemnumber); - + my $countanalytics = GetAnalyticsCount($itemnumber); # check that there is no issue on this item before deletion. - my $sth = $dbh->prepare(q{ + my $sth = $dbh->prepare( + q{ SELECT COUNT(*) FROM issues WHERE itemnumber = ? - }); + } + ); $sth->execute($itemnumber); my ($onloan) = $sth->fetchrow; my $item = GetItem($itemnumber); - if ($onloan){ - $error = "book_on_loan" + if ($onloan) { + $status = "book_on_loan"; } elsif ( defined C4::Context->userenv and !C4::Context->IsSuperLibrarian() and C4::Context->preference("IndependentBranches") and ( C4::Context->userenv->{branch} ne $item->{'homebranch'} ) ) { - $error = "not_same_branch"; + $status = "not_same_branch"; } - else{ + else { # check it doesn't have a waiting reserve - $sth = $dbh->prepare(q{ + $sth = $dbh->prepare( + q{ SELECT COUNT(*) FROM reserves WHERE (found = 'W' OR found = 'T') AND itemnumber = ? - }); + } + ); $sth->execute($itemnumber); my ($reserve) = $sth->fetchrow; - if ($reserve){ - $error = "book_reserved"; - } elsif ($countanalytics > 0){ - $error = "linked_analytics"; - } else { - DelItem( - { - biblionumber => $biblionumber, - itemnumber => $itemnumber - } - ); - return 1; + if ($reserve) { + $status = "book_reserved"; } + elsif ( $countanalytics > 0 ) { + $status = "linked_analytics"; + } + else { + $status = 1; + } + } + return $status; +} + +=head2 DelItemCheck + + DelItemCheck($dbh, $biblionumber, $itemnumber); + +Exported function (core API) for deleting an item record in Koha if there no current issue. + +DelItemCheck wraps ItemSafeToDelete around DelItem. + +It takes a database handle, biblionumber and itemnumber as arguments, and can optionally take a hashref with a 'do_not_commit' flag: + + DelItemCheck( $dbh, $biblionumber, $itemnumber, { do_not_commit => 1 } ); + +This is done so that command line scripts calling DelItemCheck have the option of doing a 'dry-run'. +=cut + +sub DelItemCheck { + my ( $dbh, $biblionumber, $itemnumber, $options ) = @_; + my $commit = ( defined $options && $options->{do_not_commit} eq 1 ) ? 0 : 1; + my $status = ItemSafeToDelete( $dbh, $biblionumber, $itemnumber ); + + if ( $status == 1 && $commit ) { + DelItem( + { + biblionumber => $biblionumber, + itemnumber => $itemnumber + } + ); } - return $error; + return $status; } =head2 _koha_modify_item diff --git a/t/db_dependent/Items_DelItemCheck.t b/t/db_dependent/Items_DelItemCheck.t new file mode 100644 index 0000000..98e267a --- /dev/null +++ b/t/db_dependent/Items_DelItemCheck.t @@ -0,0 +1,145 @@ +use Modern::Perl; + +use MARC::Record; +use C4::Biblio; +use C4::Circulation; +use C4::Members; +use t::lib::Mocks; + + +use Test::More tests => 10; + +*C4::Context::userenv = \&Mock_userenv; + +BEGIN { + use_ok('C4::Items'); +} + +my $dbh = C4::Context->dbh; +$dbh->{AutoCommit} = 0; +$dbh->{RaiseError} = 1; + +my ( $biblionumber, $bibitemnum ) = get_biblio(); + +# book_on_loan + +my ( $borrowernumber, $borrower ) = get_borrower(); +my ( $itemnumber, $item ) = get_item( $biblionumber ); +AddIssue( $borrower, 'i_dont_exist' ); + +is( + ItemSafeToDelete($dbh, $biblionumber, $itemnumber), + 'book_on_loan', + 'ItemSafeToDelete reports item on loan', +); + +is( + DelItemCheck($dbh, $biblionumber, $itemnumber), + 'book_on_loan', + 'item that is on loan cannot be deleted', +); + +AddReturn('i_dont_exist', 'CPL'); + +# book_reserved is tested in t/db_dependent/Reserves.t + +# not_same_branch +t::lib::Mocks::mock_preference('IndependentBranches', 1); +ModItem( { homebranch => 'FPL', holdingbranch => 'FPL' }, $biblionumber, $itemnumber ); + +is( + ItemSafeToDelete($dbh, $biblionumber, $itemnumber), + 'not_same_branch', + 'ItemSafeToDelete reports IndependentBranches restriction', +); + +is( + DelItemCheck($dbh, $biblionumber, $itemnumber), + 'not_same_branch', + 'IndependentBranches prevents deletion at another branch', +); + +ModItem( { homebranch => 'CPL', holdingbranch => 'CPL' }, $biblionumber, $itemnumber ); + +# linked_analytics + +{ # codeblock to limit scope of $module->mock + + my $module = Test::MockModule->new('C4::Items'); + $module->mock( GetAnalyticsCount => sub { return 1 } ); + + is( + ItemSafeToDelete($dbh, $biblionumber, $itemnumber), + 'linked_analytics', + 'ItemSafeToDelete reports linked analytics', + ); + + is( + DelItemCheck($dbh, $biblionumber, $itemnumber), + 'linked_analytics', + 'Linked analytics prevents deletion of item', + ); + +} + +is( + ItemSafeToDelete($dbh, $biblionumber, $itemnumber), + 1, + 'ItemSafeToDelete shows item safe to delete' +); + +DelItemCheck($dbh, $biblionumber, $itemnumber, { do_not_commit => 1 } ); + +my $testitem = GetItem( $itemnumber ); + +is( $testitem->{itemnumber} , $itemnumber, + "DelItemCheck should not delete item if 'do_not_commit' is set" +); + +DelItemCheck( $dbh, $biblionumber, $itemnumber ); + +$testitem = GetItem( $itemnumber ); + +is( $testitem->{itemnumber}, undef, + "DelItemCheck should delete item if 'do_not_commit' not set" +); + +# End of testing + +# Helper methods to set up Biblio, Item, and Borrower. +sub get_biblio { + my $bib = MARC::Record->new(); + $bib->append_fields( + MARC::Field->new( '100', ' ', ' ', a => 'Moffat, Steven' ), + MARC::Field->new( '245', ' ', ' ', a => 'Silence in the library' ), + ); + my ( $bibnum, $bibitemnum ) = AddBiblio( $bib, '' ); + return ( $bibnum, $bibitemnum ); +} + +sub get_item { + my $biblionumber = shift; + my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = + AddItem( { homebranch => 'CPL', holdingbranch => 'CPL', barcode => 'i_dont_exist' }, $biblionumber ); + my $item = GetItem( $itemnumber ); + return ( $itemnumber, $item ); +} + +sub get_borrower { + my $borrowernumber = AddMember( + firstname => 'my firstname', + surname => 'my surname', + categorycode => 'S', + branchcode => 'CPL', + ); + + my $borrower = GetMember( borrowernumber => $borrowernumber ); + return ( $borrowernumber, $borrower ); +} + +# C4::Context->userenv +sub Mock_userenv { + return { flags => 0, branch => 'CPL' }; +} + +$dbh->rollback; -- 2.9.2