Bug 20747 - Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library
Summary: Allow LocalHoldsPriority to fill by hierarchical groups system rather than in...
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement
Assignee: Lisette Scheer
QA Contact: Matt Blenkinsop
URL: https://bywatersolutions.com/crowdsou...
Keywords: roadmap_25_05
Depends on: 11126 16735
Blocks:
  Show dependency treegraph
 
Reported: 2018-05-09 16:36 UTC by Lisette Scheer
Modified: 2025-05-15 06:35 UTC (History)
23 users (show)

See Also:
GIT URL:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds the ability to set the "LocalHoldsPriority" system preference to more options: -`Give Library` (current behavior when on) -`Give Library and Group` -`Give Library Group` -`Don't give` (current behavior when off). The system preference will be updated in existing installations to match with the new options based on current settings. Note: The holds queue needs to be rebuilt after changing the preference to update items that have already been targeted by the queue.
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library (23.23 KB, patch)
2025-04-21 16:05 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: DB Rev (1.74 KB, patch)
2025-04-21 16:05 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: (QA follow-up) fix permissions on database update (3.96 KB, patch)
2025-04-22 20:24 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: (follow-up) fix permissions and location on database update file (4.25 KB, patch)
2025-04-22 20:53 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: (follow-up) fix atomic update (1.45 KB, patch)
2025-04-22 21:03 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library (23.26 KB, patch)
2025-04-22 21:34 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 20747: DB Rev (1.78 KB, patch)
2025-04-22 21:34 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 20747: (follow-up) fix atomic update (1.49 KB, patch)
2025-04-22 21:34 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options (22.60 KB, patch)
2025-05-06 22:00 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options (22.63 KB, patch)
2025-05-07 20:25 UTC, Jason Robb
Details | Diff | Splinter Review
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library (23.32 KB, patch)
2025-05-07 20:29 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 20747: DB Rev (1.84 KB, patch)
2025-05-07 20:29 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 20747: (follow-up) fix atomic update (1.54 KB, patch)
2025-05-07 20:29 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options (22.64 KB, patch)
2025-05-07 20:29 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library (23.39 KB, patch)
2025-05-09 15:25 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 20747: DB Rev (1.90 KB, patch)
2025-05-09 15:25 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 20747: (follow-up) fix atomic update (1.60 KB, patch)
2025-05-09 15:25 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options (22.71 KB, patch)
2025-05-09 15:25 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 20747: (QA follow-up) Fix sysprefs.sql (1.94 KB, patch)
2025-05-09 15:25 UTC, Matt Blenkinsop
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lisette Scheer 2018-05-09 16:36:51 UTC
It would be great if the LocalHoldsPriority could be used within a district in a consortium. 


>>Christopher expressed concern, adding on “grouping” – some libraries may not >>want to be effected.  Typically this is all or nothing or on or off.  Example, >>the circulation of Mobile Hotspots; hotspots are popular outside of CDA but CDA >>patrons get priority, Rathdrum is pushed down.  Consider an “Item type” or “Item >>level” opt out which would be similar to how suppression works.  If the >>modification increased features and flexibility, there may be more interest from >>the larger group in supporting the enhancement.  Group structure for settings, a >>hierarchy based system, and refinement is needed.
Comment 1 Tomás Cohen Arazi (tcohen) 2019-03-02 13:08:50 UTC
Lisette, could you please explain a bit further?
Also, can you take a look  at bug 22284 and see how it fits?
Comment 2 Lisette Scheer 2019-03-05 15:29:36 UTC
LocalHoldsPriority currently looks only at home library.
We would like if it could look at groups and look first at the whole group then outside the group for priority. 
Example: MOS and TRO are in group together and LAP is not in the same group. 
MOS has the only copy of a new book and the holds are:
1. MOS patron
2. LAP patron1
3. LAP patron2
4. TRO patron

We would like it to fill:
1. MOS Patron
2. TRO Patron
3. LAP patron1
4. LAP patron2 

Assuming no additional holds were place by MOS or TRO patrons before it got to LAP. 

Another Example might be if LAP also had a copy but the order of holds was:
1. MOS patron
2. LAP patron1
3. TRO patron
4. LAP patron2

The 2 copies could bounce around between the 3 branches taking extra courier time (LAP to TRO has to take 3 different courier routes, LAP to MOS 2) but with what we would like, the MOS copy would fill for MOS and then TRO (1 courier route) then go out to the consortium and the LAP one could stay in LAP (or its group) before going out to the consortium, potentially cutting down wait time for branches where their group has purchased copies. 


