Bug 36506 - Processing Fee should be configurable by branch
Summary: Processing Fee should be configurable by branch
Status: Passed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Nick Clemens (kidclamp)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 39802
Blocks: 35612
  Show dependency treegraph
 
Reported: 2024-04-03 18:06 UTC by Lisette Scheer
Modified: 2026-02-06 17:55 UTC (History)
3 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: Sponsored
Comma delimited list of Sponsors: The Main Library Alliance <https://www.mainlib.org/>
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds a new section to the Circulation and fines rules page, and allows the configuration of processing fees per branch and itemtype. Following the LostChargesControl syspref, the rule will be chosen based on the branch selected and items will be charged the corresponding processing fee when marked lost if there is one defined.
Version(s) released in:
Circulation function:


Attachments
Bug 36506: Add lost_item_processing_fee rule and CRUD (33.91 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Database update (3.61 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Remove API mapping (947 bytes, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Remove process fee from itemtype administration (4.19 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: DO NOT PUSH - SCHEMA UPDATES (2.33 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Use new rules in C4::Accounts (3.78 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Adjust existing tests and test builder (7.35 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Unit tests (7.26 KB, patch)
2025-10-03 16:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 36506: Add lost_item_processing_fee rule and CRUD (33.96 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Database update (3.67 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Remove API mapping (1007 bytes, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Remove process fee from itemtype administration (4.25 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: DO NOT PUSH - SCHEMA UPDATES (1.76 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Use new rules in C4::Accounts (3.84 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Adjust existing tests and test builder (7.41 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Unit tests (7.32 KB, patch)
2025-10-08 13:51 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 36506: Add lost_item_processing_fee rule and CRUD (34.03 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Database update (3.73 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Remove API mapping (1.05 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Remove process fee from itemtype administration (4.31 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: DO NOT PUSH - SCHEMA UPDATES (1.82 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Use new rules in C4::Accounts (3.91 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Adjust existing tests and test builder (13.46 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: Unit tests (7.38 KB, patch)
2026-01-29 18:21 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 36506: (QA follow-up) Add test for missing circulation rule (3.05 KB, patch)
2026-01-29 18:21 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 Lisette Scheer 2024-04-03 18:06:30 UTC
In a consortia environment, some branches might want a process fee and others might not. This could be worked around with multiple item types, but that can cause other issues.
Comment 1 Andrew Fuerste-Henry 2025-05-01 16:01:58 UTC
This enhancement will need to include a system preference to determine which branch's processing fees should be used. Further, that system preference should cover branch selection for lost item behavior generally. I've filed bug 39802.
Comment 2 Nick Clemens (kidclamp) 2025-10-03 16:28:42 UTC
Created attachment 187411 [details] [review]
Bug 36506: Add lost_item_processing_fee rule and CRUD

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 3 Nick Clemens (kidclamp) 2025-10-03 16:28:44 UTC
Created attachment 187412 [details] [review]
Bug 36506: Database update

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 4 Nick Clemens (kidclamp) 2025-10-03 16:28:47 UTC
Created attachment 187413 [details] [review]
Bug 36506: Remove API mapping

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 5 Nick Clemens (kidclamp) 2025-10-03 16:28:49 UTC
Created attachment 187414 [details] [review]
Bug 36506: Remove process fee from itemtype administration

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 6 Nick Clemens (kidclamp) 2025-10-03 16:28:52 UTC
Created attachment 187415 [details] [review]
Bug 36506: DO NOT PUSH - SCHEMA UPDATES

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 7 Nick Clemens (kidclamp) 2025-10-03 16:28:54 UTC
Created attachment 187416 [details] [review]
Bug 36506: Use new rules in C4::Accounts

This seems to be the only place processing fee is used, in chargelostitem

This patch uses get_effective_rule_value, falling back to 0 if no rule.

We also respect the new system preference, LostChargesControl, introduced in bug 39802

To test:
 1 - Apply patch, updatedatabase, restart all
 2 - Set a default/all branches  processing rule for Books itemtype to 5
 3 - Set a rule for branch 1 to 1
 4 - Set a rule for branch 2 to 2
 5 - Set a rule for branch 3 to 3
 6 - Set a rule for branch 4 to 4
 7 - Set LostChargesControl to 'library the patron is from'
 8 - Check out a Books item from a different branch to a patron from branch 1
 9 - Mark item lost, confirm processing fee is charged as 1
10 - Check out out a Books item from a different branch to a patron from branch 5
11 - Mark item lost, confirm processing fee is 5
12 - Set LostChargesControl to 'library you are logged in at'
13 - Check out a Books item from a different branch to a patron from branch 1
14 - Ensure you are logged in at branch 2
15 - Mark item lost, confirm processing fee is 2
16 - Set LostChargesControl to 'the library the item is from'
17 - Set HomeOrHoldingBranch to 'homebranch'
18 - Still at branch 2, check out a books item from branch 3 to a patron of branch 1, this ensure holdingbranch is not 3
19 - Mark item lost, confirm processing fee is 3
20 - Set HomeOrHoldingBranch to 'holdingbranch'
21 - Ensure you are logged in at branch 4
22 - Checkout an item from branch 1 to a patron from a different branch
23 - Mark item lost, confirm processing fee is 4
24 - Test other combos as you see fit
25 - Sign off!

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 8 Nick Clemens (kidclamp) 2025-10-03 16:28:57 UTC
Created attachment 187417 [details] [review]
Bug 36506: Adjust existing tests and test builder

All the previously written tests should continue to pass.

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 9 Nick Clemens (kidclamp) 2025-10-03 16:28:59 UTC
Created attachment 187418 [details] [review]
Bug 36506: Unit tests

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Comment 10 Andrew Fuerste-Henry 2025-10-08 13:51:19 UTC
Created attachment 187573 [details] [review]
Bug 36506: Add lost_item_processing_fee rule and CRUD

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 11 Andrew Fuerste-Henry 2025-10-08 13:51:22 UTC
Created attachment 187574 [details] [review]
Bug 36506: Database update

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 12 Andrew Fuerste-Henry 2025-10-08 13:51:25 UTC
Created attachment 187575 [details] [review]
Bug 36506: Remove API mapping

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 13 Andrew Fuerste-Henry 2025-10-08 13:51:28 UTC
Created attachment 187576 [details] [review]
Bug 36506: Remove process fee from itemtype administration

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 14 Andrew Fuerste-Henry 2025-10-08 13:51:31 UTC
Created attachment 187577 [details] [review]
Bug 36506: DO NOT PUSH - SCHEMA UPDATES

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 15 Andrew Fuerste-Henry 2025-10-08 13:51:34 UTC
Created attachment 187578 [details] [review]
Bug 36506: Use new rules in C4::Accounts

This seems to be the only place processing fee is used, in chargelostitem

This patch uses get_effective_rule_value, falling back to 0 if no rule.

We also respect the new system preference, LostChargesControl, introduced in bug 39802

To test:
 1 - Apply patch, updatedatabase, restart all
 2 - Set a default/all branches  processing rule for Books itemtype to 5
 3 - Set a rule for branch 1 to 1
 4 - Set a rule for branch 2 to 2
 5 - Set a rule for branch 3 to 3
 6 - Set a rule for branch 4 to 4
 7 - Set LostChargesControl to 'library the patron is from'
 8 - Check out a Books item from a different branch to a patron from branch 1
 9 - Mark item lost, confirm processing fee is charged as 1
10 - Check out out a Books item from a different branch to a patron from branch 5
11 - Mark item lost, confirm processing fee is 5
12 - Set LostChargesControl to 'library you are logged in at'
13 - Check out a Books item from a different branch to a patron from branch 1
14 - Ensure you are logged in at branch 2
15 - Mark item lost, confirm processing fee is 2
16 - Set LostChargesControl to 'the library the item is from'
17 - Set HomeOrHoldingBranch to 'homebranch'
18 - Still at branch 2, check out a books item from branch 3 to a patron of branch 1, this ensure holdingbranch is not 3
19 - Mark item lost, confirm processing fee is 3
20 - Set HomeOrHoldingBranch to 'holdingbranch'
21 - Ensure you are logged in at branch 4
22 - Checkout an item from branch 1 to a patron from a different branch
23 - Mark item lost, confirm processing fee is 4
24 - Test other combos as you see fit
25 - Sign off!

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 16 Andrew Fuerste-Henry 2025-10-08 13:51:37 UTC
Created attachment 187579 [details] [review]
Bug 36506: Adjust existing tests and test builder

All the previously written tests should continue to pass.

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 17 Andrew Fuerste-Henry 2025-10-08 13:51:41 UTC
Created attachment 187580 [details] [review]
Bug 36506: Unit tests

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>

Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Comment 18 Martin Renvoize (ashimema) 2026-01-29 17:59:17 UTC
Dare I ask.. as we're migrating from Itemtypes to Circulation Rules... why limit ourselves to Itemtype/Branchcode combinations.. it feels like it would be a tiny extra step to include the Patron Category as an optional filter too at this stage?
Comment 19 Martin Renvoize (ashimema) 2026-01-29 18:11:39 UTC
Comment on attachment 187578 [details] [review]
Bug 36506: Use new rules in C4::Accounts

Review of attachment 187578 [details] [review]:
-----------------------------------------------------------------

::: C4/Accounts.pm
@@ +78,5 @@
> +    my $lost_control_branch;
> +    if ( $lost_control_pref eq 'PatronLibrary' ) {
> +        $lost_control_branch = $patron->branchcode;
> +    } elsif ( $lost_control_pref eq 'PickupLibrary' ) {
> +        $lost_control_branch = C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;

Can you explain the reasoning here.. a lost item won't necessarily be related to a hold.. so I get we can't easily pass a PickupLibrary and I understand using the userenv.. i.e. your using 'PickupLibrary' to mean 'Library the patron is standing in' if this is getting charged via the UI.. but when that's not the case.. i.e. when running from a cron?. Does it really make sense to fall back to the 'default rule for all libraries' if set.. or should we instead fall back to the patrons home library on the assumption they'd 'Pickup' from their home branch?
Comment 20 Martin Renvoize (ashimema) 2026-01-29 18:21:14 UTC
Created attachment 192182 [details] [review]
Bug 36506: Add lost_item_processing_fee rule and CRUD

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 21 Martin Renvoize (ashimema) 2026-01-29 18:21:15 UTC
Created attachment 192183 [details] [review]
Bug 36506: Database update

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 22 Martin Renvoize (ashimema) 2026-01-29 18:21:16 UTC
Created attachment 192184 [details] [review]
Bug 36506: Remove API mapping

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 23 Martin Renvoize (ashimema) 2026-01-29 18:21:17 UTC
Created attachment 192185 [details] [review]
Bug 36506: Remove process fee from itemtype administration

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 24 Martin Renvoize (ashimema) 2026-01-29 18:21:19 UTC
Created attachment 192186 [details] [review]
Bug 36506: DO NOT PUSH - SCHEMA UPDATES

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 25 Martin Renvoize (ashimema) 2026-01-29 18:21:20 UTC
Created attachment 192187 [details] [review]
Bug 36506: Use new rules in C4::Accounts

This seems to be the only place processing fee is used, in chargelostitem

This patch uses get_effective_rule_value, falling back to 0 if no rule.

We also respect the new system preference, LostChargesControl, introduced in bug 39802

To test:
 1 - Apply patch, updatedatabase, restart all
 2 - Set a default/all branches  processing rule for Books itemtype to 5
 3 - Set a rule for branch 1 to 1
 4 - Set a rule for branch 2 to 2
 5 - Set a rule for branch 3 to 3
 6 - Set a rule for branch 4 to 4
 7 - Set LostChargesControl to 'library the patron is from'
 8 - Check out a Books item from a different branch to a patron from branch 1
 9 - Mark item lost, confirm processing fee is charged as 1
10 - Check out out a Books item from a different branch to a patron from branch 5
11 - Mark item lost, confirm processing fee is 5
12 - Set LostChargesControl to 'library you are logged in at'
13 - Check out a Books item from a different branch to a patron from branch 1
14 - Ensure you are logged in at branch 2
15 - Mark item lost, confirm processing fee is 2
16 - Set LostChargesControl to 'the library the item is from'
17 - Set HomeOrHoldingBranch to 'homebranch'
18 - Still at branch 2, check out a books item from branch 3 to a patron of branch 1, this ensure holdingbranch is not 3
19 - Mark item lost, confirm processing fee is 3
20 - Set HomeOrHoldingBranch to 'holdingbranch'
21 - Ensure you are logged in at branch 4
22 - Checkout an item from branch 1 to a patron from a different branch
23 - Mark item lost, confirm processing fee is 4
24 - Test other combos as you see fit
25 - Sign off!

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 26 Martin Renvoize (ashimema) 2026-01-29 18:21:21 UTC
Created attachment 192188 [details] [review]
Bug 36506: Adjust existing tests and test builder

All the previously written tests should continue to pass.

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 27 Martin Renvoize (ashimema) 2026-01-29 18:21:22 UTC
Created attachment 192189 [details] [review]
Bug 36506: Unit tests

Sponsored-by: The Main Library Alliance <https://www.mainlib.org/>
Signed-off-by: Trevor Diamond <trevor.diamond@mainlib.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 28 Martin Renvoize (ashimema) 2026-01-29 18:21:23 UTC
Created attachment 192190 [details] [review]
Bug 36506: (QA follow-up) Add test for missing circulation rule

This adds a test case to verify that when no lost_item_processing_fee
circulation rule exists for a branch/itemtype combination, the system
correctly defaults to 0 (no processing fee charged).

The test confirms:
- No PROCESSING account line is created when the rule is missing
- The LOST fee is still created normally (only processing fee affected)
- The // 0 fallback in C4::Accounts::chargelostitem works correctly

Test plan:
1. prove t/db_dependent/Accounts.t
2. Verify all tests pass, including the new "missing circulation rule"
   test case

Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Comment 29 Nick Clemens (kidclamp) 2026-01-29 19:12:55 UTC
(In reply to Martin Renvoize (ashimema) from comment #19)
> Comment on attachment 187578 [details] [review] [review]
> > +    } elsif ( $lost_control_pref eq 'PickupLibrary' ) {

"PickupLibrary" is really a misnomer here, I held it over from the pref we split off from in bug 39802

The interface translates PickupLibrary as 'the library you are logged in at'

If you don't have a userenv (i.e. you are a cronjob) then we assume the default rules - which is where we copy the current itemtype rules - so falling back to default/undef made sense to me here