Bug 24164

Summary: Patron emailer cronjob is not generating unique content for notices
Product: Koha Reporter: Nick Clemens <nick>
Component: Command-line UtilitiesAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: blocker    
Priority: P1 - high CC: 1joynelson, andrewfh, jonathan.druart, jzairo, kelly, lucas, mkstephens, wizzyrea
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.01
Bug Depends on: 16149    
Bug Blocks:    
Attachments: Bug 24164: Unit test
Bug 24164: Copy the letter for each email
Bug 24164: Copy the letter for each email
Bug 24164: Copy the letter for each email
Bug 24164: Unit test
Bug 24164: Copy the letter for each email

Description Nick Clemens 2019-12-03 19:53:15 UTC
The content is being copied from one patron to all notices - it is because we are nesting a hashref so end up with a pointer instead of a new copy
Comment 1 Nick Clemens 2019-12-03 19:59:19 UTC
Created attachment 95982 [details] [review]
Bug 24164: Unit test
Comment 2 Nick Clemens 2019-12-03 19:59:21 UTC
Created attachment 95983 [details] [review]
Bug 24164: Copy the letter for each email

To test:
1 - Set or have two patrons with emails - note thier borrowernumber for the report
2 - Create a report - note the report id
SELECT borrowernumber, surname, firstname WHERE borrowernumber IN (##,##);
3 - Create a notice in circulation with code TESTEMAIL
4 - The content should be "[% surname %]"
5 - perl misc/cronjobs/patron_emailer.pl --report ## --notice TESTEMAIL --module circulation --from anyone@anywhere.com --verbose
6 - Note that the emails both have the same surname
7 - Apply patch
8 - Repeat 5
9 - Emails now have correct content
Comment 3 ByWater Sandboxes 2019-12-05 14:26:02 UTC
Created attachment 96040 [details] [review]
Bug 24164: Copy the letter for each email

To test:
1 - Set or have two patrons with emails - note thier borrowernumber for the report
2 - Create a report - note the report id
SELECT borrowernumber, surname, firstname WHERE borrowernumber IN (##,##);
3 - Create a notice in circulation with code TESTEMAIL
4 - The content should be "[% surname %]"
5 - perl misc/cronjobs/patron_emailer.pl --report ## --notice TESTEMAIL --module circulation --from anyone@anywhere.com --verbose
6 - Note that the emails both have the same surname
7 - Apply patch
8 - Repeat 5
9 - Emails now have correct content

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Comment 4 Liz Rea 2019-12-11 18:55:58 UTC
Potential for the wrong data to be sent to borrowers, upping the severity/priority.
Comment 5 ByWater Sandboxes 2019-12-11 19:39:07 UTC
Created attachment 96211 [details] [review]
Bug 24164: Copy the letter for each email

To test:
1 - Set or have two patrons with emails - note thier borrowernumber for the report
2 - Create a report - note the report id
SELECT borrowernumber, surname, firstname WHERE borrowernumber IN (##,##);
3 - Create a notice in circulation with code TESTEMAIL
4 - The content should be "[% surname %]"
5 - perl misc/cronjobs/patron_emailer.pl --report ## --notice TESTEMAIL --module circulation --from anyone@anywhere.com --verbose
6 - Note that the emails both have the same surname
7 - Apply patch
8 - Repeat 5
9 - Emails now have correct content

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Jessica Zairo <jzairo@bywatersolutions.com>
Comment 6 Jessie Zairo 2019-12-11 19:41:20 UTC
Anyone else who tests, make sure to also set the following: 

In step 2 (make sure you add email to the SELECT statement and add a FROM statement

- Create a report - note the report id
SELECT borrowernumber, surname, firstname, email FROM borrowers
Comment 7 Jonathan Druart 2019-12-12 13:53:30 UTC
Created attachment 96220 [details] [review]
Bug 24164: Unit test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2019-12-12 13:53:34 UTC
Created attachment 96221 [details] [review]
Bug 24164: Copy the letter for each email

To test:
1 - Set or have two patrons with emails - note thier borrowernumber for the report
2 - Create a report - note the report id ($REPORT_ID)
SELECT borrowernumber, surname, firstname, email FROM borrowers WHERE borrowernumber IN (##,##);
3 - Create a notice in circulation with code TESTEMAIL
4 - The content should be "[% surname %]"
5 - perl misc/cronjobs/patron_emailer.pl --report $REPORT_ID --notice TESTEMAIL --module circulation --from anyone@anywhere.com --verbose
6 - Note that the emails both have the same surname
7 - Apply patch
8 - Repeat 5
9 - Emails now have correct content

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>

Signed-off-by: Jessica Zairo <jzairo@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Martin Renvoize 2019-12-13 14:07:39 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 10 Joy Nelson 2019-12-19 22:01:07 UTC
Pushed to 19.11.01
Comment 11 Lucas Gass 2019-12-20 18:05:55 UTC
backported to 19.05.x for 19.05.06