Block the author while deleting content

+100 billion. This would make the workflow for managing drupal.org SO much easier... we could allow people in the site maintainer role to delete content AND users without giving them permissions to view user email addresses and such.

This is a re-roll with a couple small re-wording changes (mentions the user's name when asking whether to delete, drupal_set_message()s that they've been banned, checkbox changed to "Also ban name?").

please, please let this get in.

This patch adds in the ability to delete authors when deleting comments as well.

Also a couple changes: check first to see if the author's user ID is > 1, as we can't ban the Anonymous user, and we don't want to ban uid 1. :P

dww rightly pointed out that other places in the UI, "ban" is called "block" and this should be consistent. I personally find the word "ban" a much better word, and more consistent with what other software uses, but that's a separate argument for a separate patch. :)

as you can probably tell, i've already looked at earlier versions of this patch and gave my input. ;) that said, the latest is definitely RTBC:

• code is sane, well commented, small, and safe.
• i tested every possible combination, and the permission does exactly what it's supposed to. everything works as advertised.
• UI is simple, consistent, and clear.

let's commit this to D6. once a core committer confirms this is good, i'd be in favor of a backport to D5 that i could apply to d.o. this would be a huge time saver for the infra team if all the content maintainers could use this to block spam users themselves.

Side note: In IRC #drupal it was suggested that this patch be done as a contrib module rather than backport to D5. However, due to the fact that comment.module uses $_POST['confirm'] to capture the confirm form button press in an if() you can't bind an additional submit handler for the comment delete confirm_form() handler as that side steps the entire fAPI submit process (which means you can't capture the "delete" button being pressed).

It's ok for nodes as node.module uses node_delete_confirm_submit() in the normal sense and it's possible to bind an additional submit handler.

So, in it's current form, to get all this functionality into D5 requires a core patch rather than a contrib. Contrib can provide a node solution just not a comment one also. Sorry about that ;)

OK, for the record I have a solution for #6 above so I now have a module which does this. Contact me via my contact tab for details if needed.

It seems more of drupal.org's needs than a worth feature.. contrib module would be better place that's my opinion.

if it's in a contrib module I'll download it, but it'd be a shame not to have this in core given it's a small amount of code, and so many spammers around ( some bots appear to be programmed to target drupal sites given some forum discussions here), anything which saves a few seconds (although this adds up to hours a year) dealing with that is great.

I agree that this reads more 'contrib' than 'core' even though it would be useful on drupal.org. Since it is perm based though, we can essentially limit who sees this option so I don't have any huge issues with this going into core.

Wouldn't the integration of the actions module, deprecate this patch, or not really?

Not really. Not any more than the "Make node sticky" action deprecates the Sticky checkbox. An action is a way to do something programmatically that can also be done manually.

Plus, what is being talked about here is manual banning of a user. An action that does this would not be helpful without condition support in actions, which is something that is probably Drupal 7ish.

So, +1 for this patch. But. The patch should also do a DELETE from {sessions} WHERE uid = %d IMO; blocked users should find themselves logged out.

@jvandyk: see core's user_save() function:

;)

so, there you have it. Mr. Actions doesn't think this is particularly action-able, everyone wants the patch on d.o, it's so tiny that doing this as contrib seems silly, it could be useful for many sites -- not just d.o, it's been reviewed a bunch, tested, it's secure, etc.

therefore, #4 is RTBC.

thanks,
-derek

Is there a reason for this to be in core? Couldn't a contrib module use hook_form_alter to add an option to the node deletion confirmation and capture the result in hook_init (or wherever) to ban the author, if requested?

If my suggested implementation is too awkward, perhaps we should go forward with certain elements of the Deletion API to allow convenient additions to deletion forms.

Another option for contrib would be developing a sort of vandalism control center that adds a link to the deletion confirmation page to a more complete form offering tools to wipe out the user and his or her posts and comments.

it should be in core because it is a common operation for a community site, which is what we offer at drupal.org. yes it could be done in contrib, but everyone working here wants to improve the experience for people right out of the box.

I reckon this a big usability improvement for social networking sites, hopefully it could still be considered for 6.x
I've updated the patch from #4 to current HEAD

Re-rolled for HEAD

Not all of the patch applies, and none cleanly:

gentoo-vm drupal6 # patch -p0 < block_15_0.patch
patching file modules/comment/comment.module
Hunk #1 succeeded at 1141 (offset 26 lines).
Hunk #2 succeeded at 1163 (offset 26 lines).
patching file modules/node/node.module
Hunk #1 FAILED at 2470.
Hunk #2 FAILED at 2494.
2 out of 2 hunks FAILED -- saving rejects to file modules/node/node.module.rej
patching file modules/user/user.module
Hunk #1 succeeded at 497 (offset 8 lines).

So I re-rolled and tested the new patch, though not thoroughly, and am convinced this still works. So if someone would care to give this a more thorough review... ? Or should this be marked for D7 by now?

DrupalTestbedBot tested Freso's patch (http://drupal.org/files/issues/block_on_delete.patch), the patch passed. For more information visit http://testing.drupal.org/node/127

No longer applies. Much as I love this patch, if it was going to get into D6, it would've happened by now.

Rerolled.

Still applies, great feature, works as intented, let's get this in !

There are some minor code-style issues in the patch (i.e. tabs instead of spaces).

I'm not sure we need a new permission for this. There is already a permission called "Administer users" with the following description: "Manage or block users, and manage their role assignments.". I think we should stick with the existing permission and keep things simple.

Personally, I like to _delete_ spammers rather than blocking them. I guess blocking them would be a useful first step so I'm supportive of this patch.

Here's a revised patch with all the code style issues I could find by visual inspection fixed.

It appears that user_perm() has style issues in CVS, and I've updated this patch to fix them as well as add the "block users" permission item.

I believe that we should add a new permission for "block users" so that content administrators can have the ability to block spammers when they delete content without also needing to be full site admins (or capable of taking such control). You don't necessarily want content administrators to have full user administration capabilities. Compared to the capabilities provided by "administer users", the capabilities provided by "block users" are easy to reverse if they're abused, and there's no ability to escalate your privileges with it.

I have extended every case of user_access('block users') to accept the broader "administer users" permission.

Tiny update to restore the array initialization strangely removed in the patch.

I second David's argumentation why we should add a new permission for this. See also webchicks first comment on this - that's an obvious use case.

The last submitted patch failed testing.

Chasing HEAD and requesting testbot review.

I agree with the extra permission for the same reason we separated edit and delete permissions for nodes - blocking is non-destructive and easily reversed so there are plenty of use cases for giving it separately. Visually the code looks fine, so RTBC.

I would highly suggest to rethink the implementation of this patch after #8: Let users cancel their accounts has landed. Slight changes to this patch would allow moderators/administrators to cancel the user account (i.e. not only block the user) by invoking the new cancel account process. Having all cancel account options would be extremely helpful in cases where you encounter malicious posts or spam, i.e. when you know that you could also simply and immediately delete the user and all his contents.

Also, this patch does not allow to skip the notification mail that is automatically sent by user_save() upon blocking an account (if the notification is enabled on admin/user/settings), which is also covered in the new cancel account process.

Setting to CNW now that #8: Let users cancel their accounts has landed.