Bug 25426

Summary:	Allow return policy to be selected via syspref and not just home library
Product:	Koha	Reporter:	Andrew Fuerste-Henry <andrew>
Component:	Circulation	Assignee:	Kyle M Hall (khall) <kyle>
Status:	CLOSED FIXED	QA Contact:	Katrin Fischer <katrin.fischer>
Severity:	normal
Priority:	P5 - low	CC:	bugzilla, caroline.cyr-la-rose, fiona.borthwick, gmcharlt, janet.mcgowan, joonas.kylmala, kyle.m.hall, kyle, lucas, rcoert
Version:	Main
Hardware:	All
OS:	All
See Also:	https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28841 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31046
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 Bug 25426: Allow return policy to be selected via syspref and not just home library Bug 25426: Allow return policy to be selected via syspref and not just home library Bug 25426: Allow return policy to be selected via syspref and not just home library Bug 25426: (QA follow-up) Small fixes to POD and INSERT IGNORE or datatabase update Bug 25426: (QA follow-up) Rephrase CircControlReturnsBranch system preference description Bug 25426: (QA follow-up) Update notes on circulation rules page for new pref

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.