Bug 10784

Summary:	SMS-related code needs improvements
Product:	Koha	Reporter:	Magnus Enger <magnus>
Component:	Notices	Assignee:	Bugs List <koha-bugs>
Status:	RESOLVED DUPLICATE	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	benjamin.rokseth, jonathan.druart, julian.maurice
Version:	Main
Hardware:	All
OS:	All
See Also:	http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13029 http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14820
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:

Description Magnus Enger 2013-08-26 17:41:24 UTC

Some more or less random findings:

sms/sms_listen.pl has hardcoded strings in it, in Turkish (according to Google translate):
http://git.koha-community.org/gitweb/?p=koha.git;a=blob;f=sms/sms_listen.pl;h=b36b25da46baa3e49b9c0ae77dbf40fa8f3715c8;hb=9c8a1e29f8f25890c341859eb943aff64f7f37ac#l58

This comment from an earlier commit sounds ominous: "SMS - widespread cleanup and moving code away from Cyprus/Turkey-specific implementation. Much more needed."
http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=9c8a1e29f8f25890c341859eb943aff64f7f37ac

To bring this in line with other functionality in Koha it would probably be a good idea to make the notices editable, in the same way that other notices are. 

C4::SMS references two sysprefs, SMSSendUsername and SMSSendPassword, but when I search for these in 3.12.x and current master I get no results.

Comment 1 Benjamin Rokseth 2014-06-02 08:47:50 UTC

Adding to Magnus' findings:

all scripts under sms/ seems to concern some Turkish SMS reply service, probably renew or reserve by SMS. However, they should hardly be part of Koha main branch.
At least this should be a module under C4::SMS/Koha::SMS.

Also it might be in order to be able to pass various parameters to the chosen SMS::Send driver from setting 'SMSSendDriver'.

Now the various drivers need to hardcode settings such as url, username, password, etc.

I don't know what would be best practice, but maybe a setting such as 'SMSSendOptions' where options could be separated with '|'

e.g. 'url=http://smsservice.com|username=john|password=smith'

The drivers available at cpan: http://search.cpan.org/search?query=sms%3A%3Asend&mode=all

uses a long range of options, url,user,username,password,ssl,apikey,login,method, etc. It would be impossible to include all in C4::SMS, but a generic option hash passed to the driver would work, as per the Send::SMS doc: 

Params with a leading underscore are "private" driver-specific options and will be passed through to the driver unchanged.

Comment 2 Jonathan Druart 2014-09-01 09:16:29 UTC

See http://git.biblibre.com/biblibre/kohac/commits/sanop/ft/MT10141 too.

Comment 3 Julian Maurice 2014-10-06 13:38:58 UTC

(In reply to Jonathan Druart from comment #2)
> See http://git.biblibre.com/biblibre/kohac/commits/sanop/ft/MT10141 too.

This has been submitted in bug 13029

Comment 4 Magnus Enger 2017-08-28 06:59:17 UTC


*** This bug has been marked as a duplicate of bug 19178 ***