Bug 20937 - PrintNoticesMaxLines is not effective for overdue notices with a print type specified where a patron has an email
Summary: PrintNoticesMaxLines is not effective for overdue notices with a print type s...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-13 18:56 UTC by Nick Clemens (kidclamp)
Modified: 2020-06-04 20:33 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.05
Circulation function:


Attachments
Bug 20937: Truncate items for print notices when user has an email (2.29 KB, patch)
2018-06-13 19:12 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 20937: Truncate items for print notices when user has an email (2.35 KB, patch)
2019-01-11 21:48 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 20937: Truncate items for print notices when user has an email (1.74 KB, patch)
2019-02-27 17:46 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 20937: Truncate items for print notices when user has an email (1.80 KB, patch)
2019-04-09 07:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 20937: Truncate items for print notices when user has an email (1.92 KB, patch)
2019-04-09 10:24 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2018-06-13 18:56:50 UTC
The logic in the script is:
if ( ( scalar(@emails_to_use) == 0 || $nomail ) && $PrintNoticesMaxLines && $j >= $PrintNoticesMaxLines )

So if we don't pass 'nomail' and the patron has an email, the items don't get kicked out.
Comment 1 Nick Clemens (kidclamp) 2018-06-13 19:12:12 UTC Comment hidden (obsolete)
Comment 2 Charles Farmer 2019-01-11 21:48:03 UTC
Created attachment 83829 [details] [review]
Bug 20937: Truncate items for print notices when user has an email

To test:
 1 - Set PrintNotciesMaxLines to 1
 2 - Checkout 2 (or more) items to a patron and make them overdue to trigger a
notice
 3 - Make sure the patron has an email
 4 - Make sure the notice you are sending is set to 'print' as the type
 5 - Run overdue_notices.pl without the 'nomail' option
 6 - Find the message and note all items have been added
 7 - Apply patch
 8 - Clear the message queue
 9 - Re-run overdue_notices.pl
10 - Confirm the message now has only 1 item.

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 3 Charles Farmer 2019-01-11 21:48:50 UTC
Test plan made it easy to reproduce, patch works has expected
Comment 4 Jonathan Druart 2019-02-26 23:58:33 UTC
Comment on attachment 83829 [details] [review]
Bug 20937: Truncate items for print notices when user has an email

Review of attachment 83829 [details] [review]:
-----------------------------------------------------------------

::: misc/cronjobs/overdue_notices.pl
@@ +683,4 @@
>                          # email or sms is requested but not exist, do a print.
>                          $effective_mtt = 'print';
>                      }
> +                    my @notice_items = @items;

Why a new variable?
Comment 5 Nick Clemens (kidclamp) 2019-02-27 17:46:23 UTC
Created attachment 85790 [details] [review]
Bug 20937: Truncate items for print notices when user has an email

To test:
 1 - Set PrintNoticesMaxLines to 1
 2 - Checkout 2 (or more) items to a patron and make them overdue to trigger a
notice
 3 - Make sure the patron has an email
 4 - Make sure the notice you are sending is set to 'print' as the type
 5 - Run overdue_notices.pl without the 'nomail' option
 6 - Find the message and note all items have been added
 7 - Apply patch
 8 - Clear the message queue
 9 - Re-run overdue_notices.pl
10 - Confirm the message now has only 1 item.
11 - Set PrintNoticesMaxLines to 0
12 - Clear the message queue
13 - Re-run overdue_notices.pl
14 - Confirm the message has 2 items as expected
Comment 6 Nick Clemens (kidclamp) 2019-02-27 17:47:13 UTC
(In reply to Jonathan Druart from comment #4)
> 
> Why a new variable?

I wasn't sure if it was used elsewhere in the script - it does not appear to be, removed
Comment 7 Martin Renvoize (ashimema) 2019-04-09 07:52:23 UTC
Created attachment 87581 [details] [review]
Bug 20937: Truncate items for print notices when user has an email

To test:
 1 - Set PrintNoticesMaxLines to 1
 2 - Checkout 2 (or more) items to a patron and make them overdue to trigger a
notice
 3 - Make sure the patron has an email
 4 - Make sure the notice you are sending is set to 'print' as the type
 5 - Run overdue_notices.pl without the 'nomail' option
 6 - Find the message and note all items have been added
 7 - Apply patch
 8 - Clear the message queue
 9 - Re-run overdue_notices.pl
10 - Confirm the message now has only 1 item.
11 - Set PrintNoticesMaxLines to 0
12 - Clear the message queue
13 - Re-run overdue_notices.pl
14 - Confirm the message has 2 items as expected

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2019-04-09 07:52:42 UTC
Works as intended, signing off.
Comment 9 Katrin Fischer 2019-04-09 10:24:07 UTC
Created attachment 87592 [details] [review]
Bug 20937: Truncate items for print notices when user has an email

To test:
 1 - Set PrintNoticesMaxLines to 1
 2 - Checkout 2 (or more) items to a patron and make them overdue to trigger a
notice
 3 - Make sure the patron has an email
 4 - Make sure the notice you are sending is set to 'print' as the type
 5 - Run overdue_notices.pl without the 'nomail' option
 6 - Find the message and note all items have been added
 7 - Apply patch
 8 - Clear the message queue
 9 - Re-run overdue_notices.pl
10 - Confirm the message now has only 1 item.
11 - Set PrintNoticesMaxLines to 0
12 - Clear the message queue
13 - Re-run overdue_notices.pl
14 - Confirm the message has 2 items as expected

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Nick Clemens (kidclamp) 2019-04-11 13:55:48 UTC
Awesome work all!

Pushed to master for 19.05
Comment 11 Martin Renvoize (ashimema) 2019-04-15 12:10:44 UTC
Pushed to 18.11.x for 18.11.05
Comment 12 Lucas Gass (lukeg) 2019-04-16 21:41:47 UTC
Backported to 18.05.x for 18.05.12