Created attachment 32011 [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).
Created attachment 32013 [details] [review] Bug 13029: Allow to pass additional parameters to SMS::Send drivers Fixed a variable name
Conflict in C4::SMS Applying: Bug 13029: Allow to pass additional parameters to SMS::Send drivers fatal: sha1 information is lacking or useless (C4/SMS.pm). probably from pushed bug #3186: https://github.com/Koha-Community/Koha/commit/03038897467f811670250977a3889f91cfa24d20
Created attachment 34403 [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).
Patch rebased.
Wouldn't it make sense to also document this new feature in the POD of C4/SMS.pm?
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.
The patch uses this: C4::Context->config('installdir') There is no trace of installdir in current Koha. According to Joubu on IRC this config variable was introduced by the Solr work and later removed. Comments 6 and 7 also need to be addressed.
Any chance of this patch getting some more attention?
I'm going to see if I can fix this up
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.
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).
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
(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? :)
(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? :)
(In reply to Jonathan Druart from comment #15) > So, Magnus maybe? :) I will have a look in Marseille, if noone beats me to it.
Created attachment 56102 [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>
Created attachment 56103 [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>
Created attachment 56104 [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>
Created attachment 56111 [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>
Created attachment 56112 [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.
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>
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.
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>
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>
Pushed to master for 17.05, thanks Julian, Chris!
This won't get backported to 16.11.x as it is an enhancement.