I may have it wrong, but it looks like bug 22284 would prevent the holds from going out of the groups? That isn't what we would want, as we still want our items to be able to go out of the district once holds for our district have been filled.
Comment 3 Joy Nelson 2019-03-26 14:42:26 UTC
Lisette-
The way that Bug 22284 would work includes functionality for 'non local/group' holds to be filled after all the local holds have been satisfied.

joy

> I may have it wrong, but it looks like bug 22284 would prevent the holds
> from going out of the groups? That isn't what we would want, as we still
> want our items to be able to go out of the district once holds for our
> district have been filled.
Comment 4 Lisette Scheer 2019-03-26 19:52:13 UTC
Joy, 
In that case, it looks like bug 22284 would work for our purposes here at Valnet. Thanks for the clarification! 
Lisette 
(In reply to Joy Nelson from comment #3)


> Lisette-
> The way that Bug 22284 would work includes functionality for 'non
> local/group' holds to be filled after all the local holds have been
> satisfied.
> 
> joy
Comment 5 Tomás Cohen Arazi (tcohen) 2019-04-15 23:05:07 UTC

*** This bug has been marked as a duplicate of bug 22284 ***
Comment 6 Lisette Scheer 2021-12-21 17:53:27 UTC
But 22284 did not fix this issue. 

It only limited completely to the group, rather than prioritizing the group and then going out of the group, similarly to Bug 12004 but to allow just group prioritization and not branch prioritization. 

The reason that using just the transportation cost matrix doesn't just do this has to do with when an item is already checked out and returned, it'll send the item far away and then when the copy for the far away branch comes in, it gets routed up to us. 

Lisette
Comment 7 Michelle Spinney 2023-07-27 13:55:29 UTC
This is really important for consortia, or any network with branches. We need this for our network as we have 2 islands that use their own intra-island delivery as well as our state delivery service. Not having this ability has caused a lot of issues during our busy summer tourist season. The transport cost matrix has been no help.
Comment 8 Eileen Chandler 2023-07-27 15:53:03 UTC
CLAMS wants 
1. individual library
2. Hierarchical group
3. larger group.
We don't want to cut out the individual library as the first choice. 
New bug perhaps? 
Cutting out the individual library and preferring a group as the first option will not work for us.
Comment 9 George Williams (NEKLS) 2024-05-29 20:12:43 UTC
I want the development to have enough flexibility to do:

1. individual library
2. hold group
3. entire system

or:

1. hold group
2. entire system

I don't see any reason it couldn't be designed to do both.

(In reply to Eileen Chandler from comment #8)
> CLAMS wants 
> 1. individual library
> 2. Hierarchical group
> 3. larger group.
> We don't want to cut out the individual library as the first choice. 
> New bug perhaps? 
> Cutting out the individual library and preferring a group as the first
> option will not work for us.
Comment 10 Chanel Wheeler 2024-08-12 19:33:07 UTC
We agree with
1. individual library
2. hold group
3. entire system

We had this in our last ILS and depended on it. We're at a loss as to what to do now.
Comment 11 Lisette Scheer 2025-04-21 16:05:34 UTC Comment hidden (obsolete)
Comment 12 Lisette Scheer 2025-04-21 16:05:38 UTC
Created attachment 181221 [details] [review]
Bug 20747: DB Rev

This patch adds a DB rev to update the system preference in existing installations.

 # Please enter the commit message for your changes. Lines starting
Comment 13 Jason Robb 2025-04-22 20:13:40 UTC
This mostly worked. When I did step 12 I had to toggle the syspref to get it to work. I also got this error when I ran the qa tools.

 FAIL   installer/data/mysql/db_revs/bug_20747-updatelocalholdspriority_syspref.pl
   FAIL   file permissions
                File must have the exec flag
Comment 14 Lisette Scheer 2025-04-22 20:24:17 UTC
Created attachment 181311 [details] [review]
Bug 20747: (QA follow-up) fix permissions on database update

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH
/Setup Groups and confirm current behavior/
1. Set the " LocalHoldsPriority " system preference to "Give", "LocalHoldsPriorityPatronControl" to "home library" and "LocalHoldsPriorityItemControl" to "home library"
2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 for pickup at Centerville (Priority 1)
  - One for 23529001000463 for pickup at fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold, not the first.

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat steps 7-8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record in the following order:
    - Pickup at Fairfield -  23529000050113
    - Pickup at Franklin - 	23529000121682
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
17. Suspend the hold for 23529001000463
18. Check in 39999000000238 should try and fill to 23529000121682

/ check library group setting /
19. Reset the holds to match step 14.
20. Change " LocalHoldsPriority " to "Give group"
21. Check in 39999000000252 - should try and fill to Fairfield
22. Check in 39999000000238 - should try and fill to Franklin

/check no priority /
23. Change " LocalHoldsPriority " to "Don't give"
24. Reset the holds to match step 14.
25. Check in 39999000000238 - Should fill the Fairfield hold
26. Check in 39999000000252 - Should fill the Fairfield hold

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)
Comment 15 Lisette Scheer 2025-04-22 20:26:46 UTC
setting back to failed qa. I'm having some trouble with the dbrev.
Comment 16 Lisette Scheer 2025-04-22 20:53:09 UTC
Created attachment 181313 [details] [review]
Bug 20747: (follow-up) fix permissions and location on database update file

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH
/Setup Groups and confirm current behavior/
1. Set the " LocalHoldsPriority " system preference to "Give", "LocalHoldsPriorityPatronControl" to "home library" and "LocalHoldsPriorityItemControl" to "home library"
2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 for pickup at Centerville (Priority 1)
  - One for 23529001000463 for pickup at fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold, not the first.

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat steps 7-8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record in the following order:
    - Pickup at Fairfield -  23529000050113
    - Pickup at Franklin - 	23529000121682
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
17. Suspend the hold for 23529001000463
18. Check in 39999000000238 should try and fill to 23529000121682

/ check library group setting /
19. Reset the holds to match step 14.
20. Change " LocalHoldsPriority " to "Give group"
21. Check in 39999000000252 - should try and fill to Fairfield
22. Check in 39999000000238 - should try and fill to Franklin

/check no priority /
23. Change " LocalHoldsPriority " to "Don't give"
24. Reset the holds to match step 14.
25. Check in 39999000000238 - Should fill the Fairfield hold
26. Check in 39999000000252 - Should fill the Fairfield hold

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Bug 20747: (follow-up) Move atomic update
Comment 17 Lisette Scheer 2025-04-22 21:03:11 UTC
Created attachment 181314 [details] [review]
Bug 20747: (follow-up) fix atomic update

This update moves the atomic update to the correct place, updates permissions, and fixes an error in the original patch.
Comment 18 Jason Robb 2025-04-22 21:34:12 UTC
Created attachment 181319 [details] [review]
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH
/Setup Groups and confirm current behavior/
1. Set the " LocalHoldsPriority " system preference to "Give", "LocalHoldsPriorityPatronControl" to "home library" and "LocalHoldsPriorityItemControl" to "home library"
2. Create 2 Library hold Groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 for pickup at Centerville (Priority 1)
  - One for 23529001000463 for pickup at fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold, not the first.

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat steps 7-8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record:
    - Pickup at Fairfield -  23529000050113
    - Pickup at Franklin - 	23529000121682
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
17. Cancel the hold for 23529001000463
18. Check in 39999000000238 should try and fill to 23529000121682

/ check library group setting /
19. Reset the holds to match step 14.
20. Check in 39999000000252 - should try and fill to Fairfield
21. Check in 39999000000238 - should try and fill to Franklin

/check no priority /
22. Reset the holds to match step 14.
23. Check in 39999000000238 - Should fill the Fairfield hold
24. Check in 39999000000252 - Should fill the Franklin hold

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 19 Jason Robb 2025-04-22 21:34:15 UTC
Created attachment 181320 [details] [review]
Bug 20747: DB Rev

This patch adds a DB rev to update the system preference in existing installations.

 # Please enter the commit message for your changes. Lines starting

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 20 Jason Robb 2025-04-22 21:34:18 UTC
Created attachment 181321 [details] [review]
Bug 20747: (follow-up) fix atomic update

This update moves the atomic update to the correct place, updates permissions, and fixes an error in the original patch.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 21 Matt Blenkinsop 2025-04-25 16:24:17 UTC
Comment on attachment 181319 [details] [review]
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library

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

QA feedback:

I've got failing tests at t/db_dependent/HoldsQueue.t
Comment 22 Matt Blenkinsop 2025-04-25 16:27:13 UTC
Tried the review feature but it hasn't saved my comments, trying again
Comment 23 Matt Blenkinsop 2025-04-25 16:28:55 UTC
Comment on attachment 181319 [details] [review]
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library

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

QA feedback:

I've got failing tests at t/db_dependent/HoldsQueue.t, plus the below comment which applies to all the calls to AddReserve in the test

::: t/db_dependent/Holds/LocalHoldsPriority.t
@@ +111,5 @@
>  
> +# Create five record level holds
> +AddReserve(
> +    {
> +        branchcode     => $library1,

We're now passing the entire library object instead of just the branchcode
Comment 24 Lisette Scheer 2025-04-29 15:04:58 UTC Comment hidden (obsolete)
Comment 25 Lisette Scheer 2025-05-06 22:00:06 UTC
Created attachment 182008 [details] [review]
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options

This fixes the holds queue and associated tests.
I also updated the sql file to fix an issue and the LocalHoldsPriority tests to fix a typo.

I'm working on final unit tests for the holds queue for all the options for QA but I wanted to get the queue changes ready for sign-off.

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH

/Tip/
I kept the following tabs open:
-The holds screen (for suspending, checking order, etc)
-Check in screen (can ignore holds to keep them all in place)
-Holds to Pull with "All" for Libraries
-System Preferences

/Setup Groups and confirm current behavior/
1. Set the following system preferences:
-" LocalHoldsPriority " system preference to "Give"
-"LocalHoldsPriorityPatronControl" to "home library"
-"LocalHoldsPriorityItemControl" to "home library"
-"RealTimeHoldsQueue" to "Enable"

2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - Changed 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113 (Jordan)
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682 (Nicole)
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 (Henry) for pickup at Centerville (Priority 1)
  - One for 23529001000463 (Edna) for pickup at Fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold (Henry), not the first (Edna.) Ignore the hold
9. Edit the record then save to trigger real time queue update
10. On the Holds Queue (submit after each set of changes):
- Centerville should be for Henry
- Fairview should be for Edna

/apply patch and confirm behavior still works/
10. Apply patch
11. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
12. Confirm that " LocalHoldsPriority " is set to "Give Library"
13. Repeat steps 8-10, behavior should be the same.

/ check library then library group setting/
14. Change " LocalHoldsPriority " to "Give library then library group"
15. Set the following record level holds for the record in the following order:
*You can add Jordan and Nicole's, then rearrange the order.
    - Pickup at Fairfield -  23529000050113 (Jordan)
    - Pickup at Franklin - 	23529000121682 (Nicole)
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
17. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
17. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
18. On the Hold Queue:
- Centerville should be for Henry
- Fairview should be for Edna
19. Suspend the hold for 23529001000463 (Edna)
20. Check in 39999000000238 should try and fill to 23529000121682 (Nicole) (ignore the hold)
21. On the Holds Queue:
- Centerville should be for Henry
- Fairview should be for Nicole

/ check library group setting /
22. Reset the holds to match step 15.
23. Change " LocalHoldsPriority " to "Give group"
24. Check in 39999000000252 - should try and fill to Fairfield (Jordan) (ignore the hold)
25. Check in 39999000000238 - should try and fill to Franklin (Nicole) (ignore the hold)
26. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/check no priority /
27. Change " LocalHoldsPriority " to "Don't give"
28. Reset the holds to match step 15.
29. Check in 39999000000238 - Should fill to Fairfield (Jordan) (ignore the hold)
30. Check in 39999000000252 - Should fill to Fairfield hold (Jordan) (ignore the hold)
31. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)
Comment 26 Lisette Scheer 2025-05-07 16:41:22 UTC Comment hidden (obsolete)
Comment 27 Lisette Scheer 2025-05-07 17:08:43 UTC Comment hidden (obsolete)
Comment 28 Lisette Scheer 2025-05-07 17:48:58 UTC
less convoluted test plan for holds without the queue being built (fresh check in/new hold no rtq) and checking the holds queue:

To Test:
BEFORE APPLYING THE PATCH

/Tip/
I kept the following tabs open:
-The holds screen (for suspending, checking order, etc) 
-Check in screen (can ignore holds to keep them all in place)
-Holds to Pull with "All" for Libraries
-System Preferences 

/Setup Groups and confirm current behavior/
1. Set the following system preferences:
-" LocalHoldsPriority " system preference to "Give"
-"LocalHoldsPriorityPatronControl" to "home library"
-"LocalHoldsPriorityItemControl" to "home library" 

2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - Changed 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113 (Jordan)
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682 (Nicole)
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 (Henry) for pickup at Centerville (Priority 1)
  - One for 23529001000463 (Edna) for pickup at Fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold (Edna), not the first (Henry.) Ignore the hold

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat step 8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record in the following order:
*You can add Jordan and Nicole's, then rearrange the order.
    - Pickup at Fairfield -  23529000050113 (Jordan)
    - Pickup at Franklin - 	23529000121682 (Nicole)
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry) (ignore the hold)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna) (ignore the hold)
17. Suspend the hold for 23529001000463 (Edna)
18. Check in 39999000000238 should try and fill to 23529000121682 (Nicole) (ignore the hold)

