Needs review
Project:
Messaging
Version:
6.x-2.x-dev
Component:
SMS Plugin
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
21 Nov 2008 at 20:16 UTC
Updated:
24 Mar 2011 at 04:32 UTC
Jump to comment: Most recent file
Comments
Comment #1
jose reyero commentedThe sending method plug-ins have gone through some changes so this one no longer applies.
However, there's an issue with this approach. Though the user sending function takes an user/uid as argument, the generic sending callback should work with user-less destinations (like a phone number) so it can be used to send messages to non users too.
Take a look at the new 'destination callback' parameter, which is intended to translate user accounts into destination's
Comment #2
jose reyero commentedComment #3
Will White commentedHey Jose. I think it makes sense just to pass on the whole $user->sms_user array to the send callback so it has direct access to it.
Is there a better way? I couldn't figure out how to use the $params variable in the send callback.
Comment #4
jose reyero commentedThanks Will, committed with a few minor changes (param checking, also did some cleaning in the old module's code.
About the $params variable, that's optional for sending methods that may need more params (like mail), no need to handle it unless it makes sense.
Comment #6
rothatboat commentedI continue to get the error with the 'carrier' not being added on for the email to send:
'Unable to send e-mail. Please contact the site administrator if the problem persists.'
The patch has not solved it, and I'm using the latest release of the sms framework. I have tried inputting the 'carrier' manually into the sms email gateway and that worked. But my users have various providers. Is there any solution to this problem on Drupal 6.13?
Thanks in advance.
Comment #7
Daryljames commentedThis is definately still a bug in 6.x-2.2
Comment #8
Daryljames commentedComment #9
Daryljames commentedI have it working if the user subscribes to tags... but not when they are subscribed to groups... it may have something to do with either notifications-lite, or organic groups notifications.
Comment #10
jose reyero commentedPlease find out which is the module causing the trouble. Then try to reproduce with a simple set up and post the steps here
Comment #11
humpdog commentedObserved Problem: When posting new content to a group, and a user is subscribed to receive notification via SMS for the newly posted content, after the content creation form is submitted, the page displays the "Unable to send e-mail. Please contact the site administrator if the problem persists." error message.
The send to email address is not complete. For example: the email address being used to send the notification is: 1234567890@
Here's the trace in the DB Log entry:
Sending SMS to 1234567890 failed.
Error sending e-mail (from to 1234567890@).
The table, sms_user.gateway, has the correct data for the carrier for the user.
Here's what works: Clicking on the "Send to Phone" link on a published content item to a group, displays a form with the phone number and the carrier for the current logged in user. When the form is submitted the SMS is sent and received on the phone.
Configuration:
Organic Groups (enabled features)
1. Organic groups access control
2. Organic Groups Notifications
3. Organic groups registration keys
4. Organic groups Views integration
SMS Framework (enabled features):
1. Gateway Configuration: E-mail
2. E-Mail Gateway
3. Send to Phone
4. SMS User
Messaging Framework 6.x.2-2 (enabled features):
1. Simple Mail
2. Simple messaging
3. SMS Messaging
Notifications 6.x-2.2 (enabled features):
1. Content Notifications
2. Notifications
3. Notifications Lite
4. Notifications UI
5. Notifications Views
Steps:
1. Organic Groups are created as Moderated or Open.
2. Content type of "Page" is allowed to be posted to the group.
3. Each group has more than one member.
4. Each member is configured to use SMS as the notification send method for each subscription they have for each group.
5. Each member has subscribed to the new content posted to the group.
6. New content of type, Page, is posted to the group.
7. Notification fails.
Thanks in advance.
Comment #12
humpdog commentedI did a little more investigation and testing. In the messaging module's messaging_sms.module,
the function call, messaging_sms_user_destination($account, $message), does not return the account's gateway information (carrier) with the phone number.
When posting new content to a group which has subscribed users who selected to receive sms notifications, this function is executed when the content creation form is submitted and I was able to trace it using the watchdog function.
Comment #13
jose reyero commentedThanks for the detailed report.
I've been doing some more tests and it seems the SMS framework needs more parameters for some gateways, like the mail gateway. Thus we need to pass on som more data besides the phone number.
I guess it will work with the other gateways. Anyway we need to find a workaround for this.
Comment #14
skram commentedI'm new to this framework/ecosystem but the following seems to be functional for me. It would be more pragmatic to have a params callback or something, but that doesn't seem to available from the main Messaging module and I figured this fix would be easier to get pushed through.
Replace the function in messaging_sms.module:
Replace the function in messaging_sms.module: (only the
returnline has been changed to reflect the new return type (now Array) from the destination callback.Please let me know if this is an appropriate solution or maybe there's a params callback in the main Messaging module?
Looking forward to getting this fixed to I can simplify the gateway module I'm getting ready to release! Thanks
Comment #15
skram commentedI just had a chance to look at this (http://drupal.org/files/issues/messaging_sms.diff) ... It's very similar to the idea I came up with just now. So what's holding this back from being implemented in a release? Just curious
Comment #16
Crom commentedsubscribing
Comment #17
NROTC_Webmaster commentedskram,
Your code worked flawlessly.
Comment #18
NROTC_Webmaster commentedI updated everything to version 2.3 and one again everything seems to be working well. I do however want to change the user page to display both the number and the gateway. I was able to correct the issue and the following code works as desired.
(line 210 in sms_user.module)
$form['sms_user']['number'] = array(
'#type' => 'item',
'#title' => t('Your number'),
- '#value' => $account->sms_user['number'],
+ '#value' => $account->sms_user['number'] . "@" . $account->sms_user['gateway']['carrier'],
'#description' => t('Your number has been confirmed.'),
);
Comment #19
buddaI fixed the problem passing the destination users sms_user array through to the sms gateway $params:
not sure if there are any negatives on this, but at least it works internationally now!
Comment #20
univate commentedInstead of using sms_send here it would be better to use sms_user_send
Messaging should not be concerned with details of sms numbers, just that its sending a sms to user id#XXX with the message 'blah'
Comment #21
univate commentedThere is an API for sending a message to a specific user on the site and this checks things like the number status. It doesn't make sense for this module to be concerned with the details of sms. This patch passes of the sms specific work to the sms module where it makes more sense.
Comment #22
univate commentedComment #23
nodecode commentedsubscribing