From 6347cbf5dfcd9900ad408eb5ed7e3bcdd6dd1142 Mon Sep 17 00:00:00 2001 From: Robin Sheat Date: Tue, 20 Dec 2011 17:05:17 +1300 Subject: [PATCH] Bug 7362 - allow returns by item number This allows the item number to be provided as a fallback if the item doesn't have a barcode when returning items. --- C4/Circulation.pm | 38 +++--- C4/Items.pm | 5 +- C4/SIP/ILS/Transaction/Checkin.pm | 6 +- circ/circulation.pl | 23 ++-- circ/returns.pl | 152 ++++++++++++-------- .../intranet-tmpl/prog/en/includes/cat-search.inc | 2 +- .../intranet-tmpl/prog/en/includes/circ-search.inc | 2 +- .../prog/en/includes/patron-search.inc | 2 +- .../prog/en/modules/circ/circulation.tt | 30 ++-- .../intranet-tmpl/prog/en/modules/circ/returns.tt | 25 +++- members/moremember.pl | 3 +- opac/sco/sco-main.pl | 2 +- reserve/renewscript.pl | 18 ++-- tools/inventory.pl | 2 +- 14 files changed, 176 insertions(+), 134 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 719c5bb..6cc99a2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -314,7 +314,7 @@ sub transferbook { # check if it is still issued to someone, return it... if ($issue->{borrowernumber}) { - AddReturn( $barcode, $fbr ); + AddReturn( $itemnumber, $fbr ); $messages->{'WasReturned'} = $issue->{borrowernumber}; } @@ -997,7 +997,7 @@ sub AddIssue { # This book is currently on loan, but not to the person # who wants to borrow it now. mark it returned before issuing to the new borrower AddReturn( - $item->{'barcode'}, + $item->{'itemnumber'}, C4::Context->userenv->{'branch'} ); } @@ -1419,13 +1419,13 @@ sub GetBranchItemRule { =head2 AddReturn ($doreturn, $messages, $iteminformation, $borrower) = - &AddReturn($barcode, $branch, $exemptfine, $dropbox); + &AddReturn($itemnumber, $branch, $exemptfine, $dropbox); Returns a book. =over 4 -=item C<$barcode> is the bar code of the book being returned. +=item C<$itemnumber> is the item number of the book being returned. =item C<$branch> is the code of the branch where the book is being returned. @@ -1449,13 +1449,13 @@ The keys of the hash are: =over 4 -=item C +=item C -No item with this barcode exists. The value is C<$barcode>. +No item with this itemnumber exists. The value is C<$itemnumber>. =item C -The book is not currently on loan. The value is C<$barcode>. +The book is not currently on loan. The value is C<$itemnumber>. =item C @@ -1465,7 +1465,8 @@ the book's home branch. =item C -This book has been withdrawn/cancelled. The value should be ignored. +This book has been withdrawn/cancelled. The value should be ignored. The +spelling is not a typo. =item C @@ -1491,7 +1492,7 @@ patron who last borrowed the book. =cut sub AddReturn { - my ( $barcode, $branch, $exemptfine, $dropbox ) = @_; + my ( $itemnumber, $branch, $exemptfine, $dropbox ) = @_; if ($branch and not GetBranchDetail($branch)) { warn "AddReturn error: branch '$branch' not found. Reverting to " . C4::Context->userenv->{'branch'}; undef $branch; @@ -1502,21 +1503,21 @@ sub AddReturn { my $biblio; my $doreturn = 1; my $validTransfert = 0; - my $stat_type = 'return'; + my $stat_type = 'return'; # get information on item - my $itemnumber = GetItemnumberFromBarcode( $barcode ); - unless ($itemnumber) { - return (0, { BadBarcode => $barcode }); # no barcode means no item or borrower. bail out. + my $item = GetItem($itemnumber); + unless ($item) { + return (0, { BadItemnumber => $itemnumber }); } my $issue = GetItemIssue($itemnumber); # warn Dumper($iteminformation); if ($issue and $issue->{borrowernumber}) { $borrower = C4::Members::GetMemberDetails($issue->{borrowernumber}) - or die "Data inconsistency: barcode $barcode (itemnumber:$itemnumber) claims to be issued to non-existant borrowernumber '$issue->{borrowernumber}'\n" + or die "Data inconsistency: itemnumber:$itemnumber claims to be issued to non-existant borrowernumber '$issue->{borrowernumber}'\n" . Dumper($issue) . "\n"; } else { - $messages->{'NotIssued'} = $barcode; + $messages->{'NotIssued'} = $itemnumber; # even though item is not on loan, it may still be transferred; therefore, get current branch info $doreturn = 0; # No issue, no borrowernumber. ONLY if $doreturn, *might* you have a $borrower later. @@ -1527,9 +1528,6 @@ sub AddReturn { } } - my $item = GetItem($itemnumber) or die "GetItem($itemnumber) failed"; - # full item data, but no borrowernumber or checkout info (no issue) - # we know GetItem should work because GetItemnumberFromBarcode worked my $hbr = C4::Context->preference("HomeOrHoldingBranchReturn") || "homebranch"; $hbr = $item->{$hbr} || ''; # item must be from items table -- issues table has branchcode and issuingbranch, not homebranch nor holdingbranch @@ -1611,7 +1609,7 @@ sub AddReturn { # fix up the accounts..... if ($item->{'itemlost'}) { - _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $barcode); # can tolerate undef $borrowernumber + _FixAccountForLostAndReturned($item->{'itemnumber'}, $borrowernumber, $item->{'barcode'}); # can tolerate undef $borrowernumber $messages->{'WasLost'} = 1; } @@ -1619,7 +1617,7 @@ sub AddReturn { if ($borrowernumber) { my $fix = _FixOverduesOnReturn($borrowernumber, $item->{itemnumber}, $exemptfine, $dropbox); defined($fix) or warn "_FixOverduesOnReturn($borrowernumber, $item->{itemnumber}...) failed!"; # zero is OK, check defined - + # fix fine days my $debardate = _FixFineDaysOnReturn( $borrower, $item, $issue->{date_due} ); $messages->{'Debarred'} = $debardate if ($debardate); diff --git a/C4/Items.pm b/C4/Items.pm index 8802a4c..1d97f39 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -2,6 +2,7 @@ package C4::Items; # Copyright 2007 LibLime, Inc. # Parts Copyright Biblibre 2010 +# Copyright 2011 Catalyst IT # # This file is part of Koha. # @@ -134,6 +135,8 @@ Return item information, for a given itemnumber or barcode. The return value is a hashref mapping item column names to values. If C<$serial> is true, include serial publication data. +If the requested item doesn't exist, C is returned. + =cut sub GetItem { @@ -160,7 +163,7 @@ sub GetItem { ($data->{'serialseq'} , $data->{'publisheddate'}) = $ssth->fetchrow_array(); } #if we don't have an items.itype, use biblioitems.itemtype. - if( ! $data->{'itype'} ) { + if( $data && ! $data->{'itype'} ) { my $sth = $dbh->prepare("SELECT itemtype FROM biblioitems WHERE biblionumber = ?"); $sth->execute($data->{'biblionumber'}); ($data->{'itype'}) = $sth->fetchrow_array; diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm index d3a4700..79d3ab7 100644 --- a/C4/SIP/ILS/Transaction/Checkin.pm +++ b/C4/SIP/ILS/Transaction/Checkin.pm @@ -45,9 +45,9 @@ sub new { sub do_checkin { my $self = shift; my $branch = @_ ? shift : 'SIP2' ; - my $barcode = $self->{item}->id; - $debug and warn "do_checkin() calling AddReturn($barcode, $branch)"; - my ($return, $messages, $iteminformation, $borrower) = AddReturn($barcode, $branch); + my $itemnumber = $self->{item}->{'itemnumber'}; + $debug and warn "do_checkin() calling AddReturn($itemnumber, $branch)"; + my ($return, $messages, $iteminformation, $borrower) = AddReturn($itemnumber, $branch); $self->alert(!$return); # ignoring messages: NotIssued, IsPermanent, WasLost, WasTransfered diff --git a/circ/circulation.pl b/circ/circulation.pl index d800525..f58e302 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -92,7 +92,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user ( my $branches = GetBranches(); -my @failedrenews = $query->param('failedrenew'); # expected to be itemnumbers +my @failedrenews = $query->param('failedrenew'); # expected to be itemnumbers my %renew_failed; for (@failedrenews) { $renew_failed{$_} = 1; } @@ -665,7 +665,7 @@ $template->param( borrower => $borrower, borrowernumber => $borrowernumber, branch => $branch, - branchname => GetBranchName($borrower->{'branchcode'}), + branchname => GetBranchName( $borrower->{'branchcode'} ), printer => $printer, printername => $printer, firstname => $borrower->{'firstname'}, @@ -673,7 +673,7 @@ $template->param( showname => $borrower->{'showname'}, category_type => $borrower->{'category_type'}, dateexpiry => format_date($newexpiry), - expiry => format_date($borrower->{'dateexpiry'}), + expiry => format_date( $borrower->{'dateexpiry'} ), categorycode => $borrower->{'categorycode'}, categoryname => $borrower->{description}, address => $address, @@ -682,7 +682,7 @@ $template->param( emailpro => $borrower->{'emailpro'}, borrowernotes => $borrower->{'borrowernotes'}, city => $borrower->{'city'}, - state => $borrower->{'state'}, + state => $borrower->{'state'}, zipcode => $borrower->{'zipcode'}, country => $borrower->{'country'}, phone => $borrower->{'phone'} || $borrower->{'mobile'}, @@ -694,20 +694,21 @@ $template->param( duedatespec => $duedatespec, message => $message, CGIselectborrower => $CGIselectborrower, - totalprice => sprintf('%.2f', $totalprice), - totaldue => sprintf('%.2f', $total), + totalprice => sprintf( '%.2f', $totalprice ), + totaldue => sprintf( '%.2f', $total ), todayissues => \@todaysissues, previssues => \@previousissues, - relissues => \@relissues, - relprevissues => \@relprevissues, - displayrelissues => $displayrelissues, + relissues => \@relissues, + relprevissues => \@relprevissues, + displayrelissues => $displayrelissues, inprocess => $inprocess, memberofinstution => $member_of_institution, CGIorganisations => $CGIorganisations, - is_child => ($borrower->{'category_type'} eq 'C'), - circview => 1, + is_child => ( $borrower->{'category_type'} eq 'C' ), + circview => 1, soundon => C4::Context->preference("SoundOn"), fast_cataloging => $fast_cataloging, + can_be_itemnumber => C4::Context->preference('CircFallbackItemnumber'), ); # save stickyduedate to session diff --git a/circ/returns.pl b/circ/returns.pl index 98d310e..24d0c83 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -3,7 +3,7 @@ # Copyright 2000-2002 Katipo Communications # 2006 SAN-OP # 2007-2010 BibLibre, Paul POULAIN -# 2010 Catalyst IT +# 2010-2011 Catalyst IT # # This file is part of Koha. # @@ -86,6 +86,7 @@ my $userenv_branch = C4::Context->userenv->{'branch'} || ''; my %returneditems; my %riduedate; my %riborrowernumber; +my %ribarcode; my @inputloop; foreach ( $query->param ) { my $counter; @@ -99,24 +100,22 @@ foreach ( $query->param ) { next; } + # These "slide up" the list of returned items, to make room for the one + # being processed now. my %input; - my $barcode = $query->param("ri-$counter"); + my $itemnumber = $query->param("ri-$counter"); + my $barcode = $query->param("bc-$counter"); my $duedate = $query->param("dd-$counter"); my $borrowernumber = $query->param("bn-$counter"); $counter++; - # decode barcode ## Didn't we already decode them before passing them back last time?? - $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace - $barcode = barcodedecode($barcode) if(C4::Context->preference('itemBarcodeInputFilter')); - - ###################### - #Are these lines still useful ? - $returneditems{$counter} = $barcode; + $returneditems{$counter} = $itemnumber; $riduedate{$counter} = $duedate; $riborrowernumber{$counter} = $borrowernumber; + $ribarcode{$counter} = $barcode; - ####################### $input{counter} = $counter; + $input{itemnumber} = $itemnumber; $input{barcode} = $barcode; $input{duedate} = $duedate; $input{borrowernumber} = $borrowernumber; @@ -168,6 +167,8 @@ my $returned = 0; my $messages; my $issueinformation; my $itemnumber; +# Depending on the setting of CircItemnumberFallback, this may also contain +# the itemnumber. my $barcode = $query->param('barcode'); my $exemptfine = $query->param('exemptfine'); my $dropboxmode = $query->param('dropboxmode'); @@ -198,21 +199,30 @@ if ($canceltransfer){ } # actually return book and prepare item table..... +my $item; if ($barcode) { $barcode =~ s/^\s*|\s*$//g; # remove leading/trailing whitespace + my $orig_barcode = $barcode; $barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter'); $itemnumber = GetItemnumberFromBarcode($barcode); - - if ( C4::Context->preference("InProcessingToShelvingCart") ) { - my $item = GetItem( $itemnumber ); + if (!$itemnumber && C4::Context->preference('CircFallbackItemnumber')) { + # See if we have a real item number instead of a barcode + $item = GetItem($orig_barcode); + $itemnumber = $item->{itemnumber} if $item; + } + else { + $item = GetItem($itemnumber); + } + # If the provided item doesn't exist, then we will fall through and + # this problem is picked up by AddReturn. + if ( C4::Context->preference("InProcessingToShelvingCart") && $item ) { if ( $item->{'location'} eq 'PROC' ) { $item->{'location'} = 'CART'; ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} ); } } - if ( C4::Context->preference("ReturnToShelvingCart") ) { - my $item = GetItem( $itemnumber ); + if ( C4::Context->preference("ReturnToShelvingCart") && $item) { $item->{'location'} = 'CART'; ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} ); } @@ -221,7 +231,7 @@ if ($barcode) { # save the return # ( $returned, $messages, $issueinformation, $borrower ) = - AddReturn( $barcode, $userenv_branch, $exemptfine, $dropboxmode); # do the return + AddReturn( $itemnumber, $userenv_branch, $exemptfine, $dropboxmode ); my $homeorholdingbranchreturn = C4::Context->preference('HomeOrHoldingBranchReturn') or 'homebranch'; # get biblio description @@ -237,23 +247,27 @@ if ($barcode) { itembarcode => $biblio->{'barcode'}, itemtype => $biblio->{'itemtype'}, ccode => $biblio->{'ccode'}, - itembiblionumber => $biblio->{'biblionumber'}, + itembiblionumber => $biblio->{'biblionumber'}, + itemnumber => $itemnumber, ); my %input = ( - counter => 0, - first => 1, - barcode => $barcode, + counter => 0, + first => 1, + barcode => $biblio->{barcode}, + itemnumber => $itemnumber, ); if ($returned) { my $duedate = $issueinformation->{'date_due'}; - $returneditems{0} = $barcode; + $returneditems{0} = $itemnumber; $riborrowernumber{0} = $borrower->{'borrowernumber'}; $riduedate{0} = $duedate; + $ribarcode{0} = $biblio->{barcode}; $input{borrowernumber} = $borrower->{'borrowernumber'}; $input{duedate} = $duedate; - $input{return_overdue} = 1 if ($duedate and $duedate lt $today->output('iso')); + $input{return_overdue} = 1 + if ( $duedate and $duedate lt $today->output('iso') ); push( @inputloop, \%input ); if ( C4::Context->preference("FineNotifyAtCheckin") ) { @@ -283,9 +297,9 @@ if ($barcode) { } } } - elsif ( !$messages->{'BadBarcode'} ) { + elsif ( !$messages->{'BadItemnumber'} ) { $input{duedate} = 0; - $returneditems{0} = $barcode; + $returneditems{0} = $itemnumber; $riduedate{0} = 0; if ( $messages->{'wthdrawn'} ) { $input{withdrawn} = 1; @@ -376,10 +390,13 @@ if ( $messages->{'ResFound'}) { ); } elsif ( $reserve->{'ResFound'} eq "Reserved" ) { $template->param( - intransit => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), - transfertodo => ($userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), - resbarcode => $barcode, - reserved => 1, + intransit => + ( $userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), + transfertodo => + ( $userenv_branch eq $reserve->{'branchcode'} ? 0 : 1 ), + resbarcode => $item->{barcode}, + resitemnumber => $item->{itemnumber}, + reserved => 1, ); } @@ -401,7 +418,7 @@ if ( $messages->{'ResFound'}) { borcnum => $borr->{'cardnumber'}, debarred => $borr->{'debarred'}, gonenoaddress => $borr->{'gonenoaddress'}, - barcode => $barcode, + barcode => $reserve->{'barcode'}, destbranch => $reserve->{'branchcode'}, borrowernumber => $reserve->{'borrowernumber'}, itemnumber => $reserve->{'itemnumber'}, @@ -415,9 +432,9 @@ my @errmsgloop; foreach my $code ( keys %$messages ) { my %err; my $exit_required_p = 0; - if ( $code eq 'BadBarcode' ) { + if ( $code eq 'BadItemnumber' ) { $err{badbarcode} = 1; - $err{msg} = $messages->{'BadBarcode'}; + $err{msg} = $messages->{'BadItemnumber'} || $barcode; } elsif ( $code eq 'NotIssued' ) { $err{notissued} = 1; @@ -499,12 +516,15 @@ if ($borrower) { my $items = $flags->{$flag}->{'itemlist'}; foreach my $item (@$items) { my $biblio = GetBiblioFromItemNumber( $item->{'itemnumber'}); - push @waitingitemloop, { - biblionum => $biblio->{'biblionumber'}, - barcode => $biblio->{'barcode'}, - title => $biblio->{'title'}, - brname => $branches->{ $biblio->{'holdingbranch'} }->{'branchname'}, - }; + push @waitingitemloop, + { + biblionum => $biblio->{'biblionumber'}, + barcode => $biblio->{'barcode'}, + itemnumber => $item->{'itemnumber'}, + title => $biblio->{'title'}, + brname => + $branches->{ $biblio->{'holdingbranch'} }->{'branchname'}, + }; } $flaginfo{itemloop} = \@waitingitemloop; } @@ -515,13 +535,16 @@ if ($borrower) { @$items ) { my $biblio = GetBiblioFromItemNumber( $item->{'itemnumber'}); - push @itemloop, { - duedate => format_date($item->{'date_due'}), - biblionum => $biblio->{'biblionumber'}, - barcode => $biblio->{'barcode'}, - title => $biblio->{'title'}, - brname => $branches->{ $biblio->{'holdingbranch'} }->{'branchname'}, - }; + push @itemloop, + { + duedate => format_date( $item->{'date_due'} ), + biblionum => $biblio->{'biblionumber'}, + barcode => $biblio->{'barcode'}, + itemnumber => $item->{'itemnumber'}, + title => $biblio->{'title'}, + brname => + $branches->{ $biblio->{'holdingbranch'} }->{'branchname'}, + }; } $flaginfo{itemloop} = \@itemloop; $flaginfo{overdue} = 1; @@ -551,7 +574,7 @@ my $shelflocations = GetKohaAuthorisedValues('items.location',''); foreach ( sort { $a <=> $b } keys %returneditems ) { my %ri; if ( $count++ < $returned_counter ) { - my $bar_code = $returneditems{$_}; + my $returned_item = $returneditems{$_}; my $duedate = $riduedate{$_}; if ($duedate) { my @tempdate = split( /-/, $duedate ); @@ -574,9 +597,13 @@ foreach ( sort { $a <=> $b } keys %returneditems ) { } # my %ri; - my $biblio = GetBiblioFromItemNumber(GetItemnumberFromBarcode($bar_code)); + my $biblio = GetBiblioFromItemNumber($returned_item); + # fix up item type for display - $biblio->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $biblio->{'itype'} : $biblio->{'itemtype'}; + $biblio->{'itemtype'} = + C4::Context->preference('item-level_itypes') + ? $biblio->{'itype'} + : $biblio->{'itemtype'}; $ri{itembiblionumber} = $biblio->{'biblionumber'}; $ri{itemtitle} = $biblio->{'title'}; $ri{itemauthor} = $biblio->{'author'}; @@ -584,12 +611,14 @@ foreach ( sort { $a <=> $b } keys %returneditems ) { $ri{itemtype} = $biblio->{'itemtype'}; $ri{itemnote} = $biblio->{'itemnotes'}; $ri{ccode} = $biblio->{'ccode'}; + $ri{barcode} = $biblio->{'barcode'}; $ri{itemnumber} = $biblio->{'itemnumber'}; - $ri{barcode} = $bar_code; - $ri{location} = $biblio->{'location'}; my $shelfcode = $ri{'location'}; - $ri{'location'} = $shelflocations->{$shelfcode} if ( defined( $shelfcode ) && defined($shelflocations) && exists( $shelflocations->{$shelfcode} ) ); + $ri{'location'} = $shelflocations->{$shelfcode} + if ( defined($shelfcode) + && defined($shelflocations) + && exists( $shelflocations->{$shelfcode} ) ); } else { @@ -599,17 +628,18 @@ foreach ( sort { $a <=> $b } keys %returneditems ) { } $template->param( - riloop => \@riloop, - genbrname => $branches->{$userenv_branch}->{'branchname'}, - genprname => $printers->{$printer}->{'printername'}, - branchname => $branches->{$userenv_branch}->{'branchname'}, - printer => $printer, - errmsgloop => \@errmsgloop, - exemptfine => $exemptfine, - dropboxmode => $dropboxmode, - dropboxdate => $dropboxdate->output(), - overduecharges => $overduecharges, - soundon => C4::Context->preference("SoundOn"), + riloop => \@riloop, + genbrname => $branches->{$userenv_branch}->{'branchname'}, + genprname => $printers->{$printer}->{'printername'}, + branchname => $branches->{$userenv_branch}->{'branchname'}, + printer => $printer, + errmsgloop => \@errmsgloop, + exemptfine => $exemptfine, + dropboxmode => $dropboxmode, + dropboxdate => $dropboxdate->output(), + overduecharges => $overduecharges, + soundon => C4::Context->preference("SoundOn"), + can_be_itemnumber => C4::Context->preference('CircFallbackItemnumber'), ); ### Comment out rotating collections for now to allow it a little more time to bake diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc index 379cc56..2f019e4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc @@ -71,7 +71,7 @@ YAHOO.util.Event.onContentReady("header_search", function() { [% IF ( CAN_user_circulate ) %]