/ check library group setting /
19. Reset the holds to match step 14.
20. Change " LocalHoldsPriority " to "Give group"
21. Check in 39999000000252 - should try and fill to Fairfield (Jordan) (ignore the hold)
22. Check in 39999000000238 - should try and fill to Franklin (Nicole) (ignore the hold)

/check no priority /
23. Change " LocalHoldsPriority " to "Don't give"
24. Reset the holds to match step 14.
25. Check in 39999000000238 - Should fill to Fairfield (Jordan) (ignore the hold)
26. Check in 39999000000252 - Should fill to Fairfield hold (Jordan) (ignore the hold)

/Check holds queue/

/"Give library then library group" hold queue - Library/
27. Repeat Step 13
28. Run perl misc/cronjobs/holds/build_holds_queue.pl
29. On the Hold Queue:
- Centerville should be for Henry
- Fairview should be for Edna

/"Give library then library group" hold queue - Group/
30. Repeat step 17
31. Run perl misc/cronjobs/holds/build_holds_queue.pl
32. On the Holds Queue:
- Centerville should be for Henry
- Fairview should be for Nicole

/"Give Group" hold queue/
33. Repeat steps 19-20
34. Run perl misc/cronjobs/holds/build_holds_queue.pl
35. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/"Don't Give" hold queue/
36. Repeat Steps 23-24
37. On the Holds Queue
- Centerville should be for Jordan
- Fairview should be for Nicole

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)
Comment 29 Lisette Scheer 2025-05-07 19:26:27 UTC
Per some discussion with Jason, I've filed a new bug for allowing a staff interface trigger of a full holds queue rebuild. The queue has to be rebuilt after the preference is changed to take effect if the items were already queued. Bug 39859
Comment 30 Jason Robb 2025-05-07 20:25:17 UTC
Created attachment 182048 [details] [review]
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options

