From d5f1cef6420c7450ce960f469fd10557ea8fcdd9 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 30 Oct 2017 15:10:21 +0100 Subject: [PATCH] fix comment sorter background: we have a flat hierarchy of comments, not a tree. therefore we can also remove again the unnecessary additions. Signed-off-by: Arthur Schiwon --- apps/comments/js/commentstabview.js | 2 +- .../lib/Collaboration/CommentersSorter.php | 15 +++- .../Collaboration/CommentersSorterTest.php | 23 ++++-- lib/private/Comments/Manager.php | 76 ------------------- lib/public/Comments/ICommentsManager.php | 22 ------ tests/lib/Comments/ManagerTest.php | 31 -------- 6 files changed, 32 insertions(+), 137 deletions(-) diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js index 7b5a25b7e8e..4d67ca90825 100644 --- a/apps/comments/js/commentstabview.js +++ b/apps/comments/js/commentstabview.js @@ -238,7 +238,7 @@ search: query, itemType: 'files', itemId: s.model.get('id'), - sorter: 'comments|share-recipients', + sorter: 'commenters|share-recipients', limit: OC.appConfig.comments.maxAutoCompleteResults }, function (data) { diff --git a/apps/comments/lib/Collaboration/CommentersSorter.php b/apps/comments/lib/Collaboration/CommentersSorter.php index 8a24592c30b..b8bb745b3b8 100644 --- a/apps/comments/lib/Collaboration/CommentersSorter.php +++ b/apps/comments/lib/Collaboration/CommentersSorter.php @@ -84,12 +84,23 @@ class CommentersSorter implements ISorter { * @return array */ protected function retrieveCommentsInformation($type, $id) { - $comments = $this->commentsManager->getForObject($type, $id, 1); + $comments = $this->commentsManager->getForObject($type, $id); if(count($comments) === 0) { return []; } - return $this->commentsManager->getActorsInTree($comments[0]->getTopmostParentId()); + $actors = []; + foreach ($comments as $comment) { + if(!isset($actors[$comment->getActorType()])) { + $actors[$comment->getActorType()] = []; + } + if(!isset($actors[$comment->getActorType()][$comment->getActorId()])) { + $actors[$comment->getActorType()][$comment->getActorId()] = 1; + } else { + $actors[$comment->getActorType()][$comment->getActorId()]++; + } + } + return $actors; } protected function compare(array $a, array $b, array $commenters) { diff --git a/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php b/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php index 95a74f118c0..0cc3e3d4b61 100644 --- a/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php +++ b/apps/comments/tests/Unit/Collaboration/CommentersSorterTest.php @@ -27,6 +27,7 @@ namespace OCA\Comments\Tests\Unit\Collaboration; use OCA\Comments\Collaboration\CommentersSorter; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; +use OCP\IConfig; use Test\TestCase; class CommentersSorterTest extends TestCase { @@ -48,13 +49,25 @@ class CommentersSorterTest extends TestCase { * @param $data */ public function testSort($data) { - $this->commentsManager->expects($this->once()) - ->method('getForObject') - ->willReturn([$this->createMock(IComment::class)]); + $commentMocks = []; + foreach($data['actors'] as $actorType => $actors) { + foreach ($actors as $actorId => $noOfComments) { + for($i=0;$i<$noOfComments;$i++) { + $mock = $this->createMock(IComment::class); + $mock->expects($this->atLeastOnce()) + ->method('getActorType') + ->willReturn($actorType); + $mock->expects($this->atLeastOnce()) + ->method('getActorId') + ->willReturn($actorId); + $commentMocks[] = $mock; + } + } + } $this->commentsManager->expects($this->once()) - ->method('getActorsInTree') - ->willReturn($data['actors']); + ->method('getForObject') + ->willReturn($commentMocks); $workArray = $data['input']; $this->sorter->sort($workArray, ['itemType' => 'files', 'itemId' => '24']); diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index 3b5eb23844c..078e1eef4d3 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -317,82 +317,6 @@ class Manager implements ICommentsManager { return $tree; } - /** - * Get the actor types and ID that commented in the tree specified by the ID - * - * @param string $id - * @return array - * @since 13.0.0 - * - * The return array looks like this: - * - * [ - * 'user' => [ - * 'alice' => 2, - * 'bob' => 3 - * ], - * 'robot' => [ - * 'r2-d2' => 5, - * 'c-3po' => 17, - * ] - * ] - */ - public function getActorsInTree($id) { - $tree = $this->getTree($id); - $actors = []; - $this->walkTree($tree, $actors, [$this, 'extractActor']); - return $actors; - } - - /** - * @param IComment $comment - * @param array &$actors - * - * build an array that looks like: - * - * [ - * 'user' => [ - * 'alice' => 2, - * 'bob' => 3 - * ], - * 'robot' => [ - * 'r2-d2' => 5, - * 'c-3po' => 17, - * ] - * ] - * - */ - protected function extractActor(IComment $comment, &$actors) { - if(!isset($actors[$comment->getActorType()])) { - $actors[$comment->getActorType()] = []; - } - if(!isset($actors[$comment->getActorType()][$comment->getActorId()])) { - $actors[$comment->getActorType()][$comment->getActorId()] = 1; - } else { - $actors[$comment->getActorType()][$comment->getActorId()] += 1; - } - } - - /** - * walks through a comment tree (as returned by getTree() and invokes a callback - * with the current IComment instance (and optionally custom parameters) - * - * @param array $node - * @param array &$results - * @param callable $callback - * @param array|null $parameters - */ - protected function walkTree($node, array &$results, callable $callback, array $parameters = null) { - if(isset($node['replies'])) { - foreach ($node['replies'] as $subNode) { - $this->walkTree($subNode, $results, $callback, $parameters); - } - } - if(isset($node['comment']) && $node['comment'] instanceof IComment) { - $callback($node['comment'], $results, $parameters); - } - } - /** * returns comments for a specific object (e.g. a file). * diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index e3ea7888ffd..61633af95cd 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/lib/public/Comments/ICommentsManager.php @@ -137,28 +137,6 @@ interface ICommentsManager { */ public function getNumberOfUnreadCommentsForFolder($folderId, IUser $user); - /** - * Get the actor types and ID that commented in the tree specified by the ID - * - * @param string $id - * @return array - * @since 13.0.0 - * - * The return array looks like this: - * - * [ - * 'users' => [ - * 'alice', - * 'bob', - * ], - * 'robots' => [ - * 'r2-d2', - * 'c-3po', - * ] - * ] - */ - public function getActorsInTree($id); - /** * creates a new comment and returns it. At this point of time, it is not * saved in the used data storage. Use save() after setting other fields diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php index df70581d6b4..671389232e2 100644 --- a/tests/lib/Comments/ManagerTest.php +++ b/tests/lib/Comments/ManagerTest.php @@ -253,37 +253,6 @@ class ManagerTest extends TestCase { } while (count($comments) > 0); } - public function testGetActorsInTree() { - $manager = $this->getManager(); - - $headId = $this->addDatabaseEntry(0, 0); - - $id = $this->addDatabaseEntry($headId, $headId, new \DateTime('-3 hours')); - $comment = $manager->get($id)->setActor('users', 'bob'); - $manager->save($comment); - - $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); - $this->addDatabaseEntry($headId, $headId, new \DateTime('-2 hours')); - - $id = $this->addDatabaseEntry($headId, $headId, new \DateTime('-1 hour')); - $comment = $manager->get($id)->setActor('users', 'bob'); - $manager->save($comment); - - $id = $this->addDatabaseEntry($headId, $headId, new \DateTime('-4 hour')); - $comment = $manager->get($id)->setActor('users', 'cynthia'); - $manager->save($comment); - - $actors = $manager->getActorsInTree($headId); - $this->assertTrue(isset($actors['users'])); - $this->assertCount(3, $actors['users']); - $this->assertTrue(isset($actors['users']['alice'])); - $this->assertTrue(isset($actors['users']['bob'])); - $this->assertTrue(isset($actors['users']['cynthia'])); - $this->assertSame(3, $actors['users']['alice']); - $this->assertSame(2, $actors['users']['bob']); - $this->assertSame(1, $actors['users']['cynthia']); - } - public function testGetForObjectWithDateTimeConstraint() { $this->addDatabaseEntry(0, 0, new \DateTime('-6 hours')); $this->addDatabaseEntry(0, 0, new \DateTime('-5 hours'));