Most of the SMS Gateways support REST API and it is common that the operator sends their report back to the service. Now it is impossible to catch reports if sms was succesfully sent or it failed since we can't pass the message_id to our report url when sending the message to the gateway.
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.