This fixes the holds queue and associated tests.
I also updated the sql file to fix an issue and the LocalHoldsPriority tests to fix a typo.

I'm working on final unit tests for the holds queue for all the options for QA but I wanted to get the queue changes ready for sign-off.

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH

/Tip/
I kept the following tabs open:
-The holds screen (for suspending, checking order, etc)
-Check in screen (can ignore holds to keep them all in place)
-Holds to Pull with "All" for Libraries
-System Preferences

/Setup Groups and confirm current behavior/
1. Set the following system preferences:
-" LocalHoldsPriority " system preference to "Give"
-"LocalHoldsPriorityPatronControl" to "home library"
-"LocalHoldsPriorityItemControl" to "home library"
-"RealTimeHoldsQueue" to "Enable"

2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - Changed 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113 (Jordan)
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682 (Nicole)
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 (Henry) for pickup at Centerville (Priority 1)
  - One for 23529001000463 (Edna) for pickup at Fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold (Henry), not the first (Edna.) Ignore the hold
9. Edit the record then save to trigger real time queue update
10. On the Holds Queue (submit after each set of changes):
- Centerville should be for Henry
- Fairview should be for Edna

/apply patch and confirm behavior still works/
10. Apply patch
11. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
12. Confirm that " LocalHoldsPriority " is set to "Give Library"
13. Repeat steps 8-10, behavior should be the same.

