Bug 4123

---- Reported by oleonard@myacpl.org 2010-02-05 21:02:15 ----

In Circulation.pm, part of sub CanBookBeRenewed:
-----------------------------------------------
                   SELECT 
                    borrowers.categorycode, biblioitems.itemtype, issues.renewals, renewalsallowed, $controlbranch
                   FROM  issuingrules, 
                   issues 
                   LEFT JOIN items USING (itemnumber) 
                   LEFT JOIN borrowers USING (borrowernumber) 
                   LEFT JOIN biblioitems USING (biblioitemnumber)
                   
                   WHERE
                    issuingrules.categorycode = borrowers.categorycode
                   AND
                    issuingrules.itemtype = $itype
                   AND
                    (issuingrules.branchcode = $controlbranch OR issuingrules.branchcode = '*') 
                   AND 
                    borrowernumber = ? 
                   AND
                    itemnumber = ?
                   ORDER BY
                    issuingrules.categorycode desc,
                    issuingrules.itemtype desc,
                    issuingrules.branchcode desc
                   LIMIT 1;
-----------------------------------------------
I see that there is a check for a wildcard under issuingrules.branchcode. Should there not be a similar check with issuingrules.categorycode and issuingrules.itemtype? i.e.

(issuingrules.categorycode = borrowers.categorycode OR issuingrules.categorycode = '*')

(issuingrules.itemtype = $itype OR issuingrules.itemtype = '*')

Without the wildcard option doesn't that mean you have to have a rule set up for each patron category/itemtype combination?



---- Additional Comments From kyle.m.hall@gmail.com 2010-03-01 17:06:27 ----

Created an attachment
This patch adds the code. Fixed our problem being unable to renew items.





---- Additional Comments From colin.campbell@ptfs-europe.com 2010-03-19 20:04:45 ----

Even with the below patch applied I found at a single branch library that renew was not being allowed because the routine was not generating valid SQL.

Problem seems to be with the controlbranch variable which does not have a valid value.

The sql query looks very baroque and far too easily broken for such basic functionality.




---- Additional Comments From ian.walls@bywatersolutions.com 2010-04-02 15:50:48 ----

(In reply to comment #2)
> 
> Problem seems to be with the controlbranch variable which does not have a valid
> value.
> 

If CircControl exists and is set to a value, this shouldn't be a problem.  But, yes, the $controlbranch may require a default value in the event that CircControl fails.  This should either be home or holding branch, but I'm not sure which (not that it matters in a single library environment, but it would if CircControl wasn't set in a multiple library install).




---- Additional Comments From gmcharlt@gmail.com 2010-04-30 12:11:37 ----

Pushed Kyle's patch to HEAD.  Besides the $controlbranch issue, I view this only as a stopgap for 3.2, as it implements a behavior that could be subtly different from how loan rules are retrieved in other contexts, e.g., by GetIssuingRule.

Straightening this out would be a Good Thing for 3.4; I know that BibLibre has sent some patches proposing an overhaul of the circ policy matrix, but more work (and test cases!) are needed.

Leaving open but downgrading to 'normal'.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:23 UTC  ---

This bug was previously known as _bug_ 4123 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4123
Imported an attachment (id=1852)

Actual time not defined. Setting to 0.0
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.
The original submitter of attachment 1852 [details] [review] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.