multi-comment delete broken

comment to get me subscribed

Busy investigating...

Lots of fapi 2 clean up also required in comment.module

Found the problem. The comments admin page is a callback to drupal_get_form() which takes 'comment_admin_overview' as an argument. In comment_admin_overview, when the operation is delete, comment_multiple_delete_confirm() returns a form array. The thing here is, the form is not rendered as the array is not passed to drupal_get_form().

Patch changes things by:
1. Setting the menu callback to comment_admin
2. Comment_admin will determine which form to render, comment_multiple_delete_confirm or comment_admin_overview
3. Comment_admin_overview is now a function that only builds the comment listing array

Please review.

Ah, confirm delete form was working in the previous patch but deleting still never took place. $edit in comment_multiple_delete_confirm_submit was not defined, this patch changed it to $form_values.

Btw, to get the patch working properly, you need to rebuild your menu.

Patch applied clean.

Hmm, this just gives me

warning: Missing argument 2 for comment_admin_overview() in /usr/home/ajk/WS/DRUPAL/DRUPAL-5-0-0-RCx1/drupal/modules/comment/comment.module on line 1013.

and still no multi-comment delete. I tried something similar along these lines (as used in user.module for multi-user delete which does work) and still it refused to budge.

Did you try resetting your menu? Just browse to the menus admin page.

Stunning, lightning storm just took out all the power for a minute (so much for the UPS, lasted less than 45secs, need a bigger one).

When I came back to this (obviously after all my machines rebooted) bingo, works a treat! I guess Mother Nature cleared the cache for me (and scared the life out of my 3yrs old who didn't like the sudden dark).

I'd say RTBC but I'm sure Flk will come along and double check it. Thx for the patch

regards,
--AjK

Patch was rerolled from Drupal root.

Using a 'comment/delete/cidhere' won't work as we are deleting multi comments. But I agree that using a specific menu callback like 'comment/delete' would be less confusing to the users and we can avoid using $_POST. That is beyond the scope of this patch IMO, and should be left for a separate task/feature report. Maybe then we can also revamp the deleting multiple users workflow to follow suit?

This patch follows the original workflow of deleting comments as close as possible to avoid changing a lot of codes. I'd say it fixes the bug as reported.

I'd tend to agree that this patch resolves this issue.

For the record, applies clean and does what it says on the tin, fixes "broken multi-comment delete".

regards,
--AjK

Committed to CVS HEAD. Thanks.