Skip to content

Commit

Permalink
FIX Forum statistics returned incorrect counts.
Browse files Browse the repository at this point in the history
  • Loading branch information
@camfindlay
camfindlay committed Aug 22, 2015
1 parent 2fc1764 commit 95a7707
Show file tree
Hide file tree
Showing 7 changed files with 157 additions and 69 deletions.
8 changes: 7 additions & 1 deletion code/model/ForumThread.php
View file
Expand Up @@ -133,7 +133,13 @@ function getFirstPost() {
* @return int
*/
function getNumPosts() {
return (int)DB::query("SELECT count(*) FROM \"Post\" WHERE \"ThreadID\" = $this->ID")->value();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT("Post"."ID")');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"ThreadID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}

/**
Expand Down
27 changes: 23 additions & 4 deletions code/pagetypes/Forum.php
View file
Expand Up @@ -357,25 +357,44 @@ function getLatestPost() {
* @return int Returns the number of topics (threads)
*/
function getNumTopics() {
return DB::query(sprintf('SELECT COUNT("ID") FROM "ForumThread" WHERE "ForumID" = \'%s\'', $this->ID))->value();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT(DISTINCT("ThreadID"))');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}

/**
* Get the number of total posts
*
* @return int
* @return int Returns the number of posts
*/
function getNumPosts() {
return DB::query(sprintf('SELECT COUNT("ID") FROM "Post" WHERE "ForumID" = \'%s\'', $this->ID))->value();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT("Post"."ID")');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}


/**
* Get the number of distinct Authors
*
* @return int
*/
function getNumAuthors() {
return DB::query(sprintf('SELECT COUNT(DISTINCT "AuthorID") FROM "Post" WHERE "ForumID" = \'%s\'', $this->ID))->value();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT(DISTINCT("AuthorID"))');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"ForumID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}

/**
Expand Down
41 changes: 24 additions & 17 deletions code/pagetypes/ForumHolder.php
View file
Expand Up @@ -222,12 +222,14 @@ public function Breadcrumbs($maxDepth = 20, $unlinked = false, $stopAtPageType =
* @return int Returns the number of posts
*/
public function getNumPosts() {
return Post::get()
->innerJoin(ForumHolder::baseForumTable(),"\"Post\".\"ForumID\" = \"ForumPage\".\"ID\"" , "ForumPage")
->filter(array(
"ForumPage.ParentID" => $this->ID
))
->count();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT("Post"."ID")');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}


Expand All @@ -237,12 +239,14 @@ public function getNumPosts() {
* @return int Returns the number of topics (threads)
*/
function getNumTopics() {
return ForumThread::get()
->innerJoin(ForumHolder::baseForumTable(),"\"ForumThread\".\"ForumID\" = \"ForumPage\".\"ID\"","ForumPage")
->filter(array(
"ForumPage.ParentID" => $this->ID
))
->count();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT(DISTINCT("ThreadID"))');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}


Expand All @@ -252,11 +256,14 @@ function getNumTopics() {
* @return int Returns the number of distinct authors
*/
public function getNumAuthors() {
return DB::query("
SELECT COUNT(DISTINCT \"Post\".\"AuthorID\")
FROM \"Post\"
JOIN \"" . ForumHolder::baseForumTable() . "\" AS \"ForumPage\" ON \"Post\".\"ForumID\"=\"ForumPage\".\"ID\"
AND \"ForumPage\".\"ParentID\" = '" . $this->ID . "'")->value();
$sqlQuery = new SQLQuery();
$sqlQuery->setFrom('"Post"');
$sqlQuery->setSelect('COUNT(DISTINCT("AuthorID"))');
$sqlQuery->addInnerJoin('Member', '"Post"."AuthorID" = "Member"."ID"');
$sqlQuery->addInnerJoin('SiteTree', '"Post"."ForumID" = "SiteTree"."ID"');
$sqlQuery->addWhere('"Member"."ForumStatus" = \'Normal\'');
$sqlQuery->addWhere('"SiteTree"."ParentID" = ' . $this->ID);
return $sqlQuery->execute()->value();
}

/**
Expand Down
4 changes: 2 additions & 2 deletions templates/Includes/ForumHeader.ss
View file
@@ -1,6 +1,6 @@
<div class="forum-header">

<% loop $ForumHolder %>

<div class="forum-header-forms">

<span class="forum-search-dropdown-icon"></span>
Expand Down Expand Up @@ -49,7 +49,7 @@
<% end_if %>

</div><!-- forum-header-forms. -->
<% end_loop %>


<h1 class="forum-heading"><a name='Header'>$HolderSubtitle</a></h1>
<p class="forum-breadcrumbs">$Breadcrumbs</p>
Expand Down
107 changes: 67 additions & 40 deletions tests/ForumHolderTest.php
View file
Expand Up @@ -4,7 +4,7 @@
* @todo Write tests to cover the RSS feeds
*/
class ForumHolderTest extends FunctionalTest {

static $fixture_file = "forum/tests/ForumTest.yml";

public function setUp() {
Expand All @@ -20,7 +20,7 @@ public function setUp() {
*
* @return unknown_type
*/
function testGetForums() {
public function testGetForums() {
$fh = $this->objFromFixture("ForumHolder", "fh");
$fh_controller = new ForumHolder_Controller($fh);

Expand All @@ -34,64 +34,91 @@ function testGetForums() {
// expects none.
$this->assertTrue($fh_controller->Categories()->First()->Forums()->Count() == 0, "fh first category has 0 forums");
$this->assertTrue($fh_controller->Categories()->Last()->Forums()->Count() == 2, "fh second category has 2 forums");

// Test ForumHolder::Categories() on 'fh2', from which we expect 2 categories
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$fh2_controller = new ForumHolder_Controller($fh2);
$this->assertTrue($fh2_controller->Categories()->Count() == 2, "fh first forum has two categories");

// Test what we got back from the two categories. Each expects 1.
$this->assertTrue($fh2_controller->Categories()->First()->Forums()->Count() == 1, "fh first category has 1 forums");
$this->assertTrue($fh2_controller->Categories()->Last()->Forums()->Count() == 1, "fh second category has 1 forums");


// plain forums (not nested in categories)
$forumHolder = $this->objFromFixture("ForumHolder", "fhNoCategories");

$this->assertEquals($forumHolder->Forums()->Count(), 1);
$this->assertEquals($forumHolder->Forums()->First()->Title, "Forum Without Category");

// plain forums with nested in categories enabled (but shouldn't effect it)
$forumHolder = $this->objFromFixture("ForumHolder", "fhNoCategories");
$forumHolder->ShowInCategories = true;
$forumHolder->write();

$this->assertEquals($forumHolder->Forums()->Count(), 1);
$this->assertEquals($forumHolder->Forums()->First()->Title, "Forum Without Category");

}

function testGetNumPosts() {
public function testGetNumPosts() {
// test holder with posts
$fh = $this->objFromFixture("ForumHolder", "fh");
$this->assertEquals($fh->getNumPosts(), 24);
$this->assertEquals(24, $fh->getNumPosts());

// test holder that doesn't have posts
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$this->assertEquals($fh2->getNumPosts(), 0);
$this->assertEquals(0, $fh2->getNumPosts());

//Mark spammer accounts and retest the posts count
$this->markGhosts();
$this->assertEquals(22, $fh->getNumPosts());



}
function testGetNumTopics() {

public function testGetNumTopics() {
// test holder with posts
$fh = $this->objFromFixture("ForumHolder", "fh");
$this->assertEquals($fh->getNumTopics(), 6);
$this->assertEquals(6, $fh->getNumTopics());

// test holder that doesn't have posts
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$this->assertEquals($fh2->getNumTopics(), 0);
$this->assertEquals(0, $fh2->getNumTopics());

//Mark spammer accounts and retest the threads count
$this->markGhosts();
$this->assertEquals(5, $fh->getNumTopics());
}
function testGetNumAuthors() {

public function testGetNumAuthors() {
// test holder with posts
$fh = $this->objFromFixture("ForumHolder", "fh");
$this->assertEquals($fh->getNumAuthors(), 4);
$this->assertEquals(4, $fh->getNumAuthors());

// test holder that doesn't have posts
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$this->assertEquals($fh2->getNumAuthors(), 0);
$this->assertEquals(0, $fh2->getNumAuthors());

//Mark spammer accounts and retest the authors count
$this->markGhosts();
$this->assertEquals(2, $fh->getNumAuthors());

}

protected function markGhosts() {
//Mark a members as a spammers
$spammer = $this->objFromFixture("Member", "spammer");
$spammer->ForumStatus = 'Ghost';
$spammer->write();

$spammer2 = $this->objFromFixture("Member", "spammer2");
$spammer2->ForumStatus = 'Ghost';
$spammer2->write();
}
function testGetRecentPosts() {

public function testGetRecentPosts() {
// test holder with posts
$fh = $this->objFromFixture("ForumHolder", "fh");

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

// check they're in the right order (well if the first and last are right its fairly safe)
$this->assertEquals($fh->getRecentPosts()->First()->Content, "This is the last post to a long thread");

// test holder that doesn't have posts
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$this->assertNull($fh2->getRecentPosts());

// test trying to get recent posts specific forum without posts
$forum = $this->objFromFixture("Forum", "forum1cat2");
$this->assertNull($fh->getRecentPosts(50, $forum->ID));

// test trying to get recent posts specific to a forum which has posts
$forum = $this->objFromFixture("Forum", "general");

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

// test trying to filter by a specific thread
$thread = $this->objFromFixture("ForumThread","Thread1");

$this->assertEquals($fh->getRecentPosts(50, null, $thread->ID)->Count(), 17);
$this->assertEquals($fh->getRecentPosts(10, null, $thread->ID)->Count(), 10);
$this->assertEquals($fh->getRecentPosts(50, null, $thread->ID)->First()->Content, 'This is the last post to a long thread');

// test limiting the response
$this->assertEquals($fh->getRecentPosts(1)->Count(), 1);
}
function testGlobalAnnouncements() {

public function testGlobalAnnouncements() {
// test holder with posts
$fh = $this->objFromFixture("ForumHolder", "fh");
$controller = new ForumHolder_Controller($fh);

// make sure all the announcements are included
$this->assertEquals($controller->GlobalAnnouncements()->Count(), 1);

// test holder that doesn't have posts
$fh2 = $this->objFromFixture("ForumHolder", "fh2");
$controller2 = new ForumHolder_Controller($fh2);

$this->assertEquals($controller2->GlobalAnnouncements()->Count(), 0);
}
function testGetNewPostsAvailable() {

public function testGetNewPostsAvailable() {
$fh = $this->objFromFixture("ForumHolder", "fh");

// test last visit. we can assume that these tests have been reloaded in the past 24 hours
// test last visit. we can assume that these tests have been reloaded in the past 24 hours
$data = array();
$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')))));

// set the last post ID (test the first post - so there should be a post, last post (false))
$fixtureIDs = $this->allFixtureIDs('Post');
$lastPostID = end($fixtureIDs);

$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data,null, 1));
$this->assertFalse(ForumHolder::new_posts_available($fh->ID, $data, null, $lastPostID));

// limit to a specific forum
$forum = $this->objFromFixture("Forum", "general");
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data, null, null, $forum->ID));
$this->assertFalse(ForumHolder::new_posts_available($fh->ID, $data, null, $lastPostID, $forum->ID));

// limit to a specific thread
$thread = $this->objFromFixture("ForumThread", "Thread1");
$this->assertTrue(ForumHolder::new_posts_available($fh->ID, $data, null, null, null, $thread->ID));
Expand Down

0 comments on commit 95a7707

Please sign in to comment.