[Koha-bugs] [Bug 14504] Add command-line script to batch delete items based on lost/withdrawn statuses

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14504

--- Comment #5 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
Comment on attachment 42978
  --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42978
[SIGNED-OFF] Bug 14504: Add delete_items.pl: a command line batch deletion tool

Review of attachment 42978:
 --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=14504&attachment=42978)
-----------------------------------------------------------------

FAIL   misc/cronjobs/delete_items.pl
   FAIL   pod
                *** WARNING: Verbatim paragraph in NAME section  in file
misc/cronjobs/delete_items.pl

::: misc/cronjobs/delete_items.pl
@@ +1,4 @@
> +#! /usr/bin/perl
> +
> +use warnings;
> +use strict;

warnings and strict are useless when Modern::Perl is already used.

@@ +8,5 @@
> +use C4::Circulation;
> +use Modern::Perl;
> +use Pod::Usage;
> +
> +my $VERSION='1.0';

How this is useful? We usually don't use it in scripts.

@@ +30,5 @@
> +          , help      => ''
> +          , manual    => ''
> +          , version   => ''
> +      }
> +};

I am not really in favor of these 2 variables.
IMO it is preferable to stick to the structure of the already existing scripts
in misc/*

@@ +39,5 @@
> +    , 'V|version'  => sub { $OPTIONS->{flags}->{version}   = 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!
> +    , 'dry-run'    => sub { $OPTIONS->{flags}->{commit}    = 0;

I don't think dry-run is useful, it's in dry-run mode if commit is not given.

@@ +51,5 @@
> +    exit;
> +}
> +
> +pod2usage( -verbose => 2 ) if  $OPTIONS->{flags}->{manual};
> +pod2usage(1) if ( $OPTIONS->{flags}->{help} || scalar @criteria == 0 );

The script should not return 1 if help is specified.
It would be good to display on error for the @criteria==0 case (see msg option
of pod2usage).

@@ +65,5 @@
> +my $where_clause = ' where ' . join ( " and ", @criteria );
> +
> +verbose "Where statement: $where_clause";
> +
> +$GLOBAL->{sth}->{target_tiems} = $dbh->prepare( $query->{target_items} . $where_clause  );

typo tiems vs items, I suppose.

@@ +71,5 @@
> +
> +DELITEM: while ( my $item = $GLOBAL->{sth}->{target_tiems}->fetchrow_hashref() ) {
> +    my $issue = GetOpenIssue( $item->{itemnumber} );
> +    if( defined $issue ) {
> +        verbose "Cannot delete '$item->{itemnumber}' -- item is checked out."

Shouldn't we also search for holds?

-- 
You are receiving this mail because:
You are watching all bug changes.