Bug 33761

Summary: Holds queue is not selecting items with completed transfers
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: critical    
Priority: P5 - low CC: acampbell, gmcharlt, jonathan.druart, Laura.escamilla, martin.renvoize, sophie.meynieux
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30580
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.07
Bug Depends on: 24295    
Bug Blocks:    
Attachments: Bug 33761: Unit test
Bug 33761: Alter query to remove items with active transfers from available list
Bug 33761: Unit test
Bug 33761: Alter query to remove items with active transfers from available list

Description Nick Clemens (kidclamp) 2023-05-18 11:06:32 UTC
Bug 24295 removed the use of GetTransfer by adding a join to branchtransfers when getting available items.

The conditions are right, datearrived and datecancelled are null - but they need to be in the join, otherwise we remove all items with completed transfers from the results
Comment 1 Nick Clemens (kidclamp) 2023-05-18 11:12:41 UTC
Created attachment 151386 [details] [review]
Bug 33761: Unit test
Comment 2 Nick Clemens (kidclamp) 2023-05-18 11:12:43 UTC
Created attachment 151387 [details] [review]
Bug 33761: Alter query to remove items with active transfers from available list

Current code removes all items without an active transfer, and any with completed transfers.

This patch moves the conditionals for transfers into the join, then adds a new
condition to remove items with active transfers.

To test:
1 - Apply unit test patch only
2 - prove -v t/db_dependent/HoldsQueue.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/HoldsQueue.t
6 - Success!
Comment 3 Martin Renvoize 2023-05-18 12:56:35 UTC
Created attachment 151413 [details] [review]
Bug 33761: Unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2023-05-18 12:56:37 UTC
Created attachment 151414 [details] [review]
Bug 33761: Alter query to remove items with active transfers from available list

Current code removes all items without an active transfer, and any with completed transfers.

This patch moves the conditionals for transfers into the join, then adds a new
condition to remove items with active transfers.

To test:
1 - Apply unit test patch only
2 - prove -v t/db_dependent/HoldsQueue.t
3 - It fails
4 - Apply second patch
5 - prove -v t/db_dependent/HoldsQueue.t
6 - Success!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2023-05-18 13:10:02 UTC
Thanks for the prompt fix here Nick.

Tested and working.. kinda silly I missed it until now.

Signing off
Comment 6 Martin Renvoize 2023-05-18 13:28:27 UTC
As this is critical.. lets go straight for PQA here.. It's a clear and simple fix with solid unit tests.

Passing QA
Comment 7 Tomás Cohen Arazi 2023-05-18 13:54:48 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 8 Laura Escamilla 2023-05-19 18:40:19 UTC
Can this be backported to 22.11.x?
Comment 9 Matt Blenkinsop 2023-06-08 16:35:25 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 10 Sophie MEYNIEUX 2023-07-19 12:06:19 UTC
what is the value of x please ? for 22.11
Comment 11 Sophie MEYNIEUX 2023-07-19 12:26:19 UTC
Found, thanks