Bug 33761 - Holds queue is not selecting items with completed transfers
Summary: Holds queue is not selecting items with completed transfers
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Nick Clemens
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 24295
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-18 11:06 UTC by Nick Clemens
Modified: 2023-12-28 20:46 UTC (History)
6 users (show)

See Also:
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


Attachments
Bug 33761: Unit test (2.08 KB, patch)
2023-05-18 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33761: Alter query to remove items with active transfers from available list (2.45 KB, patch)
2023-05-18 11:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 33761: Unit test (2.14 KB, patch)
2023-05-18 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33761: Alter query to remove items with active transfers from available list (2.52 KB, patch)
2023-05-18 12:56 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 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 2023-05-18 11:12:41 UTC
Created attachment 151386 [details] [review]
Bug 33761: Unit test
Comment 2 Nick Clemens 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