From 93504aea26054f436a9e98fcbef0274a0ac64f38 Mon Sep 17 00:00:00 2001
From: Barton Chittenden <barton@bywatersolutions.com>
Date: Thu, 15 Oct 2015 13:40:29 -0700
Subject: [PATCH] Bug 14504: (QA followup)

* Fix POD warning.
* Remove redundant 'use stric' and 'use warnings'
* Remove $VERSION and --version option.
* Remove --dry-run option
* Split test for --help and check for @criteria into two separate pod2usage calls,
    enabling -msg on the latter.
* Fix 'target_tiems' typo.
* Test for holds on items to be deleted.
* Fix whitespace
* Fix test for holds.

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 misc/cronjobs/delete_items.pl | 58 +++++++++++++----------------------
 1 file changed, 21 insertions(+), 37 deletions(-)

diff --git a/misc/cronjobs/delete_items.pl b/misc/cronjobs/delete_items.pl
index c6683ff2a7..81f11c720e 100755
--- a/misc/cronjobs/delete_items.pl
+++ b/misc/cronjobs/delete_items.pl
@@ -1,7 +1,5 @@
 #! /usr/bin/perl
 
-use warnings;
-use strict;
 use Getopt::Long;
 use C4::Context;
 use C4::Items;
@@ -9,12 +7,10 @@ use C4::Circulation;
 use Modern::Perl;
 use Pod::Usage;
 
-my $VERSION='1.0';
-
 my $dbh = C4::Context->dbh();
 
 my $query = {
-    target_items => q|SELECT itemnumber from items|
+    target_items => q|SELECT itemnumber, biblionumber from items|
 };
 
 my $GLOBAL = {
@@ -40,56 +36,52 @@ GetOptions(
     , 'h|help'     => sub { $OPTIONS->{flags}->{help}      = 1 }
     , 'm|manual'   => sub { $OPTIONS->{flags}->{manual}    = 1 }
     , 'c|commit'   => sub { $OPTIONS->{flags}->{commit}    = 1 } # aka DO-EET!
-    , 'dry-run'    => sub { $OPTIONS->{flags}->{commit}    = 0;
-                            $OPTIONS->{flags}->{verbose}   = 1 }
 );
 
 my @criteria = @{ $OPTIONS->{criteria} };
 
-if( $OPTIONS->{flags}->{version} ) {
-    say "$0 version $VERSION";
-    exit;
-}
-
 pod2usage( -verbose => 2 ) if  $OPTIONS->{flags}->{manual};
-pod2usage(1) if ( $OPTIONS->{flags}->{help} || scalar @criteria == 0 );
-
-
-
+pod2usage( -verbose => 1 ) if  $OPTIONS->{flags}->{help};
+pod2usage( -verbose => 1 -msg => 'You must supply at least one --criteria option' ) if  scalar @criteria == 0;
 
 sub verbose {
     say @_ if $OPTIONS->{flags}->{verbose};
 }
 
-
 my $where_clause = ' where ' . join ( " and ", @criteria );
 
 verbose "Where statement: $where_clause";
 
-$GLOBAL->{sth}->{target_tiems} = $dbh->prepare( $query->{target_items} . $where_clause  );
-$GLOBAL->{sth}->{target_tiems}->execute();
+$GLOBAL->{sth}->{target_items} = $dbh->prepare( $query->{target_items} . $where_clause  );
+$GLOBAL->{sth}->{target_items}->execute();
 
-DELITEM: while ( my $item = $GLOBAL->{sth}->{target_tiems}->fetchrow_hashref() ) {
+DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() ) {
     my $issue = GetOpenIssue( $item->{itemnumber} );
+    my $holds = GetItemHolds( $item->{biblionumber}, $item->{itemnumber} );
+
     if( defined $issue ) {
-        verbose "Cannot delete '$item->{itemnumber}' -- item is checked out."
-    } else {
-        verbose "Deleting '$item->{itemnumber}' ";
-        C4::Items::DelItem( { itemnumber => $item->{itemnumber} } ) if $OPTIONS->{flags}->{commit} ;
+        verbose "Cannot delete '$item->{itemnumber}' -- item is checked out.";
+        next DELITEM;
     }
+
+    if( $holds ) {
+        verbose "Cannot delete '$item->{itemnumber}' -- item has open holds.";
+        next DELITEM;
+    }
+
+    verbose "Deleting '$item->{itemnumber}' ";
+    C4::Items::DelItem( { itemnumber => $item->{itemnumber} } ) if $OPTIONS->{flags}->{commit} ;
 }
 
 =head1 NAME
 
- delete_items.pl - A batch item deletion tool, which generates a query against
-                   the items database and deletes the items matching the
-                   criteria specified in the command line arguments.
+delete_items.pl - A batch item deletion tool, which generates a query against the items database and deletes the items matching the criteria specified in the command line arguments.
 
 =head1 SYNOPSIS
 
-delete_items.pl [--help|--manual|--version]
+delete_items.pl [--help|--manual]
 
-delete_items.pl [--verbose] [--dry-run] --criteria "I<SQL CONDITIONAL EXPRESSION>" ... [--commit]
+delete_items.pl [--verbose] --criteria "I<SQL CONDITIONAL EXPRESSION>" ... [--commit]
 
 =cut
 
@@ -105,10 +97,6 @@ Show the brief help information.
 
 Read the manual, with examples.
 
-=item B<--version>
-
-Show the version number and exit.
-
 =item B<--verbose>
 
 Send the "WHERE" clause generated by the collected C<--criteria>
@@ -124,10 +112,6 @@ clause querying the items table. These are joined by C<AND>.
 
 No items will be deleted unless the C<--commit> flag is present.
 
-=item B<--dry-run>
-
-Disables C<--commit> flag and enables C<--verbose> flag.
-
 =back
 
 =cut
-- 
2.39.5