Bugzilla – Attachment 44013 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 Fixes -- use TestBuilder, remove do_not_commit
Bug-14504-QA-Fixes----use-TestBuilder-remove-donot.patch (text/plain), 9.18 KB, created by
Barton Chittenden
on 2015-10-26 17:12:24 UTC
(
hide
)
Description:
Bug 14504 QA Fixes -- use TestBuilder, remove do_not_commit
Filename:
MIME Type:
Creator:
Barton Chittenden
Created:
2015-10-26 17:12:24 UTC
Size:
9.18 KB
patch
obsolete
>From a57407c1c1bd03ca15d38a7eed25b1470af21a63 Mon Sep 17 00:00:00 2001 >From: Barton Chittenden <barton@bywatersolutions.com> >Date: Mon, 26 Oct 2015 09:01:50 -0700 >Subject: [PATCH] Bug 14504 QA Fixes -- use TestBuilder, remove do_not_commit > >Use t::lib::TestBuilder in t/db_dependent/Items_DelItemCheck.t > >Remove the option 'do_not_commit' from C4::Items::DelItemCheck. > >Whitespace cleanup. >--- > C4/Items.pm | 12 ++- > misc/cronjobs/delete_items.pl | 7 +- > t/db_dependent/Items_DelItemCheck.t | 138 ++++++++++++++++++----------------- > 3 files changed, 78 insertions(+), 79 deletions(-) > >diff --git a/C4/Items.pm b/C4/Items.pm >index 44e7300..274cfe4 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -2303,7 +2303,7 @@ sub MoveItemFromBiblio { > > Exported function (core API) for checking whether an item record is safe to delete. > >-returns 1 if the item is safe to delete, >+returns 1 if the item is safe to delete, > > "book_on_loan" if the item is checked out, > >@@ -2374,19 +2374,17 @@ Exported function (core API) for deleting an item record in Koha if there no cur > > 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: >+It takes a database handle, biblionumber and itemnumber as arguments: > >- DelItemCheck( $dbh, $biblionumber, $itemnumber, { do_not_commit => 1 } ); >+ DelItemCheck( $dbh, $biblionumber, $itemnumber ); > >-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 ( $dbh, $biblionumber, $itemnumber ) = @_; > my $status = ItemSafeToDelete( $dbh, $biblionumber, $itemnumber ); > >- if ( $status == 1 && $commit ) { >+ if ( $status == 1 ) { > DelItem( > { > biblionumber => $biblionumber, >diff --git a/misc/cronjobs/delete_items.pl b/misc/cronjobs/delete_items.pl >index 41a385b..7c2e1cf 100755 >--- a/misc/cronjobs/delete_items.pl >+++ b/misc/cronjobs/delete_items.pl >@@ -56,11 +56,10 @@ $GLOBAL->{sth}->{target_items} = $dbh->prepare( $query->{target_items} . $where_ > $GLOBAL->{sth}->{target_items}->execute(); > > DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() ) { >- my $del_check_options = $OPTIONS->{flags}->{commit} >- ? undef >- : { do_not_commit => 1 }; >- my $status = C4::Items::DelItemCheck( $dbh, $item->{itemnumber}, $item->{biblionumber}, $del_check_options ); >+ >+ my $status = C4::Items::ItemSafeToDelete( $dbh, $item->{itemnumber}, $item->{biblionumber} ); > if( $status == 1 ) { >+ C4::Items::DelItemCheck( $dbh, $item->{itemnumber}, $item->{biblionumber} ); > verbose "Deleting '$item->{itemnumber}'"; > } else { > verbose "Item '$item->{itemnumber}' not deletd: $status"; >diff --git a/t/db_dependent/Items_DelItemCheck.t b/t/db_dependent/Items_DelItemCheck.t >index 7880c44..6775a45 100644 >--- a/t/db_dependent/Items_DelItemCheck.t >+++ b/t/db_dependent/Items_DelItemCheck.t >@@ -1,13 +1,11 @@ > use Modern::Perl; > >-use MARC::Record; >-use C4::Biblio; > use C4::Circulation; >-use C4::Members; >-use t::lib::Mocks; > >+use t::lib::TestBuilder; >+use t::lib::Mocks; > >-use Test::More tests => 10; >+use Test::More tests => 9; > > *C4::Context::userenv = \&Mock_userenv; > >@@ -16,50 +14,96 @@ BEGIN { > } > > my $dbh = C4::Context->dbh; >-$dbh->{AutoCommit} = 0; >-$dbh->{RaiseError} = 1; > >-my ( $biblionumber, $bibitemnum ) = get_biblio(); >+my $builder = t::lib::TestBuilder->new(); >+ >+my $branch = $builder->build( >+ { >+ source => 'Branch', >+ } >+); >+ >+my $branch2 = $builder->build( >+ { >+ source => 'Branch', >+ } >+); >+ >+my $category = $builder->build( >+ { >+ source => 'Category', >+ } >+); >+ >+my $patron = $builder->build( >+ { >+ source => 'Borrower', >+ value => { >+ categorycode => $category->{categorycode}, >+ branchcode => $branch->{branchcode}, >+ }, >+ } >+); >+ >+my $biblio = $builder->build( >+ { >+ source => 'Biblio', >+ value => { >+ branchcode => $branch->{branchcode}, >+ }, >+ } >+); >+ >+my $item = $builder->build( >+ { >+ source => 'Item', >+ value => { >+ biblionumber => $biblio->{biblionumber}, >+ homebranch => $branch->{branchcode}, >+ holdingbranch => $branch->{branchcode}, >+ withdrawn => 0, # randomly assigned value may block return. >+ withdrawn_on => undef, >+ }, >+ } >+); > > # book_on_loan > >-my ( $borrowernumber, $borrower ) = get_borrower(); >-my ( $itemnumber, $item ) = get_item( $biblionumber ); >-AddIssue( $borrower, 'i_dont_exist' ); >+AddIssue( $patron, $item->{barcode} ); > > is( >- ItemSafeToDelete($dbh, $biblionumber, $itemnumber), >+ ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'book_on_loan', > 'ItemSafeToDelete reports item on loan', > ); > > is( >- DelItemCheck($dbh, $biblionumber, $itemnumber), >+ DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'book_on_loan', > 'item that is on loan cannot be deleted', > ); > >-AddReturn('i_dont_exist', 'CPL'); >+AddReturn( $item->{barcode}, $branch->{branchcode} ); > > # 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 ); >+t::lib::Mocks::mock_preference('IndependentBranches', 1); >+ModItem( { homebranch => $branch2->{branchcode}, holdingbranch => $branch2->{branchcode} }, $biblio->{biblionumber}, $item->{itemnumber} ); > > is( >- ItemSafeToDelete($dbh, $biblionumber, $itemnumber), >+ ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'not_same_branch', > 'ItemSafeToDelete reports IndependentBranches restriction', > ); > > is( >- DelItemCheck($dbh, $biblionumber, $itemnumber), >+ DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'not_same_branch', > 'IndependentBranches prevents deletion at another branch', > ); > >-ModItem( { homebranch => 'CPL', holdingbranch => 'CPL' }, $biblionumber, $itemnumber ); >+ModItem( { homebranch => $branch->{branchcode}, holdingbranch => $branch->{branchcode} }, $biblio->{biblionumber}, $item->{itemnumber} ); > > # linked_analytics > >@@ -69,13 +113,13 @@ ModItem( { homebranch => 'CPL', holdingbranch => 'CPL' }, $biblionumber, $itemnu > $module->mock( GetAnalyticsCount => sub { return 1 } ); > > is( >- ItemSafeToDelete($dbh, $biblionumber, $itemnumber), >+ ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'linked_analytics', > 'ItemSafeToDelete reports linked analytics', > ); > > is( >- DelItemCheck($dbh, $biblionumber, $itemnumber), >+ DelItemCheck($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 'linked_analytics', > 'Linked analytics prevents deletion of item', > ); >@@ -83,64 +127,22 @@ ModItem( { homebranch => 'CPL', holdingbranch => 'CPL' }, $biblionumber, $itemnu > } > > is( >- ItemSafeToDelete($dbh, $biblionumber, $itemnumber), >+ ItemSafeToDelete($dbh, $biblio->{biblionumber}, $item->{itemnumber} ), > 1, > 'ItemSafeToDelete shows item safe to delete' > ); > >-DelItemCheck($dbh, $biblionumber, $itemnumber, { do_not_commit => 1 } ); >+DelItemCheck( $dbh, $biblio->{biblionumber}, $item->{itemnumber} ); > >-my $testitem = GetItem( $itemnumber ); >+my $test_item = GetItem( $item->{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, >+is( $test_item->{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' }; >+ return { flags => 0, branch => $branch->{branchcode} }; > } >- >-$dbh->rollback; >- >-- >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