https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19059 --- Comment #31 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #21)
Looks good to me. Few minor points left:
FIXME in sub ModReserveFill + # FIXME Must call Koha::Hold->cancel ? => No, should call ->filled and add the correct log Not clear to me. Please clarify.
If we call ->cancel, we will log a "cancel" log. We should call a new method ->fill[ed] to add the "FILLED" log (or whatever, but not a cancel log)
sub _move_to_old The name suggests a move, but you are only copying data.
Yes, I preferred to stick on the existing pattern (Koha::Patrons), I would like to have again 1 more subroutine of this kind to know what is best.
circ/branchtransfers.pl + } # FIXME else? Can you fix it now? You add the if. Actually, the script always assumed that CancelReserve worked. So why not only test $holds->count for the call to cancel ? Same remark for other occurrences. Incl. circ/returns.pl
I wanted to avoid a crash with the $holds->next->cancel. Moreover previous code displayed a "ok" message even if something went wrong. It may be unnecessary, but I preferred to add it. -- You are receiving this mail because: You are watching all bug changes.