Bug 13029 - Allow to pass additional parameters to SMS::Send drivers
Summary: Allow to pass additional parameters to SMS::Send drivers
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Chris Cormack
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-06 13:35 UTC by Julian Maurice
Modified: 2016-12-18 20:34 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.25 KB, patch)
2014-10-06 13:36 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.25 KB, patch)
2014-10-06 14:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.66 KB, patch)
2014-12-15 10:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.64 KB, patch)
2016-09-29 20:46 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13029 : Follow up (2.83 KB, patch)
2016-09-29 20:46 UTC, Chris Cormack
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.70 KB, patch)
2016-10-10 13:40 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.70 KB, patch)
2016-10-10 13:40 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.70 KB, patch)
2016-10-10 13:41 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.70 KB, patch)
2016-10-10 14:11 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029 : Follow up (3.22 KB, patch)
2016-10-10 14:12 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.70 KB, patch)
2016-10-10 14:13 UTC, Magnus Enger
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13029 : Follow up (3.22 KB, patch)
2016-10-10 14:14 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 13029: Allow to pass additional parameters to SMS::Send drivers (2.75 KB, patch)
2016-12-14 12:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13029 : Follow up (3.26 KB, patch)
2016-12-14 12:39 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2014-10-06 13:35:49 UTC

    
Comment 1 Julian Maurice 2014-10-06 13:36:55 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2014-10-06 14:01:06 UTC Comment hidden (obsolete)
Comment 3 Benjamin Rokseth 2014-11-14 18:15:45 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-12-15 10:10:19 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2014-12-15 10:10:33 UTC Comment hidden (obsolete)
Comment 6 Magnus Enger 2015-10-09 09:16:26 UTC
Wouldn't it make sense to also document this new feature in the POD of C4/SMS.pm?
Comment 7 Magnus Enger 2015-10-09 09:28:04 UTC
How will this work for a Debian-package based installation that serves multiple Koha instances? There will only be one YAML file, right? So it is not possible to have different settings for different instances? 

