"no-op" checkins use old global behavior for floating items

[Jason Etheridge]

This happens when you use the Suppress Holds and Transits checkin modifier during Item Check In, or when you use the "Normal Checkin then Checkout" action when attempting to check out an already circulating item.

If a floating group is set on the item, the code just treats it as "true" and changes the circ library, regardless of the floating group membership.

Circ/Circulate.pm, line 2583
    } else { # no-op checkin
        if ($U->is_true( $self->copy->floating )) { # XXX floating items still stick where they are even with no-op checkin?
            $self->checkin_changed(1);
            $self->copy->circ_lib( $self->circ_lib );
            $self->update_copy;
        }
    }

[Josh Stompro (u-launchpad-stompro-org)]

I can confirm that this is an issue in 2.8.4.

We have been scratching our heads about why items were floating to a different library system occasionally. Finally figured out that it happens when an item that is already checked out, gets checked out to a new patron using the "Normal Checkin then Checkout" action. Then found this bug report.

We are a consortium of two regional library systems, one system floats everything, and the other floats nothing. We also share all holdable items between both systems.

Steps to test this issue.

1. Setup a copy with a floating group that allows floating in System A only. The copy should have a circ lib of a branch is system A.
2. Check out the item to a patron at a branch in System B.
3. Check out the item to a second patron at System B without checking it in first.
4. When you check out the item, choose the "Normal Checkin then Checkout" action.
5. View the copy to see that the circ_lib has been changed to the branch in system B.

Would the proper fix be to use the new floating groups logic, or to just not float from a no-op checkin?

Josh

[Joan Kranich (jkranich)]

I can confirm that this is an issue in 2.10.7.

Joan

[Jason Stephenson (jstephenson)]

This comment is interesting: "# XXX floating items still stick where they are even with no-op checkin?" It asks the question if copies should float on a no-op checkin.

I was going to make the no-op checkin code behave as the block above where floating is checked via the evergreen.can_float() database function. (In that case it would be handy to add a utility function so the code is in one place.) However, that comment makes me wonder if floating should be considered at all on a no-op checkin?

If no-op should not cause copies to float, then the fix for this bug is as simple as removing the else block for a no-op checkin. If the consensus is that copies should float on a no-op checkin, then we'll need to go through the evergreen.can_float function.

I'll start a branch to do the latter in the meantime.

[Kathy Lussier (klussier)]

My initial reaction is that no-op checkins should float. For the "Normal Checkin then Checkout" use case, I think either approach could work. However, for the "Suppress Holds and Transits" use case, I think the system should properly change the circulation library if it's required by the floating configuration.

[Jason Stephenson (jstephenson)]

Branch to fix this in working/user/dyrcona/lp1473054_no-op_may_float

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1473054_no-op_may_float

Pretty much a copy and paste of the float code for a normal checkin with a small edit to make sense in the context of a no-op checkin.

We're using this in production already.

[Josh Stompro (u-launchpad-stompro-org)]

I just tested this out on our test system running 2.10.6 and it worked as expected. I'm going to try it out in production tomorrow.

Signoff branch at user/stompro/lp1473054_no-op_may_float_signoff

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/stompro/lp1473054_no-op_may_float_signoff

Thanks Jason!
Josh

[Josh Stompro (u-launchpad-stompro-org)]

We are now also running this in production. I'll report back if we have any problems.

Josh

[Galen Charlton (gmc)]

Pushed to master, rel_2_11, and rel_2_10. Thanks, Jason and Josh!