/ check library then library group setting/
14. Change " LocalHoldsPriority " to "Give library then library group"
15. Set the following record level holds for the record in the following order:
*You can add Jordan and Nicole's, then rearrange the order.
    - Pickup at Fairfield -  23529000050113 (Jordan)
    - Pickup at Franklin - 	23529000121682 (Nicole)
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
17. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
17. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
18. On the Hold Queue:
- Centerville should be for Henry
- Fairview should be for Edna
19. Suspend the hold for 23529001000463 (Edna)
20. Check in 39999000000238 should try and fill to 23529000121682 (Nicole) (ignore the hold)
21. On the Holds Queue:
- Centerville should be for Henry
- Fairview should be for Nicole

/ check library group setting /
22. Reset the holds to match step 15.
23. Change " LocalHoldsPriority " to "Give group"
24. Check in 39999000000252 - should try and fill to Fairfield (Jordan) (ignore the hold)
25. Check in 39999000000238 - should try and fill to Franklin (Nicole) (ignore the hold)
26. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/check no priority /
27. Change " LocalHoldsPriority " to "Don't give"
28. Reset the holds to match step 15.
29. Check in 39999000000238 - Should fill to Fairfield (Jordan) (ignore the hold)
30. Check in 39999000000252 - Should fill to Fairfield hold (Jordan) (ignore the hold)
31. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 31 Brendan Lawlor 2025-05-07 20:29:33 UTC
Created attachment 182049 [details] [review]
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH
/Setup Groups and confirm current behavior/
1. Set the " LocalHoldsPriority " system preference to "Give", "LocalHoldsPriorityPatronControl" to "home library" and "LocalHoldsPriorityItemControl" to "home library"
2. Create 2 Library hold Groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 for pickup at Centerville (Priority 1)
  - One for 23529001000463 for pickup at fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold, not the first.

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat steps 7-8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record:
    - Pickup at Fairfield -  23529000050113
    - Pickup at Franklin - 	23529000121682
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
17. Cancel the hold for 23529001000463
18. Check in 39999000000238 should try and fill to 23529000121682

