https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34972 --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: First well done! This code is a minefield. Before passing QA, please look at my question under [1]. [1] Last question? - C4::Items::ModItemTransfer( - $itemnumber, - $item->holdingbranch, - $checkreserves->{'branchcode'}, + ModItemTransfer( $itemnumber, $userenv_branch, $diffBranchSend, 'Reserve' ); Are we sure that this new transfer should be from $userenv_branch in all cases ? (Noting that the check already referred to userenv_branch before this change.) The second call looks like: + ModItemTransfer( $itemnumber, $item->holdingbranch, $reserve->{branchcode}, 'Reserve' ); And that just convinces me more (at least, at first glance)? But the other way around: why not userenv_branch here, since that is the branch where we are checking in now? They should be equal here since we called AddReturn already. So second call is fine. [2] my ( $messages, $nextreservinfo ) = ModReserveCancelAll( $item, $borrowernumber ); Not for now, but $nextreservinfo is just a borrowernumber ! Comparing with: my ( undef, $nextreservinfo, undef ) = CheckReserves($item); Here it is a hash including a borrowernumber. [3] ModReserveCancelAll Outside scope too but the name is (also) quite confusing. -- You are receiving this mail because: You are watching all bug changes.