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: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords: Manual, release-notes-needed
Depends on:
Blocks: 23695
  Show dependency treegraph
 
Reported: 2019-06-14 18:11 UTC by Nick Clemens
Modified: 2019-10-15 15:52 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


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.