https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34972

--- Comment #33 from Marcel de Rooy <m.de.r...@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.
_______________________________________________
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to