node, comment, but not user

According to the API, this couldn't have worked since Drupal 4.7. Thanks for the catch!

A small part of the problem was a namespacing change we made earlier that we seem to have gotten overzealous on; took the opportunity to also fix the namespacing in the other content modules.

Patch enclosed.

Shouldn't the spam_nodeapi() renaming be spam_node() instead?

-function node_nodeapi(&$node, $op) {
+function spam_nodeapi(&$node, $op) {

> Shouldn't the spam_nodeapi() renaming be spam_node() instead?

Why? This is intended to invoke hook_nodeapi().

> +        // Spam/not spam status is displayed in $links -- is this necessary?
> +        //$status = db_result(db_query('SELECT status FROM {users} WHERE uid = %d', $user->uid));
> +        //$status_text = t('User status: %status', array('%status' => $status ? t('not blocked') : t('blocked')));

Agreed, this should be removed.

I applied the patch, then tried marking a user as spam -- it fails, and generates the following error:

warning: Invalid argument supplied for foreach() in /var/www/spam6/sites/all/modules/spam/filters/spam_filter_bayesian/spam_filter_bayesian.module on line 115.

This is what shows up in the spam logs:

_spam_log_trace	--
spam_mark_as_spam	marked as spam, score(99)
spam_filter_bayesian...	mark_as_spam
spam_filter_bayesian...	insert token(spammer) class(spam) probability(99)
spam_filter_bayesian...	insert token(spammer@localhost) class(spam) probability(99)
spam_filter_url_spam...	mark_as_spam
spam_unpublish	unpublished
spam_content_update	updating
spam_content_filter	bypassing filters

The user does get blocked, but does not show up as being a spam user at this point.

> > Shouldn't the spam_nodeapi() renaming be spam_node() instead?

> Why? This is intended to invoke hook_nodeapi().

I insist... 8-)

-function comment_comment(&$comment, $op) {
+function spam_comment(&$comment, $op) {
-function node_nodeapi(&$node, $op) {
+function spam_nodeapi(&$node, $op) {
-function spam_content_user($type, &$edit, &$user, $category = NULL) {
+function spam_user($type, &$edit, &$user, $category = NULL) {

But maybe you can explain this pattern better to me... 8-)

If the node content implements the spam_nodeapi() call, then it could not be a separate module which we talked about doing at some point...

Thank you.
Alexis

The user module invokes hook_user. The comment module invokes hook_comment. The node module invokes hook_nodeapi.

This is not up to the spam module, these are core Drupal hooks.

Yes. Sorry... I was thinking of the hook_spamapi() call... 8-}

So changing back to needs review for now... although Jeremy has a complain in regard to marking a user as a spammer.

Would the Bayesian error be in link with something I changed there? Or you did not test with the latest greatest version?

> Would the Bayesian error be in link with something I changed there?

Indeed, clearly this error is generated because $tokens is being passed in not as an array. Until that's solved (or explained as unrelated to this change) and clicking the 'mark as spam' link actually marks the user as spam, this "needs work".

That's a good thing, right? That we got an error back for $tokens being passed as a non-array, instead of just having it be silently empty?

I'll try to track that down.

As far as the "not marked as spam" problem: what do you mean by that? It seems from the spam user module code that blocking the user *is* marking him as spam, from the filter's perspective. There's no further marking performed. It throws the tokens in Bayesian and/or performs any actions it needs to on spam marking per-filter, and then it blocks the user. Like comments and "unpublish," there's really nothing else.

gnassar,

I suppose we could add a delayed blocking mechanism. So after 3 spam messages, you're blocked instead of blocking on the first one. That would mean adding a table for the users "module". That's certainly a feature we could think of later. Personally, I don't have the problem though. It's mainly anonymous users who spam my sites.

Thank you.
Alexis

The $tokens passing change is unrelated to this change. This change only adds the necessary UI element to fire the user content stuff, as per the bug reported. (On a side note, I do not get that error.)

Similarly, the "mark as spam" link displays, which is all that the patch addresses -- the actual content and function of it is standardized in spam.module.

The UI is clearly fixed by this patch, as was originally intended. It simply turns out that the entire user content module has never really worked, and we've never really known it.

Creating a new bug ticket with the functionality issues. We're starting to have a habit of finding really big problems and trying to deal with them all in one painfully long bug report -- I'm not convinced that's the most productive way to go about things.

----

As far as this issue goes: users can theoretically re-enable a user without going through spam. Since the "spam" or "not spam" label is tied to the item's probability here, and not to its blocked/unblocked status directly, it's probably useful to display the blocked status by the spam link. (Yes, I'm disagreeing with what I said earlier.) :)

Submitting a new patch with that block status text included.

> On a side note, I do not get that error.

I just tested again, and I'm still getting the error. Indeed, marking a user as spam a second time does indeed mark his as spam. After that 'mark as spam' and 'mark as not spam' work as expected, though I see the error each time. I'm using the latest version of the spam module code checked out from CVS (cvs update -dP). The only thing I've not (yet) tried is dropping my database and using a fresh installation, but I'm not aware of any relevant database updates.

Screenshot attached.

The steps I used to get here:
1) as uid 1 I went to admin/user/user/create and created a test user
2) I clicked on the new user to view its profile
3) I click 'mark as spam' on the user's profile

I repeated these steps to create a new user, and saw the same error.

In any case, it's a big step forward, you're welcome to commit this and track the remaining issues through new bugs if you like.

The screenshot:

Jeremy,

Do you have a better than the default filter to be able to show images inline?

I wanted to do that in the documentation if possible... with this one: http://drupal.org/files/issues/is-spam-probability.png

Thank you.
Alexis

Committed.

http://drupal.org/cvs?commit=472100