[OPEN-ILS-DEV] Possible Bugs in Circulate.pm (1.6)

[Dan Wells]

Hello all,

With the help of Jason and Dan S., it was determined that an error our library was getting when trying to circulate items with a status of 'Reserves' only happens when using in-db circ rules.  After some troubleshooting, I found at least two separate issues are at the root of this.

First, it seems that 'run_indb_circ_test()' does both patron checks and copy checks in one swoop, so you end up getting the 'COPY_NOT_AVAILABLE' permit failure when running 'run_patron_permit_scripts()'.  This particular permit failure needs a "copy object" as its payload, but since 'run_patron_permit_scripts()' is not expecting to deal with copy permits, it doesn't fill it.

I think we have a number of choices for a fix here.  The simplest to is duplicate the code near the end of 'run_copy_permit_scripts()' and add it to the end of 'run_patron_permit_scripts()':

   for (@allevents) {
      $_->{payload} = $copy if
            ($_->{textcode} eq 'COPY_NOT_AVAILABLE');
   }


This solution works around the issue but doesn't really address the bigger problem, which is that run_indb_circ_test() is returning non-patron level failures to run_patron_permit_scripts().  In addition, run_indb_circ_test() ends up getting called twice when it doesn't really need to be.  It seems like these function calls in general should be reworked in some minor way, and I am happy to do it, but it might be done more quickly and safely by someone more familiar with the reach of these functions.

Aside from this, there is another difference in the code which ends up causing further trouble for the in-db folk.  If you use legacy scripts, your $self-copy is built inside of mk_script_runner(), but if you are using in-db, $self->copy is built by mk_env().  It turns out that these "copy" variables are not being created the same.  More specifically, mk_env() leaves out a number of things, status information included.  This can be fixed by changing (within mk_env()):

    unless($self->is_noncat) {
        my $copy;
            my $flesh = {
                    flesh => 2,
                    flesh_fields => {acp => ['call_number'], acn => ['record']}
            };

to:

    unless($self->is_noncat) {
        my $copy;
            my $flesh = {
                    flesh => 2,
                    flesh_fields => {acp => ['location', 'status', 'circ_lib', 'age_protect', 'call_number'], acn => ['record']}
            };

I am not sure about needing all of that information, but this matches the object as it is built in mk_script_runner().

Thoughts?

Thanks,
Dan

-- 

*********************************************************************************
Daniel Wells, Library Programmer Analyst dbw2 at calvin.edu
Hekman Library at Calvin College
616.526.7133