SMS plugin not working - wrong/outdated function call to sms_send()
pathfinderelite - March 19, 2009 - 19:00
| Project: | Messaging |
| Version: | 6.x-2.x-dev |
| Component: | SMS Plugin |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Will White |
| Status: | closed |
Description
The SMS plugin (messaging_sms.module) calls the sms_send() function with the wrong parameters, which causes all SMS message delivery to fail. On line 66 of messaging_sms.module:
function messaging_sms_send_msg($destination, $message, $params = array()) {
// This function takes an array of destinations so
return sms_send(array($destination), $message['subject'].$message['body']);
}However, as of SMS Framework 5.x-1.2,
sms_send() takes three arguments:/**
* Sends a message using the active gateway.
*
* @param $number
* The destination number.
*
* @param $message
* The text of the messsage to send.
*
* @param $options
* An array of dditional properties as defined by gateway modules.
*/
function sms_send($number, $message, $options = array()) {In SMS Framework 5.x-1.2,
$number is treated as a string, not an array of stdObjects, which is what the SMS plugin passes in. I'm not exactly sure what all needs to be done here, but my guess would be to pass in $destination->number for the $number parameter and put everything else from $destination into an array and pass it as the optional $options parameter.
#1
Here is a patch to fix the problem
#2
#3
Then we should be fixing the _sms_send_user function in the first place.
@Will, if you can take a look at this one..
#4
Fixed, let me know whether it works now.
#5
In the mean time I've updated my site to Drupal 6, but everything seems to work fine with the 6.x-2.0 version.
#6
the third argument for sms_send() is missing in the 6.x-2.x branch, but some SMS gateways don't work without extra data passed in here (e.g. country or carrier).
#7
The patch looks good, however I don't think that changes anything in how messaging currently handles SMSs. Is it that you need it for integration with any other module?
You're right that part is missing, but also the user2sms destination mapping just retrieves the number, so we may need to completely rework that part too.
Also I've been talking with smsframework maintainer and we agree we are moving towards sms api functions that just need to get passed the mobile number, then the smsframework should be able to resolve anything else.
I mean, I'd be ok with this patch but maybe we should fix that in SMS framework instead, which is an option too.
#8
The issue is that some of the SMS Framework gateway modules cannot send an SMS with just a number. They need some other info, for example country or carrier. The latter is needed for the email gateway module, i.e., sending an SMS by sending an email to number@sms.att.net, number@vtext.com, etc.
Sending with just the number can work if the number is already in the sms_user tables, because the country or carrier could be looked up. But it doesn't work if a user wants to send a message to an arbitrary number.
So we need the ability to pass some extra params from the message object into SMS Framework.
#9
@mfb,
Agreed. However I think that is a work for the sms framework. If we need modules/users using the service to provide low level gateway parameters we completely lose the abstraction here.
That said, while waiting for the good solution from sms framework we still want the messaging plug-in to work so I guess we'll need to go with this and maybe other kind of hacks.
I see you have more knowledge about SMS and better testing set up than I do. So feel free to commit this one, maybe other fixes, just let me know if we need api changes as that can have some impact on other parts of Messaging
You've been added to the cvs commit list for messaging module, welcome :-)
#10
#11
Automatically closed -- issue fixed for 2 weeks with no activity.