Bugzilla – Attachment 33874 Details for
Bug 13330
All overdue items with fines returned via checkouts table will have fines forgiven!
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 13330 - All overdue items with fines returned via checkouts table will have fines forgiven!
Bug-13330---All-overdue-items-with-fines-returned-.patch (text/plain), 19.63 KB, created by
Kyle M Hall (khall)
on 2014-11-24 17:14:47 UTC
(
hide
)
Description:
Bug 13330 - All overdue items with fines returned via checkouts table will have fines forgiven!
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2014-11-24 17:14:47 UTC
Size:
19.63 KB
patch
obsolete
>From b6385330c730619adffd200c4f1054c771ed3674 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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<issues> row to C<old_issues> and > setting C<returndate> to the current date, or > the last non-holiday date of the branccode specified in >-C<dropbox_branch> . Assumes you've already checked that >+C<dropbox_branch> . 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<C4::Circulation>, >-not exported, but it is currently needed by one >+not exported, but it is currently needed by one > routine in C<C4::Accounts>. > > =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 = <<END_SQL; > SELECT issues.*, items.itype as itemtype, items.homebranch, TO_DAYS( date_due )-TO_DAYS( NOW() ) as days_until_due, branches.branchemail >-FROM issues >+FROM issues > LEFT JOIN items USING (itemnumber) > LEFT OUTER JOIN branches USING (branchcode) > WHERE returndate is NULL >@@ -2612,7 +2613,7 @@ HAVING days_until_due >= 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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 13330
:
33874
|
33875
|
33881
|
33883