/ check library group setting /
19. Reset the holds to match step 14.
20. Check in 39999000000252 - should try and fill to Fairfield
21. Check in 39999000000238 - should try and fill to Franklin

/check no priority /
22. Reset the holds to match step 14.
23. Check in 39999000000238 - Should fill the Fairfield hold
24. Check in 39999000000252 - Should fill the Franklin hold

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 32 Brendan Lawlor 2025-05-07 20:29:36 UTC
Created attachment 182050 [details] [review]
Bug 20747: DB Rev

This patch adds a DB rev to update the system preference in existing installations.

 # Please enter the commit message for your changes. Lines starting

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 33 Brendan Lawlor 2025-05-07 20:29:41 UTC
Created attachment 182051 [details] [review]
Bug 20747: (follow-up) fix atomic update

This update moves the atomic update to the correct place, updates permissions, and fixes an error in the original patch.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 34 Brendan Lawlor 2025-05-07 20:29:44 UTC
Created attachment 182052 [details] [review]
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options

This fixes the holds queue and associated tests.
I also updated the sql file to fix an issue and the LocalHoldsPriority tests to fix a typo.

I'm working on final unit tests for the holds queue for all the options for QA but I wanted to get the queue changes ready for sign-off.

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH

/Tip/
I kept the following tabs open:
-The holds screen (for suspending, checking order, etc)
-Check in screen (can ignore holds to keep them all in place)
-Holds to Pull with "All" for Libraries
-System Preferences

/Setup Groups and confirm current behavior/
1. Set the following system preferences:
-" LocalHoldsPriority " system preference to "Give"
-"LocalHoldsPriorityPatronControl" to "home library"
-"LocalHoldsPriorityItemControl" to "home library"
-"RealTimeHoldsQueue" to "Enable"

2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - Changed 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113 (Jordan)
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682 (Nicole)
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 (Henry) for pickup at Centerville (Priority 1)
  - One for 23529001000463 (Edna) for pickup at Fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold (Henry), not the first (Edna.) Ignore the hold
9. Edit the record then save to trigger real time queue update
10. On the Holds Queue (submit after each set of changes):
- Centerville should be for Henry
- Fairview should be for Edna

/apply patch and confirm behavior still works/
10. Apply patch
11. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
12. Confirm that " LocalHoldsPriority " is set to "Give Library"
13. Repeat steps 8-10, behavior should be the same.

/ check library then library group setting/
14. Change " LocalHoldsPriority " to "Give library then library group"
15. Set the following record level holds for the record in the following order:
*You can add Jordan and Nicole's, then rearrange the order.
    - Pickup at Fairfield -  23529000050113 (Jordan)
    - Pickup at Franklin - 	23529000121682 (Nicole)
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
17. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
17. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
18. On the Hold Queue:
- Centerville should be for Henry
- Fairview should be for Edna
19. Suspend the hold for 23529001000463 (Edna)
20. Check in 39999000000238 should try and fill to 23529000121682 (Nicole) (ignore the hold)
21. On the Holds Queue:
- Centerville should be for Henry
- Fairview should be for Nicole

/ check library group setting /
22. Reset the holds to match step 15.
23. Change " LocalHoldsPriority " to "Give group"
24. Check in 39999000000252 - should try and fill to Fairfield (Jordan) (ignore the hold)
25. Check in 39999000000238 - should try and fill to Franklin (Nicole) (ignore the hold)
26. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/check no priority /
27. Change " LocalHoldsPriority " to "Don't give"
28. Reset the holds to match step 15.
29. Check in 39999000000238 - Should fill to Fairfield (Jordan) (ignore the hold)
30. Check in 39999000000252 - Should fill to Fairfield hold (Jordan) (ignore the hold)
31. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 35 Brendan Lawlor 2025-05-07 20:40:22 UTC
Thanks for the detailed test plan, Lisette!

This worked exactly how described.

