http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3701 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- I think you are absolutely right. I have just submitted a modified version of the patch that moves this logic in AddReturn(), rather than leaving it in returns.pl I'm leaving the status as Passed QA for now. If you feel this new version needs to restart the submission process, please go ahead and reset the status to "Needs Signoff". (In reply to comment #11)

mmm... I don't like this patch, but still not sure it's wrong.

Here is why I don't like it:

You write: + if ( C4::Context->preference("ReturnToShelvingCart") && !$messages->{'WasTransfered'} ) { + my $item = GetItem( $itemnumber ); + $item->{'location'} = 'CART'; + ModItem( $item, $item->{'biblionumber'}, $item->{'itemnumber'} ); + }

in return.pl

So, you manage the ShelvingCart: * outside from AddReturn sub => what would happen if I mark return from circulation.pl (I can to that by checking the checkbox on the right of any issued item, and "renew or return checked items") ? Same thing for moremember.pl. Overall, I feel it's unsafe to manage that outside AddReturn

* you use $messages->{'WasTransfered'} to check if there has been a transfer. That's a little bit unsafe. If we remove the message, or change the use of the message, we may face side-effect

I don't failed QA this patch for now, please argue if you think i'm wrong.

-- You are receiving this mail because: You are watching all bug changes.