From d4e90ec98b76d54f9e538a4fb8c2bdd0fdbfb6e7 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Wed, 4 Dec 2024 14:28:47 +0000 Subject: [PATCH] Bug 35654: Simplify and update delete_items.pl code Rather than defining hashes and storing our options there, we can just use variables directly, improves readability IMO --- misc/cronjobs/delete_items.pl | 90 ++++++++++++++--------------------- 1 file changed, 35 insertions(+), 55 deletions(-) diff --git a/misc/cronjobs/delete_items.pl b/misc/cronjobs/delete_items.pl index 7764ca6830f..0c49076cdf0 100755 --- a/misc/cronjobs/delete_items.pl +++ b/misc/cronjobs/delete_items.pl @@ -18,7 +18,7 @@ use Modern::Perl; use Getopt::Long qw( GetOptions ); -use Pod::Usage qw( pod2usage ); +use Pod::Usage qw( pod2usage ); use Koha::Script -cron; use C4::Biblio qw( DelBiblio ); @@ -27,60 +27,41 @@ use Koha::Items; my $dbh = C4::Context->dbh(); -my $query = { - target_items => q|SELECT itemnumber, biblionumber from items| -}; - -my $GLOBAL = { - query => $query - , sth => {} -}; - -my $OPTIONS = { - where => [] - , flags => { - verbose => '' - , commit => '' - , help => '' - , manual => '' - , del_bibs => '' - } -}; +my @where = (); +my $verbose = 0; +my $help = 0; +my $manual = 0; +my $commit = 0; +my $delete_biblios = 0; GetOptions( - 'where=s' => $OPTIONS->{where} - , 'v|verbose' => sub { $OPTIONS->{flags}->{verbose} = 1 } - , 'h|help' => sub { $OPTIONS->{flags}->{help} = 1 } - , 'm|manual' => sub { $OPTIONS->{flags}->{manual} = 1 } - , 'c|commit' => sub { $OPTIONS->{flags}->{commit} = 1 } # aka DO-EET! - , 'del_bibs' => sub { $OPTIONS->{flags}->{del_bibs} = 1 } + 'where=s' => \@where, 'v|verbose' => \$verbose, + 'h|help' => \$help, + 'm|manual' => \$manual, + 'c|commit' => \$commit, + 'del_bibs' => \$delete_biblios ); -my @where = @{ $OPTIONS->{where} }; +pod2usage( -verbose => 2 ) if $manual; +pod2usage( -verbose => 1 ) if $help; +pod2usage( -verbose => 1, -message => 'You must supply at least one --where option' ) if scalar @where == 0; -pod2usage( -verbose => 2 ) if $OPTIONS->{flags}->{manual}; -pod2usage( -verbose => 1 ) if $OPTIONS->{flags}->{help}; -pod2usage( -verbose => 1 -msg => 'You must supply at least one --where option' ) if scalar @where == 0; +my $where_clause = ' where ' . join( " and ", @where ); -sub verbose { - say @_ if $OPTIONS->{flags}->{verbose}; +$verbose && say "Where statement: $where_clause"; +if ($delete_biblios) { + $verbose && say "Deleting bibliographic records when all items are deleted!"; } -my $where_clause = ' where ' . join ( " and ", @where ); - -verbose "Where statement: $where_clause"; -if( $OPTIONS->{flags}->{del_bibs} ) { - verbose "Deleting empty bib records!" -} - -print "Test run only! No data will be deleted.\n" unless $OPTIONS->{flags}->{commit}; -my $deleted_string = $OPTIONS->{flags}->{commit} ? "Deleted" : "Would have deleted"; +print "Test run only! No data will be deleted.\n" unless $commit; +my $deleted_string = $commit ? "Deleted" : "Would have deleted"; # FIXME Use Koha::Items instead -$GLOBAL->{sth}->{target_items} = $dbh->prepare( $query->{target_items} . $where_clause ); -$GLOBAL->{sth}->{target_items}->execute(); +my $query = "SELECT itemnumber, biblionumber from items "; +my $sth = $dbh->prepare( $query . $where_clause ); +$sth->execute(); -DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() ) { +DELITEM: while ( my $item = $sth->fetchrow_hashref() ) { my $item_object = Koha::Items->find( $item->{itemnumber} ); my $holdings_count = $item_object->biblio->items->count; @@ -89,23 +70,25 @@ DELITEM: while ( my $item = $GLOBAL->{sth}->{target_items}->fetchrow_hashref() ) my $safe_to_delete = $item_object->safe_to_delete; if ($safe_to_delete) { $item_object->safe_delete - if $OPTIONS->{flags}->{commit}; - verbose "$deleted_string item '$item->{itemnumber}' (of $holdings_count)"; + if $commit; + $verbose && say "$deleted_string item '$item->{itemnumber}' (of $holdings_count)"; - if ( $OPTIONS->{flags}->{del_bibs} && $holdings_count == 1 ) { # aka DO-EET for empty bibs! - $error = &DelBiblio( $item->{biblionumber} ) if $OPTIONS->{flags}->{commit}; + if ( $delete_biblios && $holdings_count == 1 ) { # aka DO-EET for empty bibs! + $error = &DelBiblio( $item->{biblionumber} ) if $commit; if ($error) { - verbose "Could not delete bib '$item->{biblionumber}': $error"; + $verbose && say "Could not delete bib '$item->{biblionumber}': $error"; } else { - verbose "No items remaining. $deleted_string bibliographic record '$item->{biblionumber}'"; + $verbose && say "No items remaining. $deleted_string bibliographic record '$item->{biblionumber}'"; } } } else { - verbose sprintf "Item '%s' (Barcode: '%s', Title: '%s') cannot deleted: %s", + say sprintf( + "Item '%s' (Barcode: '%s', Title: '%s') cannot deleted: %s", $item_object->itemnumber, $item_object->barcode, $item_object->biblio->title, - @{ $safe_to_delete->messages }[0]->message; + @{ $safe_to_delete->messages }[0]->message + ) if $verbose; } } @@ -156,7 +139,6 @@ Deletes the bibliographic record if the last item is deleted. =cut - =head1 EXAMPLES The following is an example of this script: @@ -167,14 +149,12 @@ Deletes the bibliographic record if the last item is deleted. =cut - =head1 DESCRIPTION This is a lightweight batch deletion tool for items, suitable for running in a cron job. =cut - =head1 AUTHOR Barton Chittenden -- 2.39.5