https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15986
Jonathan Druart <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #54 from Jonathan Druart <[email protected]> --- 1. 1824 my $admin_email_address = $library->{branchemail} || C4::Context->preference('KohaAdminEmailAddress'); Shouldn't it be $library->inbound_email_address instead? 2. 1844 next if ($mtt eq 'phone' and C4::Context->preference('TalkingTechItivaPhoneNotification') ); There are now 2 mtt, phone and itiva. Here you need to compare with 'itiva', and add a new condition for 'phone'. 3. Shouldn't we rename "send_notice" to "enqueue_notice" as we don't actually send it right away? 4. Why don't we insert a sample notice template and use it as the default for the script? 5. + # If passed message transports we force use those, otherwise we will use the patrons preferences + # for the 'Hold_Filled' notice + my $sending_params = @mtts ? { message_transports => \@mtts } : { message_name => "Hold_Filled" }; We should add that to the POD 6. + # Skip if we already dealt with this borrower + next if ( $done{$patron->borrowernumber} ); What if a patron has more than one hold? We won't be able to send a notice like "You have X holds waiting" 7. (not blocker) please don't forget to perltidy new files and blocks of code. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://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/
