[Koha-bugs] [Bug 16149] Generate and send custom notices based on report output

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16149

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jonathan.druart at bugs.koha-c
                   |                            |ommunity.org
             Status|Signed Off                  |Failed QA

--- Comment #16 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
1.
 FAIL   C4/Reports/Guided.pm
   FAIL   valid
                "my" variable $report masks earlier declaration in same scope 

 FAIL   misc/cronjobs/patron_emailer.pl
   FAIL   spelling
                 ommited  ==> omitted
                 sucesses  ==> successes
   FAIL   valid
                "my" variable $report masks earlier declaration in same scope 

* Commit title does not start with 'Bug XXXXX: ' - 2380dc8
* Commit title does not start with 'Bug XXXXX: ' - 579e02c

2. I think we display something more useful than the error codes

3. Please correct indentation

4. Reading the tests:
is( $result[0]{NO_EMAIL_COL}, 2, "Warning only for patron with no email");
Why 2?

5. The cronjob script has too many use statements

6. I do not think the subroutine should take "commit", maybe it should return
the notice instead?
Same for verbose actually.

7. Subroutine takes "branch" whereas the script pass "branchcode" (!)

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

9. --email, should not we use Koha::Patron->notice_email_address and remove
this option?

10.  --from                       specified email for 'from' address, report
column 'from' used if not specified

Should not we use branches.branchemail instead?

-- 
You are receiving this mail because:
You are watching all bug changes.