I´ll describe what is happening:

- A thread was started in March among 10 users
- One of them was blocked in Drupal because he´s not part of the program anymore (but we don´t want to delete him)
- The conversation is still active and that user which was blocked is still receiving email alerts for updates in the conversation

This looks like a bug for me. No?

Regards,
Wilton

Files: 
CommentFileSizeAuthor
#77 privatemsg-do-not-send-messages-to-blocked-users-834706-77-tests-only.patch4.28 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,909 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#77 privatemsg-do-not-send-messages-to-blocked-users-834706-77.patch5.08 KBptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,913 pass(es).
[ View ]
#74 privatemsg-do-not-send-messages-to-blocked-users-834706-75-tests-only.patch4.28 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,908 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#74 privatemsg-do-not-send-messages-to-blocked-users-834706-75.patch5.08 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#72 privatemsg-do-not-send-messages-to-blocked-users-834706-71.patch8.97 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#71 privatemsg-do-not-send-messages-to-blocked-users-834706-71-tests-only.patch7.43 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,908 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#71 privatemsg-do-not-send-messages-to-blocked-users-834706-71.patch8.97 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#66 privatemsg-do-not-send-messages-to-blocked-users-834706-66-tests-only.patch6.54 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,904 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#66 privatemsg-do-not-send-messages-to-blocked-users-834706-66.patch7.34 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,906 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#61 privatemsg-do-not-send-messages-to-blocked-users-834706-61-no-tests.patch6.19 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#61 privatemsg-do-not-send-messages-to-blocked-users-834706-61.patch6.99 KBptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,896 pass(es).
[ View ]
#59 privatemsg-do-not-send-messages-to-blocked-users-834706-59-tests-only.patch6.2 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#59 privatemsg-do-not-send-messages-to-blocked-users-834706-59.patch7 KBptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,896 pass(es).
[ View ]
#55 privatemsg-do-not-send-messages-to-blocked-users-834706-55.patch6.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,896 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#53 privatemsg-do-not-send-messages-to-blocked-users-834706-53-tests-only.patch6.19 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 1 exception(s).
[ View ]
#53 privatemsg-do-not-send-messages-to-blocked-users-834706-53.patch7.15 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-53.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#51 privatemsg-do-not-send-messages-to-blocked-users-834706-51.patch6.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,896 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#49 privatemsg-do-not-send-messages-to-blocked-users-834706-49.patch6.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/privatemsg/privatemsg.test.
[ View ]
#45 privatemsg-do-not-send-messages-to-blocked-users-834706-45.patch6.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 2 exception(s).
[ View ]
#43 privatemsg-do-not-send-messages-to-blocked-users-834706-43.patch7.03 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 2 exception(s).
[ View ]
#41 privatemsg-do-not-send-messages-to-blocked-users-834706-41.patch6.94 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]
#39 privatemsg-do-not-send-messages-to-blocked-users-834706-39-test-only.patch6.08 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]
#39 privatemsg-do-not-send-messages-to-blocked-users-834706-39.patch6.88 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]
#35 privatemsg-do-not-send-messages-to-blocked-users-834706-35.patch6.93 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 132 exception(s).
[ View ]
#33 privatemsg-do-not-send-messages-to-blocked-users-834706-33.patch6.89 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 133 exception(s).
[ View ]
#30 privatemsg-do-not-send-messages-to-blocked-users-834706-30.patch2.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,888 pass(es), 1 fail(s), and 133 exception(s).
[ View ]
#28 privatemsg-do-not-send-messages-to-blocked-users-834706-28.patch2.99 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#23 privatemsg-do-not-send-messages-to-blocked-users-834706-23.patch825 bytesptmkenny
FAILED: [[SimpleTest]]: [MySQL] 4,881 pass(es), 0 fail(s), and 22 exception(s).
[ View ]
#17 privatemsg-do-not-send-messages-to-blocked-users-834706-17.patch653 bytesptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,878 pass(es).
[ View ]
#15 privatemsg-do-not-send-messages-to-blocked-users-834706-15.patch623 bytesptmkenny
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 privatemsg-do-not-send-messages-to-blocked-users-834706-13.patch701 bytesptmkenny
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#10 privatemsg-do-not-send-messages-to-blocked-users-834706-10.patch751 bytesptmkenny
FAILED: [[SimpleTest]]: [MySQL] 3,902 pass(es), 95 fail(s), and 103 exception(s).
[ View ]
#8 privatemsg-do-not-send-messages-to-blocked-users-834706-8.patch1.32 KBptmkenny
FAILED: [[SimpleTest]]: [MySQL] 3,902 pass(es), 95 fail(s), and 103 exception(s).
[ View ]

