From 4c420a6c284af30270765d0d93298a16383399c2 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 6 Jun 2013 14:37:19 +0200 Subject: [PATCH] Bug 7684: QA Followup and bugfixes This followup fixes some QA issues: - replace the MySQLism SQL_CALC_FOUND_ROWS - use Koha::DateUtils instead of C4::Dates - replace "branch" and "location" with "library" - fixe wrong capitalisation on "Clear all" and "Select all" and fixes some behaviors: - the inventory tools can be used without barcode file (fixed for the csv export too). - mark as not scanned a non scanned item. - update the datelastseen 1 time per biblio (and fixes the displayed count) Signed-off-by: Mathieu Saby Signed-off-by: Koha Team Amu Signed-off-by: Kyle M Hall --- C4/Items.pm | 24 ++-- .../prog/en/modules/tools/inventory.tt | 75 +++++---- tools/ajax-inventory.pl | 22 +--- tools/inventory.pl | 167 ++++++++++---------- 4 files changed, 141 insertions(+), 147 deletions(-) diff --git a/C4/Items.pm b/C4/Items.pm index f364a48..a5e22e2 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -997,12 +997,15 @@ sub GetItemsForInventory { my $dbh = C4::Context->dbh; my ( @bind_params, @where_strings ); - my $query = <<'END_SQL'; -SELECT SQL_CALC_FOUND_ROWS items.itemnumber, barcode, itemcallnumber, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, stocknumber -FROM items - LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber - LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber -END_SQL + my $select_columns = q{ + SELECT items.itemnumber, barcode, itemcallnumber, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, stocknumber + }; + my $select_count = q{SELECT COUNT(*)}; + my $query = q{ + FROM items + LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber + LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber + }; if ($statushash){ for my $authvfield (keys %$statushash){ if ( scalar @{$statushash->{$authvfield}} > 0 ){ @@ -1057,18 +1060,19 @@ END_SQL $query .= join ' AND ', @where_strings; } $query .= ' ORDER BY items.cn_sort, itemcallnumber, title'; + my $count_query = $select_count . $query; $query .= " LIMIT $offset, $size" if ($offset and $size); + $query = $select_columns . $query; my $sth = $dbh->prepare($query); $sth->execute( @bind_params ); - my @results; + my @results = (); my $tmpresults = $sth->fetchall_arrayref({}); - $sth = $dbh->prepare("SELECT FOUND_ROWS()"); - $sth->execute(); + $sth = $dbh->prepare( $count_query ); + $sth->execute( @bind_params ); my ($iTotalRecords) = $sth->fetchrow_array(); foreach my $row (@$tmpresults) { - $row->{datelastseen} = format_date( $row->{datelastseen} ); # Auth values foreach (keys %$row) { 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 1052714..3da9df7 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt @@ -1,3 +1,4 @@ +[% USE KohaDates %] [% INCLUDE 'doc-head-open.inc' %] Koha › Tools › Inventory [% INCLUDE 'doc-head-close.inc' %] @@ -12,12 +13,16 @@ $(document).ready(function(){ inventorydt = $('#inventoryt').dataTable($.extend(true, {}, dataTablesDefaults, { 'sPaginationType': 'full_numbers', - "aoColumnDefs": [ { "bSortable": false, "aTargets": [ 0 ] } ] + [% IF compareinv2barcd %] + "aoColumnDefs": [ { "bSortable": false, "aTargets": [ 1 ] } ] + [% ELSE %] + "aoColumnDefs": [ { "bSortable": false, "aTargets": [ 0 ] } ] + [% END %] } )); - $("#continuewithoutmarkingbutton").click(function(){ - inventorydt.fnPageChange( 'next' ); + $("#continuewithoutmarkingbutton").click(function(){ + inventorydt.fnPageChange( 'next' ); return false; }); @@ -47,7 +52,6 @@ $(document).ready(function(){ }); - $(".checkall").click(function(){ $(".checkboxed").checkCheckboxes(); return false; @@ -56,16 +60,6 @@ $(document).ready(function(){ $(".checkboxed").unCheckCheckboxes(); return false; }); -[% IF ( offset ) %]$("#markseen").before(" ");[% END %] -[% IF ( nextoffset ) %]$("#markseen").after(" ");[% END %] - $("#markback").click(function(){ - $(".checkboxed").find("input").filter("[name=offset]").attr("value","[% prevoffset %]"); - return true; - }); - $("#marknext").click(function(){ - $(".checkboxed").find("input").filter("[name=offset]").attr("value","[% nextoffset %]"); - return true; - }); }); //]]> @@ -82,7 +76,7 @@ $(document).ready(function(){

Inventory/Stocktaking

- [% IF (moddatecount) %]
[% moddatecount %] items modified : datelastseen set to [% date %]
[% END %] + [% IF (moddatecount) %]
[% moddatecount %] items modified : datelastseen set to [% date | $KohaDates %]
[% END %] [% IF (errorfile) %]
[% errorfile %] can't be opened
[% END %] [% FOREACH error IN errorloop %]
@@ -100,7 +94,7 @@ $(document).ready(function(){ Use a barcode file
    2. -
  2. +
@@ -109,10 +103,10 @@ $(document).ready(function(){
  1. Home library - Current location + Current library
  2. +
  3. [% IF (ignoreissued) %] @@ -204,15 +198,17 @@ $(document).ready(function(){ - - - + + [% UNLESS compareinv2barcd %] + + [% END %] + - + [% UNLESS compareinv2barcd %][% END %] - + @@ -224,9 +220,11 @@ $(document).ready(function(){ [% FOREACH result IN loop %] - + [% UNLESS compareinv2barcd %] + + [% END %] @@ -246,21 +244,30 @@ $(document).ready(function(){ [% result.damaged | html %] [% END %]
    SeenSeenBarcodeLocationLibrary Title Status Lost
    - - + + [% result.barcode | html %] - [% result.datelastseen | html %] + [% result.datelastseen | html | $KohaDates%] - [% IF (result.wrongplace) %]

    Item should not have been scanned

    [% ELSIF (result.missingitem) %]

    Item missing

    [% ELSIF (result.changestatus) %]

    Change item status

    [% END %] + [% IF result.problem == 'wrongplace' %] +

    Item should not have been scanned

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

    Item missing

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

    Change item status

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

    Item should have been scanned

    + [% END %]
    -

    - - - - + [% UNLESS compareinv2barcd %] + + + + + [% END %]
diff --git a/tools/ajax-inventory.pl b/tools/ajax-inventory.pl index 24739b9..63f371a 100755 --- a/tools/ajax-inventory.pl +++ b/tools/ajax-inventory.pl @@ -1,27 +1,9 @@ #!/usr/bin/perl -# use Modern::Perl; - use CGI; -use JSON; - use C4::Auth; -use C4::Circulation qw/CanBookBeRenewed/; -use C4::Context; -use C4::Koha qw/getitemtypeimagelocation/; -use C4::Reserves qw/CheckReserves/; -use C4::Utils::DataTables; -use C4::Output; -use C4::Biblio; -use C4::Items; -use C4::Dates qw/format_date format_date_in_iso/; -use C4::Koha; -use C4::Branch; # GetBranches -use C4::Reports::Guided; #_get_column_defs -use C4::Charset; -use List::MoreUtils qw /none/; - +use C4::Items qw( ModDateLastSeen ); my $input = new CGI; @@ -38,6 +20,4 @@ foreach ( @seent ) { /SEEN-(.+)/ and &ModDateLastSeen($1); } - print $input->header('application/json'); - diff --git a/tools/inventory.pl b/tools/inventory.pl index 9a80017..dbd73ac 100755 --- a/tools/inventory.pl +++ b/tools/inventory.pl @@ -31,13 +31,13 @@ use C4::Context; use C4::Output; use C4::Biblio; use C4::Items; -use C4::Dates qw/format_date format_date_in_iso/; use C4::Koha; use C4::Branch; # GetBranches use C4::Circulation; use C4::Reports::Guided; #_get_column_defs use C4::Charset; -use List::MoreUtils qw/none/; +use Koha::DateUtils; +use List::MoreUtils qw( none ); my $minlocation=$input->param('minlocation') || ''; @@ -47,19 +47,14 @@ 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 $offset = $input->param('offset'); my $markseen = $input->param('markseen'); -$offset=0 unless $offset; -my $pagesize = $input->param('pagesize'); -$pagesize=50 unless $pagesize; my $branchcode = $input->param('branchcode') || ''; my $branch = $input->param('branch'); my $op = $input->param('op'); my $compareinv2barcd = $input->param('compareinv2barcd'); -my $res; #contains the results loop my ( $template, $borrowernumber, $cookie ) = get_template_and_user( - { template_name => "tools/inventory.tmpl", + { template_name => "tools/inventory.tt", query => $input, type => "intranet", authnotrequired => 0, @@ -134,15 +129,13 @@ $statussth =~ s, and $,,g; $template->param( branchloop => \@branch_loop, authorised_values => \@authorised_value_list, - today => C4::Dates->today(), + today => dt_from_string, minlocation => $minlocation, maxlocation => $maxlocation, location => $location, ignoreissued => $ignoreissued, branchcode => $branchcode, branch => $branch, - offset => $offset, - pagesize => $pagesize, datelastseen => $datelastseen, compareinv2barcd => $compareinv2barcd, notforloanlist => $notforloanlist @@ -153,14 +146,12 @@ if (defined $notforloanlist) { @notforloans = split(/,/, $notforloanlist); } - - -my @brcditems; -my $barcodelist; +my @scanned_items; my @errorloop; if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { my $dbh = C4::Context->dbh; - my $date = format_date_in_iso( $input->param('setdate') ) || C4::Dates->today('iso'); + my $date = dt_from_string( $input->param('setdate') ); + $date = output_pref( $date, 'iso' ); my $strsth = "select * from issues, items where items.itemnumber=issues.itemnumber and items.barcode =?"; my $qonloan = $dbh->prepare($strsth); @@ -171,14 +162,14 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { while (my $barcode=<$uploadbarcodes>){ $barcode =~ s/\r?\n$//; - $barcodelist .= ($barcodelist) ? '|' . $barcode : $barcode; + next unless $barcode; if ( $qwithdrawn->execute($barcode) && $qwithdrawn->rows ) { push @errorloop, { 'barcode' => $barcode, 'ERR_WTHDRAWN' => 1 }; } else { my $item = GetItem( '', $barcode ); if ( defined $item && $item->{'itemnumber'} ) { ModItem( { datelastseen => $date }, undef, $item->{'itemnumber'} ); - push @brcditems, $item; + push @scanned_items, $item; $count++; $qonloan->execute($barcode); if ($qonloan->rows){ @@ -196,96 +187,103 @@ if ( $uploadbarcodes && length($uploadbarcodes) > 0 ) { } } - $qonloan->finish; - $qwithdrawn->finish; - $template->param( date => format_date($date), Number => $count ); - $template->param( errorloop => \@errorloop ) if (@errorloop); + $template->param( date => $date, Number => $count ); + $template->param( errorloop => \@errorloop ) if (@errorloop); } -$template->param(barcodelist => $barcodelist); # now build the result list: inventoried items if requested, and mis-placed items -always- my $inventorylist; +my @items_with_problems; if ( $markseen or $op ) { # retrieve all items in this range. my $totalrecords; ($inventorylist, $totalrecords) = GetItemsForInventory($minlocation, $maxlocation, $location, $itemtype, $ignoreissued, '', $branchcode, $branch, 0, undef , $staton); +} - # Real copy - my @res_copy; - foreach (@$inventorylist) { - push @res_copy, $_; +# 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 choosen datelastseen (cdls) + my $dls = output_pref( dt_from_string( $datelastseen ), 'iso' ); + foreach my $item ( @$inventorylist ) { + my $cdls = output_pref( dt_from_string( $_->{datelastseen} ), '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 }; + } } - $res = \@res_copy; } -# set "missing" flags for all items with a datelastseen before the choosen datelastseen -foreach (@$res) { $_->{missingitem}=1 if C4::Dates->new($_->{datelastseen})->output('iso') lt C4::Dates->new($datelastseen)->output('iso'); } -# removing missing items from loop if "Compare barcodes list to results" has not been checked -@$res = grep {!$_->{missingitem} == 1 } @$res if (!$input->param('compareinv2barcd')); # 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 choosen for comparison -foreach my $temp (@brcditems) { +my $moddatecount = 0; +foreach my $item ( @scanned_items ) { # Saving notforloan code before it's replaced by it's authorised value for later comparison - $temp->{'notforloancode'} = $temp->{'notforloan'}; + $item->{notforloancode} = $item->{notforloan}; # Populating with authorised values - foreach (keys %$temp) { + foreach my $field ( keys %$item ) { # If the koha field is mapped to a marc field - my $fc = $temp->{'frameworkcode'} || ''; - my ($f, $sf) = GetMarcFromKohaField("items.$_", $fc); + my $fc = $item->{'frameworkcode'} || ''; + my ($f, $sf) = GetMarcFromKohaField("items.$field", $fc); if ($f and $sf) { # We replace the code with it's description my $authvals = C4::Koha::GetKohaAuthorisedValuesFromField($f, $sf, $fc); - if ($authvals and defined $temp->{$_} and defined $authvals->{$temp->{$_}}) { - $temp->{$_} = $authvals->{$temp->{$_}}; + if ($authvals and defined $item->{$field} and defined $authvals->{$item->{$field}}) { + $item->{$field} = $authvals->{$item->{$field}}; } } } - next if $temp->{onloan}; # skip checked out items + next if $item->{onloan}; # skip checked out items # If we have scanned items with a non-matching notforloan value - if (none { $temp->{'notforloancode'} eq $_ } @notforloans) { - $temp->{'changestatus'} = 1; - my $biblio = C4::Biblio::GetBiblioData($temp->{biblionumber}); - $temp->{title} = $biblio->{title}; - $temp->{author} = $biblio->{author}; - $temp->{datelastseen} = format_date($temp->{datelastseen}); - push @$res, $temp; - + if (none { $item->{'notforloancode'} eq $_ } @notforloans) { + $item->{problem} = 'changestatus'; + push @items_with_problems, { %$item }; } - if (none { $temp->{barcode} eq $_->{barcode} && !$_->{'onloan'} } @$inventorylist) { - my $temp2 = { %$temp }; - $temp2->{wrongplace}=1; - my $biblio = C4::Biblio::GetBiblioData($temp->{biblionumber}); - $temp2->{title} = $biblio->{title}; - $temp2->{author} = $biblio->{author}; - $temp2->{datelastseen} = format_date($temp->{datelastseen}); - push @$res, $temp2; + if (none { $item->{barcode} eq $_->{barcode} && !$_->{'onloan'} } @$inventorylist) { + $item->{problem} = 'wrongplace'; + push @items_with_problems, { %$item }; } + + # Modify date last seen for scanned items + ModDateLastSeen($_->{'itemnumber'}); + $moddatecount++; } -# Finally, modifying datelastseen for remaining items -my $moddatecount = 0; -foreach (@$res) { - unless ($_->{'missingitem'}) { - ModDateLastSeen($_->{'itemnumber'}); - $moddatecount++; +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 }; + } } } -# Removing items that don't have any problems from loop -@$res = grep { $_->{missingitem} || $_->{wrongplace} || $_->{changestatus} } @$res; +for my $item ( @items_with_problems ) { + 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 => $res, - nextoffset => ( $offset + $pagesize ), - prevoffset => ( $offset ? $offset - $pagesize : 0 ), + loop => \@results, op => $op ); @@ -316,35 +314,40 @@ if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){ / ) { push @translated_keys, $columns_def_hashref->{$key}; } + push @translated_keys, 'problem' if $uploadbarcodes; $csv->combine(@translated_keys); print $csv->string, "\n"; my @keys = qw / title author barcode itemnumber homebranch location itemcallnumber notforloan lost damaged stocknumber /; - for my $re (@$res) { + for my $item ( @results ) { my @line; for my $key (@keys) { - push @line, $re->{$key}; + push @line, $item->{$key}; } - if ($re->{wrongplace}) { - push @line, "wrong place"; - } elsif ($re->{missingitem}) { - push @line, "missing item"; - } elsif ($re->{changestatus}) { - push @line, "change item status"; + 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"; + } } $csv->combine(@line); print $csv->string, "\n"; } # Adding not found barcodes foreach my $error (@errorloop) { - my @line; - if ($error->{'ERR_BARCODE'}) { - push @line, map { $_ eq 'barcode' ? $error->{'barcode'} : ''} @keys; - push @line, "barcode not found"; - $csv->combine(@line); - print $csv->string, "\n"; - } + my @line; + if ($error->{'ERR_BARCODE'}) { + push @line, map { $_ eq 'barcode' ? $error->{'barcode'} : ''} @keys; + push @line, "barcode not found"; + $csv->combine(@line); + print $csv->string, "\n"; + } } exit; } -- 1.7.2.5