Bug 27834 - CircControl syspref is used wrong in TooMany function C4::Circulation
Summary: CircControl syspref is used wrong in TooMany function C4::Circulation
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
: 21045 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-03-02 14:06 UTC by Joonas Kylmälä
Modified: 2025-10-02 15:06 UTC (History)
10 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 27834: Add CheckoutLimitScope system preference (3.75 KB, patch)
2025-09-22 06:51 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27834: Fix CircControl behavior in TooMany function (13.34 KB, patch)
2025-09-22 06:51 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27834: Database update - Add CircControlCheckoutLimitScope system preference (3.98 KB, patch)
2025-10-02 15:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27834: Use new CircControlCheckoutLimitScope system preference (27.14 KB, patch)
2025-10-02 15:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2021-03-02 14:06:25 UTC
The CircControl syspref description says

> Use the checkout and fines rules of
>   PickupLibrary: the library you are logged in at.
>   PatronLibrary: the library the patron is from.
>   ItemHomeLibrary: the library the item is from.

This is mostly true but there is one exception in the TooMany function where it actually doesn't just change which libraries rules are used but how the the calculation is performed.

With PatronLibrary set the behaviour is as expected by reading the syspref description. However with the setting PickupLibrary the behaviour changes now, instead of just using the rules of the pickup library we also only take into account the loans in the pickup library when calculating whether we are over the checkout limit or not. The same happens with ItemHomeLibrary where we instead calculate the patron's checkouts in the home library of the item being checked out.

We should really try to fix this either by

a) showing that ItemHomeLibrary and PickupLibrary is unacceptable and no one actually wants that, that would allow then making the behaviour similar as what it is to PatronLibrary (so just selecting the right circ rule instead of changing calculation method).

b) Provide a new syspref for changing the calcutation method when PickupLibrary or ItemHomeLibrary CircControl values are used, and set it on upgrade enabled as default.


Ps. svc/letters/preview might use CirControl wrongly as well, I'm not 100% I didn't quite understand but as for other usages of CircControl syspref I grepped where it was used and it correctly only changed the circ rules to be used instead of changing behaviour. So in addition to TooMany we might have to do some changes to svc/letters/preview if it turns out it changes behaviour as well.
Comment 1 Joonas Kylmälä 2021-03-02 14:10:33 UTC
IMHO, easiest way would be to go with option b):

b) Provide a new syspref for changing the calcutation method when PickupLibrary or ItemHomeLibrary CircControl values are used, and set it on upgrade enabled as default.

We might miss some corner case even if we deem option a) safe and so if this new syspref introduced by option b) turns out unused for all users then we can very easily remove it in future and revert it if a corner case comes there. With option a) reverting is not as simple.
Comment 2 Lisette Scheer 2023-06-06 14:49:07 UTC
*** Bug 21045 has been marked as a duplicate of this bug. ***
Comment 3 Emily Lamancusa (emlam) 2024-07-23 19:28:47 UTC
Still valid. It seems strange to me that only checkout limit behaves this way, but all other limits apply globally (if I am understanding correctly). That being said, I could see a consortium wanting to enforce checkout limits per branch, rather than total checkout limits, in a situation where the libraries are mostly independent but are sharing a Koha instance. I agree that adding a syspref would be a good fix - it's pretty straightforward, would preserve existing behavior for libraries that want it, and would allow CircControl's behavior to work the way the description makes it sound like it should for libraries who want that.
Comment 4 Ray Delahunty 2025-08-07 13:27:31 UTC
I wish we had known in 2015 when we were selecting a new LMS that Koha circcontrol works this way. We set maximum loans globally (differing by patron group) rather than max loans being a multiplier of the number of libraries, and the only way we can do this is to set circcontrol to use the rules (and calendar!) from the library the patron belongs to. Our users move around the libraries and carry their rules with them and impose them on the libraries they visit(!).
 
This means that hourly loans (like our laptops that loan for 5 hours) have never worked correctly coming up to library closing time and at weekends where some of our libraries are open, and some are not. This effects of this bug are now much more visible as it destroys the advantages offered in 24.11 with ConsiderLibraryHoursInCirulation and the addition of opening and closing times.
 
Our support company does not see this as a bug which should be fixed as routine support and are suggesting we fund development to possibly add a system preference as Emily suggests. Without this fixed Koha circulation will never work correctly for hourly loan at our institution (or others that operate like our do). I have no idea if we can find development funds, but am about to find out. Depending on the work involved we may need help… in a joint funding situation.
Comment 5 Martin Renvoize (ashimema) 2025-09-22 06:51:03 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize (ashimema) 2025-09-22 06:51:05 UTC Comment hidden (obsolete)
Comment 7 Ray Delahunty 2025-09-22 13:35:26 UTC
I'm looking forward to configuring all this and testing the fix out but in my Open Fifth sandbox I cannot run the updatedatabase.pl script. Once our support staff run the script for me (assuming that can be done on a sandbox, and a KTD isn't needed) I hope to be able to see the new syspref and then I can get testing.
Comment 8 Martin Renvoize (ashimema) 2025-10-02 15:03:17 UTC
Created attachment 187296 [details] [review]
Bug 27834: Database update - Add CircControlCheckoutLimitScope system preference

