Splitting this functionality out of bug 20844 to make the functionality changes clearer in the release notes and simplify QA
Created attachment 81695 [details] [review] Bug 21732: Clean up outstanding transfers on lost items When an item is marked as lost, the routine should also clean up any outstanding transfers.
Created attachment 81696 [details] [review] Bug 21732: (QA follow-up) Performance Improvement We don't really need to lookup transfers before deleting them here.
I'm not actually sure this should be governed by a system preference at all. It feels to me like a bug that transfers are not cleaned up when an Item is lost.. I'll ask for a second QA opinion on that.
Also.. I'd like to see a test added to t/db_dependent/Circulation.t for this.
Created attachment 81701 [details] [review] Bug 21732: Clean up outstanding transfers on lost items When an item is marked as lost, the routine should also clean up any outstanding transfers. Test plan: 1. Find a item which is in transfer, i.e. find an item with the text in the 'Status' field of the table in detail.pl that indicates it is in transfer 2. Now enable the new 'LostItemCancelOutstandingTransfers' syspref. This will now cancel any outstanding transfers on the item when it is marked as lost. 3. Set the item to 'Lost' either by clicking on Edit->Edit items from the detail.pl page OR clicking on the Items tab on the left side of the detail.pl page 4. Notice that the transfer is now cancelled for the item
Created attachment 81702 [details] [review] Bug 21732: (QA follow-up) Performance Improvement We don't really need to lookup transfers before deleting them here.
Created attachment 81703 [details] [review] Bug 21732: Clean up outstanding transfers on lost items When an item is marked as lost, the routine should also clean up any outstanding transfers. Test plan: 1. Find a item which is in transfer, i.e. find an item with the text in the 'Status' field of the table in detail.pl that indicates it is in transfer 2. Now enable the new 'LostItemCancelOutstandingTransfers' syspref. This will now cancel any outstanding transfers on the item when it is marked as lost. 3. Set the item to 'Lost' either by clicking on Edit->Edit items from the detail.pl page OR clicking on the Items tab on the left side of the detail.pl page 4. Notice that the transfer is now cancelled for the item Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 81704 [details] [review] Bug 21732: (QA follow-up) Performance Improvement We don't really need to lookup transfers before deleting them here.
So the questions: When the item is set to lost, when in transfer, which is the holding library? The library where it was in transit from or to?
Hi Katrin and Martin Thanks for the feedback. I am writing the unit test in t/db_dependent/Circulation.t now. Additionally, do you have any suggestions/preferences on how to address the questions to consider in comment #9 on which is the holding library of items set to lost whilst in transfer? Cheers, Alex
I think I'd go with the 'from' library for the holding library of the lost item. To me, until the 'to' library accepts arrival the 'from' library is responsible.
(In reply to Martin Renvoize from comment #3) > I'm not actually sure this should be governed by a system preference at all. Not sure either, if it's a bug fix we should not add a new pref. Maybe we should ask on the list? I do not manage to know how bad it is. Should we alert during the update? Create a new check in the data inconsistencies script?
Created attachment 81782 [details] [review] Bug 21732: (follow-up) Added tests to t/db_dependent/Circulation.t Tests check: * If transfer is deleted when item is marked as lost with the LostItemCancelOutstandingTransfer enabled and disabled * If the items holdingbranch changes when item with transfers on it is marked as lost. Also added call to ModItem() when transfers are deleted (if the LostItemCancelOutstandingTransfers syspref is enabled and a transfer did exist on the item) to set the items holdingbranch to the transfer source branch so the 'from' library is responsible for the lost item. Sponsored-By: Brimbank Library, Australia
Hi there I have added tests to t/db_dependent/Circulation.t and a change of the items holdingbranch to the 'from' branch when the syspref is enabled. I am not sure what data inconsistencies script you are referring to Jonathan? I will set this to 'Needs signoff' for now as I have addressed Martin's point in comment #3 and Katrin's point in comment #9. Please let me know if you'd like me to make further changes. Cheers, Alex
Hi there I have made another bug report Bug 21754 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21754) which does not use a system preference to determine if outstanding transfers on an item marked as lost are cancelled. Instead, my patch on bug 21754 automatically cancels outstanding transfers on lost items and sets the items holding branch to the cancelled transfers source ('from') branch. Therefore with bug 21732 and bug 21754 we should be covered for whatever responses/concensus comes back to your questions in the Koha mailing list Martin. Bug 21752 also contains tests in the t/db_dependent/Circulation.t Could bugs 21732 and 21754 please be tested and QA? Cheers, Alex
*** This bug has been marked as a duplicate of bug 21754 ***