Thanks for this module. I'm currently implementing this for a customer.

On unsubscribe it seems unnecessary to require the name. Just the e-mail should do. The user might not even remember the name he/she registered with.

Could you make "name required" a setting?

(Even nicer would be to adjust the requirement based on the subscribe/unsubscribe toggle, but this would need some javascript magic.)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sgabe’s picture

Status: Active » Needs review
FileSize
1.74 KB

Sure, absolutely. This can be easily done with Chaos tool suite's dependent tool and some modification based on Skip Validation. The attached patch hides the "Name" field and skips the validation on unsubscription. Please, test it and report back.

Note that you need to install and enable Chaos tool suite to toggle the visibility of the "Name" field!

sgabe’s picture

Committed to both branches.

Note that the visibility toogle for the "Name" field works only in the 1.x branch, since only that uses radios to choose to subscribe or unsubscribe. However the "Name" field is not validated as required field if the user choose to unsubscribe from a newsletter.

See the patches attached to easier review, but you can test the development snapshots.

gaele’s picture

Thanks. I'll test tomorrow, when the new dev snapshots are published.

gaele’s picture

Version: 6.x-1.0-alpha2 » 6.x-1.x-dev

Hi, I tested with 6.x-1.x.dev. Showing/hiding the Name field seems to work as expected. However I encountered two bugs.

- logged in: after subscribing followed by unsubscribing the corresponding row in table simplenews_realname is not deleted.

- not logged in: subscribing yields this error: warning: mail() expects parameter 1 to be string, array given in [...]/includes/mail.inc on line 193.

sgabe’s picture

About the first one, were you subscribed for multiple newsletters?

gaele’s picture

No, only one newsletter is available.

sgabe’s picture

Okay, I found the problem. Actually the submit callback never runs for registered users on unsubscription. Please test the attached patch.

gaele’s picture

Status: Needs review » Needs work

The first bug is solved. The second still exists.

In simplenews_realname_mail_alter():

    // Replace receiver's name if the message sent through Mime Mail.
    if (module_exists('mimemail')) {
      if (isset($message['params']['context']['account'])) {
        $message['params']['context']['account']->name = $realname;
      }
      $recipient = array('name' => $realname, 'mail' => $message['to']);
      $message['to'] = $recipient;
    }

$recipient should be a string. Perhaps you where thinking of "User <user@example.com>"?

sgabe’s picture

What Mime Mail version and mail engine are you using?

gaele’s picture

sendmail

But this looks like a PHP limitation. mail.inc calls PHP mail. http://php.net/manual/en/function.mail.php:
bool mail ( string $to , string $subject , string $message [, string $additional_headers [, string $additional_parameters ]] )

sgabe’s picture

I mean what mail engine for Mime Mail? Default, SMTP, PHPMailer etc? Mime Mail accepts array for the recipient and converts it to string.

gaele’s picture

Ah I see. mimemail 6.x-1.0-alpha6 with default engine ('mimemail').

gaele’s picture

The subscription mail sent by simplenews is just plain text, though. Mimemail isn't used.

gaele’s picture

This is working for me (taken from mimemail_address()):

$recipient = '"'. addslashes(mime_header_encode($realname)) .'" <'. $message['to'] .'>';
sgabe’s picture

Note that Windows based systems will reject formatted addresses, we need to check that first.

sgabe’s picture

Status: Needs work » Needs review
sgabe’s picture

Status: Needs review » Fixed

Committed to both branches. I hope all of this is fixed now. Thanks!

gaele’s picture

Thank you sgabe!

Status: Fixed » Closed (fixed)

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