The following log is confusing:
node_age_spam_filter, retrieved nid () from database

This message should only be displayed if a nid was actually retrieved from the database.

CommentFileSizeAuthor
#4 spam-540830-r1.patch2.23 KBgnassar
#3 spam-540830-r0.patch1.89 KBgnassar

Comments

jeremy’s picture

jeremy commented
Status: Active » Fixed

Fixed:
http://drupal.org/cvs?commit=247178

gnassar’s picture

gnassar commented

Seems like by the code gets to that point, there should never be a case where a nid cannot be recovered.

Most likely case that would happen is where the content type isn't a comment -- so shouldn't that be checked for first?

The fix I have (and have been testing on my site) does that, as well as more accurately describes the errors at each if, upping severity to SPAM_VERBOSE if something that "shouldn't happen" happens. I'll re-roll that against the current changed HEAD and post in just a minute so you can take a look and see what you think.

gnassar’s picture

gnassar commented
StatusFileSize
new spam-540830-r0.patch1.89 KB
gnassar’s picture

gnassar commented
StatusFileSize
new spam-540830-r1.patch2.23 KB

OK, that's what I get for doing stuff so late. That was neither an unified diff nor was it against HEAD. Fixed.

jeremy’s picture

jeremy commented
Status: Fixed » Reviewed & tested by the community

Yes, your fix is _much_ better than my fix. I've applied it locally and tested, it works as designed.

gnassar’s picture

gnassar commented

Great. I'll commit as soon as I get comments on #532158: Consistent logging across modules to make the patch compliant with the proposed logging settings there.

gnassar’s picture

gnassar commented
Status: Reviewed & tested by the community » Fixed

Committed: http://drupal.org/cvs?commit=247568

Status: Fixed » Closed (fixed)

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