Code cannot rely on issueinformation being populated.
Note there is room for better efficiency to have AddReturn also provide the
itemnumber (where existing) so that GetItemnumberFromBarcode is not called
at both levels. Unfortunately there is discrepancy between this idea (for
efficiency) and the stated purpose of the $iteminformation object returned,
since $iteminformation is specifically the info from the issues table and
MUST be empty when the item was not in fact issued.
Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
+C<$iteminformation> is a reference-to-hash, giving information about the
+returned item from the issues table.
+
C<$borrower> is a reference-to-hash, giving information about the
patron who last borrowed the book.
C<$borrower> is a reference-to-hash, giving information about the
patron who last borrowed the book.
sub AddReturn {
my ( $barcode, $branch, $exemptfine, $dropbox ) = @_;
sub AddReturn {
my ( $barcode, $branch, $exemptfine, $dropbox ) = @_;
- my $dbh = C4::Context->dbh;
+ if ($branch and not GetBranchDetail($branch)) {
+ warn "AddReturn error: branch '$branch' not found. Reverting to " . C4::Context->userenv->{'branch'};
+ undef $branch;
+ }
+ $branch = C4::Context->userenv->{'branch'} unless $branch; # we trust userenv to be a safe fallback/default
# get information on item
# get information on item
- my $iteminformation = GetItemIssue( GetItemnumberFromBarcode($barcode));
- my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
+ my $itemnumber = GetItemnumberFromBarcode( $barcode );
+ my $iteminformation = GetItemIssue($itemnumber);
+ my $biblio = GetBiblioItemData($iteminformation->{'biblioitemnumber'});
# use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);
# use Data::Dumper;warn Data::Dumper::Dumper($iteminformation);
- unless ($iteminformation->{'itemnumber'} ) {
$messages->{'BadBarcode'} = $barcode;
$doreturn = 0;
} else {
$messages->{'BadBarcode'} = $barcode;
$doreturn = 0;
} else {
- # find the borrower
- if ( ( not $iteminformation->{borrowernumber} ) && $doreturn ) {
+ if ( not $iteminformation ) {
$messages->{'NotIssued'} = $barcode;
# even though item is not on loan, it may still
# be transferred; therefore, get current branch information
my $curr_iteminfo = GetItem($iteminformation->{'itemnumber'});
$messages->{'NotIssued'} = $barcode;
# even though item is not on loan, it may still
# be transferred; therefore, get current branch information
my $curr_iteminfo = GetItem($iteminformation->{'itemnumber'});
- $iteminformation->{'homebranch'} = $curr_iteminfo->{'homebranch'};
+ $iteminformation->{'homebranch'} = $curr_iteminfo->{'homebranch'};
$iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
$iteminformation->{'holdingbranch'} = $curr_iteminfo->{'holdingbranch'};
- $iteminformation->{'itemlost'} = $curr_iteminfo->{'itemlost'};
+ $iteminformation->{'itemlost'} = $curr_iteminfo->{'itemlost'};
+ # These lines patch up $iteminformation enough so it can be used below for other messages
+
+ my $hbr = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")} || '';
# check if the book is in a permanent collection....
# check if the book is in a permanent collection....
- my $hbr = $iteminformation->{C4::Context->preference("HomeOrHoldingBranch")};
- my $branches = GetBranches();
- # FIXME -- This 'PE' attribute is largely undocumented. afaict, there's no user interface that reflects this functionality.
- if ( $hbr && $branches->{$hbr}->{'PE'} ) {
- $messages->{'IsPermanent'} = $hbr;
+ # FIXME -- This 'PE' attribute is largely undocumented. afaict, there's no user interface that reflects this functionality.
+ if ( $hbr ) {
+ my $branches = GetBranches(); # a potentially expensive call for a non-feature.
+ $branches->{$hbr}->{PE} and $messages->{'IsPermanent'} = $hbr;
-
- # if independent branches are on and returning to different branch, refuse the return
- if ($hbr ne C4::Context->userenv->{'branch'} && C4::Context->preference("IndependantBranches")){
- $messages->{'Wrongbranch'} = 1;
- $doreturn=0;
- }
-
- # check that the book has been cancelled
- if ( $iteminformation->{'wthdrawn'} ) {
+
+ # if indy branches and returning to different branch, refuse the return
+ if ($hbr ne $branch && C4::Context->preference("IndependantBranches")){
+ $messages->{'Wrongbranch'} = 1;
+ $doreturn = 0;
+ }
+
+ if ( $iteminformation->{'wthdrawn'} ) { # book has been cancelled
$messages->{'wthdrawn'} = 1;
$doreturn = 0;
}
$messages->{'wthdrawn'} = 1;
$doreturn = 0;
}
-
- # new op dev : if the book returned in an other branch update the holding branch
-
- # update issues, thereby returning book (should push this out into another subroutine
+
+ # if the book returned in an other branch, update the holding branch
+ # update issues, thereby returning book (should push this out into another subroutine
$borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
$borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
-
- # case of a return of document (deal with issues and holdingbranch)
+
+ # case of a return of document (deal with issues and holdingbranch)
if ($doreturn) {
my $circControlBranch;
if ($doreturn) {
my $circControlBranch;
# don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
if (C4::Context->preference('CircControl') eq 'ItemHomeBranch' ) {
# don't allow dropbox mode to create an invalid entry in issues (issuedate > returndate) FIXME: actually checks eq, not gt
undef($dropbox) if ( $iteminformation->{'issuedate'} eq C4::Dates->today('iso') );
if (C4::Context->preference('CircControl') eq 'ItemHomeBranch' ) {
MarkIssueReturned($borrower->{'borrowernumber'}, $iteminformation->{'itemnumber'},$circControlBranch);
$messages->{'WasReturned'} = 1; # FIXME is the "= 1" right?
MarkIssueReturned($borrower->{'borrowernumber'}, $iteminformation->{'itemnumber'},$circControlBranch);
$messages->{'WasReturned'} = 1; # FIXME is the "= 1" right?
# continue to deal with returns cases, but not only if we have an issue
# the holdingbranch is updated if the document is returned in an other location .
# continue to deal with returns cases, but not only if we have an issue
# the holdingbranch is updated if the document is returned in an other location .
- if ( $iteminformation->{'holdingbranch'} ne C4::Context->userenv->{'branch'} ) {
- UpdateHoldingbranch(C4::Context->userenv->{'branch'},$iteminformation->{'itemnumber'});
- # reload iteminformation holdingbranch with the userenv value
- $iteminformation->{'holdingbranch'} = C4::Context->userenv->{'branch'};
+ if ( $iteminformation->{'holdingbranch'} ne $branch ) {
+ UpdateHoldingbranch($branch, $iteminformation->{'itemnumber'});
+ $iteminformation->{'holdingbranch'} = $branch; # update iteminformation holdingbranch too
}
ModDateLastSeen( $iteminformation->{'itemnumber'} );
ModItem({ onloan => undef }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
}
ModDateLastSeen( $iteminformation->{'itemnumber'} );
ModItem({ onloan => undef }, $biblio->{'biblionumber'}, $iteminformation->{'itemnumber'});
- if ($iteminformation->{borrowernumber}){
- ($borrower) = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 );
+ if ($iteminformation->{borrowernumber}){
+ $borrower = C4::Members::GetMemberDetails( $iteminformation->{borrowernumber}, 0 ); # FIXME: we shouldn't need to make the same call twice
}
}
# fix up the accounts.....
}
}
# fix up the accounts.....
$messages->{'WasLost'} = 1;
}
$messages->{'WasLost'} = 1;
}
- # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
- # check if we have a transfer for this document
+ # check if we have a transfer for this document
my ($datesent,$frombranch,$tobranch) = GetTransfers( $iteminformation->{'itemnumber'} );
my ($datesent,$frombranch,$tobranch) = GetTransfers( $iteminformation->{'itemnumber'} );
- # if we have a transfer to do, we update the line of transfers with the datearrived
+ # if we have a transfer to do, we update the line of transfers with the datearrived
- if ( $tobranch eq C4::Context->userenv->{'branch'} ) {
- my $sth =
- $dbh->prepare(
- "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
- );
- $sth->execute( $iteminformation->{'itemnumber'} );
- $sth->finish;
- # now we check if there is a reservation with the validate of transfer if we have one, we can set it with the status 'W'
- C4::Reserves::ModReserveStatus( $iteminformation->{'itemnumber'},'W' );
+ if ( $tobranch eq $branch ) {
+ my $sth = C4::Context->dbh->prepare(
+ "UPDATE branchtransfers SET datearrived = now() WHERE itemnumber= ? AND datearrived IS NULL"
+ );
+ $sth->execute( $iteminformation->{'itemnumber'} );
+ # if we have a reservation with the validate of transfer, we can set it's status to 'W'
+ C4::Reserves::ModReserveStatus($iteminformation->{'itemnumber'}, 'W');
+ } else {
+ $messages->{'WrongTransfer'} = $tobranch;
+ $messages->{'WrongTransferItem'} = $iteminformation->{'itemnumber'};
- else {
- $messages->{'WrongTransfer'} = $tobranch;
- $messages->{'WrongTransferItem'} = $iteminformation->{'itemnumber'};
- }
- $validTransfert = 1;
- # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
# fix up the accounts.....
if ($iteminformation->{'itemlost'}) {
# fix up the accounts.....
if ($iteminformation->{'itemlost'}) {
- FixAccountForLostAndReturned($iteminformation, $borrower);
- $messages->{'WasLost'} = 1;
+ FixAccountForLostAndReturned($iteminformation, $borrower);
+ $messages->{'WasLost'} = 1;
# fix up the overdues in accounts...
FixOverduesOnReturn( $borrower->{'borrowernumber'},
$iteminformation->{'itemnumber'}, $exemptfine, $dropbox );
# fix up the overdues in accounts...
FixOverduesOnReturn( $borrower->{'borrowernumber'},
$iteminformation->{'itemnumber'}, $exemptfine, $dropbox );
- # find reserves.....
- # if we don't have a reserve with the status W, we launch the Checkreserves routine
- my ( $resfound, $resrec ) =
- C4::Reserves::CheckReserves( $iteminformation->{'itemnumber'} );
+ # find reserves.....
+ # if we don't have a reserve with the status W, we launch the Checkreserves routine
+ my ( $resfound, $resrec ) = C4::Reserves::CheckReserves( $iteminformation->{'itemnumber'} );
- $resrec->{'ResFound'} = $resfound;
+ $resrec->{'ResFound'} = $resfound;
$messages->{'ResFound'} = $resrec;
$reserveDone = 1;
}
$messages->{'ResFound'} = $resrec;
$reserveDone = 1;
}
) {
ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
$messages->{'WasTransfered'} = 1;
) {
ModItemTransfer($iteminformation->{'itemnumber'}, C4::Context->userenv->{'branch'}, $iteminformation->{'homebranch'});
$messages->{'WasTransfered'} = 1;
$messages->{'NeedsTransfer'} = 1;
}
}
$messages->{'NeedsTransfer'} = 1;
}
}
my $borrowernumber = $query->param("bn-$counter");
$counter++;
my $borrowernumber = $query->param("bn-$counter");
$counter++;
+ # decode barcode ## Didn't we already decode them before passing them back last time??
$barcode = barcodedecode($barcode) if(C4::Context->preference('itemBarcodeInputFilter'));
######################
$barcode = barcodedecode($barcode) if(C4::Context->preference('itemBarcodeInputFilter'));
######################
my $returned = 0;
my $messages;
my $issueinformation;
my $returned = 0;
my $messages;
my $issueinformation;
my $barcode = $query->param('barcode');
my $exemptfine = $query->param('exemptfine');
my $dropboxmode = $query->param('dropboxmode');
my $barcode = $query->param('barcode');
my $exemptfine = $query->param('exemptfine');
my $dropboxmode = $query->param('dropboxmode');
my $today_iso = $today->output('iso');
my $dropboxdate = $calendar->addDate($today, -1);
if ($dotransfer){
my $today_iso = $today->output('iso');
my $dropboxdate = $calendar->addDate($today, -1);
if ($dotransfer){
- # An item has been returned to a branch other than the homebranch, and the librarian has choosen to initiate a transfer
+ # An item has been returned to a branch other than the homebranch, and the librarian has chosen to initiate a transfer
my $transferitem = $query->param('transferitem');
my $tobranch = $query->param('tobranch');
ModItemTransfer($transferitem, $userenv_branch, $tobranch);
my $transferitem = $query->param('transferitem');
my $tobranch = $query->param('tobranch');
ModItemTransfer($transferitem, $userenv_branch, $tobranch);
# actually return book and prepare item table.....
if ($barcode) {
$barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter');
# actually return book and prepare item table.....
if ($barcode) {
$barcode = barcodedecode($barcode) if C4::Context->preference('itemBarcodeInputFilter');
+ $itemnumber = GetItemnumberFromBarcode($barcode);
+
( $returned, $messages, $issueinformation, $borrower ) =
( $returned, $messages, $issueinformation, $borrower ) =
- AddReturn( $barcode, $userenv_branch, $exemptfine, $dropboxmode);
+ AddReturn( $barcode, $userenv_branch, $exemptfine, $dropboxmode); # do the return
+
- my $biblio = GetBiblioFromItemNumber($issueinformation->{'itemnumber'});
+ my $biblio = GetBiblioFromItemNumber($itemnumber);
# fix up item type for display
$biblio->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $biblio->{'itype'} : $biblio->{'itemtype'};
# fix up item type for display
$biblio->{'itemtype'} = C4::Context->preference('item-level_itypes') ? $biblio->{'itype'} : $biblio->{'itemtype'};
- $returneditems{0} = $barcode;
- $riborrowernumber{0} = $borrower->{'borrowernumber'};
- $riduedate{0} = $issueinformation->{'date_due'};
+ my $duedate = $issueinformation->{'date_due'};
+ $returneditems{0} = $barcode;
+ $riborrowernumber{0} = $borrower->{'borrowernumber'};
+ $riduedate{0} = $duedate;
$input{borrowernumber} = $borrower->{'borrowernumber'};
$input{borrowernumber} = $borrower->{'borrowernumber'};
- $input{duedate} = $issueinformation->{'date_due'};
- $input{return_overdue} = 1 if ($issueinformation->{'date_due'} lt $today->output('iso'));
+ $input{duedate} = $duedate;
+ $input{return_overdue} = 1 if ($duedate and $duedate lt $today->output('iso'));
push( @inputloop, \%input );
push( @inputloop, \%input );
-
- # check if the branch is the same as homebranch
- # if not, we want to put a message
- if ( $biblio->{'homebranch'} ne $userenv_branch ) {
- $template->param( homebranch => $biblio->{'homebranch'} );
- }
}
elsif ( !$messages->{'BadBarcode'} ) {
$input{duedate} = 0;
}
elsif ( !$messages->{'BadBarcode'} ) {
$input{duedate} = 0;
$riborrowernumber{0} = 'Item Cancelled';
}
else {
$riborrowernumber{0} = 'Item Cancelled';
}
else {
- $input{borrowernumber} = ' ';
+ $input{borrowernumber} = ' '; # This seems clearly bogus.
$riborrowernumber{0} = ' ';
}
push( @inputloop, \%input );
$riborrowernumber{0} = ' ';
}
push( @inputloop, \%input );
$template->param(
found => 1,
needstransfer => 1,
$template->param(
found => 1,
needstransfer => 1,
- itemnumber => $issueinformation->{'itemnumber'}
+ itemnumber => $itemnumber,
-# adding a case of wrong transfert, if the document wasn't transfered in the good library (according to branchtransfer (tobranch) BDD)
+# case of wrong transfert, if the document wasn't transfered to the right library (according to branchtransfer (tobranch) BDD)
if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) {
$template->param(
if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) {
$template->param(
#
# reserve found and item arrived at the expected branch
#
#
# reserve found and item arrived at the expected branch
#
# Error Messages
my @errmsgloop;
foreach my $code ( keys %$messages ) {
# Error Messages
my @errmsgloop;
foreach my $code ( keys %$messages ) {
my %err;
my $exit_required_p = 0;
if ( $code eq 'BadBarcode' ) {
my %err;
my $exit_required_p = 0;
if ( $code eq 'BadBarcode' ) {
- die "Unknown error code $code"; # XXX
+ die "Unknown error code $code"; # note we need all the (empty) elsif's above, or we die.
+ # This forces the issue of staying in sync w/ Circulation.pm
}
if (%err) {
push( @errmsgloop, \%err );
}
if (%err) {
push( @errmsgloop, \%err );
- <!-- case of a return of item, but with no reservation after, if the item must be returned to its homebranch -->
<!-- TMPL_IF Name="transfer" -->
<!-- TMPL_IF Name="transfer" -->
+ <!-- transfer: item with no reservation, must be returned to its homebranch -->
<div class="dialog message">
<div class="dialog message">
- <h3>Please return <a href="/cgi-bin/koha/catalogue/detail.pl?type=intra&biblionumber=<!-- TMPL_VAR NAME="itembiblionumber" -->"><!-- TMPL_VAR Name="title" escape="html" --></a> to <!-- TMPL_VAR Name="homebranch" --></h3></div><!-- /TMPL_IF -->
+ <h3>Please return <a href="/cgi-bin/koha/catalogue/detail.pl?type=intra&biblionumber=<!-- TMPL_VAR NAME="itembiblionumber" -->"><!-- TMPL_VAR NAME="title" escape="html" DEFAULT="item" --></a> to <!-- TMPL_VAR NAME="homebranch" DEFAULT="homebranch" --></h3></div><!-- /TMPL_IF -->
<!-- TMPL_IF Name="needstransfer" -->
<!-- TMPL_IF Name="needstransfer" -->
<div class="dialog message"><h3> This item needs to be transfered to <!-- TMPL_VAR Name="homebranch" --></h3>
Transfer Now?<br />
<form method="post" action="returns.pl" name="mainform" id="mainform">
<div class="dialog message"><h3> This item needs to be transfered to <!-- TMPL_VAR Name="homebranch" --></h3>
Transfer Now?<br />
<form method="post" action="returns.pl" name="mainform" id="mainform">