bug 14504: (QA followup) fixing DelItemCheck arguments

Remove $dbh as argument to C4::Items::DelItemCheck
and C4::Items::ItemSafeToDelete, also change all
calls to these functions throughout the codebase.

Also remove remaining reference to 'do_not_commit' in
t/db_dependent/Items_DelItemCheck.t

Fixed doubled "$$" in C4/ImportBatch.pm

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

C4/Acquisition.pm

@@ -1800,7 +1800,7 @@ sub DelOrder {
 
     my @itemnumbers = GetItemnumbersFromOrder( $ordernumber );
     foreach my $itemnumber (@itemnumbers){
-        my $delcheck = C4::Items::DelItemCheck( $dbh, $bibnum, $itemnumber );
+        my $delcheck = C4::Items::DelItemCheck( $bibnum, $itemnumber );
 
         if($delcheck != 1) {
             $error->{'delitem'} = 1;

C4/ImportBatch.pm

@@ -924,7 +924,7 @@ sub BatchRevertItems {
     $sth->bind_param(1, $import_record_id);
     $sth->execute();
     while (my $row = $sth->fetchrow_hashref()) {
-        my $error = DelItemCheck($dbh, $biblionumber, $row->{'itemnumber'});
+        my $error = DelItemCheck( $biblionumber, $row->{'itemnumber'});
         if ($error == 1){
             my $updsth = $dbh->prepare("UPDATE import_items SET status = ? WHERE import_items_id = ?");
             $updsth->bind_param(1, 'reverted');

C4/Items.pm

@@ -2220,7 +2220,7 @@ sub MoveItemFromBiblio {
 
 =head2 ItemSafeToDelete
 
-   ItemSafeToDelete($dbh, $biblionumber, $itemnumber);
+   ItemSafeToDelete( $biblionumber, $itemnumber);
 
 Exported function (core API) for checking whether an item record is safe to delete.
 
@@ -2237,9 +2237,9 @@ returns 1 if the item is safe to delete,
 =cut
 
 sub ItemSafeToDelete {
-    my ( $dbh, $biblionumber, $itemnumber ) = @_;
+    my ( $biblionumber, $itemnumber ) = @_;
     my $status;
-    $dbh ||= C4::Context->dbh;
+    my $dbh = C4::Context->dbh;
 
     my $error;
 
@@ -2293,21 +2293,17 @@ sub ItemSafeToDelete {
 
 =head2 DelItemCheck
 
-   DelItemCheck($dbh, $biblionumber, $itemnumber);
+   DelItemCheck( $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:
-
-    DelItemCheck(  $dbh, $biblionumber, $itemnumber );
-
 =cut
 
 sub DelItemCheck {
-    my ( $dbh, $biblionumber, $itemnumber ) = @_;
-    my $status = ItemSafeToDelete( $dbh, $biblionumber, $itemnumber );
+    my ( $biblionumber, $itemnumber ) = @_;
+    my $status = ItemSafeToDelete( $biblionumber, $itemnumber );
 
     if ( $status == 1 ) {
         DelItem(

cataloguing/additem.pl

@@ -628,7 +628,7 @@ if ($op eq "additem") {
 } elsif ($op eq "delitem") {
 #-------------------------------------------------------------------------------
     # check that there is no issue on this item before deletion.
-    $error = &DelItemCheck($dbh,$biblionumber,$itemnumber);
+    $error = &DelItemCheck( $biblionumber,$itemnumber);
     if($error == 1){
         print $input->redirect("additem.pl?biblionumber=$biblionumber&frameworkcode=$frameworkcode&searchid=$searchid");
     }else{
@@ -645,7 +645,7 @@ if ($op eq "additem") {
         my $items = &GetItemsByBiblioitemnumber( $biblioitem->{biblioitemnumber} );
 
         foreach my $item (@$items) {
-            $error =&DelItemCheck( $dbh, $biblionumber, $item->{itemnumber} );
+            $error =&DelItemCheck( $biblionumber, $item->{itemnumber} );
             $itemfail =$item;
         if($error == 1){
             next

misc/cronjobs/delete_items.pl

@@ -57,9 +57,9 @@ $GLOBAL->{sth}->{target_items}->execute();
 
 DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() ) {
 
-    my $status = C4::Items::ItemSafeToDelete( $dbh, $item->{itemnumber}, $item->{biblionumber} );
+    my $status = C4::Items::ItemSafeToDelete( $item->{itemnumber}, $item->{biblionumber} );
     if( $status == 1 )  {
-        C4::Items::DelItemCheck( $dbh, $item->{itemnumber}, $item->{biblionumber} );
+        C4::Items::DelItemCheck( $item->{itemnumber}, $item->{biblionumber} );
         verbose "Deleting '$item->{itemnumber}'";
     } else {
         verbose "Item '$item->{itemnumber}' not deletd: $status";

t/db_dependent/Circulation/IsItemIssued.t

@@ -44,7 +44,7 @@ AddIssue($borrower, 'i_dont_exist');
 is ( IsItemIssued( $item->{itemnumber} ), 1, "item is now on loan" );
 
 is(
-    DelItemCheck($dbh, $biblionumber, $itemnumber),
+    DelItemCheck( $biblionumber, $itemnumber),
     'book_on_loan',
     'item that is on loan cannot be deleted',
 );
@@ -53,7 +53,7 @@ AddReturn('i_dont_exist', $library->{branchcode});
 is ( IsItemIssued( $item->{itemnumber} ), 0, "item has been returned" );
 
 is(
-    DelItemCheck($dbh, $biblionumber, $itemnumber),
+    DelItemCheck( $biblionumber, $itemnumber),
     1,
     'item that is not on loan can be deleted',
 );

t/db_dependent/Items_DelItemCheck.t

@@ -72,13 +72,13 @@ my $item = $builder->build(
 AddIssue( $patron, $item->{barcode} );
 
 is(
-    ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
     'book_on_loan',
     'ItemSafeToDelete reports item on loan',
 );
 
 is(
-    DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+    DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
     'book_on_loan',
     'item that is on loan cannot be deleted',
 );
@@ -92,13 +92,13 @@ t::lib::Mocks::mock_preference('IndependentBranches', 1);
 ModItem( { homebranch => $branch2->{branchcode}, holdingbranch => $branch2->{branchcode} }, $biblio->{biblionumber}, $item->{itemnumber} );
 
 is(
-    ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
     'not_same_branch',
     'ItemSafeToDelete reports IndependentBranches restriction',
 );
 
 is(
-    DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+    DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
     'not_same_branch',
     'IndependentBranches prevents deletion at another branch',
 );
@@ -113,13 +113,13 @@ ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branc
     $module->mock( GetAnalyticsCount => sub { return 1 } );
 
     is(
-        ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+        ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
         'linked_analytics',
         'ItemSafeToDelete reports linked analytics',
     );
 
     is(
-        DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+        DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} ),
         'linked_analytics',
         'Linked analytics prevents deletion of item',
     );
@@ -127,17 +127,17 @@ ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branc
 }
 
 is(
-    ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ),
+    ItemSafeToDelete( $biblio->{biblionumber}, $item->{itemnumber} ),
     1,
     'ItemSafeToDelete shows item safe to delete'
 );
 
-DelItemCheck( $dbh, $biblio->{biblionumber}, $item->{itemnumber} );
+DelItemCheck( $biblio->{biblionumber}, $item->{itemnumber} );
 
 my $test_item = GetItem( $item->{itemnumber} );
 
 is( $test_item->{itemnumber}, undef,
-    "DelItemCheck should delete item if 'do_not_commit' not set"
+    "DelItemCheck should delete item if ItemSafeToDelete returns true"
 );
 
 # End of testing

t/db_dependent/Reserves.t

@@ -371,7 +371,7 @@ is($new_count, $hold_notice_count + 1, 'patron not notified a second time (bug 1
 # avoiding the not_same_branch error
 t::lib::Mocks::mock_preference('IndependentBranches', 0);
 is(
-    DelItemCheck($dbh, $bibnum, $itemnumber),
+    DelItemCheck( $bibnum, $itemnumber),
     'book_reserved',
     'item that is captured to fill a hold cannot be deleted',
 );

tools/batchMod.pl

@@ -166,7 +166,7 @@ if ($op eq "action") {
 		$job->progress($i) if $runinbackground;
 		my $itemdata = GetItem($itemnumber);
         if ( $del ){
-			my $return = DelItemCheck(C4::Context->dbh, $itemdata->{'biblionumber'}, $itemdata->{'itemnumber'});
+            my $return = DelItemCheck( $itemdata->{'biblionumber'}, $itemdata->{'itemnumber'});
 			if ($return == 1) {
 			    $deleted_items++;
 			} else {

tools/batch_delete_records.pl

@@ -159,7 +159,7 @@ if ( $op eq 'form' ) {
             # Delete items
             my @itemnumbers = @{ C4::Items::GetItemnumbersForBiblio( $biblionumber ) };
             ITEMNUMBER: for my $itemnumber ( @itemnumbers ) {
-                my $error = eval { C4::Items::DelItemCheck( $dbh, $biblionumber, $itemnumber ) };
+                my $error = eval { C4::Items::DelItemCheck( $biblionumber, $itemnumber ) };
                 if ( $error != 1 or $@ ) {
                     push @messages, {
                         type => 'error',