I have an add-on module that allows the admin to manage the tokens for the Spam module. Should I contribute it as a completely separate module or have it included as part of the base distribution?

Comments

jeremy’s picture

Status: Active » Needs work

I finally extracted your zip and installed it locally.

General feedback, without actually looking at the code:

  • without being able to sort the various columns, the module is basically unusable for any real installation. As is, the tokens are always formatted by tid, meaning they are formatted in the order the tokens are added to the database. With over 50,000 tokens in my KernelTrap.org spam_tokens table, I'd never be able to find anything.
  • for that reason, it would be nice to add a filter allowing you to search for a token starting with or ending with specified characters
  • the "mark as" column is an automatically generated value, it should not be manually edited. If the probability of a token is > then the threshold, it is then used to mark content as spam. If the probability of a token is < then the threshold, it is used to mark content as not spam. Editing this value only makes it possible to break the logic used by the module. It is a very bad idea.
  • I also see no need to edit the "last seen column". I think it's only useful to be able to edit a token, delete a token, and edit its probability.
  • Your UI is nonstandard. I expect to find an "operations" column with and "edit" field. It actually took me a while to realize I was expected to click on the name of the token to edit it.

If you continue working on this, please just attach the files uncompressed. It is a tiny module and won't hurt anything. If it's a zip, well, you saw how quickly I respond to them.

nancydru’s picture

These comments are why I sent it to you.

1) It's in order by the token name on my site. Maybe I changed that after I sent it to you.
2) I'll see if I can do this.
3) I can remove the "mark as" from the edit screen. Should it remain on the add screen? I have used this to pro-actively add tokens before they were encountered.
4) I will change it to a comment line. I like to see it. But it will default to the time a token is added in the add logic.
5) Okay.

jeremy’s picture

"2) I'll see if I can do this."

It's worth the effort, as it makes the module much more usable. Sifting through 50,000+ tokens is basically impossible otherwise, no matter how they are ordered.

"3) I can remove the "mark as" from the edit screen. Should it remain on the add screen? I have used this to pro-actively add tokens before they were encountered."

It should be auto-calculated. If you want a token to be marked as spam, set the probability to 80 or greater. If you want the token to be marked as not spam, set it to a probability less than 80. Your _submit action should then check the probability and set the "mark as" field automatically. The "mark as" field is only in that table as a search optimization, nothing more.

"4) I will change it to a comment line. I like to see it. But it will default to the time a token is added in the add logic."

Sure, providing the information is certainly useful. You can also provide the "mark as" field as information too, perhaps along with an explanation. "This token has a probability of 85, so its occurrence in submitted content makes it more likely to be automatically marked as spam" or something like that.

jeremy’s picture

"It should be auto-calculated. If you want a token to be marked as spam, set the probability to 80 or greater. If you want the token to be marked as not spam, set it to a probability less than 80. Your _submit action should then check the probability and set the "mark as" field automatically. The "mark as" field is only in that table as a search optimization, nothing more."

Sorry, bad information.

The spam_tokens table has the following columns: tid, token, spam, notspam, probability, last.

The spam field is the number of times this token has shown up in spam content. The notspam field is the number of times this token has show up in not-spam content. The probability is then calculated from these two fields. ie, if spam=5 and notspam=0, then probability = 99 (there is no 100). if spam=5 and notspam=5, then probability = 50.

Your code is incorrect as it specifies that a token can't have a value in both spam and notspam -- but obviously not only can it but in real world usages it will more often than not.

nancydru’s picture

Okay, disregard what I just sent you...

nancydru’s picture

Assigned: Unassigned » nancydru
Status: Needs work » Fixed
Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.