Bug 16149 - Generate and send custom notices based on report output
Summary: Generate and send custom notices based on report output
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Testopia
URL: http://devs.bywatersolutions.com/proj...
Whiteboard:
Keywords:
: 15507 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-03-28 13:40 UTC by Nicole C. Engard
Modified: 2018-09-24 17:24 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 16149 - Generate and send custom notices based on report output (8.45 KB, patch)
2017-04-17 15:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Generate and send custom notices based on report output (9.26 KB, patch)
2017-05-12 21:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Unit tests (3.89 KB, patch)
2017-05-12 21:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Generate and send custom notices based on report output (9.01 KB, patch)
2017-08-23 22:48 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Unit tests (3.92 KB, patch)
2017-08-23 22:48 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Generate and send custom notices based on report output (9.01 KB, patch)
2017-09-27 12:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Unit tests (3.92 KB, patch)
2017-09-27 12:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Generate and send custom notices based on report output (9.06 KB, patch)
2018-09-15 20:44 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149 - Unit tests (3.97 KB, patch)
2018-09-15 20:44 UTC, Nick Clemens
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16149 - Generate and send custom notices based on report output (9.13 KB, patch)
2018-09-15 20:56 UTC, Claire Gravely
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16149 - Unit tests (4.04 KB, patch)
2018-09-15 20:56 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 16149: Generate and send custom notices based on report output (10.29 KB, patch)
2018-09-24 17:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149: Unit tests (3.92 KB, patch)
2018-09-24 17:16 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16149: Generate and send custom notices based on report output (10.30 KB, patch)
2018-09-24 17:19 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2016-03-28 13:40:37 UTC
Work to be done: Write a cron script that will
a) Accept parameters for
1) a report id
2) a notice code
3) a notice module
4a) a branchcode
4b) or database column from which to select the branchcode
5) a column from which to select the email address to send to ( if not set, Koha will use the first available email address for the patron )
6a) A "from" email address
6b) A column from which to select the "from" email address
b) The script will run the report and for each row of results will generate a notice and queue it for sending. All and only the columns from the report will be available to be used in the notice text.

Looking for sponsors: http://devs.bywatersolutions.com/projects/custom-notices-based-report-output/
Comment 1 Nicole C. Engard 2016-06-01 18:03:35 UTC
*** Bug 15507 has been marked as a duplicate of this bug. ***
Comment 2 Nick Clemens 2017-04-17 15:24:38 UTC
Created attachment 62217 [details] [review]
Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 3 Nick Clemens 2017-05-12 21:14:41 UTC
Created attachment 63454 [details] [review]
Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 4 Nick Clemens 2017-05-12 21:14:45 UTC
Created attachment 63455 [details] [review]
Bug 16149 - Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t
Comment 5 Hugo Agud 2017-08-23 08:34:20 UTC
Bug 16149 - Generate and send custom notices based on report output

63454 - Bug 16149 - Generate and send custom notices based on report output
63455 - Bug 16149 - Unit tests

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16149 - Generate and send custom notices based on report output
Using index info to reconstruct a base tree...
M	C4/Reports/Guided.pm
Falling back to patching base and 3-way merge...
Auto-merging C4/Reports/Guided.pm
Applying: Bug 16149 - Unit tests
Using index info to reconstruct a base tree...
M	t/db_dependent/Reports/Guided.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Reports/Guided.t
CONFLICT (content): Merge conflict in t/db_dependent/Reports/Guided.t                                                                                                                         6d20h 1.30 2x2.8GHz 3.7G85% 2017-08-23 08:09:18
Failed to merge in the changes.
Patch failed at 0001 Bug 16149 - Unit tests
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16149---Unit-tests-IoVGvi.patch
Comment 6 Nick Clemens 2017-08-23 22:48:30 UTC
Created attachment 66398 [details] [review]
Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 7 Nick Clemens 2017-08-23 22:48:33 UTC
Created attachment 66399 [details] [review]
Bug 16149 - Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t
Comment 8 Hugo Agud 2017-08-24 07:47:52 UTC
Hi,

I have been able to apply the patch but I am not able to send any mail

I have enabled koha mail and I have install postfix... 

may you help me on this, please?

kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ perl patron_emailer.pl -report=1 -notice=ODUE -module=circulation -library=orex -from=info@orex.es -email=email -verbose
LETTER_FOUND => 1
REPORT_FOUND => 1
REPORT_SUCCEED => 1
SENDING_SUCCEED => 0
kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ perl patron_emailer.pl -report=1 -notice=CHECKIN -module=circulation -library=orex -from=info@orex.es -email=email -verbose
LETTER_FOUND => 1
REPORT_FOUND => 1
REPORT_SUCCEED => 1
SENDING_SUCCEED => 0
kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ perl patron_emailer.pl -report=1 -notice=DUE -module=circulation -library=orex -from=info@orex.es -email=email -verbose
LETTER_FOUND => 1
REPORT_FOUND => 1
REPORT_SUCCEED => 1
SENDING_SUCCEED => 0
kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ perl patron_emailer.pl -report=1 -notice=PREDUE -module=circulation -library=orex -from=info@orex.es -email=email -verbose
LETTER_FOUND => 1
REPORT_FOUND => 1
REPORT_SUCCEED => 1
SENDING_SUCCEED => 0
kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/cronjobs$ exit
Comment 9 Nick Clemens 2017-09-27 12:51:53 UTC
Created attachment 67392 [details] [review]
Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 10 Nick Clemens 2017-09-27 12:51:56 UTC
Created attachment 67393 [details] [review]
Bug 16149 - Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t
Comment 11 Nick Clemens 2017-09-27 12:53:35 UTC
(In reply to Hugo Agud from comment #8)
> SENDING_SUCCEED => 0

Heh, the line above is reporting the success of row #0, not a failure. Poor design.

I altered the patch to count up from 1.

Additionally you need the '-commit' option to actually populate the message queue.

Please retest and let me know if that works :-)
Comment 12 Nick Clemens 2018-09-15 20:44:52 UTC
Created attachment 78890 [details] [review]
Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 13 Nick Clemens 2018-09-15 20:44:56 UTC
Created attachment 78891 [details] [review]
Bug 16149 - Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t
Comment 14 Claire Gravely 2018-09-15 20:56:26 UTC
Created attachment 78896 [details] [review]
[SIGNED-OFF] Bug 16149 - Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 15 Claire Gravely 2018-09-15 20:56:31 UTC
Created attachment 78897 [details] [review]
[SIGNED-OFF] Bug 16149 - Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 16 Jonathan Druart 2018-09-22 13:39:57 UTC
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?
Comment 17 Nick Clemens 2018-09-24 17:16:18 UTC Comment hidden (obsolete)
Comment 18 Nick Clemens 2018-09-24 17:16:23 UTC
Created attachment 79329 [details] [review]
Bug 16149: Unit tests

To test:
prove -v t/db_dependent/Reports/Guided.t

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 19 Nick Clemens 2018-09-24 17:19:56 UTC
Created attachment 79330 [details] [review]
Bug 16149: Generate and send custom notices based on report output

Ths patch add an EmailReport function to C4::Reports::Guided

It accepts a notice (module, code, branch) and a report and attempts to
email notices to patron, generating content using report content.

Notice must be in template toolkit syntax, only columns in report are
available for notice.

To test:
1 - Specify various options
2 - Ensure errors are returned if options are incomplete or incorrect
3 - Pass a report containing 'from' and 'email' and 'borrowernumber'
columns and ensure message queue populated as expected
Comment 20 Nick Clemens 2018-09-24 17:24:31 UTC
(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