Bug 25724

Summary: Transferred item checked in to shelving cart has cart location removed when transfer is filled
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Nick Clemens <nick>
Status: Pushed to oldstable --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: aleisha, george, gmcharlt, kyle.m.hall, lucas
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Who signed the patch off: Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03, 19.11.09
Attachments: Bug 25724: Unit tests
Bug 25724: Do not call ModReserveStatus when completing transfer
Bug 25724: Unit tests
Bug 25724: Do not call ModReserveStatus when completing transfer
Bug 25724: Unit tests
Bug 25724: Do not call ModReserveStatus when completing transfer

Description Nick Clemens 2020-06-12 10:50:53 UTC
To recreate:
1 - Set  UpdateItemLocationOnCheckin  to:
    _ALL_: CART
2 - SetAutomaticItemReturn = Do
3 - Check an item in at a different branch than it's homebranch to create a transfer
4 - Check the item in at it's homebranch
5 - View the item details page
6 - Item is not in CART location
Comment 1 Nick Clemens 2020-06-12 11:16:18 UTC
Created attachment 105790 [details] [review]
Bug 25724: Unit tests
Comment 2 Nick Clemens 2020-06-12 11:16:20 UTC
Created attachment 105791 [details] [review]
Bug 25724: Do not call ModReserveStatus when completing transfer

I can not see how this code is useful here. It checks for a reserve with priority 0 and found = NULL
That is not a status that should occur when filling a transfer. Either the found is 'T' if we are transferring due
to the hold, or the hold was placed after the transfer was initiated, and so the priority is not 0

Additional, AddReturn checks for reserves later and asks the staff to confirm waiting status.

ModReserveStatus also calls CartToShelf regardless of what happens here.

To test:
1 - Set  UpdateItemLocationOnCheckin  to:
    _ALL_: CART
2 - SetAutomaticItemReturn = Do
3 - Check an item in at a different branch than it's homebranch to create a transfer
4 - Check the item in at it's homebranch
5 - View the item details page
6 - Item is not in CART location
7 - Apply patch
8 - Repeat
9 - Item is in CART location after completion of transfer
Comment 3 Andrew Fuerste-Henry 2020-06-29 18:32:17 UTC
Created attachment 106416 [details] [review]
Bug 25724: Unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 4 Andrew Fuerste-Henry 2020-06-29 18:32:20 UTC
Created attachment 106417 [details] [review]
Bug 25724: Do not call ModReserveStatus when completing transfer

I can not see how this code is useful here. It checks for a reserve with priority 0 and found = NULL
That is not a status that should occur when filling a transfer. Either the found is 'T' if we are transferring due
to the hold, or the hold was placed after the transfer was initiated, and so the priority is not 0

Additional, AddReturn checks for reserves later and asks the staff to confirm waiting status.

ModReserveStatus also calls CartToShelf regardless of what happens here.

To test:
1 - Set  UpdateItemLocationOnCheckin  to:
    _ALL_: CART
2 - SetAutomaticItemReturn = Do
3 - Check an item in at a different branch than it's homebranch to create a transfer
4 - Check the item in at it's homebranch
5 - View the item details page
6 - Item is not in CART location
7 - Apply patch
8 - Repeat
9 - Item is in CART location after completion of transfer

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jason Robb <jrobb@sekls.org>
Comment 5 Katrin Fischer 2020-07-26 14:43:20 UTC
Created attachment 107372 [details] [review]
Bug 25724: Unit tests

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jason Robb <jrobb@sekls.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2020-07-26 14:43:24 UTC
Created attachment 107373 [details] [review]
Bug 25724: Do not call ModReserveStatus when completing transfer

I can not see how this code is useful here. It checks for a reserve with priority 0 and found = NULL
That is not a status that should occur when filling a transfer. Either the found is 'T' if we are transferring due
to the hold, or the hold was placed after the transfer was initiated, and so the priority is not 0

Additional, AddReturn checks for reserves later and asks the staff to confirm waiting status.

ModReserveStatus also calls CartToShelf regardless of what happens here.

To test:
1 - Set  UpdateItemLocationOnCheckin  to:
    _ALL_: CART
2 - SetAutomaticItemReturn = Do
3 - Check an item in at a different branch than it's homebranch to create a transfer
4 - Check the item in at it's homebranch
5 - View the item details page
6 - Item is not in CART location
7 - Apply patch
8 - Repeat
9 - Item is in CART location after completion of transfer

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jason Robb <jrobb@sekls.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Jonathan Druart 2020-07-27 15:25:59 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 8 Lucas Gass 2020-07-31 20:50:47 UTC
backported to 20.05.x for 20.05.03
Comment 9 Aleisha Amohia 2020-08-03 23:46:59 UTC
backported to 19.11.x for 19.11.09