Summary: | Pass message_id to SMS drivers | ||
---|---|---|---|
Product: | Koha | Reporter: | Johanna Räisä <johanna.raisa> |
Component: | Notices | Assignee: | Johanna Räisä <johanna.raisa> |
Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | andrew, david, kyle, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32381 | ||
GIT URL: | Change sponsored?: | Sponsored | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 31481: pass message_id to SMS drivers
Bug 31481: pass message_id to SMS drivers Bug 31481: pass message_id to SMS drivers Bug 31481: pass message_id to SMS drivers Bug 31481: pass message_id to SMS drivers Bug 31481: Pass message_id to SMS drivers |
Description
Johanna Räisä
2022-08-29 06:07:33 UTC
Created attachment 139938 [details] [review] Bug 31481: pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy Created attachment 140507 [details] [review] Bug 31481: pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> +++ b/t/SMS.t @@ -57,6 +57,7 @@ $send_sms = C4::SMS->send_sms({ destination => '+33123456789', message => 'my message', driver => 'Test', + message_id => 1, Please explain what this should prove ? Created attachment 158114 [details] [review] Bug 31481: pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy (In reply to Marcel de Rooy from comment #3) > +++ b/t/SMS.t > @@ -57,6 +57,7 @@ $send_sms = C4::SMS->send_sms({ > destination => '+33123456789', > message => 'my message', > driver => 'Test', > + message_id => 1, > > Please explain what this should prove ? I'm not sure anymore. I fixed the tests and message_id is required parameter. Created attachment 160986 [details] [review] Bug 31481: pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy Signed-off-by: matthias le gac <matthias.le-gac@inlibro.com> Created attachment 167206 [details] [review] Bug 31481: pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy Signed-off-by: matthias le gac <matthias.le-gac@inlibro.com> The patch is now fixed. Created attachment 167330 [details] [review] Bug 31481: Pass message_id to SMS drivers This patch adds message_id to _send_message_by_sms and sms_send so the driver could be build to catch SMS gateways delivery report. Test plan: 1) Apply the patch 2) prove t/SMS.t Sponsored-by: Koha-Suomi Oy Signed-off-by: matthias le gac <matthias.le-gac@inlibro.com> Signed-off-by: David Nind <david@davidnind.com> I have very mild concerns that this may break some SMS::Send drivers. I spot checked three drivers: Twilio, RedOxygen, and WebSMS. WebSMS is the only driver that passes all arbitrary options through to the rest api it calls. The safest solution would be to make sending this parameter configurable. |