Bug 24839

Summary: branchtransfers does not deal with holds
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: CirculationAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: 1joynelson, gmcharlt, kyle.m.hall, lucas, martin.renvoize, sally.healey
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05
Bug Depends on:    
Bug Blocks: 24620    
Attachments: Bug 24839: Return hold info to transfer script and set borrowernumber
Bug 24839: unit tests
Bug 24839: Return hold info to transfer script and set borrowernumber
Bug 24839: unit tests
Bug 24839: Return hold info to transfer script and set borrowernumber
Bug 24839: unit tests
Bug 24839: Return hold info to transfer script and set borrowernumber
Bug 24839: (QA follow-up) Remove unused variables

Description Nick Clemens (kidclamp) 2020-03-10 13:37:14 UTC
This code looks to have been broken for some time, we check reserves, but we don't pass the variable back to the script
Comment 1 Nick Clemens (kidclamp) 2020-03-10 13:59:45 UTC
Created attachment 100459 [details] [review]
Bug 24839: Return hold info to transfer script and set borrowernumber
Comment 2 Nick Clemens (kidclamp) 2020-03-10 13:59:48 UTC
Created attachment 100460 [details] [review]
Bug 24839: unit tests
Comment 3 Nick Clemens (kidclamp) 2020-03-10 14:02:38 UTC
Created attachment 100461 [details] [review]
Bug 24839: Return hold info to transfer script and set borrowernumber

To test:
1 - Place a hold on an item
2 - Go to Circulation->Transfers
3 - Attempt to transfer item to a branch it is not expected at
4 - No warning
5 - Apply patches
6 - Repeate
7 - You get a notice that there is a hold and must deal with the hold (or ignore)
Comment 4 David Nind 2020-03-10 19:56:59 UTC
Created attachment 100481 [details] [review]
Bug 24839: unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2020-03-10 19:57:03 UTC
Created attachment 100482 [details] [review]
Bug 24839: Return hold info to transfer script and set borrowernumber

To test:
1 - Place a hold on an item
2 - Go to Circulation->Transfers
3 - Attempt to transfer item to a branch it is not expected at
4 - No warning
5 - Apply patches
6 - Repeate
7 - You get a notice that there is a hold and must deal with the hold (or ignore)

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 Martin Renvoize 2020-03-11 11:23:13 UTC
transferbook is also called in rotating collections.. should the return value be used there too?
Comment 7 Martin Renvoize 2020-03-11 11:24:32 UTC
Created attachment 100542 [details] [review]
Bug 24839: unit tests

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2020-03-11 11:24:35 UTC
Created attachment 100543 [details] [review]
Bug 24839: Return hold info to transfer script and set borrowernumber

To test:
1 - Place a hold on an item
2 - Go to Circulation->Transfers
3 - Attempt to transfer item to a branch it is not expected at
4 - No warning
5 - Apply patches
6 - Repeate
7 - You get a notice that there is a hold and must deal with the hold (or ignore)

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-03-11 11:26:04 UTC
(In reply to Martin Renvoize from comment #6)
> transferbook is also called in rotating collections.. should the return
> value be used there too?

I think this can be handled in a follow-up bug report as currently Rotating Collections just ignores the response from transfer book entirely.

At some point soon I'd really like to actually see transferbook removed and the Koha Objects alternative used.. but that's down the line..

Passing QA
Comment 10 Martin Renvoize 2020-03-11 11:28:46 UTC
Created attachment 100544 [details] [review]
Bug 24839: (QA follow-up) Remove unused variables

Whilst QAing I spotted a couple of unused variables in the test.. no
harm in removing them ;)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-03-11 13:49:58 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-04-02 20:39:29 UTC
Backported to 19.11.x for 19.11.05
Comment 13 Lucas Gass 2020-04-10 15:32:31 UTC
tests would need to be rebased if this is needed in 19.05.x