From b6385330c730619adffd200c4f1054c771ed3674 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 24 Nov 2014 09:53:00 -0500 Subject: [PATCH] Bug 13330 - All overdue items with fines returned via checkouts table will have fines forgiven! Due to a disconnect between how booleans are handled in Perl vs JSON, all checkins done from the checkouts table will have fines forgiven! This is because the parameters sent to svc/checkin are coded in JSON and the JSON false is treated in Perl as the string 'false' which evaluates to true! Test Plan: 1) Find a patron with an overdue item that has a fine 2) Return the item via the checkouts table on circulation.pl or moremember.pl 3) Note the fine is forgiven 4) Apply this patch 5) Repeat steps 1-2 6) Note the fine is no longer forgiven --- C4/Circulation.pm | 129 +++++++++++++++++++++++++++-------------------------- svc/checkin | 4 ++ 2 files changed, 69 insertions(+), 64 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index f24f2f7..b8c5ce6 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -149,11 +149,11 @@ Generic filter function for barcode string. Called on every circ if the System Pref itemBarcodeInputFilter is set. Will do some manipulation of the barcode for systems that deliver a barcode to circulation.pl that differs from the barcode stored for the item. -For proper functioning of this filter, calling the function on the +For proper functioning of this filter, calling the function on the correct barcode string (items.barcode) should return an unaltered barcode. -The optional $filter argument is to allow for testing or explicit -behavior that ignores the System Pref. Valid values are the same as the +The optional $filter argument is to allow for testing or explicit +behavior that ignores the System Pref. Valid values are the same as the System Pref options. =cut @@ -243,7 +243,7 @@ sub decode { =head2 transferbook - ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, + ($dotransfer, $messages, $iteminformation) = &transferbook($newbranch, $barcode, $ignore_reserves); Transfers an item to a new branch. If the item is currently on loan, it is automatically returned before the actual transfer. @@ -259,7 +259,7 @@ Returns three values: =over -=item $dotransfer +=item $dotransfer is true if the transfer was successful. @@ -382,10 +382,10 @@ sub TooMany { my $branch; # Get which branchcode we need $branch = _GetCircControlBranch($item,$borrower); - my $type = (C4::Context->preference('item-level_itypes')) + my $type = (C4::Context->preference('item-level_itypes')) ? $item->{'itype'} # item-level : $item->{'itemtype'}; # biblio-level - + # given branch, patron category, and item type, determine # applicable issuing rule my $issuing_rule = GetIssuingRule($cat_borrower, $type, $branch); @@ -410,8 +410,8 @@ sub TooMany { AND (categorycode = ? OR categorycode = ?) AND itemtype <> '*' ) "; - } else { - $count_query .= " JOIN biblioitems USING (biblionumber) + } else { + $count_query .= " JOIN biblioitems USING (biblionumber) WHERE biblioitems.itemtype NOT IN ( SELECT itemtype FROM issuingrules WHERE branchcode = ? @@ -427,8 +427,8 @@ sub TooMany { # specific item type if (C4::Context->preference('item-level_itypes')) { $count_query .= " WHERE items.itype = ? "; - } else { - $count_query .= " JOIN biblioitems USING (biblionumber) + } else { + $count_query .= " JOIN biblioitems USING (biblionumber) WHERE biblioitems.itemtype= ? "; } push @bind_params, $type; @@ -601,7 +601,7 @@ sub itemissues { =head2 CanBookBeIssued - ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $borrower, + ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $borrower, $barcode, $duedatespec, $inprocess, $ignore_reserves ); Check if a book can be issued. @@ -630,7 +630,7 @@ Possible values are : =back -=head3 INVALID_DATE +=head3 INVALID_DATE sticky due date is invalid @@ -662,7 +662,7 @@ item withdrawn. item is restricted (set by ??) -C<$needsconfirmation> a reference to a hash. It contains reasons why the loan +C<$needsconfirmation> a reference to a hash. It contains reasons why the loan could be prevented, but ones that can be overriden by the operator. Possible values are : @@ -702,7 +702,7 @@ sub CanBookBeIssued { my $item = GetItem(GetItemnumberFromBarcode( $barcode )); my $issue = GetItemIssue($item->{itemnumber}); my $biblioitem = GetBiblioItemData($item->{biblioitemnumber}); - $item->{'itemtype'}=$item->{'itype'}; + $item->{'itemtype'}=$item->{'itype'}; my $dbh = C4::Context->dbh; # MANDATORY CHECKS - unless item exists, nothing else matters @@ -741,7 +741,7 @@ sub CanBookBeIssued { # # BORROWER STATUS # - if ( $borrower->{'category_type'} eq 'X' && ( $item->{barcode} )) { + if ( $borrower->{'category_type'} eq 'X' && ( $item->{barcode} )) { # stats only borrower -- add entry to statistics table, and return issuingimpossible{STATS} = 1 . &UpdateStats({ branch => C4::Context->userenv->{'branch'}, @@ -1210,13 +1210,13 @@ sub AddIssue { # find which item we issue my $item = GetItem('', $barcode) or return; # if we don't get an Item, abort. my $branch = _GetCircControlBranch($item,$borrower); - + # get actual issuing if there is one my $actualissue = GetItemIssue( $item->{itemnumber}); - + # get biblioinformation for this item my $biblio = GetBiblioFromItemNumber($item->{itemnumber}); - + # # check if we just renew the issue. # @@ -1247,7 +1247,7 @@ sub AddIssue { # updating line of branchtranfert to finish it, and changing the to branch value, implement a comment for visibility of this case (maybe for stats ....) my $sth = $dbh->prepare( - "UPDATE branchtransfers + "UPDATE branchtransfers SET datearrived = now(), tobranch = ?, comments = 'Forced branchtransfer' @@ -1457,7 +1457,7 @@ sub GetHardDueDate { my $irule = &GetIssuingRule($borrowertype,$itemtype,branchcode) FIXME - This is a copy-paste of GetLoanLength -as a stop-gap. Do not wish to change API for GetLoanLength +as a stop-gap. Do not wish to change API for GetLoanLength this close to release. Get the issuing rule for an itemtype, a borrower type and a branch @@ -1512,7 +1512,7 @@ sub GetIssuingRule { my $branch_cat_rule = GetBranchBorrowerCircRule($branchcode, $categorycode); Retrieves circulation rule attributes that apply to the given -branch and patron category, regardless of item type. +branch and patron category, regardless of item type. The return value is a hashref containing the following key: maxissueqty - maximum number of loans that a @@ -1520,11 +1520,11 @@ patron of the given category can have at the given branch. If the value is undef, no limit. This will first check for a specific branch and -category match from branch_borrower_circ_rules. +category match from branch_borrower_circ_rules. If no rule is found, it will then check default_branch_circ_rules (same branch, default category). If no rule is found, -it will then check default_borrower_circ_rules (default +it will then check default_borrower_circ_rules (default branch, same category), then failing that, default_circ_rules (default branch, default category). @@ -1574,7 +1574,7 @@ sub GetBranchBorrowerCircRule { if ($result = $sth->fetchrow_hashref()) { return $result; } - + # try default branch, default borrower category my $default_query = "SELECT maxissueqty FROM default_circ_rules"; @@ -1583,7 +1583,7 @@ sub GetBranchBorrowerCircRule { if ($result = $sth->fetchrow_hashref()) { return $result; } - + # built-in default circulation rule return { maxissueqty => undef, @@ -1650,7 +1650,7 @@ sub GetBranchItemRule { $result->{'holdallowed'} = $search_result->{'holdallowed'} unless ( defined $result->{'holdallowed'} ); $result->{'returnbranch'} = $search_result->{'returnbranch'} unless ( defined $result->{'returnbranch'} ); } - + # built-in default circulation rule $result->{'holdallowed'} = 2 unless ( defined $result->{'holdallowed'} ); $result->{'returnbranch'} = 'homebranch' unless ( defined $result->{'returnbranch'} ); @@ -1737,6 +1737,7 @@ patron who last borrowed the book. sub AddReturn { my ( $barcode, $branch, $exemptfine, $dropbox, $return_date ) = @_; + warn "AddReturn( $barcode, $branch, $exemptfine, $dropbox, $return_date )"; if ($branch and not GetBranchDetail($branch)) { warn "AddReturn error: branch '$branch' not found. Reverting to " . C4::Context->userenv->{'branch'}; @@ -1947,7 +1948,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 - + if ( $issue->{overdue} && $issue->{date_due} ) { # fix fine days my ($debardate,$reminder) = _debar_user_on_return( $borrower, $item, $issue->{date_due}, $today ); @@ -2008,10 +2009,10 @@ sub AddReturn { branch => $branch, }); } - + logaction("CIRCULATION", "RETURN", $borrowernumber, $item->{'itemnumber'}) if C4::Context->preference("ReturnLog"); - + # Remove any OVERDUES related debarment if the borrower has no overdues if ( $borrowernumber && $borrower->{'debarred'} @@ -2051,7 +2052,7 @@ Unconditionally marks an issue as being returned by moving the C row to C and setting C to the current date, or the last non-holiday date of the branccode specified in -C . Assumes you've already checked that +C . Assumes you've already checked that it's safe to do this, i.e. last non-holiday > issuedate. if C<$returndate> is specified (in iso format), it is used as the date @@ -2061,7 +2062,7 @@ C<$privacy> contains the privacy parameter. If the patron has set privacy to 2, the old_issue is immediately anonymised Ideally, this function would be internal to C, -not exported, but it is currently needed by one +not exported, but it is currently needed by one routine in C. =cut @@ -2193,7 +2194,7 @@ C<$brn> borrowernumber C<$itm> itemnumber -C<$exemptfine> BOOL -- remove overdue charge associated with this issue. +C<$exemptfine> BOOL -- remove overdue charge associated with this issue. C<$dropboxmode> BOOL -- remove lastincrement on overdue charge associated with this issue. Internal function, called only by AddReturn @@ -2340,7 +2341,7 @@ sub _FixAccountForLostAndReturned { my $circ_control_branch = _GetCircControlBranch($iteminfos, $borrower); -Internal function : +Internal function : Return the library code to be used to determine which circulation policy applies to a transaction. Looks up the CircControl and @@ -2425,7 +2426,7 @@ Returns a hashref sub GetOpenIssue { my ( $itemnumber ) = @_; return unless $itemnumber; - my $dbh = C4::Context->dbh; + my $dbh = C4::Context->dbh; my $sth = $dbh->prepare( "SELECT * FROM issues WHERE itemnumber = ? AND returndate IS NULL" ); $sth->execute( $itemnumber ); return $sth->fetchrow_hashref(); @@ -2515,7 +2516,7 @@ Returns reference to an array of hashes sub GetItemIssues { my ( $itemnumber, $history ) = @_; - + my $today = DateTime->now( time_zome => C4::Context->tz); # get today date $today->truncate( to => 'minute' ); my $sql = "SELECT * FROM issues @@ -2604,7 +2605,7 @@ sub GetUpcomingDueIssues { my $statement = <= 0 AND days_until_due <= ? END_SQL my @bind_parameters = ( $params->{'days_in_advance'} ); - + my $sth = $dbh->prepare( $statement ); $sth->execute( @bind_parameters ); my $upcoming_dues = $sth->fetchall_arrayref({}); @@ -2813,7 +2814,7 @@ sub AddRenewal { # of how many times it has been renewed. my $renews = $issuedata->{'renewals'} + 1; $sth = $dbh->prepare("UPDATE issues SET date_due = ?, renewals = ?, lastreneweddate = ? - WHERE borrowernumber=? + WHERE borrowernumber=? AND itemnumber=?" ); @@ -2829,7 +2830,7 @@ sub AddRenewal { my $accountno = getnextacctno( $borrowernumber ); my $item = GetBiblioFromItemNumber($itemnumber); my $manager_id = 0; - $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; + $manager_id = C4::Context->userenv->{'number'} if C4::Context->userenv; $sth = $dbh->prepare( "INSERT INTO accountlines (date, borrowernumber, accountno, amount, manager_id, @@ -2892,7 +2893,7 @@ sub GetRenewCount { my $renewsleft = 0; my $borrower = C4::Members::GetMember( borrowernumber => $bornum); - my $item = GetItem($itemno); + my $item = GetItem($itemno); # Look in the issues table for this item, lent to this borrower, # and not yet returned. @@ -2908,9 +2909,9 @@ sub GetRenewCount { $renewcount = $data->{'renewals'} if $data->{'renewals'}; # $item and $borrower should be calculated my $branchcode = _GetCircControlBranch($item, $borrower); - + my $issuingrule = GetIssuingRule($borrower->{categorycode}, $item->{itype}, $branchcode); - + $renewsallowed = $issuingrule->{'renewalsallowed'}; $renewsleft = $renewsallowed - $renewcount; if($renewsleft < 0){ $renewsleft = 0; } @@ -3286,13 +3287,13 @@ sub SendCirculationAlert { $items = updateWrongTransfer($itemNumber,$borrowernumber,$waitingAtLibrary,$FromLibrary); -This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation +This function validate the line of brachtransfer but with the wrong destination (mistake from a librarian ...), and create a new line in branchtransfer from the actual library to the original library of reservation =cut sub updateWrongTransfer { my ( $itemNumber,$waitingAtLibrary,$FromLibrary ) = @_; - my $dbh = C4::Context->dbh; + my $dbh = C4::Context->dbh; # first step validate the actual line of transfert . my $sth = $dbh->prepare( @@ -3428,15 +3429,15 @@ This function checks if the date due is a repeatable holiday C<$date_due> = returndate calculate with no day check C<$itemnumber> = itemnumber -C<$branchcode> = localisation of issue +C<$branchcode> = localisation of issue =cut sub CheckRepeatableHolidays{ my($itemnumber,$week_day,$branchcode)=@_; my $dbh = C4::Context->dbh; -my $query = qq|SELECT count(*) - FROM repeatable_holidays +my $query = qq|SELECT count(*) + FROM repeatable_holidays WHERE branchcode=? AND weekday=?|; my $sth = $dbh->prepare($query); @@ -3456,14 +3457,14 @@ C<$years> = the years of datedue C<$month> = the month of datedue C<$day> = the day of datedue C<$itemnumber> = itemnumber -C<$branchcode> = localisation of issue +C<$branchcode> = localisation of issue =cut sub CheckSpecialHolidays{ my ($years,$month,$day,$itemnumber,$branchcode) = @_; my $dbh = C4::Context->dbh; -my $query=qq|SELECT count(*) +my $query=qq|SELECT count(*) FROM `special_holidays` WHERE year=? AND month=? @@ -3485,14 +3486,14 @@ This function check if the date is a repeatble special holidays C<$month> = the month of datedue C<$day> = the day of datedue C<$itemnumber> = itemnumber -C<$branchcode> = localisation of issue +C<$branchcode> = localisation of issue =cut sub CheckRepeatableSpecialHolidays{ my ($month,$day,$itemnumber,$branchcode) = @_; my $dbh = C4::Context->dbh; -my $query=qq|SELECT count(*) +my $query=qq|SELECT count(*) FROM `repeatable_holidays` WHERE month=? AND day=? @@ -3509,8 +3510,8 @@ return $countspecial; sub CheckValidBarcode{ my ($barcode) = @_; my $dbh = C4::Context->dbh; -my $query=qq|SELECT count(*) - FROM items +my $query=qq|SELECT count(*) + FROM items WHERE barcode=? |; my $sth = $dbh->prepare($query); @@ -3531,21 +3532,21 @@ sub IsBranchTransferAllowed { my ( $toBranch, $fromBranch, $code ) = @_; if ( $toBranch eq $fromBranch ) { return 1; } ## Short circuit for speed. - - my $limitType = C4::Context->preference("BranchTransferLimitsType"); + + my $limitType = C4::Context->preference("BranchTransferLimitsType"); my $dbh = C4::Context->dbh; - + my $sth = $dbh->prepare("SELECT * FROM branch_transfer_limits WHERE toBranch = ? AND fromBranch = ? AND $limitType = ?"); $sth->execute( $toBranch, $fromBranch, $code ); my $limit = $sth->fetchrow_hashref(); - + ## If a row is found, then that combination is not allowed, if no matching row is found, then the combination *is allowed* if ( $limit->{'limitId'} ) { return 0; } else { return 1; } -} +} =head2 CreateBranchTransferLimit @@ -3559,9 +3560,9 @@ sub CreateBranchTransferLimit { my ( $toBranch, $fromBranch, $code ) = @_; return unless defined($toBranch) && defined($fromBranch); my $limitType = C4::Context->preference("BranchTransferLimitsType"); - + my $dbh = C4::Context->dbh; - + my $sth = $dbh->prepare("INSERT INTO branch_transfer_limits ( $limitType, toBranch, fromBranch ) VALUES ( ?, ?, ? )"); return $sth->execute( $code, $toBranch, $fromBranch ); } @@ -3602,9 +3603,9 @@ sub LostItem{ my ($itemnumber, $mark_returned) = @_; my $dbh = C4::Context->dbh(); - my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title - FROM issues - JOIN items USING (itemnumber) + my $sth=$dbh->prepare("SELECT issues.*,items.*,biblio.title + FROM issues + JOIN items USING (itemnumber) JOIN biblio USING (biblionumber) WHERE issues.itemnumber=?"); $sth->execute($itemnumber); diff --git a/svc/checkin b/svc/checkin index 2ca539c..c10a1b8 100755 --- a/svc/checkin +++ b/svc/checkin @@ -47,6 +47,10 @@ my $exempt_fine = $input->param('exempt_fine'); my $branchcode = $input->param('branchcode') || C4::Context->userenv->{'branch'}; +# Expect these inputs to come in as JSON boolean values +$override_limit = $override_limit ? $override_limit eq 'true' : undef; +$exempt_fine = $exempt_fine ? $exempt_fine eq 'true' : undef; + my $barcode = GetBarcodeFromItemnumber($itemnumber); my $data; -- 1.7.2.5