From 6da97c7c8713bb16cbf6cf079df18534c3661414 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 3 Apr 2017 15:16:42 +0200 Subject: [PATCH] Bug 14399: Numerous small refinements to the inventory script This patch contains the following changes: [01] Label "Inventory date" reworded to "Last inventory date", adding a small explanation for its purpose. [02] Restructured the results: it was an array with items and possible error messages. Multiple messages duplicated individual items. Now the results are in a hash, pulling all error messages for one item together. At the end of the script they are copied to an array. (A helper sub additemtoresults is added in this regard.) We no longer use array @items_with_problems. [03] Both datepickers are no longer connected to the same class. This prevents changing the set date by filling the last inventory date. [04] Input markseen in the template and $markseen in the script are no longer needed. [05] The paragraph before the detail link in the results table in the Title column has been removed. Same for problems column. This makes vertical spacing consistent. [06] Problem status 'missingitem' is no longer used; the missing items are marked as 'not_scanned'. Two additional statuses are: no_barcode and checkedout. [07] Removed unused $itemtype, $totalrecords and $count. We use variable $moddatecount to report a count to the template. [08] The script updated scanned items twice. The first time with ModItem and the second time with ModDateLastSeen. The second call is removed. [09] If a book is checked in, we do no longer return an error message when the checkin is successful (ERR_ONLOAN_RET). The updated datelastseen is passed to the results. [10] $wrongplacelist is renamed to $rightplacelist. It is only built when we need it. (Same for inventorylist now.) [11] Datelastseen (last inventory date) is always used for building the inventory list. It allows you to process partial barcode lists or make a list of items not seen after some date. We do no longer use variable $paramdatelastseen. [12] The section where items.datelastseen was compared with the inventory date has been removed. Scanned items were already updated; to get items seen before some date, you can now use last inventory date without passing barcodes. The form can mainly be used for the following three cases: [1] Prepare an inventory list or csv file; we do not upload barcodes. [2] Update items for uploaded barcodes without comparing to inventory. Last inventory date is useless in this case. Errors wrongplace, checkedout and changestatus are reported. Use this scenario for partial scanned barcode lists (all but last). [3] Update items for uploaded barcodes and compare to inventory, filtered by an optional last inventory date. Apart from the errors mentioned under [2], this also reports not_scanned ("missing") and no_barcode. Use this scenario too for the last partial barcode file (together with inventory date). Test plan: See next patch ("Interface changes"). Signed-off-by: Marcel de Rooy Signed-off-by: Josef Moravec Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall --- .../prog/en/modules/tools/inventory.tt | 31 +-- tools/inventory.pl | 201 ++++++++---------- 2 files changed, 112 insertions(+), 120 deletions(-) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt index 650a51088e..7f31c93e6c 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt @@ -128,7 +128,7 @@ $(document).ready(function(){ Use a barcode file
    2. -
  2. +
@@ -200,8 +200,9 @@ $(document).ready(function(){
    [% END %] -
  1. - +
  2. + + (Skip records marked as seen on or after this date.)
  3. [% IF (ignoreissued) %] @@ -232,7 +233,6 @@ $(document).ready(function(){ [% END %] [% IF (op) %]
    - @@ -276,7 +276,8 @@ $(document).ready(function(){ [% result.location | html %] -

    [% result.title | html %]

    [% result.author | html %]

    + [% result.title | html %] +

    [% result.author | html %]

    [% result.notforloan | html %] @@ -294,14 +295,18 @@ $(document).ready(function(){ [% result.datelastseen | $KohaDates | html %] - [% IF result.problem == 'wrongplace' %] -

    Item should not have been scanned

    - [% ELSIF result.problem == 'missingitem' %] -

    Item missing

    - [% ELSIF result.problem == 'changestatus' %] -

    Unknown not-for-loan status

    - [% ELSIF result.problem == 'not_scanned' %] -

    Item should have been scanned

    + [% FOREACH problem IN result.problems %] + [% IF problem.key == 'wrongplace' %] + Found in wrong place
    + [% ELSIF problem.key == 'changestatus' %] + Unknown not-for-loan status
    + [% ELSIF problem.key == 'not_scanned' %] + Missing (not scanned)
    + [% ELSIF problem.key == 'checkedout' %] + Still checked out
    + [% ELSIF problem.key == 'no_barcode' %] + No barcode
    + [% END %] [% END %] diff --git a/tools/inventory.pl b/tools/inventory.pl index 9efa8c5b6d..76a09252fa 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -1,7 +1,7 @@ #!/usr/bin/perl # Copyright 2000-2009 Biblibre S.A -# John Soros +# John Soros # # This file is part of Koha. # @@ -18,8 +18,7 @@ # You should have received a copy of the GNU General Public License # along with Koha; if not, see . -use strict; -use warnings; +use Modern::Perl; #need to open cgi and get the fh before anything else opens a new cgi context (see C4::Auth) use CGI qw ( -utf8 ); @@ -40,15 +39,12 @@ use Koha::AuthorisedValues; use Koha::BiblioFrameworks; use List::MoreUtils qw( none ); - my $minlocation=$input->param('minlocation') || ''; my $maxlocation=$input->param('maxlocation'); $maxlocation=$minlocation.'Z' unless ( $maxlocation || ! $minlocation ); my $location=$input->param('location') || ''; -my $itemtype=$input->param('itemtype'); # FIXME note, template does not currently supply this my $ignoreissued=$input->param('ignoreissued'); -my $datelastseen = $input->param('datelastseen'); -my $markseen = $input->param('markseen'); +my $datelastseen = $input->param('datelastseen'); # last inventory date my $branchcode = $input->param('branchcode') || ''; my $branch = $input->param('branch'); my $op = $input->param('op'); @@ -105,9 +101,8 @@ for my $statfield (qw/items.notforloan items.itemlost items.withdrawn items.dama } } - $template->param( statuses => $statuses ); -my $staton = {}; #authorized values that are ticked +my $staton = {}; #authorized values that are ticked for my $authvfield (@$statuses) { $staton->{$authvfield->{fieldname}} = []; for my $authval (@{$authvfield->{values}}){ @@ -130,8 +125,11 @@ $template->param( compareinv2barcd => $compareinv2barcd, ); +# Walk through uploaded barcodes, report errors, mark as seen, check in +my $results = {}; my @scanned_items; my @errorloop; +my $moddatecount = 0; if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { my $dbh = C4::Context->dbh; my $date = dt_from_string( scalar $input->param('setdate') ); @@ -142,8 +140,6 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { $strsth="select * from items where items.barcode =? and items.withdrawn = 1"; my $qwithdrawn = $dbh->prepare($strsth); - my $count = 0; - my @barcodes; my @uploadedbarcodes; @@ -190,101 +186,77 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { } else { my $item = GetItem( '', $barcode ); if ( defined $item && $item->{'itemnumber'} ) { - ModItem( { datelastseen => $date }, undef, $item->{'itemnumber'} ); - push @scanned_items, $item; - $count++; + # Modify date last seen for scanned items, remove lost status + ModItem( { itemlost => 0, datelastseen => $date }, undef, $item->{'itemnumber'} ); + $moddatecount++; + # update item hash accordingly + $item->{itemlost} = 0; + $item->{datelastseen} = $date; unless ( $dont_checkin ) { $qonloan->execute($barcode); if ($qonloan->rows){ my $data = $qonloan->fetchrow_hashref; my ($doreturn, $messages, $iteminformation, $borrower) =AddReturn($barcode, $data->{homebranch}); - if ($doreturn){ - push @errorloop, {'barcode'=>$barcode,'ERR_ONLOAN_RET'=>1} + if( $doreturn ) { + $item->{onloan} = undef; + $item->{datelastseen} = dt_from_string; } else { - push @errorloop, {'barcode'=>$barcode,'ERR_ONLOAN_NOT_RET'=>1} + push @errorloop, { barcode => $barcode, ERR_ONLOAN_NOT_RET => 1 }; } } } + push @scanned_items, $item; } else { - push @errorloop, {'barcode'=>$barcode,'ERR_BARCODE'=>1}; + push @errorloop, { barcode => $barcode, ERR_BARCODE => 1 }; } } - } - - $template->param( date => $date, Number => $count ); + $template->param( date => $date ); $template->param( errorloop => \@errorloop ) if (@errorloop); } -# now build the result list: inventoried items if requested, and mis-placed items -always- -my $inventorylist; -my $wrongplacelist; -my @items_with_problems; -if ( $markseen or $op ) { - # retrieve all items in this range. - my $totalrecords; - - # We use datelastseen only when comparing the results to the barcode file. - my $paramdatelastseen = ($compareinv2barcd) ? $datelastseen : ''; - ($inventorylist, $totalrecords) = GetItemsForInventory( { +# Build inventorylist: used as result list when you do not pass barcodes +# This list is also used when you want to compare with barcodes +my ( $inventorylist, $rightplacelist ); +if ( $op && ( !$uploadbarcodes || $compareinv2barcd )) { + ( $inventorylist ) = GetItemsForInventory({ minlocation => $minlocation, maxlocation => $maxlocation, location => $location, - itemtype => $itemtype, ignoreissued => $ignoreissued, - datelastseen => $paramdatelastseen, + datelastseen => $datelastseen, branchcode => $branchcode, branch => $branch, offset => 0, - size => undef, statushash => $staton, - } ); - - # For the items that may be marked as "wrong place", we only check the location (callnumbers, location and branch) - ($wrongplacelist, $totalrecords) = GetItemsForInventory( { + }); +} +# Build rightplacelist used to check if a scanned item is in the right place. +if( @scanned_items ) { + ( $rightplacelist ) = GetItemsForInventory({ minlocation => $minlocation, maxlocation => $maxlocation, location => $location, - itemtype => undef, ignoreissued => undef, datelastseen => undef, branchcode => $branchcode, branch => $branch, offset => 0, - size => undef, statushash => undef, - } ); - -} - -# If "compare barcodes list to results" has been checked, we want to alert for missing items -if ( $compareinv2barcd ) { - # set "missing" flags for all items with a datelastseen (dls) before the chosen datelastseen (cdls) - my $dls = output_pref( { dt => dt_from_string( $datelastseen ), - dateformat => 'iso' } ); - foreach my $item ( @$inventorylist ) { - my $cdls = output_pref( { dt => dt_from_string( $item->{datelastseen} ), - dateformat => 'iso' } ); - if ( $cdls lt $dls ) { - $item->{problem} = 'missingitem'; - # We have to push a copy of the item, not the reference - push @items_with_problems, { %$item }; - } - } + }); + # Convert the structure to a hash on barcode + $rightplacelist = { + map { $_->{barcode} ? ( $_->{barcode}, $_ ) : (); } @$rightplacelist + }; } - - -# insert "wrongplace" to all scanned items that are not supposed to be in this range -# note this list is always displayed, whatever the librarian has chosen for comparison -my $moddatecount = 0; +# Report scanned items that are on the wrong place, or have a wrong notforloan +# status, or are still checked out. foreach my $item ( @scanned_items ) { + $item->{notforloancode} = $item->{notforloan}; # save for later use - # Saving notforloan code before it's replaced by it's authorised value for later comparison - $item->{notforloancode} = $item->{notforloan}; - - # Populating with authorised values - foreach my $field ( keys %$item ) { + # Populating with authorised values + foreach my $field ( keys %$item ) { # If the koha field is mapped to a marc field my $fc = $item->{'frameworkcode'} || ''; my ($f, $sf) = GetMarcFromKohaField("items.$field", $fc); @@ -299,54 +271,57 @@ foreach my $item ( @scanned_items ) { } } - next if $item->{onloan}; # skip checked out items - # If we have scanned items with a non-matching notforloan value - if (none { $item->{'notforloancode'} eq $_ } @notforloans) { - $item->{problem} = 'changestatus'; - push @items_with_problems, { %$item }; - } - if (none { $item->{barcode} eq $_->{barcode} && !$_->{'onloan'} } @$wrongplacelist) { - $item->{problem} = 'wrongplace'; - push @items_with_problems, { %$item }; + if( none { $item->{'notforloancode'} eq $_ } @notforloans ) { + $item->{problems}->{changestatus} = 1; + additemtoresults( $item, $results ); } - # Modify date last seen for scanned items - ModDateLastSeen($item->{'itemnumber'}); - $moddatecount++; + # Report an item that is checked out (unusual!) or wrongly placed + if( $item->{onloan} ) { + $item->{problems}->{checkedout} = 1; + additemtoresults( $item, $results ); + next; # do not modify item + } elsif( !exists $rightplacelist->{ $item->{barcode} } ) { + $item->{problems}->{wrongplace} = 1; + additemtoresults( $item, $results ); + } } +# Compare barcodes with inventory list, report no_barcode and not_scanned. +# not_scanned can be interpreted as missing if ( $compareinv2barcd ) { my @scanned_barcodes = map {$_->{barcode}} @scanned_items; - for my $should_be_scanned ( @$inventorylist ) { - my $barcode = $should_be_scanned->{barcode}; - unless ( grep /^$barcode$/, @scanned_barcodes ) { - $should_be_scanned->{problem} = 'not_scanned'; - push @items_with_problems, { %$should_be_scanned }; + for my $item ( @$inventorylist ) { + my $barcode = $item->{barcode}; + if( !$barcode ) { + $item->{problems}->{no_barcode} = 1; + } elsif ( grep /^$barcode$/, @scanned_barcodes ) { + next; + } else { + $item->{problems}->{not_scanned} = 1; } + additemtoresults( $item, $results ); } } -for my $item ( @items_with_problems ) { +# Construct final results, add biblio information +my $loop = $uploadbarcodes + ? [ map { $results->{$_} } keys %$results ] + : $inventorylist // []; +for my $item ( @$loop ) { my $biblio = C4::Biblio::GetBiblioData($item->{biblionumber}); $item->{title} = $biblio->{title}; $item->{author} = $biblio->{author}; } -# If a barcode file is given, we want to show problems, else all items -my @results; -@results = $uploadbarcodes - ? @items_with_problems - : $op - ? @$inventorylist - : (); - $template->param( moddatecount => $moddatecount, - loop => \@results, - op => $op + loop => $loop, + op => $op, ); +# Export to csv if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ eval {use Text::CSV}; my $csv = Text::CSV->new or @@ -381,22 +356,27 @@ if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ print $csv->string, "\n"; my @keys = qw / title author barcode itemnumber homebranch location itemcallnumber notforloan lost damaged withdrawn stocknumber /; - for my $item ( @results ) { + for my $item ( @$loop ) { my @line; for my $key (@keys) { push @line, $item->{$key}; } - if ( defined $item->{problem} ) { - if ( $item->{problem} eq 'wrongplace' ) { - push @line, "wrong place"; - } elsif ( $item->{problem} eq 'missingitem' ) { - push @line, "missing item"; - } elsif ( $item->{problem} eq 'changestatus' ) { - push @line, "change item status"; - } elsif ($item->{problem} eq 'not_scanned' ) { - push @line, "item not scanned"; + my $errstr = ''; + foreach my $key ( keys %{$item->{problems}} ) { + if( $key eq 'wrongplace' ) { + $errstr .= "wrong place,"; + } elsif( $key eq 'changestatus' ) { + $errstr .= "unknown notforloan status,"; + } elsif( $key eq 'not_scanned' ) { + $errstr .= "missing,"; + } elsif( $key eq 'no_barcode' ) { + $errstr .= "no barcode,"; + } elsif( $key eq 'checkedout' ) { + $errstr .= "checked out,"; } } + $errstr =~ s/,$//; + push @line, $errstr; $csv->combine(@line); print $csv->string, "\n"; } @@ -414,3 +394,10 @@ if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ } output_html_with_http_headers $input, $cookie, $template->output; + +sub additemtoresults { + my ( $item, $results ) = @_; + my $itemno = $item->{itemnumber}; + # since the script appends to $item, we can just overwrite the hash entry + $results->{$itemno} = $item; +} -- 2.39.5