FIX: Implement paginated list for RSS feed. Fixes #31.

Includes functional tests for the RSSFeed but currently commented out until that feature lands in the main framework.

_config.php

@@ -21,6 +21,9 @@
  *		'require_login' => true
  *	));
  * </code>
+ *
+ * To see all the configuration options read docs/en/Configuration.md or
+ * consult the Commenting class.
  */
 
 Commenting::add('SiteTree');

_config/routes.yml

@@ -5,5 +5,5 @@ After: framework/routes#coreroutes
 Director:
   rules:
     # handle old 2.4 style urls
-    'PageComments/$Action/$ID': 'CommentingController'
-    'PageComments_Controller/$Action/$ID': 'CommentingController'
+    'PageComments/$Action/$ID/$OtherID': 'CommentingController'
+    'PageComments_Controller/$Action/$ID/$OtherID': 'CommentingController'

code/Commenting.php

@@ -114,8 +114,8 @@ class Commenting {
 	 * @throws Exception 
 	 * @return mixed
 	 */
-	public static function get_config_value($class, $key) {
-		if(isset(self::$enabled_classes[$class])) {
+	public static function get_config_value($class = null, $key) {
+		if(!$class || isset(self::$enabled_classes[$class])) {
 			// custom configuration
 			if(isset(self::$enabled_classes[$class][$key])) return self::$enabled_classes[$class][$key];
 			

code/controllers/CommentingController.php

@@ -15,7 +15,7 @@ class CommentingController extends Controller {
 		'CommentsForm',
 		'doPostComment'
 	);
-	
+
 	private $baseClass = "";
 	private $ownerRecord = "";
 	private $ownerController = "";
@@ -54,19 +54,30 @@ class CommentingController extends Controller {
 	}
 	
 	/**
-	 * Return an RSS feed of comments for a given set of comments or all 
+	 * Outputs the RSS feed of comments
+	 *
+	 * @return XML
+	 */
+	public function rss() {
+		return $this->getFeed($this->request)->outputToBrowser();
+	}
+
+	/**
+	 * Return an RSSFeed of comments for a given set of comments or all 
 	 * comments on the website.
 	 *
 	 * To maintain backwards compatibility with 2.4 this supports mapping
 	 * of PageComment/rss?pageid= as well as the new RSS format for comments
 	 * of CommentingController/rss/{classname}/{id}
 	 *
-	 * @return RSS
+	 * @param SS_HTTPRequest
+	 *
+	 * @return RSSFeed
 	 */
-	public function rss() {
+	public function getFeed(SS_HTTPRequest $request) {
 		$link = $this->Link('rss');
-		$class = $this->urlParams['ID'];
-		$id = $this->urlParams['OtherID'];
+		$class = $request->param('ID');
+		$id = $request->param('OtherID');
 
 		if(isset($_GET['pageid'])) {
 			$id =  Convert::raw2sql($_GET['pageid']);
@@ -99,22 +110,22 @@ class CommentingController extends Controller {
 				return $this->httpError(404);
 			}
 		} else {
-			$comments = Comment::get();
+			$comments = Comment::get()->where("Moderated = 1 AND IsSpam = 0");
 		}
 
 		$title = _t('CommentingController.RSSTITLE', "Comments RSS Feed");
 
-		$feed = new RSSFeed($comments, $link, $title, $link, 'Title', 'Comment', 'AuthorName');
-		$feed->outputToBrowser();
+		$comments = new PaginatedList($comments, $request);
+		$comments->setPageLength(Commenting::get_config_value(null, 'comments_per_page'));
+
+		return new RSSFeed($comments, $link, $title, $link, 'Title', 'Comment', 'AuthorName');
 	}
 
 	/**
 	 * Deletes a given {@link Comment} via the URL.
-	 *
-	 * @param SS_HTTPRequest
 	 */
-	public function delete($request) {
-		if(!$this->checkSecurityToken($request)) {
+	public function delete() {
+		if(!$this->checkSecurityToken($this->request)) {
 			return $this->httpError(400);
 		}
 
@@ -129,11 +140,9 @@ class CommentingController extends Controller {
 
 	/**
 	 * Marks a given {@link Comment} as spam. Removes the comment from display
-	 *
-	 * @param SS_HTTPRequest
 	 */
 	public function spam() {
-		if(!$this->checkSecurityToken($request)) {
+		if(!$this->checkSecurityToken($this->request)) {
 			return $this->httpError(400);
 		}
 
@@ -152,11 +161,9 @@ class CommentingController extends Controller {
 
 	/**
 	 * Marks a given {@link Comment} as ham (not spam).
-	 *
-	 * @param SS_HTTPRequest
 	 */
-	public function ham($request) {
-		if(!$this->checkSecurityToken($request)) {
+	public function ham() {
+		if(!$this->checkSecurityToken($this->request)) {
 			return $this->httpError(400);
 		}
 
@@ -175,11 +182,9 @@ class CommentingController extends Controller {
 
 	/**
 	 * Marks a given {@link Comment} as approved.
-	 *
-	 * @param SS_HTTPRequest
 	 */
-	public function approve($request) {
-		if(!$this->checkSecurityToken($request)) {
+	public function approve() {
+		if(!$this->checkSecurityToken($this->request)) {
 			return $this->httpError(400);
 		}
 
@@ -197,7 +202,8 @@ class CommentingController extends Controller {
 	}
 	
 	/**
-	 * Returns the comment referenced in the URL (by ID).
+	 * Returns the comment referenced in the URL (by ID). Permission checking
+	 * should be done in the callee.
 	 *
 	 * @return Comment|false
 	 */
@@ -370,15 +376,10 @@ class CommentingController extends Controller {
 			
 			return true;
 		}
-		
-		if($comment->NeedsModeration){
-			$this->sessionMessage($moderationMsg, 'good');
-		}
-		
-		// build up the return link. Ideally redirect to 
+
 		$holder = Commenting::get_config_value($comment->BaseClass, 'comments_holder_id');
 
-		$hash = ($comment->NeedsModeration) ? $holder : $comment->Permalink();
+		$hash = ($moderated) ? $comment->Permalink() : $holder;
 		$url = (isset($data['ReturnURL'])) ? $data['ReturnURL'] : false;
 			
 		return ($url) ? $this->redirect($url .'#'. $hash) : $this->redirectBack();

code/dataobjects/Comment.php

@@ -29,7 +29,8 @@ class Comment extends DataObject {
 	public static $many_many = array();
 	
 	public static $defaults = array(
-		"Moderated" => true
+		"Moderated" => 1,
+		"IsSpam" => 0
 	);
 	
 	public static $casting = array(
@@ -167,10 +168,9 @@ class Comment extends DataObject {
 		if($extended !== null) return $extended;
 		
 		$page = $this->getParent();
-		return (
-			($page && $page->ProvideComments)
-			|| (bool)Permission::checkMember($member, 'CMS_ACCESS_CommentAdmin')
-		);
+		$admin = (bool) Permission::checkMember($member, 'CMS_ACCESS_CommentAdmin');
+
+		return (($page && $page->ProvideComments && $page->canView($member)) || $admin);
 	}
 	
 	/**
@@ -229,9 +229,9 @@ class Comment extends DataObject {
 		if($this->canDelete()) {
 			$token = SecurityToken::inst();
 
-			return DBField::create_field("Varchar", $token->addToUrl(sprintf(
+			return DBField::create_field("Varchar", Director::absoluteURL($token->addToUrl(sprintf(
 				"CommentingController/delete/%s", (int) $this->ID
-			)));
+			))));
 		}
 	}
 	
@@ -242,9 +242,9 @@ class Comment extends DataObject {
 		if($this->canEdit() && !$this->IsSpam) {
 			$token = SecurityToken::inst();
 
-			return DBField::create_field("Varchar", $token->addToUrl(sprintf(
+			return DBField::create_field("Varchar", Director::absoluteURL($token->addToUrl(sprintf(
 				"CommentingController/spam/%s", (int) $this->ID
-			)));
+			))));
 		}
 	}
 	
@@ -255,9 +255,9 @@ class Comment extends DataObject {
 		if($this->canEdit() && $this->IsSpam) {
 			$token = SecurityToken::inst();
 
-			return DBField::create_field("Varchar", $token->addToUrl(sprintf(
+			return DBField::create_field("Varchar", Director::absoluteURL($token->addToUrl(sprintf(
 				"CommentingController/ham/%s", (int) $this->ID
-			)));
+			))));
 		}
 	}
 	
@@ -268,9 +268,9 @@ class Comment extends DataObject {
 		if($this->canEdit() && !$this->Moderated) {
 			$token = SecurityToken::inst();
 
-			return DBField::create_field("Varchar", $token->addToUrl(sprintf(
+			return DBField::create_field("Varchar", Director::absoluteURL($token->addToUrl(sprintf(
 				"CommentingController/approve/%s", (int) $this->ID
-			)));
+			))));
 		}
 	}
 	
@@ -278,9 +278,9 @@ class Comment extends DataObject {
 	 * @return string
 	 */
 	public function SpamClass() {
-		if($this->getField('IsSpam')) {
+		if($this->IsSpam) {
 			return 'spam';
-		} else if($this->getField('NeedsModeration')) {
+		} else if(!$this->Moderated) {
 			return 'unmoderated';
 		} else {
 			return 'notspam';
@@ -291,7 +291,7 @@ class Comment extends DataObject {
 	 * @return string
 	 */
 	public function getTitle() {
-		$title = sprintf(_t('Comment.COMMENTBY', "Comment by '%s'", 'Name'), $this->getAuthorName());
+		$title = sprintf(_t('Comment.COMMENTBY', "Comment by %s", 'Name'), $this->getAuthorName());
 
 		if($parent = $this->getParent()) {
 			if($parent->Title) {

code/extensions/CommentsExtension.php

@@ -65,6 +65,8 @@ class CommentsExtension extends DataExtension {
 	 * @return PaginatedList
 	 */
 	public function Comments() {
+		$controller = Controller::curr();
+
 		$order = Commenting::get_config_value($this->ownerBaseClass, 'order_comments_by');
 		
 		$list = new PaginatedList(Comment::get()->where(sprintf(
@@ -75,15 +77,14 @@ class CommentsExtension extends DataExtension {
 			$this->ownerBaseClass, 'comments_per_page'
 		));
 
-		$controller = Controller::curr();		
+
+		$controller = Controller::curr();	
 		$list->setPageStart($controller->request->getVar("commentsstart". $this->owner->ID));
 		$list->setPaginationGetVar("commentsstart". $this->owner->ID);
-		$list->MoreThanOnePage();
 
 		return $list;
 	}
 	
-	
 	/**
 	 * Comments interface for the front end. Includes the CommentAddForm and the composition
 	 * of the comments display. 

tests/CommentingControllerTests.php

@@ -0,0 +1,61 @@
+<?php
+
+/**
+ * @package comments
+ * @subpackage tests
+ */
+class CommentingControllerTests extends FunctionalTest {
+	
+	public static $fixture_file = 'comments/tests/CommentsTest.yml';
+
+	protected $extraDataObjects = array(
+		'CommentableItem'
+	);
+
+	public function setUp() {
+		parent::setUp();
+
+		Commenting::add('CommentableItem');
+	}
+
+	public function testRSS() {
+		$this->markTestIncomplete("Waiting for https://github.com/silverstripe/sapphire/pull/686 to land");
+
+		$item = $this->objFromFixture('CommentableItem', 'first');
+
+		// comments sitewide
+		$response = $this->get('CommentingController/rss');
+		$this->assertEquals(10, substr_count($response->getBody(), "<item>"), "10 approved, non spam comments on page 1");
+
+		$response = $this->get('CommentingController/rss?start=10');
+		$this->assertEquals(3, substr_count($response->getBody(), "<item>"), "3 approved, non spam comments on page 2");
+
+		// all comments on a type
+		$response = $this->get('CommentingController/rss/CommentableItem');
+		$this->assertEquals(10, substr_count($response->getBody(), "<item>"));
+
+		$response = $this->get('CommentingController/rss/CommentableItem?start=10');
+		$this->assertEquals(3, substr_count($response->getBody(), "<item>"), "3 approved, non spam comments on page 2");
+
+		// specific page
+		$response = $this->get('CommentingController/rss/CommentableItem/'.$item->ID);
+		$this->assertEquals(1, substr_count($response->getBody(), "<item>"));
+		$this->assertContains('<dc:creator>FA</dc:creator>', $response->getBody());
+
+		// test accessing comments on a type that doesn't exist
+		$response = $this->get('CommentingController/rss/Fake');
+		$this->assertEquals(404, $response->getStatusCode());
+	}
+
+	public function testRSSSecuredCommentsForm() {
+		$this->markTestIncomplete("Not implemented");
+	}
+
+	public function testCommentsForm() {
+		$this->markTestIncomplete("Not implemented");
+	}
+
+	public function testDoCommentsForm() {
+		$this->markTestIncomplete("Not implemented");
+	}
+}

tests/CommentsTest.php

@@ -5,15 +5,26 @@
  */
 class CommentsTest extends FunctionalTest {
 	
-	static $fixture_file = 'comments/tests/CommentsTest.yml';
+	public static $fixture_file = 'comments/tests/CommentsTest.yml';
 	
-	function testCanView() {
+	protected $extraDataObjects = array(
+		'CommentableItem'
+	);
+
+	public function setUp() {
+		parent::setUp();
+
+		Commenting::add('CommentableItem');
+	}
+	
+
+	public function testCanView() {
 		$visitor = $this->objFromFixture('Member', 'visitor');
 		$admin = $this->objFromFixture('Member', 'commentadmin');
 		$comment = $this->objFromFixture('Comment', 'firstComA');
-		
+
 		$this->assertTrue($comment->canView($visitor), 
-			'Unauthenticated members can view comments associated to a page with ProvideComments=1'
+			'Unauthenticated members can view comments associated to a object with ProvideComments=1'
 		);
 		$this->assertTrue($comment->canView($admin),
 			'Admins with CMS_ACCESS_CommentAdmin permissions can view comments associated to a page with ProvideComments=1'
@@ -22,14 +33,15 @@ class CommentsTest extends FunctionalTest {
 		$disabledComment = $this->objFromFixture('Comment', 'disabledCom');
 		
 		$this->assertFalse($disabledComment->canView($visitor),
-		'Unauthenticated members can not view comments associated to a page with ProvideComments=0'
+			'Unauthenticated members can not view comments associated to a object with ProvideComments=0'
 		);
+
 		$this->assertTrue($disabledComment->canView($admin),
 			'Admins with CMS_ACCESS_CommentAdmin permissions can view comments associated to a page with ProvideComments=0'
 		);
 	}
 	
-	function testCanEdit() {
+	public function testCanEdit() {
 		$visitor = $this->objFromFixture('Member', 'visitor');
 		$admin = $this->objFromFixture('Member', 'commentadmin');
 		$comment = $this->objFromFixture('Comment', 'firstComA');
@@ -38,7 +50,7 @@ class CommentsTest extends FunctionalTest {
 		$this->assertTrue($comment->canEdit($admin));
 	}
 	
-	function testCanDelete() {
+	public function testCanDelete() {
 		$visitor = $this->objFromFixture('Member', 'visitor');
 		$admin = $this->objFromFixture('Member', 'commentadmin');
 		$comment = $this->objFromFixture('Comment', 'firstComA');
@@ -47,8 +59,14 @@ class CommentsTest extends FunctionalTest {
 		$this->assertTrue($comment->canEdit($admin));
 	}
 	
-	function testDeleteComment() {
-		$firstPage = $this->objFromFixture('Page', 'first');
+	public function testDeleteComment() {
+		$comment = $this->objFromFixture('Comment', 'firstComA');
+		$this->assertNull($comment->DeleteLink(), 'No permission to see delete link');
+		$delete = $this->get('CommentingController/delete/'.$comment->ID);
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertTrue($check && $check->exists());
+
+		$firstPage = $this->objFromFixture('CommentableItem', 'first');
 		$this->autoFollowRedirection = false;
 		$this->logInAs('commentadmin');
 		
@@ -56,11 +74,71 @@ class CommentsTest extends FunctionalTest {
 		$firstCommentID = $firstComment->ID;
 		Director::test($firstPage->RelativeLink(), null, $this->session());
 		$delete = $this->get('CommentingController/delete/'.$firstComment->ID);
-	
-		$this->assertFalse(DataObject::get_by_id('Comment', $firstCommentID));
+		$check = DataObject::get_by_id('Comment', $firstCommentID);
+		$this->assertFalse($check && $check->exists());
+	}
+
+	public function testSpamComment() {
+		$comment = $this->objFromFixture('Comment', 'firstComA');
+		$this->assertNull($comment->SpamLink(), 'No permission to see mark as spam link');
+		$spam = $this->get('CommentingController/spam/'.$comment->ID);
+
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(0, $check->IsSpam, 'No permission to mark as spam');
+
+		$this->autoFollowRedirection = false;
+		$this->logInAs('commentadmin');
+
+		$this->assertContains('CommentingController/spam/'. $comment->ID, $comment->SpamLink()->getValue());
+
+		$spam = $this->get('CommentingController/spam/'.$comment->ID);
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(1, $check->IsSpam);
+
+		$this->assertNull($check->SpamLink());
+	}
+
+	public function testHamComment() {
+		$comment = $this->objFromFixture('Comment', 'secondComC');
+		$this->assertNull($comment->HamLink(), 'No permission to see mark as ham link');
+		$ham = $this->get('CommentingController/ham/'.$comment->ID);
+
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(1, $check->IsSpam, 'No permission to mark as ham');
+
+		$this->autoFollowRedirection = false;
+		$this->logInAs('commentadmin');
+
+		$this->assertContains('CommentingController/ham/'. $comment->ID, $comment->HamLink()->getValue());
+
+		$ham = $this->get('CommentingController/ham/'.$comment->ID);
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(0, $check->IsSpam);
+
+		$this->assertNull($check->HamLink());
 	}
 	
-	function testCommenterURLWrite() {
+	public function testApproveComment() {
+		$comment = $this->objFromFixture('Comment', 'secondComB');
+		$this->assertNull($comment->ApproveLink(), 'No permission to see mark as approved link');
+		$ham = $this->get('CommentingController/approve/'.$comment->ID);
+
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(0, $check->Moderated, 'No permission to mark as approved');
+
+		$this->autoFollowRedirection = false;
+		$this->logInAs('commentadmin');
+
+		$this->assertContains('CommentingController/approve/'. $comment->ID, $comment->ApproveLink()->getValue());
+
+		$ham = $this->get('CommentingController/approve/'.$comment->ID);
+		$check = DataObject::get_by_id('Comment', $comment->ID);
+		$this->assertEquals(1, $check->Moderated);
+
+		$this->assertNull($check->ApproveLink());
+	}
+
+	public function testCommenterURLWrite() {
 		$comment = new Comment();
 		// We only care about the CommenterURL, so only set that
 		// Check a http and https URL. Add more test urls here as needed.
@@ -69,6 +147,7 @@ class CommentsTest extends FunctionalTest {
 			'Https',
 		);
 		$url = '://example.com';
+
 		foreach($protocols as $protocol) {
 			$comment->CommenterURL = $protocol . $url;
 			// The protocol should stay as if, assuming it is valid
@@ -77,3 +156,47 @@ class CommentsTest extends FunctionalTest {
 		}
 	}
 }
+
+
+/**
+ * @package comments
+ * @subpackage tests
+ */
+class CommentableItem extends DataObject implements TestOnly {
+
+	public static $db = array(
+		'ProvideComments' => 'Boolean',
+		'Title' => 'Varchar'
+	);
+
+	public function RelativeLink() {
+		return "CommentableItem_Controller";
+	}
+
+	public function canView($member = null) {
+		return true;
+	}
+
+	public function Link() {
+		return $this->RelativeLink();
+	}
+
+	public function AbsoluteLink() {
+		return Director::absoluteURL($this->RelativeLink());
+	}
+}
+
+/**
+ * @package comments
+ * @subpackage tests
+ */
+class CommentableItem_Controller extends Controller implements TestOnly {
+	
+	public static $allowed_actions = array(
+		"*" => true
+	);
+	
+	public function index() {
+		return CommentableItem::get()->first()->CommentsForm();
+	}
+}

tests/CommentsTest.yml

@@ -14,59 +14,127 @@ Permission:
 		Code: CMS_ACCESS_CommentAdmin
 		Group: =>Group.commentadmins
 
-Page:
+CommentableItem:
 	first:
-		Title: First page
-		URLSegment: first-page
+		Title: First
 		ProvideComments: 1
 	second:
-		Title: Second page
-		URLSegment: second-page
+		Title: Second
  		ProvideComments: 1
 	third:
-		Title: Third page
-		URLSegment:third-page
+		Title: Third
 		ProvideComments: 1
-	pageNoComments:
+	nocomments:
 		Title: No comments
-		URLSegment: no-comments
 		ProvideComments: 0
 
 Comment:
 	firstComA:
-		ParentID: =>Page.first
+		ParentID: =>CommentableItem.first
 		Name: FA
 		Comment: textFA
+		BaseClass: CommentableItem
 		Moderated: 1
+		IsSpam: 0
 	secondComA:
-		ParentID: =>Page.second
+		ParentID: =>CommentableItem.second
 		Name: SA
 		Comment: textSA
 		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
 	secondComB:
-		ParentID: =>Page.second
+		ParentID: =>CommentableItem.second
+		Name: SB
+		Comment: textSB
+		Moderated: 0
+		IsSpam: 0
+		BaseClass: CommentableItem
+	secondComC:
+		ParentID: =>CommentableItem.second
 		Name: SB
 		Comment: textSB
 		Moderated: 1
+		IsSpam: 1
+		BaseClass: CommentableItem
 	thirdComA:
-		ParentID: =>Page.third
+		ParentID: =>CommentableItem.third
 		Name: TA
 		Comment: textTA
 		Moderated: 1
-		IsSpam: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
 	thirdComB:
-		ParentID: =>Page.third
+		ParentID: =>CommentableItem.third
 		Name: TB
 		Comment: textTB
-		Moderated: 0
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
 	thirdComC:
-		ParentID: =>Page.third
+		ParentID: =>CommentableItem.third
 		Name: TC
 		Comment: textTC
-		Moderated: 0
-		
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComD:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		BaseClass: CommentableItem
+	thirdComE:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		BaseClass: CommentableItem
+	thirdComF:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComG:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComH:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComI:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComJ:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem
+	thirdComK:
+		ParentID: =>CommentableItem.third
+		Name: TC
+		Comment: textTC
+		Moderated: 1
+		IsSpam: 0
+		BaseClass: CommentableItem						
 	disabledCom:
-		ParentID: =>Page.pageNoComments
+		ParentID: =>CommentableItem.nocomments
 		Name: Disabled
 		Moderated: 0
-		IsSpam: 1
+		IsSpam: 1
+		BaseClass: CommentableItem