From 3d41eaa9798bcde460daea4671133b31ea20c0e4 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 followup) 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.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
C4/Items.pm | 10 ++-
misc/cronjobs/delete_items.pl | 7 +-
t/db_dependent/Items_DelItemCheck.t | 135 ++++++++++++++++++------------------
3 files changed, 76 insertions(+), 76 deletions(-)
diff --git a/C4/Items.pm b/C4/Items.pm
index 6b518c6..f2aad89 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -2299,19 +2299,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 040bdc5..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 98e267a..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 );
+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,63 +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;
--
2.9.2