Bug 4123 - Error in checking whether a checkout can be renewed
Summary: Error in checking whether a checkout can be renewed
Status: CLOSED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Galen Charlton
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-05 09:02 UTC by Owen Leonard
Modified: 2016-06-21 21:37 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
This patch adds the code. Fixed our problem being unable to renew items. (1.18 KB, patch)
2010-03-01 17:06 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:23:24 UTC


---- 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.

Comment 1 Owen Leonard 2011-05-18 15:53:20 UTC
As gmcharlt said "Straightening this out would be a Good Thing for 3.4" I think this bug is still valid.
Comment 2 Jared Camins-Esakov 2012-02-12 20:27:36 UTC
Comment on attachment 1852 [details] [review]
This patch adds the code. Fixed our problem being unable to renew items.

Patch already pushed.
Comment 3 Katrin Fischer 2015-06-06 19:40:38 UTC
CanBookBeRenewed on current master looks very different to the code posted here. There are no known problems I am aware of. I am closing this 'RESOLVED WORKSFORME'.