|
Description
Kyle M Hall (khall)
2023-09-06 19:51:40 UTC
Created attachment 155325 [details] [review] 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 Created attachment 155326 [details] [review] Bug 34731: Don't call SendQueuedMessages if message_id is bad Created attachment 155327 [details] [review] Bug 34731: Use Koha::Notice::Message in EnqueueLetter Test Plan: 1) Apply this patch 2) prove t/db_dependent/Letters.t 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. Created attachment 155459 [details] [review] 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> Created attachment 155460 [details] [review] Bug 34731: Don't call SendQueuedMessages if message_id is bad Signed-off-by: David Nind <david@davidnind.com> Created attachment 155461 [details] [review] 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> 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>
I've had a go at the release notes - but I'm not sure I've gotten it right! Created attachment 155478 [details] [review] 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> Created attachment 155479 [details] [review] 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> Created attachment 155480 [details] [review] 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> Created attachment 155481 [details] [review] Bug 34731: (follow-up) Fix conditional variable declaration Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Working well, good fix. Passing QA Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.04 Missing code from bug 33360 in 22.11.x - not backporting Nice work everyone! Created attachment 158178 [details] [review] [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> Created attachment 158179 [details] [review] [22.11.x] 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> Created attachment 158180 [details] [review] [22.11.x] Bug 34731: (follow-up) Fix conditional variable declaration Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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! Thanks Kyle, backported to 22.11.x |