This patch adds the database components for the new
CircControlCheckoutLimitScope system preference.

The preference provides three explicit options for controlling how
checkout limits (maxissueqty and patron_maxissueqty) are calculated:

* 'all' (default) - Count all patron checkouts across all libraries
* 'item' - Count only checkouts of items from the same library as the
  item being checked out (follows HomeOrHoldingBranch preference)
* 'checkout' - Count only checkouts made at the same library as the
  current checkout

This gives libraries explicit control over checkout limit scope,
independent of the CircControl preference.

Sponsored-by: University of the Arts London
Comment 9 Martin Renvoize (ashimema) 2025-10-02 15:03:20 UTC
Created attachment 187297 [details] [review]
Bug 27834: Use new CircControlCheckoutLimitScope system preference

This patch updates the TooMany function to use the new
CircControlCheckoutLimitScope system preference, providing explicit
control over how checkout limits (maxissueqty and patron_maxissueqty)
are calculated, independent of the CircControl preference.

Previously, the TooMany function conflated two separate concerns:
1. CircControl - which circulation rules to apply
2. Checkout counting scope - which checkouts count toward limits

This caused the checkout counting behavior to change based on CircControl,
even when libraries wanted consistent limit enforcement regardless of
which circulation rules applied.

The new CircControlCheckoutLimitScope preference offers three options:

* 'all' (default) - Count all patron checkouts across all libraries
* 'item' - Count only checkouts of items from the same library as the
  item being checked out (follows HomeOrHoldingBranch preference)
* 'checkout' - Count only checkouts made at the same library as the
  current checkout

Implementation note: When scope is 'item', the system counts checkouts
of items from the same library as the item being checked out (using the
item's homebranch or holdingbranch). This applies regardless of whether
the matching circulation rule is branch-specific or general (branch=*),
ensuring consistent behavior across all rule types.

This gives libraries explicit, flexible control over checkout limit scope
while maintaining CircControl's intended purpose of rule selection.

Test plan:
1. Apply patches and update database:
   perl installer/data/mysql/updatedatabase.pl

2. Verify new preference exists:
   Administration > System preferences > Circulation
   Search for "CircControlCheckoutLimitScope"
   Default value should be 'all'

3. Set up test environment:
   - Create two branches (Branch1, Branch2)
   - Create a patron category and item type
   - Create a patron in Branch1
   - Create items from both branches
   - Set circulation rules for both branches:
     * Branch1: maxissueqty = 1 for itemtype/category
     * Branch2: maxissueqty = 1 for itemtype/category

4. Test scope = 'checkout':
   a. Set CircControlCheckoutLimitScope = 'checkout'
   b. Log in as staff from Branch1
   c. Check out item from Branch1 to patron → succeeds
   d. Try to check out another item from Branch1 → blocked (limit: 1)
   e. Log in as staff from Branch2
   f. Check out item from Branch2 to same patron → succeeds
   g. Expected: Checkouts at different libraries don't count together

5. Test scope = 'all':
   a. Set CircControlCheckoutLimitScope = 'all'
   b. Return all items
   c. Log in as staff from Branch1
   d. Check out item from Branch1 to patron → succeeds
   e. Log in as staff from Branch2
   f. Try to check out item from Branch2 to same patron → blocked
   g. Expected: All patron checkouts count toward limit regardless of location

6. Test scope = 'item':
   a. Set CircControlCheckoutLimitScope = 'item'
   b. Set HomeOrHoldingBranch = 'homebranch'
   c. Return all items
   d. Check out item from Branch1 to patron → succeeds
   e. Try to check out another item from Branch1 → blocked (limit: 1)
   f. Check out item from Branch2 to same patron → succeeds
   g. Expected: Only items from same home library count together
   h. Note: When checking out an item, only checkouts of items from
      the same library as that item count toward the limit

7. Test scope applies to all rule types:
   a. With scope='item', test with a general rule (branch=*)
   b. Check out items from different branches
   c. Expected: Scope still applies - only items from the same branch
      as the item being checked out are counted

8. Test independence from CircControl:
   a. Test each scope with CircControl = 'PickupLibrary'
   b. Test each scope with CircControl = 'ItemHomeLibrary'
   c. Test each scope with CircControl = 'PatronLibrary'
   d. Expected: CircControlCheckoutLimitScope behavior is consistent
      regardless of CircControl setting

9. Run tests:
   prove t/db_dependent/Circulation/TooMany.t

10. Test with patron_maxissueqty:
    - Set up patron-level limits instead of item-type limits
    - Verify all three scopes work correctly
    - Test with both rule types active

11. Verify existing functionality:
    - Test on-site checkouts still work
    - Test item type hierarchies still work
    - Test unlimited ("") limits still work
    - prove t/db_dependent/Circulation/

Sponsored-by: University of the Arts London
Comment 10 Martin Renvoize (ashimema) 2025-10-02 15:06:56 UTC
OK.. so after a chat with Nick around this, I found I needed to think a little more deeply and hense the new patch.

I'll get this onto a test system for you imminently Ray.