Bug 31481

Summary: Pass message_id to SMS drivers
Product: Koha Reporter: Johanna Räisä <johanna.raisa>
Component: NoticesAssignee: 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
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.
Comment 1 Johanna Räisä 2022-08-29 07:39:28 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
Comment 2 David Nind 2022-09-12 20:39:40 UTC
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>
Comment 3 Marcel de Rooy 2022-09-30 09:55:45 UTC
+++ 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 ?
Comment 4 Johanna Räisä 2023-10-31 12:14:39 UTC
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
Comment 5 Johanna Räisä 2023-10-31 12:17:19 UTC
(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.
Comment 6 Matthias Le Gac 2024-01-12 16:07:05 UTC
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>
Comment 7 Johanna Räisä 2024-05-28 06:06:04 UTC
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>
Comment 8 Johanna Räisä 2024-05-28 06:06:54 UTC
The patch is now fixed.
Comment 9 David Nind 2024-06-02 23:52:33 UTC
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>
Comment 10 Kyle M Hall (khall) 2024-10-04 16:56:04 UTC
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.