[Koha-bugs] [Bug 3701] If the ReturnToShelvingCart syspref is on, and something needs to go in transit, the shelving cart setting is overriding the transit.

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3701

--- Comment #13 from Kyle M Hall <kyle at 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.