From 6d3597095ff1b39ac938076a9f9abab94168c198 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 31 Jul 2012 20:45:29 +1200 Subject: [PATCH] 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 | 3 + _config/routes.yml | 4 +- code/Commenting.php | 4 +- code/controllers/CommentingController.php | 65 +++++----- code/dataobjects/Comment.php | 32 ++--- code/extensions/CommentsExtension.php | 7 +- tests/CommentingControllerTests.php | 61 +++++++++ tests/CommentsTest.php | 147 ++++++++++++++++++++-- tests/CommentsTest.yml | 110 ++++++++++++---- 9 files changed, 345 insertions(+), 88 deletions(-) create mode 100644 tests/CommentingControllerTests.php diff --git a/_config.php b/_config.php index 9d2fb46..eb5c24c 100644 --- a/_config.php +++ b/_config.php @@ -21,6 +21,9 @@ * 'require_login' => true * )); * + * + * To see all the configuration options read docs/en/Configuration.md or + * consult the Commenting class. */ Commenting::add('SiteTree'); \ No newline at end of file diff --git a/_config/routes.yml b/_config/routes.yml index f3bc68e..c480a71 100644 --- a/_config/routes.yml +++ b/_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' \ No newline at end of file + 'PageComments/$Action/$ID/$OtherID': 'CommentingController' + 'PageComments_Controller/$Action/$ID/$OtherID': 'CommentingController' \ No newline at end of file diff --git a/code/Commenting.php b/code/Commenting.php index 322b552..2de8072 100644 --- a/code/Commenting.php +++ b/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]; diff --git a/code/controllers/CommentingController.php b/code/controllers/CommentingController.php index 32bbdeb..8c4bfdf 100644 --- a/code/controllers/CommentingController.php +++ b/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(); diff --git a/code/dataobjects/Comment.php b/code/dataobjects/Comment.php index c5d8f2e..40a72e0 100755 --- a/code/dataobjects/Comment.php +++ b/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) { diff --git a/code/extensions/CommentsExtension.php b/code/extensions/CommentsExtension.php index 4512637..767ee8e 100644 --- a/code/extensions/CommentsExtension.php +++ b/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. diff --git a/tests/CommentingControllerTests.php b/tests/CommentingControllerTests.php new file mode 100644 index 0000000..484331c --- /dev/null +++ b/tests/CommentingControllerTests.php @@ -0,0 +1,61 @@ +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(), ""), "10 approved, non spam comments on page 1"); + + $response = $this->get('CommentingController/rss?start=10'); + $this->assertEquals(3, substr_count($response->getBody(), ""), "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(), "")); + + $response = $this->get('CommentingController/rss/CommentableItem?start=10'); + $this->assertEquals(3, substr_count($response->getBody(), ""), "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(), "")); + $this->assertContains('FA', $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"); + } +} diff --git a/tests/CommentsTest.php b/tests/CommentsTest.php index 7b34651..4276d47 100644 --- a/tests/CommentsTest.php +++ b/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(); + } +} diff --git a/tests/CommentsTest.yml b/tests/CommentsTest.yml index 84ff8df..8f0e20f 100644 --- a/tests/CommentsTest.yml +++ b/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 \ No newline at end of file + IsSpam: 1 + BaseClass: CommentableItem \ No newline at end of file