Bug 24164 - Patron emailer cronjob is not generating unique content for notices
Summary: Patron emailer cronjob is not generating unique content for notices
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All All
: P1 - high blocker (vote)
Assignee: Nick Clemens
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on: 16149
Blocks:
  Show dependency treegraph
 
Reported: 2019-12-03 19:53 UTC by Nick Clemens
Modified: 2020-06-04 20:37 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.01


Attachments
Bug 24164: Unit test (1.92 KB, patch)
2019-12-03 19:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24164: Copy the letter for each email (1.32 KB, patch)
2019-12-03 19:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 24164: Copy the letter for each email (1.38 KB, patch)
2019-12-05 14:26 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24164: Copy the letter for each email (1.44 KB, patch)
2019-12-11 19:39 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 24164: Unit test (2.00 KB, patch)
2019-12-12 13:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24164: Copy the letter for each email (1.55 KB, patch)
2019-12-12 13:53 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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