Bug 23129 - Items holdingbranch should be set to the originating library when generating a transfer
Summary: Items holdingbranch should be set to the originating library when generating ...
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:
: 15439 15892 (view as bug list)
Depends on:
Blocks: 27692 23695 25883 26078
  Show dependency treegraph
 
Reported: 2019-06-14 18:11 UTC by Nick Clemens
Modified: 2021-12-13 21:10 UTC (History)
11 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:
19.11.00, 19.05.16


Attachments
Bug 23129: Unit test (994 bytes, patch)
2019-06-14 18:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23129: Set holdingbranch to library that initiates transfer (1.09 KB, patch)
2019-06-14 18:20 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 23129: Unit test (1.03 KB, patch)
2019-07-12 13:41 UTC, Claire Gravely
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 23129: Set holdingbranch to library that initiates transfer (1.15 KB, patch)
2019-07-12 13:41 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 23129: Unit test (1.08 KB, patch)
2019-09-20 19:31 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23129: Set holdingbranch to library that initiates transfer (1.20 KB, patch)
2019-09-20 19:31 UTC, Bouzid Fergani
Details | Diff | Splinter Review
Bug 23129: Unit test (1.14 KB, patch)
2019-09-30 09:50 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23129: Set holdingbranch to library that initiates transfer (1.26 KB, patch)
2019-09-30 09:50 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23129: (RM follow-up) Fix failing test (1.57 KB, patch)
2019-10-15 15:51 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 2019-06-14 18:11:59 UTC
In C4::Items::ModItemTransfer we call ModItem to update the holdingbranch of the item.

Currently we set this to the transfer destination. By definition we are generating a transfer because the item has not yet arrived. We should set the holdingbranch to the current location of the item
Comment 1 Nick Clemens 2019-06-14 18:20:24 UTC
Created attachment 90619 [details] [review]
Bug 23129: Unit test
Comment 2 Nick Clemens 2019-06-14 18:20:27 UTC
Created attachment 90620 [details] [review]
Bug 23129: Set holdingbranch to library that initiates transfer

To test:
1 - Find an item
2 - Check it in at anothe rbranch and initiate a transfer
3 - View the item details or check db - note that holdingbranch is set as the destination library
4 - Complete or cancel the transfer
5 - Apply patch
6 - Initiate a new transfer
7 - Note the holdingbranch is set to the initiating library
Comment 3 Claire Gravely 2019-07-12 13:41:28 UTC
Created attachment 91488 [details] [review]
[SIGNED-OFF] Bug 23129: Unit test

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 4 Claire Gravely 2019-07-12 13:41:32 UTC
Created attachment 91489 [details] [review]
[SIGNED-OFF] Bug 23129: Set holdingbranch to library that initiates transfer

To test:
1 - Find an item
2 - Check it in at anothe rbranch and initiate a transfer
3 - View the item details or check db - note that holdingbranch is set as the destination library
4 - Complete or cancel the transfer
5 - Apply patch
6 - Initiate a new transfer
7 - Note the holdingbranch is set to the initiating library

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 5 Bouzid Fergani 2019-09-20 19:31:21 UTC
Created attachment 93043 [details] [review]
Bug 23129: Unit test

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 6 Bouzid Fergani 2019-09-20 19:31:36 UTC
Created attachment 93045 [details] [review]
Bug 23129: Set holdingbranch to library that initiates transfer

To test:
1 - Find an item
2 - Check it in at anothe rbranch and initiate a transfer
3 - View the item details or check db - note that holdingbranch is set as the destination library
4 - Complete or cancel the transfer
5 - Apply patch
6 - Initiate a new transfer
7 - Note the holdingbranch is set to the initiating library

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Comment 7 Katrin Fischer 2019-09-30 09:49:49 UTC
This works for transfers initiated by holds and on returning the item at 'not the home library'. What's missing is the same behavior manual transfers initiated from the circulation home page. I have talked to Nick and will file a new bug for that. I suggest that both should be fixed/pushed pre-release so we have the same beahviour for all kinds of transfers:

Bug 23695 - Items holdingbranch should be set to the originating library when generating a manual transfer
Comment 8 Katrin Fischer 2019-09-30 09:50:29 UTC
Created attachment 93227 [details] [review]
Bug 23129: Unit test

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2019-09-30 09:50:33 UTC
Created attachment 93228 [details] [review]
Bug 23129: Set holdingbranch to library that initiates transfer

To test:
1 - Find an item
2 - Check it in at anothe rbranch and initiate a transfer
3 - View the item details or check db - note that holdingbranch is set as the destination library
4 - Complete or cancel the transfer
5 - Apply patch
6 - Initiate a new transfer
7 - Note the holdingbranch is set to the initiating library

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2019-10-14 06:00:43 UTC
Apparently I forgot to switch the status...
Comment 11 Martin Renvoize 2019-10-15 13:16:35 UTC
Nice work!

Pushed to master for 19.11.00
Comment 12 Martin Renvoize 2019-10-15 15:14:54 UTC
Hmm, so this breaks a test which expects the opposite behaviour when dealing with items getting marked as lost from the transfers window.

Question is, is the test incorrect or the functionality?
Comment 13 Martin Renvoize 2019-10-15 15:18:59 UTC
CC'd Alex Buckley as he wrote the tests that's currently failing.. I'd be interested in his thoughts regarding what the holdingbranch of the item should end up as during lost in transfer processes.
Comment 14 Martin Renvoize 2019-10-15 15:51:11 UTC
Created attachment 94219 [details] [review]
Bug 23129: (RM follow-up) Fix failing test

This patch corrects a test that was testing for the oposite behaviour to
that introduced in this patchset.  We have agreed on the bug that the
new behaviour is desired and so I have updated the test to follow.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2019-10-15 15:52:01 UTC
Discussed with Nick, the tests was wrong and so I have updated it in a followup.
Comment 16 Katrin Fischer 2020-01-08 22:33:54 UTC
*** Bug 15439 has been marked as a duplicate of this bug. ***
Comment 17 Martha Fuerst 2020-01-08 23:08:49 UTC
Any thoughts on backporting to 19.05?

-Marti
Huntsville-Madison County Public Library
Comment 18 Christopher Brannon 2020-01-08 23:17:30 UTC
(In reply to Martha Fuerst from comment #17)
> Any thoughts on backporting to 19.05?
> 
> -Marti
> Huntsville-Madison County Public Library

+1
Comment 19 Katrin Fischer 2020-01-08 23:44:53 UTC
*** Bug 15892 has been marked as a duplicate of this bug. ***
Comment 20 Victor Grousset/tuxayo 2020-09-20 17:18:19 UTC
(In reply to Martha Fuerst from comment #17)
> Any thoughts on backporting to 19.05?

Hi next (i.e. current) 19.05.x maintainer here. Is there there still interest in seeing this backported?
I could try to backport this and bug 26078 if you, Christopher or anyone else would be available to test that it doesn't cause regressions.
Comment 21 Martha Fuerst 2020-09-21 15:27:49 UTC
Yes, please. As many people won't see 19.11 until next spring and this fix greatly affects systems that do lots of transfers, I think it would be a very good idea to backport.

Thanks!
Comment 22 Victor Grousset/tuxayo 2020-10-11 11:11:03 UTC
Backported to 19.05.x branch for 19.05.16