https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16149 --- Comment #20 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #16)
1. QA tool complaints fixed
2. I think we display something more useful than the error codes
Okay, we now have readable errors
3. Please correct indentation
Ah, I think i fixed it
4. Reading the tests: is( $result[0]{NO_EMAIL_COL}, 2, "Warning only for patron with no email"); Why 2?
2 is the row with the error - the returned has includes the error and referenced row (unless borrowernumber not found, then it is the wrong borrowernumber)
5. The cronjob script has too many use statements
Yes, CP error, cleaned
6. I do not think the subroutine should take "commit", maybe it should return the notice instead? Same for verbose actually.
Okay, moved those and the sending into the cronjob
7. Subroutine takes "branch" whereas the script pass "branchcode" (!)
Oops, fixed
8. POD says "--report Specify a saved SQL report in the Koha system to user for the emails." It's actually the *id* (saved_sql.id), maybe it should be more explicit
Fixed
9. --email, should not we use Koha::Patron->notice_email_address and remove this option?
This is designed to be more flexible and let the report determine the content - perhaps the library w3ants to check secondary emails or something?
10. --from specified email for 'from' address, report column 'from' used if not specified Should not we use branches.branchemail instead?
Again, designed for flexibility in allowing report to set this - perhaps we want to send emails for patron clubs and they have their own email? This was designed to so that it is a cronjob now, but also so it could be used in a script in the GUI. This was a major refactoring, back to NSO -- You are receiving this mail because: You are watching all bug changes.