Bug 25426 - Allow return policy to be selected via syspref and not just home library
Summary: Allow return policy to be selected via syspref and not just home library
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Kyle M Hall (khall)
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-08 13:06 UTC by Andrew Fuerste-Henry
Modified: 2023-12-28 20:42 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/665
Text to go in the release notes:
This enhancement adds a new system preference, CircControlReturnsBranch. Previously circulation rules for return policies always used the item's home library. This new preference allows choosing between the item's home library, the item's holding library, and the logged in library when selecting the return policy from the circulation rules.
Version(s) released in:
22.11.00
Circulation function:

Attachments
Bug 25426: Allow return policy to be selected via syspref and not just home library (13.09 KB, patch)
2022-08-25 15:03 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 25426: Allow return policy to be selected via syspref and not just home library (13.09 KB, patch)
2022-08-25 18:23 UTC, Andrew Fuerste-Henry 		Details | Diff | Splinter Review
Bug 25426: Allow return policy to be selected via syspref and not just home library (13.15 KB, patch)
2022-08-29 17:51 UTC, Kyle M Hall (khall) 		Details | Diff | Splinter Review
Bug 25426: Allow return policy to be selected via syspref and not just home library (13.21 KB, patch)
2022-10-03 09:46 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 25426: (QA follow-up) Small fixes to POD and INSERT IGNORE or datatabase update (2.29 KB, patch)
2022-10-03 09:46 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 25426: (QA follow-up) Rephrase CircControlReturnsBranch system preference description (1.78 KB, patch)
2022-10-03 09:46 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 25426: (QA follow-up) Update notes on circulation rules page for new pref (2.13 KB, patch)
2022-10-03 10:16 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2020-05-08 13:06:48 UTC 
In 19.11 and Master, Koha is not giving precedence to branch-specific return policy rules.

To recreate:
1: set default return policy by item type for itype BOOK for all libraries to Item Floats
2: set return policy by item type for itype BOOK for Branch A to Item Returns to Issuing Library
3: set CircControl to "the library you're logged in at"
4: set library to Branch B
5: find a BOOK item currrently at Branch B, check it out to a Branch B patron
6: set library to Branch A
7: check item in
8: no transfer is generated, item remains at branch A

In 19.05, the same process results in a transfer back to Branch B.
Comment 1 Andrew Fuerste-Henry 2020-05-08 15:48:30 UTC 
Turns out there is not a change in behavior between versions here, just a messy snarl of unexpected rules.

Turns out Return Policy is always enforced based on the item's home library. Is that what it *should* do? Should it pick a branch based on CircControl instead?
Comment 2 Katrin Fischer 2020-05-23 14:26:05 UTC 
Hi Andrew,

reading through your test plan the behavior appears to be correct for me, can you explain the problem a bit differently? This is what I think happens:

- The rule to apply is "the library you are logged in at"
- When checking in the item, you are logged in at libary A.
- You haven't set a rule for library A, so there is a fallback to the 'all libraries' rule. 
- All libraries rule is set to 'item floats'.
- Checkin in the item it floats - no transfer is created, it remains at the library.
Comment 3 Andrew Fuerste-Henry 2020-08-11 16:17:03 UTC 
(In reply to Katrin Fischer from comment #2)
> Hi Andrew,
> 
> reading through your test plan the behavior appears to be correct for me,
> can you explain the problem a bit differently? This is what I think happens:
> 
> - The rule to apply is "the library you are logged in at"
> - When checking in the item, you are logged in at libary A.
> - You haven't set a rule for library A, so there is a fallback to the 'all
> libraries' rule. 
> - All libraries rule is set to 'item floats'.
> - Checkin in the item it floats - no transfer is created, it remains at the
> library.

Koha's not looking at the rule for the branch you're logged in at. Instead, it's looking at the rule for the item's homebranch.

In the test plan, I'm checking the item in at Branch A, but Koha is looking at rules for Branch B, the item homebranch. Not finding something for Branch B, it falls back to rules for all libraries.
Comment 4 Andrew Fuerste-Henry 2021-04-22 17:56:24 UTC 
Thinking on this further after running into it with a second library, I believe this should be its own system preference for ReturnControlBranch.
Comment 5 Kyle M Hall (khall) 2022-08-25 15:03:02 UTC 
Created attachment 139798 [details] [review]
Bug 25426: Allow return policy to be selected via syspref and not just home library

1) Apply this patch
2) Run updatedatabase.pl
3) Verify CircControlReturnsBranch is set to home library by default
4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch )
6) Set a Return polity for Branch C to "Item floats" ( noreturn )
7) Create an item with homebranch of Branch A, holding branch of branch B
8) Log in as Branch C
9) Set CircControlReturnsBranch to "the library the item is currently held by"
10) Check the item in, note it should be returned to the holding library
11) Set CircControlReturnsBranch to "the library the item is owned by"
12) Check the item in, note it should be returned to the home library
13) Set CircControlReturnsBranch to "the library you are logged in at"
14) Check the item in, note it should float
Comment 6 Andrew Fuerste-Henry 2022-08-25 18:23:09 UTC 
Created attachment 139802 [details] [review]
Bug 25426: Allow return policy to be selected via syspref and not just home library