Notes:
Once during testing the holds were filled differently than what was expected from the plan, but I realized that it was because I was checking all the items into Centerville. When I reset the item's current library to match the home library the holds were fulfilled as expected from the test plan.

To help with getting set up to test this you can use this SQL and then continue on from step 7:

-- set initial system preferences
UPDATE systempreferences SET value = '1' WHERE variable = 'LocalHoldsPriority';
UPDATE systempreferences SET value = 'HomeLibrary' WHERE variable = 'LocalHoldsPriorityPatronControl';
UPDATE systempreferences SET value = 'homebranch' WHERE variable = 'LocalHoldsPriorityItemControl';

-- create library groups
INSERT INTO library_groups (title,  ft_local_hold_group) VALUES ('Group 1', 1);
INSERT INTO library_groups (title,  ft_local_hold_group) VALUES ('Group 2', 1);
-- add libraries to groups 
INSERT INTO library_groups (parent_id,  branchcode) VALUES (1, 'CPL');
INSERT INTO library_groups (parent_id,  branchcode) VALUES (1, 'FFL');
INSERT INTO library_groups (parent_id,  branchcode) VALUES (2, 'FPL');
INSERT INTO library_groups (parent_id,  branchcode) VALUES (2, 'FRL');

-- set up patrons
UPDATE borrowers SET branchcode = 'CPL' WHERE cardnumber = '23529000035676';
UPDATE borrowers SET branchcode = 'FFL' WHERE cardnumber = '23529000050113'; 
UPDATE borrowers SET branchcode = 'FPL' WHERE cardnumber = '23529001000463'; 
UPDATE borrowers SET branchcode = 'FRL' WHERE cardnumber = '23529000121682';

-- set up items 
UPDATE items SET homebranch = 'FPL' WHERE barcode = '39999000000238';
Comment 36 Matt Blenkinsop 2025-05-09 15:25:21 UTC
Created attachment 182191 [details] [review]
Bug 20747: Allow LocalHoldsPriority to fill by hierarchical groups system rather than individual library

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH
/Setup Groups and confirm current behavior/
1. Set the " LocalHoldsPriority " system preference to "Give", "LocalHoldsPriorityPatronControl" to "home library" and "LocalHoldsPriorityItemControl" to "home library"
2. Create 2 Library hold Groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 for pickup at Centerville (Priority 1)
  - One for 23529001000463 for pickup at fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold, not the first.

/apply patch and confirm behavior still works/
9. Apply patch
10. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
11. Confirm that " LocalHoldsPriority " is set to "Give Library"
12. Repeat steps 7-8, behavior should be the same.

/ check library then library group setting/
13. Change " LocalHoldsPriority " to "Give library then library group"
14. Set the following record level holds for the record:
    - Pickup at Fairfield -  23529000050113
    - Pickup at Franklin - 	23529000121682
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
15. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
16. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
17. Cancel the hold for 23529001000463
18. Check in 39999000000238 should try and fill to 23529000121682

/ check library group setting /
19. Reset the holds to match step 14.
20. Check in 39999000000252 - should try and fill to Fairfield
21. Check in 39999000000238 - should try and fill to Franklin

/check no priority /
22. Reset the holds to match step 14.
23. Check in 39999000000238 - Should fill the Fairfield hold
24. Check in 39999000000252 - Should fill the Franklin hold

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 37 Matt Blenkinsop 2025-05-09 15:25:27 UTC
Created attachment 182192 [details] [review]
Bug 20747: DB Rev

This patch adds a DB rev to update the system preference in existing installations.

 # Please enter the commit message for your changes. Lines starting

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 38 Matt Blenkinsop 2025-05-09 15:25:31 UTC
Created attachment 182193 [details] [review]
Bug 20747: (follow-up) fix atomic update

This update moves the atomic update to the correct place, updates permissions, and fixes an error in the original patch.

Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 39 Matt Blenkinsop 2025-05-09 15:25:36 UTC
Created attachment 182194 [details] [review]
Bug 20747: (follow-up) Fix hold queue to work with LocalHoldPriority Group options

This fixes the holds queue and associated tests.
I also updated the sql file to fix an issue and the LocalHoldsPriority tests to fix a typo.

I'm working on final unit tests for the holds queue for all the options for QA but I wanted to get the queue changes ready for sign-off.

UPDATED TEST PLAN

This patch adds new options to the LocalHoldsPriority system preference to allow for the following options:
'None'                 the equivalent to 'Don't' pre-patch
'GiveLibrary'          the equivalent to 'Do' pre-patch
'GiveLibraryAndGroup'  Allows for checking the library first, then hold groups
'GiveGroup'            Allows to prioritize within a group without prioritizing any library first.

