Similarly to #355905: node_delete_multiple() has to use batch API, I discovered that there are no functions for mass-updating comments in a batch yet.
Aside from the fact that an administrator can delete many comments using the node mass-operations on admin/content/comment, the new account cancellation methods also allow administrators to delete all contents of a user. When an administrator is forced to delete a long-time user with all of his content, then this could easily result in thousands of comments having to be deleted.
We have two options to solve this issue:
array('cid' => NULL) can only mean that comments shall be deleted.
Comments
Comment #1
dave reidDuplicate of #152603: Revise comment admin interface like content admin, add hook_comment_operations()
Comment #2
sunNope. That other issue wants to revamp the entire UI.
This bug is critical and we cannot release without it.
Comment #3
moshe weitzman commentedNot sure I understand this. We introduced comment_delete_multiple() a while back. Having said that, we don't let all add-on modules do multiple delete. We do module_invoke_all('comment_delete', $comment); for each comment. Same for nodes.
Comment #4
sunHm. Those *_delete_multiple() functions do not use Batch API. As mentioned in the related issue #355905: node_delete_multiple() has to use batch API: when a user account is cancelled, it is possible that thousands of nodes and comments need to be updated or deleted - depending on the user account cancellation method.
- Since both nodes and comments are fieldable now, and there can be zero to unlimited enabled modules that integrate with them, a single update or delete can take quite some time.
- node_user_cancel() and comment_user_cancel() look fairly similar.
- However, node_mass_update() adds to a batch to prevent a timeout. There is no comment_mass_update(), though hook_comment_update() is invoked during comment_save().
- That said, comment_user_cancel() directly updates comments in the database and is not invoking any other modules in case for an update, which looks totally wrong to me.
Comment #5
sunThis may lead to a max_execution_time WSOD when updating/deleting thousands of contents due to a user cancelling the account, and that's critical, but that doesn't block a release.
Comment #6
catchThere's a few related issues here, some of which are duplicate of #89181: Use queue API for node and comment, user, node multiple deletes but leaving both open for now.
Comment #7
catchMarking duplicate per #6, we only need one issue for this.
Comment #8
sun#705306: user_cancel_delete method calls into a "standard" user_delete_multiple API
Comment #9
sunComment #10
catchStill a duplicate. #89181: Use queue API for node and comment, user, node multiple deletes
Comment #11
catchComment #12
catchActually we do need this for updates, even though deletes are handled in the other issue. #699596: (Un)publishing several comments doesn't work fixes a critical bug in comment module, but means we save 50 comments without a batch operation. That's acceptable, but not ideal, so we should add a proper mass_update with batch support and call that from the admin submit. There may well be other places where we do indeed need to update arbitrary numbers of comments, but I've not looked yet.
Comment #13
marcingy commentedThis is a task not a bug.
Comment #26
andypostI find it duplicate and it now only about user cancellation #3043725: Provide a Entity Handler for user cancelation