admin/content/comment/list/spam page broken

I've reviewed and tested the patch. It looks correct and works as expected.

When you say "completely broken", what's the behaviour? I have a similar problem, not sure if it's the same one though

Ok, still not sure if this is exactly the same, but here goes:

For me, admin/content/comment contains no "spam" tab what-so-ever.

admin/content/comment/list/spam gives "access denied" for all users, including user 1.

I applied the patch to the current 6.x-1.x-dev module (2009-02-13). patch applied correctly (I manually checked all the changes). Still no joy. Nothing changed on my site, admin/content/comment/list/spam is still completely inaccessible.

Obviously this makes it difficult to administer spam.

[[EDIT]]: patch did work. make sure to run update.php after patching

This patch did make the "spam" page appear, but it was a mess. Guessing the form constructor isn't getting called properly.

Partial commit -- some parts seemed unnecessary. To note here, for discussion:

1) the spam comments admin menu callback -- I don't see why it would need to load the comment admin include at all. It would seem to be independent, wouldn't it?

2) I went ahead and included the theme_spam_comment_admin_overview patch, because calling theme() is probably the "better" way to do it, though I doubt it makes much of a difference to this particular issue.

3) the patch to spam.module was also not included. Yes, that page should be themable. But the point of the spam_comments module is to be modular. If we want the submodules to be included in hook_theme (a good idea), we should come up with a way to do that through the submodule's spamapi call (or something like that) instead of hard-coding it into the root module. Since we're in dev for now, we can get away for a while with it looking unstyled until we have a proper solution.

(I should note -- I did try applying #3 in the hopes that it would make the spam page look like less of a mess. It didn't. Basically, the fields aren't getting put into tables for some reason. The patch to spam.module didn't seem to make a difference.)

If I missed some good reasons for including these, please let me know. (Preferably rerolling a patch against the current dev would be ideal.)

Will take a look at why we're not getting a table out of the spam page and possibly open a new issue. Wanted to get this one in, even if only partially, since it'd been so long and it does fix a few issues other than this one. Thank you for the patch.

Ok, so the answer to my #1 is because we're reusing submit functions and such from the comment module. Only problem is, those submit functions aren't working. So whether we need them or we just need to rewrite is, I suppose, still an open issue.

If anybody's up to it, a patch for the page functionality against current dev would be nice.

(I'll create a separate issue for #3 on my list. #2 can pretty much be ignored.)

Separate issue for #2/3 is at #494502: Let spam modules use spam_theme, and committed to CVS.

This patch did make the "spam" page appear, but it was a mess
What do you mean by a mess? Everything looks fine in mine. I've diffed my module and it's a bit different, but it's a bit of a mongrel, and I'm not sure what specific difference is relevant.

"A mess" refered to both the fact that no actions from the drop-down actually work, as well as that the theming is hard-coded into spam.module -- which it shouldn't be, but it looks ugly without it. The corrected fix for the latter has already been committed to CVS, as I mentioned in #7. The former is what I'm currently working on.

ok, well what ever patches I applied previously, those links work for me now. Would you like a diff against the current dev version?

The links are working fine. It's the mass-edit form submits that aren't.

And if the diff you want to run is against the patched version that's in #376266: Why almost all recent issues (patches) were neglected?!, which you indicated you were using, no, thank you. Like I mentioned over there, I already identified numerous problems with the "all-in-one" patch, addressed them in their original issue lists, and committed fixes. I'm not looking to regress other bugs just to fix this one.

If you have a solution to this *specific* problem as a patch to the *current* dev version, I'm all ears. But I already presented the case for why we're not doing kitchen-sink patches by various different developers and are instead dealing with individual issues, and the fact that this particular all-in-one patch had some clear bugs in it is only one of those reasons.

I've installed the latest 6.x-1.x-dev version (2009-Jun-18), and the link: /admin/content/comment/list/spam definitely doesn't work fine.

I am getting the following error:
warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, 'theme_comment_admin_overview' was given in .../includes/theme.inc on line 617.

That is correct. You'll note that the page still looks the same as it did right after I took out the inappropriate spam_theme edit out of the patch, and created a new issue to do it properly with an API callback. When I submitted the API callback patch and closed the issue, the theme could then run -- but the theming itself has an unnecessary dependency on a comment module include file, which was previously not noticeable and which is the result for the warning you're getting now. It's just a warning. You'll be fine. :-) This page was last on the priority list, since it duplicates functionality you can get out of the admin->content->spam menu and is mainly a convenience. It is what I'm working on now. (I thought about temporarily just redirecting content->comments->spam to content->spam while I worked on it, but it wasn't worth the effort when this page will only be like this for about a day.)

Theme and content edit callbacks are now properly usable across all content types; mass edit comment pages are finally hooked into spam.module correctly. The dependency on the comment admin stuff remains in spam_comment, but is completely out of spam.module. The .incs are selectively loaded based on enabled modules anyway, so that might be fine.

Rock on. Thanks for your hard work gnassar!