Bug 25724 - Transferred item checked in to shelving cart has cart location removed when transfer is filled
Summary: Transferred item checked in to shelving cart has cart location removed when t...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-06-12 10:50 UTC by Nick Clemens
Modified: 2021-06-14 21:31 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
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 (1.61 KB, patch)
2020-06-12 11:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25724: Do not call ModReserveStatus when completing transfer (1.78 KB, patch)
2020-06-12 11:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25724: Unit tests (1.72 KB, patch)
2020-06-29 18:32 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25724: Do not call ModReserveStatus when completing transfer (1.89 KB, patch)
2020-06-29 18:32 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25724: Unit tests (1.78 KB, patch)
2020-07-26 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25724: Do not call ModReserveStatus when completing transfer (1.95 KB, patch)
2020-07-26 14:43 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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
Comment 10 Victor Grousset/tuxayo 2020-09-18 20:26:45 UTC
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.