bug 14504: split logic from DelItemCheck() into ItemSafeToDelete()

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

diff --git a/C4/Items.pm b/C4/Items.pm
index 81a8263..6b518c6 100644 (file)
--- 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 (file)
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;