Bugzilla – Attachment 45789 Details for
Bug 14504
Add command-line script to batch delete items based on data in items table
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
bug 14504: QA Followup -- fixing DelItemCheck arguments
bug-14504-QA-Followup----fixing-DelItemCheck-argum.patch (text/plain), 10.84 KB, created by
Barton Chittenden
on 2015-12-17 22:58:05 UTC
(
hide
)
Description:
bug 14504: QA Followup -- fixing DelItemCheck arguments
Filename:
MIME Type:
Creator:
Barton Chittenden
Created:
2015-12-17 22:58:05 UTC
Size:
10.84 KB
patch
obsolete
>From 3ff3158f5de1341187bfdc351c355a8a2680886e Mon Sep 17 00:00:00 2001 >From: Barton Chittenden <barton@bywatersolutions.com> >Date: Thu, 29 Oct 2015 06:06:08 -0700 >Subject: [PATCH] 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 >--- > C4/Acquisition.pm | 2 +- > C4/ImportBatch.pm | 2 +- > C4/Items.pm | 15 ++++++--------- > cataloguing/additem.pl | 4 ++-- > misc/cronjobs/delete_items.pl | 4 ++-- > t/db_dependent/Circulation/IsItemIssued.t | 4 ++-- > t/db_dependent/Items_DelItemCheck.t | 18 +++++++++--------- > t/db_dependent/Reserves.t | 2 +- > tools/batchMod.pl | 2 +- > tools/batch_delete_records.pl | 2 +- > 10 files changed, 26 insertions(+), 29 deletions(-) > >diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm >index 0f2b161..9c2c834 100644 >--- a/C4/Acquisition.pm >+++ b/C4/Acquisition.pm >@@ -1813,7 +1813,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; >diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm >index cb1a231..2bacac9 100644 >--- a/C4/ImportBatch.pm >+++ b/C4/ImportBatch.pm >@@ -925,7 +925,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'); >diff --git a/C4/Items.pm b/C4/Items.pm >index 274cfe4..a9d97b4 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -2299,7 +2299,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. > >@@ -2316,9 +2316,10 @@ returns 1 if the item is safe to delete, > =cut > > sub ItemSafeToDelete { >- my ( $dbh, $biblionumber, $itemnumber ) = @_; >+ my ( $biblionumber, $itemnumber ) = @_; > my $status; > >+ my $dbh = C4::Context->dbh; > my $countanalytics = GetAnalyticsCount($itemnumber); > > # check that there is no issue on this item before deletion. >@@ -2368,21 +2369,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( >diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl >index 19279b3..8f33e9b 100755 >--- a/cataloguing/additem.pl >+++ b/cataloguing/additem.pl >@@ -589,7 +589,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{ >@@ -606,7 +606,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 >diff --git a/misc/cronjobs/delete_items.pl b/misc/cronjobs/delete_items.pl >index 7c2e1cf..f3d3de6 100755 >--- a/misc/cronjobs/delete_items.pl >+++ b/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"; >diff --git a/t/db_dependent/Circulation/IsItemIssued.t b/t/db_dependent/Circulation/IsItemIssued.t >index 18ea984..45b62df 100644 >--- a/t/db_dependent/Circulation/IsItemIssued.t >+++ b/t/db_dependent/Circulation/IsItemIssued.t >@@ -36,7 +36,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', > ); >@@ -45,7 +45,7 @@ AddReturn('i_dont_exist', 'CPL'); > 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', > ); >diff --git a/t/db_dependent/Items_DelItemCheck.t b/t/db_dependent/Items_DelItemCheck.t >index 6775a45..abe6600 100644 >--- a/t/db_dependent/Items_DelItemCheck.t >+++ b/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 >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 7301757..ca15a64 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -367,7 +367,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', > ); >diff --git a/tools/batchMod.pl b/tools/batchMod.pl >index 055e407..6e5a820 100755 >--- a/tools/batchMod.pl >+++ b/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 { >diff --git a/tools/batch_delete_records.pl b/tools/batch_delete_records.pl >index b0523d5..d20dc3a 100755 >--- a/tools/batch_delete_records.pl >+++ b/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', >-- >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 14504
:
40830
|
42978
|
43445
|
43533
|
43788
|
43789
|
44013
|
44149
|
45789
|
49648
|
49649
|
49650
|
49651
|
49652
|
49653
|
52425
|
53224
|
53225
|
54514
|
54515
|
54516
|
54517
|
54518
|
54519
|
54520
|
54521
|
54522
|
54525
|
54530
|
54531
|
54691
|
54692
|
54693
|
54694
|
54695
|
54696
|
54697
|
54698
|
54699
|
54700
|
54701
|
54702
|
54703