Comments

Yep, I guess you're right.

However, I'd say we don't want to just block email alerts but the private messages in the first place. This also means that the pm_mail_notify module will never see the blocked recipients and will therefore not send email alerts to them.

Great Berdir.

I'll work next days on a patch. Will let you know further updates...

Thanks,
Wilton

Great.

Should be just a few lines added to http://blog.worldempire.ch/api/function/_privatemsg_validate_message/1

After the check if recipients is an array("if (empty($message['recipients']) || !is_array($message['recipients']))"), you can loop over the recipients and remove the recipient + add a warning message similiar to other validation checks.

This will cover both the API functions and message sent through the form. A simple test would be awesome too...

Dear Berdir,

I´ve added the following code on privatemsg.module line 1831:

foreach ($message['recipients'] as $key=>$rec){
if(!$rec->status){
unset($message['recipients'][$key]);
drupal_set_message(t("Message not sent to ".$rec->name.": blocked"), 'warning');
}
}

I´ve tested and works fine. Could you please test?

Thanks!

Status:Active» Needs review

The code should look like this (untested):

<?php
foreach ($message['recipients'] as $key => $recipient) {
  if (!
$recipient->status) {
    unset(
$message['recipients'][$key]);
   
$messages['warning'][] = t('Disallowed to send message to blocked recipient !name.', array('!name' => theme('username', $recipient));
  }
}
?>

- Whitespaces
- Don't directly print the message but instead add it as a warning to the $messages array. There might be multiple recipients and even if one is blocked, the message will still be sent to the others
- Don't abbreviate variable names ($recipient instead of $rec)
- Use placeholders in translatable strings (also important for security)

Please test this and you're welcome to create a patch, see http://drupal.org/patch/create.

Status:Needs review» Needs work

Version:6.x-1.2» 7.x-2.x-dev
Status:Needs work» Needs review

Hi, this affects my site so I'm attempting to update this for 7.x

I attempted to modify _privatemsg_validate_message as follows:

  if (!empty($message->recipients) && is_array($message->recipients)) {
    foreach (module_invoke_all('privatemsg_block_message', $message->author, $message->recipients, (array)$message) as $blocked) {
      unset($message->recipients[$blocked['recipient']]);
      if ($form) {
        drupal_set_message($blocked['message'], 'warning');
      }
      else {
        $messages['warning'][] = $blocked['message'];
      }
    }
    //do not send private messages to blocked users
    foreach ($message->recipients as $key => $recipient) {
      if (!$recipient->status) {
        unset($message->recipients[$key]);
        //$key->name shows the user's private name if RealName module enabled-
        //must fix compatibility
        $messages['warning'][] = t('You cannot send a message to !name because that account has been cancelled.', array('!name' => $recipient->name));
      }
    }
  }

However, as noted in the comment, if the RealName module is enabled, the user's actual username will be revealed instead of the value that has been specified to be shown.

Also, if a user account has been cancelled, I don't think the reply form should be shown in the first place. So I made the following modification in privatemsg_view() in privatemsg.pages.inc:

  // Display the reply form if user is allowed to use it.
  if (privatemsg_user_access('write privatemsg') || privatemsg_user_access('reply only privatemsg')) {
    //the reply form should be hidden if any of the user accounts have been
    //cancelled
    $hide_reply = 0;
    foreach ($thread['participants'] as $key => $participant) {
      if (!$participant->status) {
        $hide_reply = 1;
      }
    }
    if ($hide_reply == 0) {
      $content['reply'] = drupal_get_form('privatemsg_form_reply', $thread);
      $content['reply']['#weight'] = 5;
    }
  }

I'd appreciate any feedback that you can provide on this code.

StatusFileSize
new1.32 KB
FAILED: [[SimpleTest]]: [MySQL] 3,902 pass(es), 95 fail(s), and 103 exception(s).
[ View ]

Attached is a patch that simply gives an error if the user attempts to send a message to a blocked user. The warning does not include the username of the blocked user, because for RealName compatibility I tried format_username($account), but that returns Anonymous if the user account has been cancelled/blocked.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-8.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new751 bytes
FAILED: [[SimpleTest]]: [MySQL] 3,902 pass(es), 95 fail(s), and 103 exception(s).
[ View ]

Updating patch to remove "Sent messages" revision which is a separate issue

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-10.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new701 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

New patch, this time without dpm() (thanks testbot!)

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new623 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-15.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ok, trying again.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-15.patch, failed testing.

StatusFileSize
new653 bytes
PASSED: [[SimpleTest]]: [MySQL] 4,878 pass(es).
[ View ]

Not sure why the last one didn't work. Trying again.

Status:Needs work» Needs review

Marked https://drupal.org/node/2007868 as a duplicate

Added feature request for hiding the reply form as per the second half of #7 here: https://drupal.org/node/2008096

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

+++ b/privatemsg.moduleundefined
@@ -1785,6 +1785,13 @@ function _privatemsg_validate_message(&$message, $form = FALSE) {
+      //do not send private messages to blocked users

Comment should start with a space and then an uppercase character and end with a "."

+++ b/privatemsg.moduleundefined
@@ -1785,6 +1785,13 @@ function _privatemsg_validate_message(&$message, $form = FALSE) {
+        if (!$recipient->status) {

This needs to check the recipient type as well to make sure it's a user.

If you implement this check within privatemsg_privatemsg_block_message(), then it should take care of unsetting the user and it should also make the reply form work as expected *I think*.

And a test would be great to cover this.

For Drupal 6.x-2x-dev:

Line 1780 and below (sorry, no patch):

  if (!empty($message['recipients']) && is_array($message['recipients'])) {
    foreach (module_invoke_all('privatemsg_block_message', $message['author'], $message['recipients'], $message) as $blocked) {
      unset($message['recipients'][$blocked['recipient']]);
      if ($form) {
        drupal_set_message($blocked['message'], 'warning');
      }
      else {
        $messages['warning'][] = $blocked['message'];
      }
    }
    //do not send private messages to blocked users
    foreach ($message['recipients'] as $key => $recipient) {
      if (!$recipient->status) {
        unset($message['recipients'][$key]);
        $msg = t('A message could not be sent because !user has already cancelled his/her account.', array('!user'=>$recipient->name));
        if ($form) {
          drupal_set_message($msg, 'warning');
        }
        else {
          $messages['warning'][] = $msg;
        }
      }
    }
}

StatusFileSize
new825 bytes
FAILED: [[SimpleTest]]: [MySQL] 4,881 pass(es), 0 fail(s), and 22 exception(s).
[ View ]

Attaching a new patch that makes the revisions requested in #21. I will add tests as soon as I can but I haven't yet figured out how to block a user created in the test.

As per Berdir's comment, moving the validation to privatemsg_block_message() causes the reply form to be hidden properly, so I'm closing https://drupal.org/node/2008096 as a duplicate.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-23.patch, failed testing.

Blocking a user should work like this in 7.x: user_save($user, array('status' => 0)). Yes, that's a strange API :)

And good, now you can actually see the notices that I expected already in the previous patch. You just need to switch the conditions, so that it checks that it's a user before it accesses the user-specific status property.

Status:Needs work» Needs review
StatusFileSize
new2.99 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Ok, trying again as per #27 and now with a test.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.99 KB
FAILED: [[SimpleTest]]: [MySQL] 4,888 pass(es), 1 fail(s), and 133 exception(s).
[ View ]

Re-roll of #28

+++ b/privatemsg.testundefined
@@ -242,6 +242,10 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
+    $blocked_recipient = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg')); // set up user with read/write privatemsg permissions

Comments should always be a on a separate line, above the code they explain. Also uppercase first character and "." at the end.

Tests looks good. Would be nice to also have a test for the reply form. You could pick one of the existing messages that were sent, block the author and check that the recipient no longer sees the reply form.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-30.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.89 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 133 exception(s).
[ View ]

Ok, I added another test at the end of the message tests to make sure that the reply form is not shown if the author is blocked. I also updated the comments throughout the test file (there were many comments with the incorrect style).

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-33.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.93 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 132 exception(s).
[ View ]

Re-roll. Remove %recipients from error message because it doesn't seem to display properly with blocked users.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-35.patch, failed testing.

Hm, sorry, that is messier than I remembered it to be ;)

Users don't always have ->type set. Do this instead:

<?php
elseif (isset($recipient->status) && !$recipient->status) {
?>

Also, in the test, you need 'recipient' => $blocked_recipient->name, you're missing the ->name.

Also, when you re-roll, please add a -test-only.patch first, the idea is to show that the new tests to fail without the fix and then the patch with the tests and the fix to make sure that it's green.

Status:Needs work» Needs review
StatusFileSize
new6.88 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]
new6.08 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-39.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.94 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 1 fail(s), and 2 exception(s).
[ View ]

Ok, I think I catch the error message correctly this time. The second message is about all recipients being blocked.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.03 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Still learning how to write tests. Hopefully I got it right this time.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-43.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.99 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 2 exception(s).
[ View ]

Trying @recipients instead of %recipient.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-45.patch, failed testing.

I'm sorry, I don't understand why this is still failing. I created some user accounts manually on my local machine and tested this; the error messages appear properly (i.e., I get "User1 has disabled his or her account." and then "You are not allowed to send this message because all recipients are blocked."

+++ b/privatemsg.testundefined
@@ -112,12 +113,15 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
-    $user_no_write_msg = $this->drupalCreateUser(); // set up user with default permissions (meaning: no read privatemsg permission
+    // Set up a user with the default permissions (no "read privatemsg"
+    // permission).
+    $user_no_write_msg = $this->drupalCreateUser();

It's fine now but be careful with changing too much. It makes patches harder to review and increases the chance of conflicts with other patches, see http://webchick.net/please-stop-eating-baby-kittens

+++ b/privatemsg.testundefined
@@ -271,6 +279,12 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
+      'recipient'   => $blocked_recipient,

Seee #37, you're passing it the user object, but you want the user name, see examples above and below. That will fix the tests.

Status:Needs work» Needs review
StatusFileSize
new6.99 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/privatemsg/privatemsg.test.
[ View ]

Ah, I was changing the wrong line to add -> name. Thanks for all your help! Hopefully this does it...

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-49.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.99 KB
FAILED: [[SimpleTest]]: [MySQL] 4,896 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Fixed PHP error in tests.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-51.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new7.15 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-do-not-send-messages-to-blocked-users-834706-53.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new6.19 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 1 exception(s).
[ View ]

Oops... Put the parentheses in the wrong place. This should be it.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-53.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new6.99 KB
FAILED: [[SimpleTest]]: [MySQL] 4,896 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Attaching again to fix the mis-included file in #53.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-55.patch, failed testing.

Hmmm... I don't understand what that last exception (in #55) has to do with my patch. Any thoughts?

+++ b/privatemsg.testundefined
@@ -324,6 +338,10 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
+    $this->assertText(t('@recipients has disabled his or her account.', array('@recipients' => $editblocked)), 'Message about blocked user displayed.');

This, $editblocked is an array, you can't put that into a placeholder, you again want $blocked_recipient->name here.

Status:Needs work» Needs review
StatusFileSize
new7 KB
PASSED: [[SimpleTest]]: [MySQL] 4,896 pass(es).
[ View ]
new6.2 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Sorry I'm so bad at this. Here it is again.

Don't worry, we're getting there :)

The second part of the test about the reply box doesn't seem to fail in the test-only patch?

StatusFileSize
new6.99 KB
PASSED: [[SimpleTest]]: [MySQL] 4,896 pass(es).
[ View ]
new6.19 KB
FAILED: [[SimpleTest]]: [MySQL] 4,894 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Good point. I tested this manually and it seems that "Reply to thread" is not shown on the page, just "Reply". I have updated the test accordingly. (If this is in fact the solution, perhaps the other tests that check for the string "Reply to thread" should be updated too?)

EDIT: Oops, named the file "-no-tests" when it fact it is "-tests-only".

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-61.patch, failed testing.

Status:Needs work» Needs review

Hmmm... Not sure why it's still not failing. "Reply" is definitely shown alongside the textarea to reply when testing manually.

StatusFileSize
new7.34 KB
FAILED: [[SimpleTest]]: [MySQL] 4,906 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new6.54 KB
FAILED: [[SimpleTest]]: [MySQL] 4,904 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Ok, I reworked the test that wasn't failing properly. A few different changes were necessary, but I think I got it.

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-66.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-66.patch, failed testing.

StatusFileSize
new8.97 KB
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new7.43 KB
FAILED: [[SimpleTest]]: [MySQL] 4,908 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Ok, I re-did the test again and I finally got my local environment doing simpletest properly so it should be correct this time. The problem with the last test was that one of the recipients was unblocked so the reply form was still shown. To simplify things, I created a new message for the next set of tests in order to ensure that no unexpected behavior occurs.

Status:Needs work» Needs review
StatusFileSize
new8.97 KB
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

EDIT: double post please ignore

Status:Needs review» Needs work

Great, thanks for pushing this forward :)

+++ b/privatemsg.pages.incundefined
@@ -393,7 +393,7 @@ function privatemsg_form_reply($form, &$form_state, $thread) {
-      $blocked = t('You can not reply to this conversation because all recipients are blocked.');
+      $blocked = t('You cannot reply to this conversation because all recipients have been blocked.');

That is not the same meaning and IMHO unrelated.

The meaning of this error is that the recipients are blocked *for you*, you are not allowed to write them.

What you are changing it into means that the users were blocked and that's why you can't write them, but there can be many reasons that prevent you from writing them.

So if you want to improve this message, lets open a separate issue to discuss that.

+++ b/privatemsg.testundefined
@@ -76,7 +76,8 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
-    $user_no_read_msg = $this->drupalCreateUser(); // set up user with default permissions (meaning: no read privatemsg permission
+    // Set up a user with the default permissions.
+    $user_no_read_msg = $this->drupalCreateUser();

This will conflict heavily with the permission name issue. I suggest you leave these changes out here as well and fix the comments there.

+++ b/privatemsg.testundefined
@@ -416,16 +444,28 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
+    $this->assertNoText(t('Reply'), 'Reply form is not displayed.');
+    $this->assertText(t('You cannot reply to this conversation because all recipients have been blocked.'), 'All recipients blocked message is displayed.');

No need for an assertion message on the second line ( the second argument), it's almost the same as the first and by default, an assertion message is generated containing the actual text, adding one just makes it harder to find out what we are looking for if the test fails.

The first one is fine, because 'Reply' is quite short and not self-explanatory.

+++ b/privatemsg.testundefined
@@ -416,16 +444,28 @@ class PrivatemsgTestCase extends PrivatemsgBaseTestCase {
-    $enableduser = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg')); // set up user with read/write privatemsg permissions
-    $enableduser2 = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg')); // set up user with read/write privatemsg permissions
-    $disableduser = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg', 'allow disabling privatemsg')); // set up user with read/write privatemsg permissions
+    // Set up a user with "read/write privatemsg" permissions.
+    $enableduser = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg'));
+    // Set up a user with "read/write privatemsg" permissions.
+    $enableduser2 = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg'));
+    // Set up a user with "read/write privatemsg" permissions.
+    $disableduser = $this->drupalCreateUser(array('read privatemsg', 'write privatemsg', 'allow disabling privatemsg'));

Same here, let's not change it here as we have another important issue where we can do this that touches these lines anyway.

StatusFileSize
new5.08 KB
FAILED: [[SimpleTest]]: [MySQL] 4,912 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
new4.28 KB
FAILED: [[SimpleTest]]: [MySQL] 4,908 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Ok, I didn't realize what the first error message meant, so I removed that change. I also removed the changes to the comments that can be done in the other patch. This version should have all the changes discussed in #73.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, privatemsg-do-not-send-messages-to-blocked-users-834706-75.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new5.08 KB
PASSED: [[SimpleTest]]: [MySQL] 4,913 pass(es).
[ View ]
new4.28 KB
FAILED: [[SimpleTest]]: [MySQL] 4,909 pass(es), 4 fail(s), and 0 exception(s).
[ View ]

Wrong recipient name in the test. Fixed now.

Version:7.x-2.x-dev» 6.x-2.x-dev
Status:Needs review» Patch (to be ported)

Thanks for your work on this, committed and pushed to 7.x-1.x and 7.x-2.x.

Thank you for all the feedback. This is a much better solution than the one I came up with originally on my own!

Previous 6.x thread here: https://drupal.org/node/1509436