Skip to content

Commit 2068a95

Browse files
dhensbydhensby
committed
Merge pull request #145 from camfindlay/forumstatistics
FIX Forum statistics exclude spammers
2 parents 2fc1764 + 95a7707 commit 2068a95

File tree

7 files changed

+157
-69
lines changed

7 files changed

+157
-69
lines changed

‎code/model/ForumThread.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,13 @@ function getFirstPost() {
133133
* @return int
134134
*/
135135
function getNumPosts() {
136-
return (int)DB::query("SELECT count(*) FROM \"Post\" WHERE \"ThreadID\" = $this->ID")->value();
136+
$sqlQuery = new SQLQuery();
137+
$sqlQuery->setFrom('"Post"');
138+
$sqlQuery->setSelect('COUNT("Post"."ID")');
139+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
140+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
141+
$sqlQuery->addWhere('"ThreadID" = ' . $this->ID);
142+
return $sqlQuery->execute()->value();
137143
}
138144

139145
/**

‎code/pagetypes/Forum.php‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,25 +357,44 @@ function getLatestPost() {
357357
* @return int Returns the number of topics (threads)
358358
*/
359359
function getNumTopics() {
360-
return DB::query(sprintf('SELECT COUNT("ID") FROM "ForumThread" WHERE "ForumID" = \'%s\'', $this->ID))->value();
360+
$sqlQuery = new SQLQuery();
361+
$sqlQuery->setFrom('"Post"');
362+
$sqlQuery->setSelect('COUNT(DISTINCT("ThreadID"))');
363+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
364+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
365+
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
366+
return $sqlQuery->execute()->value();
361367
}
362368

363369
/**
364370
* Get the number of total posts
365371
*
366-
* @return int
372+
* @return int Returns the number of posts
367373
*/
368374
function getNumPosts() {
369-
return DB::query(sprintf('SELECT COUNT("ID") FROM "Post" WHERE "ForumID" = \'%s\'', $this->ID))->value();
375+
$sqlQuery = new SQLQuery();
376+
$sqlQuery->setFrom('"Post"');
377+
$sqlQuery->setSelect('COUNT("Post"."ID")');
378+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
379+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
380+
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
381+
return $sqlQuery->execute()->value();
370382
}
371383

384+
372385
/**
373386
* Get the number of distinct Authors
374387
*
375388
* @return int
376389
*/
377390
function getNumAuthors() {
378-
return DB::query(sprintf('SELECT COUNT(DISTINCT "AuthorID") FROM "Post" WHERE "ForumID" = \'%s\'', $this->ID))->value();
391+
$sqlQuery = new SQLQuery();
392+
$sqlQuery->setFrom('"Post"');
393+
$sqlQuery->setSelect('COUNT(DISTINCT("AuthorID"))');
394+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
395+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
396+
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
397+
return $sqlQuery->execute()->value();
379398
}
380399

381400
/**

‎code/pagetypes/ForumHolder.php‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -222,12 +222,14 @@ public function Breadcrumbs($maxDepth = 20, $unlinked = false, $stopAtPageType =
222222
* @return int Returns the number of posts
223223
*/
224224
public function getNumPosts() {
225-
return Post::get()
226-
->innerJoin(ForumHolder::baseForumTable(),"\"Post\".\"ForumID\" = \"ForumPage\".\"ID\"" , "ForumPage")
227-
->filter(array(
228-
"ForumPage.ParentID" => $this->ID
229-
))
230-
->count();
225+
$sqlQuery = new SQLQuery();
226+
$sqlQuery->setFrom('"Post"');
227+
$sqlQuery->setSelect('COUNT("Post"."ID")');
228+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
229+
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
230+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
231+
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
232+
return $sqlQuery->execute()->value();
231233
}
232234

233235

@@ -237,12 +239,14 @@ public function getNumPosts() {
237239
* @return int Returns the number of topics (threads)
238240
*/
239241
function getNumTopics() {
240-
return ForumThread::get()
241-
->innerJoin(ForumHolder::baseForumTable(),"\"ForumThread\".\"ForumID\" = \"ForumPage\".\"ID\"","ForumPage")
242-
->filter(array(
243-
"ForumPage.ParentID" => $this->ID
244-
))
245-
->count();
242+
$sqlQuery = new SQLQuery();
243+
$sqlQuery->setFrom('"Post"');
244+
$sqlQuery->setSelect('COUNT(DISTINCT("ThreadID"))');
245+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
246+
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
247+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
248+
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
249+
return $sqlQuery->execute()->value();
246250
}
247251

248252

@@ -252,11 +256,14 @@ function getNumTopics() {
252256
* @return int Returns the number of distinct authors
253257
*/
254258
public function getNumAuthors() {
255-
return DB::query("
256-
SELECT COUNT(DISTINCT \"Post\".\"AuthorID\")
257-
FROM \"Post\"
258-
JOIN \"" . ForumHolder::baseForumTable() . "\" AS \"ForumPage\" ON \"Post\".\"ForumID\"=\"ForumPage\".\"ID\"
259-
AND \"ForumPage\".\"ParentID\" = '" . $this->ID . "'")->value();
259+
$sqlQuery = new SQLQuery();
260+
$sqlQuery->setFrom('"Post"');
261+
$sqlQuery->setSelect('COUNT(DISTINCT("AuthorID"))');
262+
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
263+
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
264+
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
265+
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
266+
return $sqlQuery->execute()->value();
260267
}
261268

262269
/**

‎templates/Includes/ForumHeader.ss‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<div class="forum-header">
22

3-
<% loop $ForumHolder %>
3+
44
<div class="forum-header-forms">
55

66
<span class="forum-search-dropdown-icon"></span>
@@ -49,7 +49,7 @@
4949
<% end_if %>
5050

5151
</div><!-- forum-header-forms. -->
52-
<% end_loop %>
52+
5353

5454
<h1 class="forum-heading"><a name='Header'>$HolderSubtitle</a></h1>
5555
<p class="forum-breadcrumbs">$Breadcrumbs</p>

‎tests/ForumHolderTest.php‎

Lines changed: 67 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* @todo Write tests to cover the RSS feeds
55
*/
66
class ForumHolderTest extends FunctionalTest {
7-
7+
88
static $fixture_file = "forum/tests/ForumTest.yml";
99

1010
public function setUp() {
@@ -20,7 +20,7 @@ public function setUp() {
2020
*
2121
* @return unknown_type
2222
*/
23-
function testGetForums() {
23+
public function testGetForums() {
2424
$fh = $this->objFromFixture("ForumHolder", "fh");
2525
$fh_controller = new ForumHolder_Controller($fh);
2626

@@ -34,64 +34,91 @@ function testGetForums() {
3434
// expects none.
3535
$this->assertTrue($fh_controller->Categories()->First()->Forums()->Count() == 0, "fh first category has 0 forums");
3636
$this->assertTrue($fh_controller->Categories()->Last()->Forums()->Count() == 2, "fh second category has 2 forums");
37-
37+
3838
// Test ForumHolder::Categories() on 'fh2', from which we expect 2 categories
3939
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
4040
$fh2_controller = new ForumHolder_Controller($fh2);
4141
$this->assertTrue($fh2_controller->Categories()->Count() == 2, "fh first forum has two categories");
42-
42+
4343
// Test what we got back from the two categories. Each expects 1.
4444
$this->assertTrue($fh2_controller->Categories()->First()->Forums()->Count() == 1, "fh first category has 1 forums");
4545
$this->assertTrue($fh2_controller->Categories()->Last()->Forums()->Count() == 1, "fh second category has 1 forums");
46-
47-
46+
47+
4848
// plain forums (not nested in categories)
4949
$forumHolder = $this->objFromFixture("ForumHolder", "fhNoCategories");
50-
50+
5151
$this->assertEquals($forumHolder->Forums()->Count(), 1);
5252
$this->assertEquals($forumHolder->Forums()->First()->Title, "Forum Without Category");
53-
53+
5454
// plain forums with nested in categories enabled (but shouldn't effect it)
5555
$forumHolder = $this->objFromFixture("ForumHolder", "fhNoCategories");
5656
$forumHolder->ShowInCategories = true;
5757
$forumHolder->write();
58-
58+
5959
$this->assertEquals($forumHolder->Forums()->Count(), 1);
6060
$this->assertEquals($forumHolder->Forums()->First()->Title, "Forum Without Category");
6161

6262
}
6363

64-
function testGetNumPosts() {
64+
public function testGetNumPosts() {
6565
// test holder with posts
6666
$fh = $this->objFromFixture("ForumHolder", "fh");
67-
$this->assertEquals($fh->getNumPosts(), 24);
68-
67+
$this->assertEquals(24, $fh->getNumPosts());
68+
6969
// test holder that doesn't have posts
7070
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
71-
$this->assertEquals($fh2->getNumPosts(), 0);
71+
$this->assertEquals(0, $fh2->getNumPosts());
72+
73+
//Mark spammer accounts and retest the posts count
74+
$this->markGhosts();
75+
$this->assertEquals(22, $fh->getNumPosts());
76+
77+
78+
7279
}
73-
74-
function testGetNumTopics() {
80+
81+
public function testGetNumTopics() {
7582
// test holder with posts
7683
$fh = $this->objFromFixture("ForumHolder", "fh");
77-
$this->assertEquals($fh->getNumTopics(), 6);
78-
84+
$this->assertEquals(6, $fh->getNumTopics());
85+
7986
// test holder that doesn't have posts
8087
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
81-
$this->assertEquals($fh2->getNumTopics(), 0);
88+
$this->assertEquals(0, $fh2->getNumTopics());
89+
90+
//Mark spammer accounts and retest the threads count
91+
$this->markGhosts();
92+
$this->assertEquals(5, $fh->getNumTopics());
8293
}
83-
84-
function testGetNumAuthors() {
94+
95+
public function testGetNumAuthors() {
8596
// test holder with posts
8697
$fh = $this->objFromFixture("ForumHolder", "fh");
87-
$this->assertEquals($fh->getNumAuthors(), 4);
88-
98+
$this->assertEquals(4, $fh->getNumAuthors());
99+
89100
// test holder that doesn't have posts
90101
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
91-
$this->assertEquals($fh2->getNumAuthors(), 0);
102+
$this->assertEquals(0, $fh2->getNumAuthors());
103+
104+
//Mark spammer accounts and retest the authors count
105+
$this->markGhosts();
106+
$this->assertEquals(2, $fh->getNumAuthors());
107+
108+
}
109+
110+
protected function markGhosts() {
111+
//Mark a members as a spammers
112+
$spammer = $this->objFromFixture("Member", "spammer");
113+
$spammer->ForumStatus = 'Ghost';
114+
$spammer->write();
115+
116+
$spammer2 = $this->objFromFixture("Member", "spammer2");
117+
$spammer2->ForumStatus = 'Ghost';
118+
$spammer2->write();
92119
}
93-
94-
function testGetRecentPosts() {
120+
121+
public function testGetRecentPosts() {
95122
// test holder with posts
96123
$fh = $this->objFromFixture("ForumHolder", "fh");
97124

@@ -100,15 +127,15 @@ function testGetRecentPosts() {
100127

101128
// check they're in the right order (well if the first and last are right its fairly safe)
102129
$this->assertEquals($fh->getRecentPosts()->First()->Content, "This is the last post to a long thread");
103-
130+
104131
// test holder that doesn't have posts
105132
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
106133
$this->assertNull($fh2->getRecentPosts());
107-
134+
108135
// test trying to get recent posts specific forum without posts
109136
$forum = $this->objFromFixture("Forum", "forum1cat2");
110137
$this->assertNull($fh->getRecentPosts(50, $forum->ID));
111-
138+
112139
// test trying to get recent posts specific to a forum which has posts
113140
$forum = $this->objFromFixture("Forum", "general");
114141

@@ -117,49 +144,49 @@ function testGetRecentPosts() {
117144

118145
// test trying to filter by a specific thread
119146
$thread = $this->objFromFixture("ForumThread","Thread1");
120-
147+
121148
$this->assertEquals($fh->getRecentPosts(50, null, $thread->ID)->Count(), 17);
122149
$this->assertEquals($fh->getRecentPosts(10, null, $thread->ID)->Count(), 10);
123150
$this->assertEquals($fh->getRecentPosts(50, null, $thread->ID)->First()->Content, 'This is the last post to a long thread');
124-
151+
125152
// test limiting the response
126153
$this->assertEquals($fh->getRecentPosts(1)->Count(), 1);
127154
}
128-
129-
function testGlobalAnnouncements() {
155+
156+
public function testGlobalAnnouncements() {
130157
// test holder with posts
131158
$fh = $this->objFromFixture("ForumHolder", "fh");
132159
$controller = new ForumHolder_Controller($fh);
133160

134161
// make sure all the announcements are included
135162
$this->assertEquals($controller->GlobalAnnouncements()->Count(), 1);
136-
163+
137164
// test holder that doesn't have posts
138165
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
139166
$controller2 = new ForumHolder_Controller($fh2);
140167

141168
$this->assertEquals($controller2->GlobalAnnouncements()->Count(), 0);
142169
}
143-
144-
function testGetNewPostsAvailable() {
170+
171+
public function testGetNewPostsAvailable() {
145172
$fh = $this->objFromFixture("ForumHolder", "fh");
146173

147-
// test last visit. we can assume that these tests have been reloaded in the past 24 hours
174+
// test last visit. we can assume that these tests have been reloaded in the past 24 hours
148175
$data = array();
149176
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data, date('Y-m-d H:i:s', mktime(0, 0, 0, date('m'), date('d')-1, date('Y')))));
150-
177+
151178
// set the last post ID (test the first post - so there should be a post, last post (false))
152179
$fixtureIDs = $this->allFixtureIDs('Post');
153180
$lastPostID = end($fixtureIDs);
154-
181+
155182
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data,null, 1));
156183
$this->assertFalse(ForumHolder::new_posts_available($fh->ID, $data, null, $lastPostID));
157-
184+
158185
// limit to a specific forum
159186
$forum = $this->objFromFixture("Forum", "general");
160187
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data, null, null, $forum->ID));
161188
$this->assertFalse(ForumHolder::new_posts_available($fh->ID, $data, null, $lastPostID, $forum->ID));
162-
189+
163190
// limit to a specific thread
164191
$thread = $this->objFromFixture("ForumThread", "Thread1");
165192
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data, null, null, null, $thread->ID));

0 commit comments

Comments
 (0)