1) Apply this patch
2) Run updatedatabase.pl
3) Verify CircControlReturnsBranch is set to home library by default
4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch )
6) Set a Return polity for Branch C to "Item floats" ( noreturn )
7) Create an item with homebranch of Branch A, holding branch of branch B
8) Log in as Branch C
9) Set CircControlReturnsBranch to "the library the item is currently held by"
10) Check the item in, note it should be returned to the holding library
11) Set CircControlReturnsBranch to "the library the item is owned by"
12) Check the item in, note it should be returned to the home library
13) Set CircControlReturnsBranch to "the library you are logged in at"
14) Check the item in, note it should float
Comment 7 Andrew Fuerste-Henry 2022-08-25 18:25:48 UTC 
(In reply to Andrew Fuerste-Henry from comment #6)
> Created attachment 139802 [details] [review] [review]
> Bug 25426: Allow return policy to be selected via syspref and not just home
> library
> 
> 1) Apply this patch
> 2) Run updatedatabase.pl
> 3) Verify CircControlReturnsBranch is set to home library by default
> 4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
> 5) Set a Return polity for Branch B to "Item returns to issuing library" (
> holdingbranch )
> 6) Set a Return polity for Branch C to "Item floats" ( noreturn )
> 7) Create an item with homebranch of Branch A, holding branch of branch B
> 8) Log in as Branch C
> 9) Set CircControlReturnsBranch to "the library the item is currently held
> by"
> 10) Check the item in, note it should be returned to the holding library
> 11) Set CircControlReturnsBranch to "the library the item is owned by"
> 12) Check the item in, note it should be returned to the home library
> 13) Set CircControlReturnsBranch to "the library you are logged in at"
> 14) Check the item in, note it should float

I'm not sure why this didn't get a signed off note from Koha Testing Docker. 
This is signed off by me!
Comment 8 Kyle M Hall (khall) 2022-08-29 17:51:18 UTC 
Created attachment 139964 [details] [review]
Bug 25426: Allow return policy to be selected via syspref and not just home library

1) Apply this patch
2) Run updatedatabase.pl
3) Verify CircControlReturnsBranch is set to home library by default
4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch )
6) Set a Return polity for Branch C to "Item floats" ( noreturn )
7) Create an item with homebranch of Branch A, holding branch of branch B
8) Log in as Branch C
9) Set CircControlReturnsBranch to "the library the item is currently held by"
10) Check the item in, note it should be returned to the holding library
11) Set CircControlReturnsBranch to "the library the item is owned by"
12) Check the item in, note it should be returned to the home library
13) Set CircControlReturnsBranch to "the library you are logged in at"
14) Check the item in, note it should float

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 9 Joonas Kylmälä 2022-08-30 16:47:15 UTC 
Are all three options needed for the new syspref or just some of them?
Comment 10 Kyle M Hall (khall) 2022-08-31 09:42:23 UTC 
(In reply to Joonas Kylmälä from comment #9)
> Are all three options needed for the new syspref or just some of them?

I believe all three options are good and necessary for librarians to be able to set their circ returns rules as they would like them to behave.
Comment 11 Katrin Fischer 2022-10-03 09:46:23 UTC 
Created attachment 141224 [details] [review]
Bug 25426: Allow return policy to be selected via syspref and not just home library

1) Apply this patch
2) Run updatedatabase.pl
3) Verify CircControlReturnsBranch is set to home library by default
4) Set a Return policy for Branch A to "Item returns home" ( homebranch )
5) Set a Return polity for Branch B to "Item returns to issuing library" ( holdingbranch )
6) Set a Return polity for Branch C to "Item floats" ( noreturn )
7) Create an item with homebranch of Branch A, holding branch of branch B
8) Log in as Branch C
9) Set CircControlReturnsBranch to "the library the item is currently held by"
10) Check the item in, note it should be returned to the holding library
11) Set CircControlReturnsBranch to "the library the item is owned by"
12) Check the item in, note it should be returned to the home library
13) Set CircControlReturnsBranch to "the library you are logged in at"
14) Check the item in, note it should float

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2022-10-03 09:46:28 UTC 
Created attachment 141225 [details] [review]
Bug 25426: (QA follow-up) Small fixes to POD and INSERT IGNORE or datatabase update

* Makes sure the system preference is added with INSERT IGNORE
* Some smaller fixes/additions to the POD of get_return_branch_policy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2022-10-03 09:46:33 UTC 
Created attachment 141226 [details] [review]
Bug 25426: (QA follow-up) Rephrase CircControlReturnsBranch system preference description

Rephrased in hope to make its use a little clearer:
* 'logged in at' replaced with 'checked in at' keeping self checks in mind
* 'At checkin' rephrased a bit to explain possible options

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2022-10-03 10:16:37 UTC 
Created attachment 141227 [details] [review]
Bug 25426: (QA follow-up) Update notes on circulation rules page for new pref

This adds a note to the circulation rules page that already
explained about ReservesControlBranch to also include information
about the new CircControlReturnsBranch system preference.
Comment 15 Katrin Fischer 2022-10-03 10:21:08 UTC 
I might have gotten carried away a little here, but hope you all approve:

1) System preference description updated a bit. 
2) Added a note to the circulation rules page... please also check that!
3) The option itemtype '*' described in the POD can currently not be set in the GUI. As we also set We cannot just add it,  'Hold policy' and 'Hold pickup library match' in the same row, this might require some more work, but happy to have the method already support it.
4) I set release-notes-needed as while I like and agree with this change it will mean a change in behavior for some libraries and we should document that.
Comment 16 Tomás Cohen Arazi (tcohen) 2022-10-18 13:30:41 UTC 
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 17 Lucas Gass (lukeg) 2022-12-05 20:41:35 UTC 
Unless I am mistaken this is an enhancement and shouldn't be backported to 22.05.x.