[Bug 18478] New: Hold Notices sent via SMS gateway fail
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Bug ID: 18478 Summary: Hold Notices sent via SMS gateway fail Change sponsored?: --- Product: Koha Version: 16.11 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: jzairo@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Hold filled notices sent via SMS gateway fail when notifying patron. Other messages including item checkout, item checkin, send successfully via SMS gateway. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Lisette <lisetteslatah@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lisetteslatah@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |WORKSFORME CC| |kyle@bywatersolutions.com --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Appears to be a configuration issue and not a bug. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED CC| |nick@bywatersolutions.com Resolution|WORKSFORME |--- --- Comment #2 from Nick Clemens <nick@bywatersolutions.com> --- Oops, confirmed -- 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=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=18478 --- Comment #3 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63487 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63487&action=edit Bug 18478 - Hold Notices sent via SMS gateway fail It seems that for HOLD notices we rely on C4::Letters::SendQueuedMessages to populate the correct address. The sms code in this sub, however, assumed the smsalertnumebr is populated. Ths patch changes C4::Reserves::_koha_notify_reserve to use C4::Message::enqueue for SMS and email messages in order to ensure the correct addrress is populated To test: 1 - Define a messaging prefs for a patron to recieve hold notices via SMS 2 - Ensure you have defined an SMS message for 'HOLD' letter 3 - Set an SMS alert number for patron 4 - Set the SMS::Send driver to 'Email' 5 - Fill a hold for the patron 6 - Check the db and note the address is null 7 - Apply patch 8 - Fill another hold 9 - Check db and note address is populated 10 - Repeat for email notices and see the same change -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63488 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63488&action=edit Bug 18478 - Unit tests To test: 1 - Apply this patch first 2 - Prove t/db_dependent/Reserves.t 3 - Last tests fail 4 - Apply other patch 5 - All tests should pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63487|0 |1 is obsolete| | Attachment #63488|0 |1 is obsolete| | --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63504 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63504&action=edit Bug 18478 - Some notices sent via SMS gateway fail It seems that for HOLD and DUE (and maybe more) notices we rely on C4::Letters::SendQueuedMessages to populate the correct address. This patch adjust that subroutine to correctly populate the field and/or fail messages if no SMS provider available To test: 1 - Define a messaging prefs for a patron to recieve hold notices via SMS 2 - Ensure you have defined an SMS message for 'HOLD' letter 3 - Set an SMS alert number for patron 4 - Set the SMS::Send driver to 'Email' 5 - Fill a hold for the patron 6 - Check the db and note the address is null 7 - run process_message_queue.pl 8 - Check db - address is null and message pending 9 - Apply patch 10 - run process_message_queue 11 - Message to_address should be populated and message sent -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63505 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63505&action=edit Bug 18478 - Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63506&action=edit Bug 18478 - Additional unit tests This method was not previously covered, we don't change it , but no reason to throw away these tests to ensure messages are created as expected To test: 1 - Apply this patch first 2 - Prove t/db_dependent/Reserves.t 3 - Last tests fail 4 - Apply other patch 5 - All tests should pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Hold Notices sent via SMS |Some notices sent via SMS |gateway fail |gateway fail -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |18620 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18620 [Bug 18620] t/db_dependent/Letters.t failing on master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|16.11 |master CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- One test fails for me prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 55/72 No reserves HOLD letter transported by sms at /home/vagrant/kohaclone/C4/Letters.pm line 697. Could not find a letter called 'HOLD' for sms in the 'reserves' module at /home/vagrant/kohaclone/C4/Reserves.pm line 1883. # No tests run! # Failed test 'No tests run for subtest "_koha_notify_reserve() tests"' # at t/db_dependent/Reserves.t line 779. Can't call method "to_address" on an undefined value at t/db_dependent/Reserves.t line 763. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- This: $message->{to_address} = $message->{to_address} ? $message->{to_address} : $member->{'smsalertnumber'}; is actually: $message->{to_address} ||= $member->{smsalertnumber}; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The call to _update_message_to_address just before _send_message_by_email should not be needed, it is already called from _send_message_by_email. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63674&action=edit Bug 18478 - QA Followup Make sure to build necessary letters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63674|0 |1 is obsolete| | --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63675&action=edit Bug 18478 - QA Followup Make sure to build necessary letters Fix awkward construction -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #13 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #10)
The call to _update_message_to_address just before _send_message_by_email should not be needed, it is already called from _send_message_by_email.
I think it is only called there is the address was blank, we are fixing it before that call so need to update -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Jason Robb <jrobb@sekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jrobb@sekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Nick Clemens from comment #13)
(In reply to Jonathan Druart from comment #10)
The call to _update_message_to_address just before _send_message_by_email should not be needed, it is already called from _send_message_by_email.
I think it is only called there is the address was blank, we are fixing it before that call so need to update
Yes, thats what I understand too. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63504|0 |1 is obsolete| | --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 63734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63734&action=edit Bug 18478 - Some notices sent via SMS gateway fail It seems that for HOLD and DUE (and maybe more) notices we rely on C4::Letters::SendQueuedMessages to populate the correct address. This patch adjust that subroutine to correctly populate the field and/or fail messages if no SMS provider available To test: 1 - Define a messaging prefs for a patron to recieve hold notices via SMS 2 - Ensure you have defined an SMS message for 'HOLD' letter 3 - Set an SMS alert number for patron 4 - Set the SMS::Send driver to 'Email' 5 - Fill a hold for the patron 6 - Check the db and note the address is null 7 - run process_message_queue.pl 8 - Check db - address is null and message pending 9 - Apply patch 10 - run process_message_queue 11 - Message to_address should be populated and message sent Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63505|0 |1 is obsolete| | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 63735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63735&action=edit Bug 18478 - Unit tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63506|0 |1 is obsolete| | --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 63736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63736&action=edit Bug 18478 - Additional unit tests This method was not previously covered, we don't change it , but no reason to throw away these tests to ensure messages are created as expected To test: 1 - Apply this patch first 2 - Prove t/db_dependent/Reserves.t 3 - Last tests fail 4 - Apply other patch 5 - All tests should pass Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #63675|0 |1 is obsolete| | --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 63737 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63737&action=edit Bug 18478 - QA Followup Make sure to build necessary letters Fix awkward construction Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 63738 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63738&action=edit Bug 18620: [QA Follow-up] Move the final rollback to the very end The new subtest in Reserves.t does not need its own transaction. Move the original rollback to the very end of the test. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | 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=18478 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #20 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 17.05, thanks Nick, Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Bug 18478 depends on bug 18620, which changed state. Bug 18620 Summary: t/db_dependent/Letters.t failing on master https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18620 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is this needed in 16.11.x? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #21)
Is this needed in 16.11.x?
Yes, I believe so, the unit tests here caused the dependency on 18620 - but also depends on 9021 so problem exists in 16.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #23 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Nick, could you provide rebased patches for 16.11.x please? Conflicts with the tests mostly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63983 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63983&action=edit [16.11.x Rebase] Bug 18478 - Some notices sent via SMS gateway fail It seems that for HOLD and DUE (and maybe more) notices we rely on C4::Letters::SendQueuedMessages to populate the correct address. This patch adjust that subroutine to correctly populate the field and/or fail messages if no SMS provider available To test: 1 - Define a messaging prefs for a patron to recieve hold notices via SMS 2 - Ensure you have defined an SMS message for 'HOLD' letter 3 - Set an SMS alert number for patron 4 - Set the SMS::Send driver to 'Email' 5 - Fill a hold for the patron 6 - Check the db and note the address is null 7 - run process_message_queue.pl 8 - Check db - address is null and message pending 9 - Apply patch 10 - run process_message_queue 11 - Message to_address should be populated and message sent Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #25 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63984 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63984&action=edit [16.11.x rebase] Bug 18478 - Unit tests Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #26 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 63985 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63985&action=edit [16.11.x rebase] Bug 18478 - QA Followup Make sure to build necessary letters Fix awkward construction Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 --- Comment #27 from Nick Clemens <nick@bywatersolutions.com> --- rebased, removed changes to reserves.t as they had dependencies and removed use of build_object -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Thx Nick! These patches have been pushed to 16.11.x and will be in 16.11.09. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com --- Comment #29 from Mason James <mtj@kohaaloha.com> --- Pushed to 16.05.x, for 16.05.15 release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18478 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18990 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18990 [Bug 18990] Overdue Notices are not sending through SMS correctly -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org