Bug 24235 - /misc/cronjobs/advance_notices.pl DUEDGST does NOT send sms, just e-mail
Summary: /misc/cronjobs/advance_notices.pl DUEDGST does NOT send sms, just e-mail
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P3 major (vote)
Assignee: Radek Šiman (R-Bit Technology, s.r.o.)
QA Contact: Nick Clemens
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-12-12 22:54 UTC by Michal Denar
Modified: 2020-01-07 22:45 UTC (History)
9 users (show)

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


Attachments
Bug 24235: Do not use hardcoded message_name (1.08 KB, patch)
2019-12-13 08:20 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 24235: Do not use hardcoded message_name (1.13 KB, patch)
2019-12-13 08:45 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off (2.47 KB, patch)
2019-12-13 17:41 UTC, Radek Šiman (R-Bit Technology, s.r.o.)
Details | Diff | Splinter Review
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off (2.51 KB, patch)
2019-12-13 18:17 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off (2.57 KB, patch)
2019-12-16 10:55 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 Michal Denar 2019-12-12 22:54:54 UTC
Advanced notices DUEDGST does NOT send sms, just e-mail.
Comment 1 Radek Šiman (R-Bit Technology, s.r.o.) 2019-12-12 23:05:00 UTC
Function send_digests contains hard-coded value "advance_notice" in messge_name (line 552 of advance_notice.pl), but it should be passed-in as a parameter, because it's called for Advance_Notice as well as for Item_Due message attributes.
Comment 2 Radek Šiman (R-Bit Technology, s.r.o.) 2019-12-12 23:10:05 UTC
To reproduce this bug, please turn OFF sms PREDUEDGST, but have sms DUEDGST turned ON for a given borrower. Sms version of DUEDGST is never generated into the message queue.
Comment 3 Josef Moravec 2019-12-13 08:20:12 UTC
Created attachment 96238 [details] [review]
Bug 24235: Do not use hardcoded message_name

Test plan:
0) Do not apply the patch and reproduce the bug
    - turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower
    - Sms version of DUEDGST is never generated into the message queue.
1) Apply the patch
2) Ensure that the bug is fixed
Comment 4 Michal Denar 2019-12-13 08:45:02 UTC
Created attachment 96240 [details] [review]
Bug 24235: Do not use hardcoded message_name

Test plan:
0) Do not apply the patch and reproduce the bug
    - turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower
    - Sms version of DUEDGST is never generated into the message queue.
1) Apply the patch
2) Ensure that the bug is fixed

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 5 Radek Šiman (R-Bit Technology, s.r.o.) 2019-12-13 09:47:47 UTC
Please add missing parameters to all 4 calls of "send_digests".
Comment 6 Jonathan Druart 2019-12-13 10:04:38 UTC
Are you sure about this patch? It sounds like a duplicate of bug 24072.

I setup: https://snipboard.io/NfnZeG.jpg

Then run advance_notices.pl

Withtout this patch:

MariaDB [koha_kohadev]> select * from message_queue\G
*************************** 1. row ***************************
            message_id: 33
        borrowernumber: 5
               subject: Item Due Reminder
               content: You have 1 items due
              metadata:
           letter_code: DUEDGST
message_transport_type: email
                status: pending
           time_queued: 2019-12-13 10:02:38
            to_address: NULL
          from_address: root@localhost
          content_type: NULL
*************************** 2. row ***************************
            message_id: 34
        borrowernumber: 5
               subject: duegst
               content: duedgst sms
              metadata:
           letter_code: DUEDGST
message_transport_type: sms
                status: pending
           time_queued: 2019-12-13 10:02:38
            to_address: NULL
          from_address: root@localhost
          content_type: NULL
2 rows in set (0.00 sec)


With this patch: nothing is generated
Comment 7 Radek Šiman (R-Bit Technology, s.r.o.) 2019-12-13 17:41:45 UTC
Created attachment 96279 [details] [review]
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off

DUEDGST notice required PREDUEDGST to be on, but both messages have to
be generated independently

Test plan:
0) Do not apply the patch and reproduce the bug
    - turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower
    - Sms version of DUEDGST is never generated into the message queue.
1) Apply the patch
2) Ensure that the bug is fixed
Comment 8 Michal Denar 2019-12-13 18:15:13 UTC
Jonathan, 
plase follow test plan:
- turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower

On your screenshot is PREDUEDGST ON.
Comment 9 Michal Denar 2019-12-13 18:17:18 UTC
Created attachment 96283 [details] [review]
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off

DUEDGST notice required PREDUEDGST to be on, but both messages have to
be generated independently

Test plan:
0) Do not apply the patch and reproduce the bug
    - turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower
    - Sms version of DUEDGST is never generated into the message queue.
1) Apply the patch
2) Ensure that the bug is fixed

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 10 Nick Clemens 2019-12-16 10:55:59 UTC
Created attachment 96308 [details] [review]
Bug 24235: DUEDGST not sent if PREDUDGST checkbox is off

DUEDGST notice required PREDUEDGST to be on, but both messages have to
be generated independently

Test plan:
0) Do not apply the patch and reproduce the bug
    - turn OFF sms PREDUEDGST and turn ON sms DUEDGST for a given borrower
    - Sms version of DUEDGST is never generated into the message queue.
1) Apply the patch
2) Ensure that the bug is fixed

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Martin Renvoize 2019-12-16 21:08:10 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Joy Nelson 2020-01-03 16:47:02 UTC
Pushed to 19.11.x branch for 19.11.02
Comment 13 Lucas Gass 2020-01-07 22:45:48 UTC
backported to 19.05.x for 19.05.07