This patch updates the SMS sending method to be compatible with the newest SMS Framework API on Drupal 6.

Comments

jose reyero’s picture

Priority: Normal » Critical
Status: Needs review » Needs work

The 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

jose reyero’s picture

Issue tags: +Release blocker
Will White’s picture

StatusFileSize
new1.44 KB

Hey 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.

jose reyero’s picture

Status: Needs work » Fixed

Thanks 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.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

rothatboat’s picture

I 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.

Daryljames’s picture

Version: 6.x-1.x-dev » 6.x-2.2

This is definately still a bug in 6.x-2.2

Daryljames’s picture

Status: Closed (fixed) » Active
Daryljames’s picture

I 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.

jose reyero’s picture

Category: bug » support
Priority: Critical » Normal
Status: Active » Postponed (maintainer needs more info)

Please find out which is the module causing the trouble. Then try to reproduce with a simple set up and post the steps here

humpdog’s picture

Observed 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.

humpdog’s picture

I 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.

function messaging_sms_user_destination($account, $message) {
  // Check for active mobile infomation. Simply return it so that the send
  // callback has a destination array and access everything.
  if (!empty($account->sms_user) && $account->sms_user[0]['status'] == 2 && !empty($account->sms_user[0]['number'])) {
    return $account->sms_user[0]['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.

jose reyero’s picture

Category: support » bug
Status: Postponed (maintainer needs more info) » Active

Thanks 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.

skram’s picture

Status: Active » Needs review

I'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:

function messaging_sms_user_destination($account, $message) {
  // Check for active mobile infomation. Simply return it so that the send
  // callback has a destination array and access everything.
	$destination = array();
  if (!empty($account->sms_user) && $account->sms_user[0]['status'] == 2 && !empty($account->sms_user[0]['number']))
	{
		$destination['number'] = $account->sms_user[0]['number'];
		if(!empty($account->sms_user[0]['gateway'])){
			$destination['gateway'] = $account->sms_user[0]['gateway'];
		}
		return $destination;
	}
}

Replace the function in messaging_sms.module: (only the return line has been changed to reflect the new return type (now Array) from the destination callback.

function messaging_sms_send_msg($destination, $message, $params = array()) {
  $text = messaging_text_build($message, ' ');
  return sms_send($destination['number'], $text, $destination['gateway']);
}

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

skram’s picture

I 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

Crom’s picture

subscribing

NROTC_Webmaster’s picture

skram,

Your code worked flawlessly.

NROTC_Webmaster’s picture

I 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.'),
);

budda’s picture

I fixed the problem passing the destination users sms_user array through to the sms gateway $params:

<?php
function messaging_sms_send_msg($destination, $message, $params = array()) {

  $text = messaging_text_build($message, ' ');

  return sms_send($destination, $text, $message->account->sms_user[0]['gateway']);
}
?>

not sure if there are any negatives on this, but at least it works internationally now!

univate’s picture

Instead 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'

univate’s picture

Version: 6.x-2.2 » 6.x-2.x-dev
StatusFileSize
new1.79 KB

There 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.

univate’s picture

Title: Update SMS sending method for new SMS Framework » Use sms_user_send function
nodecode’s picture

subscribing