The included patches will update the preference in existing installations to maintain current behavior.

To Test:
BEFORE APPLYING THE PATCH

/Tip/
I kept the following tabs open:
-The holds screen (for suspending, checking order, etc)
-Check in screen (can ignore holds to keep them all in place)
-Holds to Pull with "All" for Libraries
-System Preferences

/Setup Groups and confirm current behavior/
1. Set the following system preferences:
-" LocalHoldsPriority " system preference to "Give"
-"LocalHoldsPriorityPatronControl" to "home library"
-"LocalHoldsPriorityItemControl" to "home library"
-"RealTimeHoldsQueue" to "Enable"

2. Create 2 Library Groups
  - Set them to be hold groups
  - I called mine group 1 & group 2
3. Add 2 libraries each to each group.
  - Group 1
    - Centerville
    - Fairfield
  - Group 2
    - Fairview
    - Franklin
4. Find a patron for each branch in the groups
  - Centerville - Changed 23529000035676 (Henry)
  - Fairfield - Changed 23529000050113 (Jordan)
  - Fairview - Changed 23529001000463 (Edna)
  - Franklin - 	23529000121682 (Nicole)
5. Find a record with at least 2 items
6. If needed change the home and holding libraries so you have at least one item for each hold group
  - Centerville - 39999000000252
  - Fairview - Changed 39999000000238
7. Place 2 record level holds for the record.
  - One for 23529000035676 (Henry) for pickup at Centerville (Priority 1)
  - One for 23529001000463 (Edna) for pickup at Fairview (Priority 2)
8. Check in 39999000000238. The hold popup should be for the second hold (Henry), not the first (Edna.) Ignore the hold
9. Edit the record then save to trigger real time queue update
10. On the Holds Queue (submit after each set of changes):
- Centerville should be for Henry
- Fairview should be for Edna

/apply patch and confirm behavior still works/
10. Apply patch
11. Rebuild the database then restart_all (in sandboxes it's an action, in ktd " dbic && updatedatabase && restart_all ")
12. Confirm that " LocalHoldsPriority " is set to "Give Library"
13. Repeat steps 8-10, behavior should be the same.

/ check library then library group setting/
14. Change " LocalHoldsPriority " to "Give library then library group"
15. Set the following record level holds for the record in the following order:
*You can add Jordan and Nicole's, then rearrange the order.
    - Pickup at Fairfield -  23529000050113 (Jordan)
    - Pickup at Franklin - 	23529000121682 (Nicole)
    - Pickup at Centerville - 23529000035676 (Henry)
    - Pickup at Fairview -  23529001000463 (Edna)
17. Check in 39999000000252 - should try and fill to Centerville - 23529000035676 (Henry)
17. Check in 39999000000238 - should try and fill to Fairview -  23529001000463 (Edna)
18. On the Hold Queue:
- Centerville should be for Henry
- Fairview should be for Edna
19. Suspend the hold for 23529001000463 (Edna)
20. Check in 39999000000238 should try and fill to 23529000121682 (Nicole) (ignore the hold)
21. On the Holds Queue:
- Centerville should be for Henry
- Fairview should be for Nicole

/ check library group setting /
22. Reset the holds to match step 15.
23. Change " LocalHoldsPriority " to "Give group"
24. Check in 39999000000252 - should try and fill to Fairfield (Jordan) (ignore the hold)
25. Check in 39999000000238 - should try and fill to Franklin (Nicole) (ignore the hold)
26. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/check no priority /
27. Change " LocalHoldsPriority " to "Don't give"
28. Reset the holds to match step 15.
29. Check in 39999000000238 - Should fill to Fairfield (Jordan) (ignore the hold)
30. Check in 39999000000252 - Should fill to Fairfield hold (Jordan) (ignore the hold)
31. On the Holds Queue:
- Centerville should be for Jordan
- Fairview should be for Nicole

/Also check/
Item level holds behave as expected
Different combinations of LocalHoldsPriorityItemControl and LocalHoldsPriorityPatronControl

Sponsored by: Cape Libraries Automated Materials Sharing
Sponsored by: Northeast Kansas Library System (NEKLS)

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 40 Matt Blenkinsop 2025-05-09 15:25:41 UTC
Created attachment 182195 [details] [review]
Bug 20747: (QA follow-up) Fix sysprefs.sql

The SQL file doesn't load properly due to a missing comma - this patch fixes that

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 41 Katrin Fischer 2025-05-15 06:35:41 UTC
Pushed for 25.05!

Well done everyone, thank you!