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
Created attachment 90619 [details] [review] Bug 23129: Unit test
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
Created attachment 91488 [details] [review] [SIGNED-OFF] Bug 23129: Unit test Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
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>
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>
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>
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
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>
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>
Apparently I forgot to switch the status...
Nice work! Pushed to master for 19.11.00
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?
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.
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>
Discussed with Nick, the tests was wrong and so I have updated it in a followup.
*** Bug 15439 has been marked as a duplicate of this bug. ***
Any thoughts on backporting to 19.05? -Marti Huntsville-Madison County Public Library
(In reply to Martha Fuerst from comment #17) > Any thoughts on backporting to 19.05? > > -Marti > Huntsville-Madison County Public Library +1
*** Bug 15892 has been marked as a duplicate of this bug. ***
(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.
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!
Backported to 19.05.x branch for 19.05.16