Skip to main content

This site requires you to update your browser. Your browsing experience maybe affected by not having the most up to date version.

Forum Module

Discuss the Forum Module.

Moderators: martimiz, Sean, biapar, Willr, Ingo, swaiba, simon_w

ForumRole::NumPosts() includes spam posts


Reply

4 Posts   537 Views

Avatar
johnmblack

2 November 2011 at 7:00am Community Member, 61 Posts

ForumRole::NumPosts() looks like:

function NumPosts() {
   if(is_numeric($this->owner->ID)) {
      return (int)DB::query("SELECT count(*) FROM \"Post\" WHERE \"AuthorID\" = '" . $this->owner->ID . "'")->value();
   } else {
      return 0;
   }
}

But this returns not just *approved* posts -- it will also include any spam that has not yet been reviewed (and spam that was rejected.) A spammer can therefore rack up X number of not-yet-approved posts, and bypass any logic that anyone writes against NumPosts.

I just added the following into my own member extension:

   function NumApprovedPosts()
   {
      if(is_numeric($this->owner->ID)) {
         return (int)DB::query("SELECT count(*) FROM \"Post\" WHERE \"AuthorID\" = '" . $this->owner->ID . "' AND \"Status\" != 'Awaiting' AND \"Status\" != 'Rejected'")->value();
      } else {
         return 0;
      }
   }

Does that look right? (Is there a better way to do this?) I'd prefer negative awaiting/rejected instead of affirmative on the others, since it is more likely that status might be added in the future that are more related to valid posts than invalid.

Avatar
Willr

4 November 2011 at 6:45pm Forum Moderator, 5511 Posts

Good catch! But wouldn't it be better to only count the moderated posts rather than not awaiting and not rejected?

return (int) DB::query("
SELECT COUNT(*) FROM \"Post\"
WHERE \"AuthorID\" = '" . $this->owner->ID . "' AND \"Status\" = 'Moderated'")->value();

If you submit the fix to github I'll add that in. If you want to earn extra points, add a unit test showing that it works (doesn't count awaiting posts) :D

Thanks!

Avatar
johnmblack

4 November 2011 at 6:55pm Community Member, 61 Posts

I considered that -- there are currently 4 statuses on the Post class:

"Status" => "Enum('Awaiting, Moderated, Rejected, Archived', 'Moderated')",

I don't know when Archived comes into play but it seems like it would be another type of "valid" post.

It would take two conditions one way or another -- either we say "not awaiting or rejected", or we say "yes archived or moderated". Now consider that as people extend the forum module (or enhance its core) there may be additional statuses, and this has the potential to be a "gotcha" in that regard. The question is -- do we think it is more likely for additional statuses to be of the Valid kind, or of the Invalid kind? If we think it's more likely for extended statuses to be Valid ones, then it is safer to code against the "not awaiting or rejected".

Avatar
johnmblack

15 November 2011 at 5:36am Community Member, 61 Posts

Hi Will, I can't get GitHub working right now.... spent all day on it last Friday and it's not cooperating with something on my system (all my site dev is on a Win7 laptop.) Can't spend any more time on it at this point. Should I open a ticket on this instead?