[Bug 34731] New: C4::Letters::SendQueuedMessages can be triggered with an undef message_id
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Bug ID: 34731 Summary: C4::Letters::SendQueuedMessages can be triggered with an undef message_id Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: kyle@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org I've discovered that sometimes C4::Letters::EnqueueLetter will return undef instead of a valid message id. In Koha::Patron::set_password we call C4::Letters::EnqueueLetter and assume a message_id exists which we pass to C4::Letters::SendQueuedMessages. Because message_id is undef, we then *process all pending messages*. This is very very bad considering libraries often have very specific time frames they wish to send message ( especially sms and phone ). In addition, this bypasses the before_send_messages hook. I have not been able to recreate on demand yet, but I think it is either random or caused by calls to the /api/v1/patrons/<patron_id>/password rest api endpoint. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155325&action=edit Bug 34731: Throw exception if SendQueuedMessages is passed a bad message_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155326 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155326&action=edit Bug 34731: Don't call SendQueuedMessages if message_id is bad -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 155327 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155327&action=edit Bug 34731: Use Koha::Notice::Message in EnqueueLetter Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- We have found a way to recreate this bug. If you update the PASSWORD_CHANGE letter to something like: [%- IF 0 %][% END %] or anything else that would generate a notice, Koha attempts to enqueue the blank notice in Koha::Patron::set_password, but EnqueueLetter returns if the rendered content is empty. Then we call SendQueuedMessages with message_id => undef! This patch set will resolve this issue for the affected code and prevent any regressions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155325|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 155459 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155459&action=edit Bug 34731: Throw exception if SendQueuedMessages is passed a bad message_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155326|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 155460 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155460&action=edit Bug 34731: Don't call SendQueuedMessages if message_id is bad Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155327|0 |1 is obsolete| | --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 155461 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155461&action=edit Bug 34731: Use Koha::Notice::Message in EnqueueLetter Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #8 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Set up KTD so that email can be sent - see [1]. 2. Add an email address to the koha user for an account you can access. 3. Test that emails are queued: . Edit the koha user patron messaging preferences so that an email is sent when an item is checked out. . Checkout an item to the koha user. . Note that under notices for the koha user that there is a Checkouts notice with status of pending. OR . Check the message queue (koha-mysql kohadev + select * from message_queue;) and note that the email status is pending. 4. Edit the CART notice so that the contents for email message is: [%- IF 0 %][% END %] 5. Add some records to the cart and send the cart to an email address. 6. You will receive a message about the checkout, even though the cronjob for sending messages wasn't run (you can also check that in the notices tab for the koha user that the checkouts notice is now showing as sent). In addition, you won't receive a message with the cart details. 7. Apply the patch etc., and repeat steps 3 + 5: this time the checkouts notice isn't sent - the status under notices for the koha user and in the database message queue will stay as pending, and you won't have received the checkout notice. Note: Possibly don't need to set up email sending - probably possible to only look at the notices tab for the koha user. [1] To test sending emails using a Google account, edit /etc/koha/sites/kohadev/koha-conf.xml and add this configuration near the end (where <user_name> = your Google email address; <password> = your APP password, not your Google account password), and then restart_all: <smtp_server> <host>smtp.gmail.com</host> <port>587</port> <timeout>5</timeout> <ssl_mode>STARTTLS</ssl_mode> <user_name>GOOGLEACCOUNTUSER</user_name> <password>GOOGLEAPPPASSWORD</password> <debug>1</debug> </smtp_server> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes an issue where release notes| |generating a notice that is | |undefined (for example, | |where it is empty) will | |trigger the sending of any | |pending messages, even | |though the message queue | |cronjob isn't run. This can | |cause an issue for | |libraries that expect | |emails and SMS messages to | |be processed at specific | |times. --- Comment #9 from David Nind <david@davidnind.com> --- I've had a go at the release notes - but I'm not sure I've gotten it right! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155459|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 155478 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155478&action=edit Bug 34731: Throw exception if SendQueuedMessages is passed a bad message_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155460|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 155479 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155479&action=edit Bug 34731: Don't call SendQueuedMessages if message_id is bad Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155461|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 155480 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155480&action=edit Bug 34731: Use Koha::Notice::Message in EnqueueLetter Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 155481 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=155481&action=edit Bug 34731: (follow-up) Fix conditional variable declaration Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Working well, good fix. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable Version(s)|23.11.00 |23.11.00,23.05.04 released in| | --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.05.x for 23.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED CC| |matt.blenkinsop@ptfs-europe | |.com --- Comment #17 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Missing code from bug 33360 in 22.11.x - not backporting Nice work everyone! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 158178 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158178&action=edit [22.11.x] Bug 34731: Throw exception if SendQueuedMessages is passed a bad message_id Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155480|0 |1 is obsolete| | --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 158179 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158179&action=edit Bug 34731: Use Koha::Notice::Message in EnqueueLetter Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #155481|0 |1 is obsolete| | --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 158180 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=158180&action=edit Bug 34731: (follow-up) Fix conditional variable declaration Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158179|Bug 34731: Use |[22.11.x] Bug 34731: Use description|Koha::Notice::Message in |Koha::Notice::Message in |EnqueueLetter |EnqueueLetter -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #158180|Bug 34731: (follow-up) Fix |[22.11.x] Bug 34731: description|conditional variable |(follow-up) Fix conditional |declaration |variable declaration -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Matt Blenkinsop from comment #17)
Missing code from bug 33360 in 22.11.x - not backporting
Nice work everyone!
22.11.x editions attached, please consider backporting! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|FIXED |--- Status|RESOLVED |REOPENED --- Comment #22 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Thanks Kyle, backported to 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|REOPENED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34731 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to oldstable -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org