Jump to:

1016 Posts in 567 Topics by 309 members

Forum Module

SilverStripe Forums » Forum Module » ForumRole::NumPosts() includes spam posts

Discuss the Forum Module.

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

Page: 1
Go to End
Author Topic: 420 Views
  • johnmblack
    Avatar
    Community Member
    61 Posts

    ForumRole::NumPosts() includes spam posts Link to this post

    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.

  • Willr
    Avatar
    Forum Moderator
    5462 Posts

    Re: ForumRole::NumPosts() includes spam posts Link to this post

    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)

    Thanks!

  • johnmblack
    Avatar
    Community Member
    61 Posts

    Re: ForumRole::NumPosts() includes spam posts Link to this post

    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".

  • johnmblack
    Avatar
    Community Member
    61 Posts

    Re: ForumRole::NumPosts() includes spam posts Link to this post

    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?

    420 Views
Page: 1
Go to Top

Want to know more about the company that brought you SilverStripe? Then check out SilverStripe.com

Comments on this website? Please give feedback.