ModItemTransfer is slowly getting removed and replaced by calls to the relevant Koha::Item and Koha::Item::Transfer methods.
Created attachment 160625 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit.
Created attachment 160631 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today.
Created attachment 160632 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today.
Created attachment 160649 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today.
Created attachment 160665 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 160691 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com>
Dep does not apply.
Created attachment 168279 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 168280 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 172777 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
What's another rebase between old friends aye...
(In reply to Martin Renvoize (ashimema) from comment #11) > What's another rebase between old friends aye... Trying to get here soon
-use C4::Items qw( ModItemTransfer ); Please grep for ModItemTransfer in this script again. We have multiple calls here left.
Looks like bug 34972 added two new occurrences.. I'll work on re-removing them again here now
Created attachment 173043 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173044 [details] [review] Bug 35721: Replace newly introduced ModItemTransfer calls
FAIL circ/returns.pl FAIL valid Global symbol "$reserve" requires explicit package name (did you forget to declare "my $reserve"?) circ/returns.pl had compilation errors. Please change to NSO after fixing. This needs to be tested.
Created attachment 173356 [details] [review] Bug 35721: Replace newly introduced ModItemTransfer calls
Corrected.. main moved again. Back to NSO as requested..
Created attachment 173359 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 173360 [details] [review] Bug 35721: Replace newly introduced ModItemTransfer calls Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org>
Created attachment 173370 [details] [review] Bug 35721: (QA follow-up) replace find() with pickup_library()
Which road map item does this relate to? (just wondering spotting this on dashboard)
(In reply to Katrin Fischer from comment #23) > Which road map item does this relate to? (just wondering spotting this on > dashboard) It's been carried from roadmap to roadmap for years.. if it's not in this next release I give up.
(In reply to Martin Renvoize (ashimema) from comment #24) > (In reply to Katrin Fischer from comment #23) > > Which road map item does this relate to? (just wondering spotting this on > > dashboard) > > It's been carried from roadmap to roadmap for years.. if it's not in this > next release I give up. Looking tomorrow
Looking here now
Created attachment 173853 [details] [review] Bug 35721: ModItemTransfer -> Koha::Item->request_transfer - returns.pl This patch replaces the call to ModItemTransfer in circ/returns.pl with a call to Koha::Item->request_transfer and Koha::Item::Transfer->transit. Test plan 1) Set 'AutomaticItemReturn' to "Don't" 2) Check an item in at a location other than it's 'home' or 'holding' branch 3) Note you should be asked a about returning the item to another branch 4) Opting to not proceed should NOT result in a row in the branchtransfers table 5) Option to proceed (either with a print slip or just 'OK') should result in a branchtransfer from the current checkin branch to the home or holding branch depending on the circulation rules. 5a) The item should be marked as immediately in transit in, i.e. the 'datesent' set to today. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 173854 [details] [review] Bug 35721: Replace newly introduced ModItemTransfer calls Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 173855 [details] [review] Bug 35721: (QA follow-up) replace find() with pickup_library() Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
QA comment coming
Passing QA but this still raises questions with me. Not completely confident that this will work in some edge cases: my $transferitem = $query->param('transferitem'); + my $item = Koha::Items->find( $transferitem ); + my $transfer = $item->request_transfer({ to => $tobranch, reason => $trigger }); No blocker for now, but this looks like a potential crash. Query param not found in items? Boom. Another thing: see the other calls too. Why dont you add enqueue (or replace) here ? Could this potentially throw an exception? - ModItemTransfer( $itemnumber, $userenv_branch, $diffBranchSend, 'Reserve' ); + my $tobranch = Koha::Libraries->find( $hold->branchcode ); - my $tobranch = Koha::Libraries->find( $hold->branchcode ); + my $tobranch = $hold->pickup_library(); Why dont you use diffBranchSend here ? my $diffBranchReturned = $query->param('diffBranch'); my $diffBranchSend = ($userenv_branch ne $diffBranchReturned) ? $diffBranchReturned : undef; template: 2x <input type="hidden" name="diffBranch" value="[% destbranch | html %]" /> script: destbranch => $reserve->{'branchcode'}, Looks like we first pass the reserve branchcode, we get it back and we do not use it but look it up again? @@ -170,8 +170,13 @@ + my $transfer = $item->request_transfer( { to => $tobranch, reason => 'Reserve', enqueue => 1 } ); request_transfer has: frombranch => $self->holdingbranch original call had userenv_branch instead of item holding branch: ModItemTransfer( $itemnumber, $userenv_branch, $diffBranchSend, 'Reserve' ) Question: When does this make a difference? Since we are returning an item, arent we receiving it at userenv branch? Same for the @@ -612,7 +617,10 @@ change
I can answer that last one...I chose to use $userenv_branch rather than the item's holding branch in that spot on bug 34972 because I felt it made the code easier to follow, but in practice it won't make a difference unless we introduce a bug somewhere else. We've already called AddReturn by the time we reach that part of the code, so the item's holding branch would already have been set to the userenv branch by that point.
Pushed for 24.11! Well done everyone, thank you!