Disabling private messages for user creates PDO error

I can confirm the same error when a user with 'allow disabling privatemsg' permissions (e.g. admin) tries to disable private messages (repeatedly) for another user that doesn't has this specific permission.

To reproduce the error:

1. Use an account that has the permission to edit other user's accounts and 'allow disabling privatemsg' (e.g. admin user)
2. Go to the user edit page of a user that doesn't has the 'allow disabling privatemsg' permission and uncheck the "Enable private messages" box.
3. After saving the changes, you'll notice that the "Enable private messages" box is still checked, but if you are going to look in the {pm_disable} table, you'll see the change has been registered.
4. Repeat step 2 and you'll get the error.

Attached is a patch for the 7.x-1.x-dev. It's my first one, hopefully it's a good one. Please review it and let me know. Thanks.

It looks like my first patch didn't work out. I used WinMerge to create it :).
The current patch is done using git, hopefully this will make it.

As you can see in the tests, that permission check is necessary. The problem is that if you remove that permission from a role and users currently have privatemsg disabled, they have no way of turning it on again. If that check results in the error you described, we need to find another solution to fix it.

This patch fixes the issue for me.

Hi, I think we need another permission for disabling other users' private message option.

Hi, I think we need another permission for disabling other users' private message option. We also need to modify the permission checking in privatemsg_is_disabled() function.

Sorry, I uploaded the wrong file format in my comment above.

Here is a new patch from me. I add the new checking condition. If the current user is not the same as the edited user, the checkbox for disabling the private message will be only appeared if the current user has permission for disabling other users' private message.

+++ b/privatemsg.moduleundefined
@@ -47,6 +47,10 @@ function privatemsg_permission() {
     ),
+    'allow disabling other users privatemsg' => array(
+      'title' => t('Allow disabling other users private messages'),
+      'description' => t("Allows user to disable privatemsg for other users so that they can't recieve or send any private messages.")

Do we really need a specific permission for this? We could also just use the normal administration permission...

Yes, you're right. We could use administer privatemsg settings for that. Here is the new patch

Hi, this is another patch from me. I move the permission for checking if user allowed to disable other users' private message into a function. This function will be called when the update user form is altered (in privatemsg_form_alter) and when the user gets updated (in privatemsg_user_update).
Sorry, for changing the patch repeatedly in a day :).

+++ b/privatemsg.moduleundefined
@@ -373,7 +373,7 @@ function privatemsg_is_disabled($account) {
-    if ((!empty($account->roles) || $account = user_load($account->uid)) && user_access('allow disabling privatemsg', $account)) {
+    if ((!empty($account->roles) || $account = user_load($account->uid)) && (user_access('allow disabling privatemsg', $account) || user_access('administer privatemsg settings'))) {

To make this consistent, we also need to pass $account to the new user_access() call.

+++ b/privatemsg.moduleundefined
@@ -1355,10 +1358,28 @@ function privatemsg_account_fieldset_remove_if_empty($element) {
+ * Checks if user is allowed to disable/enable private message.

.. if *the* user...

And should have @param and @return documentation, see other non-hook functions for examples.

+++ b/privatemsg.moduleundefined
@@ -1355,10 +1358,28 @@ function privatemsg_account_fieldset_remove_if_empty($element) {
+function privatemsg_is_allow_disable($account) {

I think just privatemsg_allow_disable() is enough, the "is" is not necessary.

+++ b/privatemsg.moduleundefined
@@ -1355,10 +1358,28 @@ function privatemsg_account_fieldset_remove_if_empty($element) {
+  // Get the edited user id.
+  $this_user_id = isset($account->uid) ? $account->uid : 0;

This shouldn't be necessary. Even for an anonymous user (for which this function should not be called, as you can't edit that one), $account->uid is set and is 0.

So this can be simplified.

+++ b/privatemsg.moduleundefined
@@ -373,7 +373,7 @@ function privatemsg_is_disabled($account) {
-    if ((!empty($account->roles) || $account = user_load($account->uid)) && user_access('allow disabling privatemsg', $account)) {
+    if ((!empty($account->roles) || $account = user_load($account->uid)) && (user_access('allow disabling privatemsg', $account) || user_access('administer privatemsg settings'))) {

The first and second user_access call are different matter. The first for is checking if the edited user account has 'allow disabling privatemsg' permission and the second is for checking if current user has 'administer privatemsg settings' permission. So if the edited user account and the current user are different (eg. admin edit the user1 which has no 'administer privatemsg settings' permission) and we pass the $account to user_access, it will give the wrong result.

Right, but it means that the function returns different results depending on the current user. Which is a problem, because it means the admin can change the setting for another user as long as he wants, it will not actually change for the user if we does not have the permission to change it himself as well.

Which means that there are only two ways this can work.

- Do not allow admins to change it if the user would not be able to change it. Probably easier to fix.
- Allow admins to disable it for users even if they can't do it themself, e.g. as a way to ban users without their knowing. This is arguably a new feature and would be easier to implement in 7.x-2.x because we will need to store this as a separate setting.

I understand your point of view. So, for the fastest solution to solve this issue, I choose the first option. The admin can only disable the users' private message if only the users are able to enable it again.
Here is my new patch.

Potentially related issue (Could be dupe): http://drupal.org/node/1087514

A related issue was fixed:
https://drupal.org/node/1605190

Please confirm that this issue is still active; the patches look almost identical to me.

Since there has been no activity for a month, I'm closing.

Issue is still present. Fix #6 works for me. But I don't know what is with tests there.

I fixed privatemsg_is_disabled() instead.
user_access('allow disabling privatemsg', $account) was always executed for the checked account and not for current account.

Please check the patch

- Do not allow admins to change it if the user would not be able to change it. Probably easier to fix.

afaics, this is done by the patch and was the intention of the code so far. Fix solves the PDO error also.