https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19059
--- Comment #31 from Jonathan Druart <[email protected]> --- (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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
