Block the author while deleting content

chx - May 22, 2007 - 21:08
Project:Drupal
Version:7.x-dev
Component:node system
Category:feature request
Priority:normal
Assigned:chx
Status:patch (code needs work)
Description

When we find a spam post, a node admin can delete it but needs a user admin to ban the user. And it takes too many clicks. Esp for book pages. This here adds a checkbox to the node delete form -- one click block.

AttachmentSize
node_ban.patch2.03 KB

#1

webchick - May 22, 2007 - 22:11

+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?").

AttachmentSize
node_ban_0.patch2.49 KB

#2

catch - May 22, 2007 - 22:14

please, please let this get in.

#3

webchick - May 22, 2007 - 22:32
Title:Ban the author while deleting a node» Ban the author while deleting content

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

AttachmentSize
ban.patch4.07 KB

#4

webchick - May 22, 2007 - 22:44

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. :)

AttachmentSize
block_13.patch4.09 KB

#5

dww - May 22, 2007 - 22:59
Status:patch (code needs review)» patch (reviewed & tested by the community)

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.

#6

AjK - May 23, 2007 - 08:46

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 ;)

#7

AjK - May 23, 2007 - 13:46

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.

#8

Gurpartap Singh - May 23, 2007 - 15:27

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

#9

catch - May 23, 2007 - 16:36

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.

#10

RobRoy - May 23, 2007 - 21:26

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.

#11

Dries - May 24, 2007 - 05:32

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

#12

jvandyk - May 25, 2007 - 13:24

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.

#13

dww - May 25, 2007 - 17:23

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

<?php
   
// Delete a blocked user's sessions to kick them if they are online.
   
if (isset($array['status']) && $array['status'] == 0) {
     
sess_destroy_uid($account->uid);
    }
?>

;)

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

#14

David Strauss - May 27, 2007 - 23:50

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?

#15

David Strauss - May 27, 2007 - 23:58

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.

#16

moshe weitzman - June 14, 2007 - 16:20

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.

#17

asimmonds - July 3, 2007 - 00:39
Status:patch (reviewed & tested by the community)» patch (code needs review)

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

AttachmentSize
block_14_0.patch4.13 KB

#18

asimmonds - July 5, 2007 - 22:55

Re-rolled for HEAD

AttachmentSize
block_15_0.patch3.71 KB

#19

Freso - October 11, 2007 - 12:30
Title:Ban the author while deleting content» Block the author while deleting content

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?

AttachmentSize
block_on_delete.patch3.75 KB

#20

System Message - October 11, 2007 - 17:22

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

#21

catch - October 29, 2007 - 12:55
Version:6.x-dev» 7.x-dev
Status:patch (code needs review)» patch (code needs work)

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

#22

Jody Lynn - October 18, 2008 - 03:19
Status:patch (code needs work)» patch (code needs review)

Rerolled.

AttachmentSize
block-users.patch3.65 KB
Testbed results
block-users.patchpassedPassed: 7204 passes, 0 fails, 0 exceptions a href=http://testing.drupal.org/pifr/file/1/block-users.patchDetailed results/a

#23

swentel - November 11, 2008 - 13:47
Status:patch (code needs review)» patch (reviewed & tested by the community)

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

#24

Dries - November 15, 2008 - 11:57
Status:patch (reviewed & tested by the community)» patch (code needs work)

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.

 
 

Drupal is a registered trademark of Dries Buytaert.