When trying to unsubscribe an e-mail address that was not present in the database, the following error popped up:

    * Notice: Trying to get property of non-object in simplenews_confirmation_add_combined() (line 2978 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Trying to get property of non-object in simplenews_confirmation_send_combined() (line 3021 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Trying to get property of non-object in simplenews_confirmation_send_combined() (line 3021 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Trying to get property of non-object in simplenews_build_unsubscribe_mail() (line 839 of /sites/all/modules/simplenews/includes/simplenews.mail.inc).
    * Notice: Trying to get property of non-object in simplenews_tokens() (line 2155 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Undefined property: stdClass::$activated in simplenews_subscriber_save() (line 1690 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Undefined property: stdClass::$mail in simplenews_subscriber_save() (line 1691 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Undefined property: stdClass::$uid in simplenews_subscriber_save() (line 1692 of /sites/all/modules/simplenews/simplenews.module).
    * Notice: Undefined property: stdClass::$language in simplenews_subscriber_save() (line 1693 of /sites/all/modules/simplenews/simplenews.module).
    * PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'activated' cannot be null: INSERT INTO {simplenews_subscriber} (activated, mail, uid, language, changes) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => [:db_insert_placeholder_2] => [:db_insert_placeholder_3] => [:db_insert_placeholder_4] => a:1:{i:168;s:11:"unsubscribe";} ) in simplenews_subscriber_save() (line 1696 of /sites/all/modules/simplenews/simplenews.module).

What's even worse, the error appeared on a totally screwed up layout, but I'd guess that's something difficult to avoid. Please at least better catch the error.

CommentFileSizeAuthor
#7 this_time_maybe.patch3.58 KBberdir
#5 fix_newline_test_fail.patch3.56 KBberdir
#3 fix_error_when_not_subscribed.patch3.54 KBberdir

Comments

moniuch’s picture

moniuch commented

Just as a quick resort, I modified the source code (grrrh) simplenews.module, line 1395:

function simplenews_unsubscribe_user($mail, $tid, $confirm = TRUE, $source = 'unknown') {
  $subscriber = simplenews_subscriber_load_by_mail($mail);

to:

function simplenews_unsubscribe_user($mail, $tid, $confirm = TRUE, $source = 'unknown') {
  $subscriber = simplenews_subscriber_load_by_mail($mail);
  if(!$subscriber) return TRUE;

The site course behaves as if the address existed, but it's better than a crash.
Sorry, I'm no git-savvy yet.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: Site crash when unsubscribing an email not in db » PDO Exception when unsubscribing an email that is not yet subscribed
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new fix_error_when_not_subscribed.patch3.54 KB

Confirmed, the attached patch should fix this and adds test coverage for it. Not trivial...

Status: Needs review » Needs work

The last submitted patch, fix_error_when_not_subscribed.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Priority: Critical » Major
Status: Needs work » Needs review
StatusFileSize
new fix_newline_test_fail.patch3.56 KB

D'oh.

Mail texts are split at 80 characters and the split on the testbot was inbetween the assertion text because of the longer hostname. Removing line breaks now before doing the assert. this one should pass.

And yes, the patch is ugly, because we have to work with faked simplenews subscribers in multiple places.

Status: Needs review » Needs work

The last submitted patch, fix_newline_test_fail.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new this_time_maybe.patch3.58 KB

that concatenated words together, let's try to replace it with a space instead of an empty string.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Commited.

Status: Fixed » Closed (fixed)

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