Hi Michael, The patch generally looks OK. I have a couple comments, one of which I think requires that the patch be changed and resubmitted:
On Tue, Apr 28, 2009 at 12:18 PM, Michael Hafen <[email protected]> wrote: > + my $branch = (C4::Context->preference('CircControl') eq > 'PickupLibrary') ? C4::Context->userenv->{'branch'} : > + (C4::Context->preference('CircControl') eq > 'PatronLibrary') ? $borrower->{'branchcode'} : > + $item->{'homebranch'}; # fallback to item's > homebranch It's time for there to be a helper function that determines the applicable branch based on CircControl, the patron, and the item - this bit of code has been copied and pasted in a couple other places in C4::Circulation. [snip] > + $issuingimpossible{INVALID_DATE} = $duedate->output('syspref') > unless ( $duedate && $duedate->output('iso') ge C4::Dates->today('iso') ); > + } Shouldn't this be moved outside the conditional block? Sometimes a due date is passed to CanBookBeIssued(), and it presumably ought to be checked as well. Regards, Galen -- Galen Charlton VP, Research & Development, LibLime [email protected] p: 1-888-564-2457 x709 skype: gmcharlt _______________________________________________ Koha-patches mailing list [email protected] http://lists.koha.org/mailman/listinfo/koha-patches