For SMS::Send::Telenor (https://github.com/eskaaren/SMS-Send-Telenor) it is possible to pass in a sender, but that will have to be a different sender for each Koha instance.
Comment 8 Magnus Enger 2015-12-08 13:11:03 UTC Comment hidden (obsolete)
Comment 9 Magnus Enger 2016-08-01 18:51:44 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2016-09-29 19:38:47 UTC Comment hidden (obsolete)
Comment 11 Chris Cormack 2016-09-29 20:12:10 UTC
My plan is to put the path to the yaml file in the koha-conf.xml
(Blank by default)

That way it will work with multiple instances.
Comment 12 Chris Cormack 2016-09-29 20:46:23 UTC
Created attachment 55942 [details] [review]
Bug 13029: Allow to pass additional parameters to SMS::Send drivers

C4::SMS::send_sms now reads a YAML file and pass all parameters in the
file to SMS::Send::new.

The config file is read in <installdir>/etc/sms/driver/<driver>.yaml.
For instance, if the driver used is SMS::Send::UK::Kapow, the config
file has to be in <installdir>/etc/sms/driver/UK/Kapow.yaml

A underscore character is prepended to all parameter names so they are
treated as driver-specific options (leading underscore must not appear
in config file).
Comment 13 Chris Cormack 2016-09-29 20:46:47 UTC
Created attachment 55943 [details] [review]
Bug 13029 : Follow up

Adding POD, changing the config files to live in a path pointed to by
koha-conf.xml

This means multiple instances can have their own config

Please test the 2 patches together
Comment 14 Jonathan Druart 2016-09-30 08:10:09 UTC
(In reply to Chris Cormack from comment #13)
> Created attachment 55943 [details] [review] [review]
> Bug 13029 : Follow up

Since this patch is stuck for almost 2 years (and I have written it 4 years ago...) It would have been better to make it moving forward and improve it on another bug report, don't you think?
Who is going to add a signoff on it now? :)
Comment 15 Jonathan Druart 2016-09-30 08:11:29 UTC
(In reply to Jonathan Druart from comment #14)
> (In reply to Chris Cormack from comment #13)
> > Created attachment 55943 [details] [review] [review] [review]
> > Bug 13029 : Follow up
> 
> Since this patch is stuck for almost 2 years (and I have written it 4 years
> ago...) It would have been better to make it moving forward and improve it
> on another bug report, don't you think?
> Who is going to add a signoff on it now? :)

Ok forget that, I did not see the installdir var.
So, Magnus maybe? :)
Comment 16 Magnus Enger 2016-10-03 08:46:19 UTC
(In reply to Jonathan Druart from comment #15)
> So, Magnus maybe? :)

I will have a look in Marseille, if noone beats me to it.
Comment 17 Magnus Enger 2016-10-10 13:40:25 UTC Comment hidden (obsolete)
Comment 18 Magnus Enger 2016-10-10 13:40:53 UTC Comment hidden (obsolete)
Comment 19 Magnus Enger 2016-10-10 13:41:17 UTC Comment hidden (obsolete)
Comment 20 Magnus Enger 2016-10-10 14:11:04 UTC Comment hidden (obsolete)
Comment 21 Magnus Enger 2016-10-10 14:12:16 UTC Comment hidden (obsolete)
Comment 22 Magnus Enger 2016-10-10 14:13:53 UTC
Created attachment 56113 [details] [review]
[SIGNED-OFF] Bug 13029: Allow to pass additional parameters to SMS::Send drivers

C4::SMS::send_sms now reads a YAML file and pass all parameters in the
file to SMS::Send::new.

The config file is read in <installdir>/etc/sms/driver/<driver>.yaml.
For instance, if the driver used is SMS::Send::UK::Kapow, the config
file has to be in <installdir>/etc/sms/driver/UK/Kapow.yaml

A underscore character is prepended to all parameter names so they are
treated as driver-specific options (leading underscore must not appear
in config file).

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Comment 23 Magnus Enger 2016-10-10 14:14:02 UTC
Created attachment 56116 [details] [review]
[SIGNED-OFF] Bug 13029 : Follow up

Adding POD, changing the config files to live in a path pointed to by
koha-conf.xml

This means multiple instances can have their own config

Please test the 2 patches together

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Arbitrary arguments can be passed to SMS:Send
drivers. If an argument is provided that has already been set by
SMS::Send or the driver, it will be overwritten by the value from
the YAML file. My only suggestion for an improvement would be an
example of what the YAML should look like, but that is a minor thing.
Comment 24 Jonathan Druart 2016-12-14 12:38:39 UTC
Created attachment 58170 [details] [review]
Bug 13029: Allow to pass additional parameters to SMS::Send drivers

C4::SMS::send_sms now reads a YAML file and pass all parameters in the
file to SMS::Send::new.

The config file is read in <installdir>/etc/sms/driver/<driver>.yaml.
For instance, if the driver used is SMS::Send::UK::Kapow, the config
file has to be in <installdir>/etc/sms/driver/UK/Kapow.yaml

A underscore character is prepended to all parameter names so they are
treated as driver-specific options (leading underscore must not appear
in config file).

Signed-off-by: Magnus Enger <magnus@libriotech.no>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 25 Jonathan Druart 2016-12-14 12:39:03 UTC
Created attachment 58172 [details] [review]
Bug 13029 : Follow up

Adding POD, changing the config files to live in a path pointed to by
koha-conf.xml

This means multiple instances can have their own config

Please test the 2 patches together

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. Arbitrary arguments can be passed to SMS:Send
drivers. If an argument is provided that has already been set by
SMS::Send or the driver, it will be overwritten by the value from
the YAML file. My only suggestion for an improvement would be an
example of what the YAML should look like, but that is a minor thing.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 26 Kyle M Hall 2016-12-16 11:33:46 UTC
Pushed to master for 17.05, thanks Julian, Chris!
Comment 27 Katrin Fischer 2016-12-18 20:34:11 UTC
This won't get backported to 16.11.x as it is an enhancement.