From 1d16a53c480ea7eb93ba118f6ffd69131eb5f3c5 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 21 Aug 2016 20:36:16 -0400 Subject: [PATCH] Store comment sorting direction in user metadata --- ChangeLog | 1 + app/Controller/BoardTooltipController.php | 5 +- app/Controller/CommentController.php | 20 ++- app/Controller/TaskViewController.php | 4 +- app/Core/Base.php | 1 + app/Core/Cache/BaseCache.php | 35 +---- app/Core/Cache/CacheInterface.php | 45 +++++++ app/Core/User/UserSession.php | 22 --- app/Decorator/MetadataCacheDecorator.php | 96 +++++++++++++ app/Model/UserMetadataModel.php | 2 + app/ServiceProvider/CacheProvider.php | 10 ++ tests/units/Core/User/UserSessionTest.php | 9 -- .../Decorator/MetadataCacheDecoratorTest.php | 127 ++++++++++++++++++ 13 files changed, 304 insertions(+), 73 deletions(-) create mode 100644 app/Core/Cache/CacheInterface.php create mode 100644 app/Decorator/MetadataCacheDecorator.php create mode 100644 tests/units/Decorator/MetadataCacheDecoratorTest.php diff --git a/ChangeLog b/ChangeLog index 145f5fd8e..4462c05aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,7 @@ New features: Improvements: +* Store comment sorting direction in the database * Avoid tags overlapping on the board * Show project name in notifications * Allow priority changes for inverted priority scales diff --git a/app/Controller/BoardTooltipController.php b/app/Controller/BoardTooltipController.php index 134d728e4..79b9b509f 100644 --- a/app/Controller/BoardTooltipController.php +++ b/app/Controller/BoardTooltipController.php @@ -2,6 +2,8 @@ namespace Kanboard\Controller; +use Kanboard\Model\UserMetadataModel; + /** * Board Tooltip * @@ -75,10 +77,11 @@ class BoardTooltipController extends BaseController public function comments() { $task = $this->getTask(); + $commentSortingDirection = $this->userMetadataCacheDecorator->get(UserMetadataModel::KEY_COMMENT_SORTING_DIRECTION, 'ASC'); $this->response->html($this->template->render('board/tooltip_comments', array( 'task' => $task, - 'comments' => $this->commentModel->getAll($task['id'], $this->userSession->getCommentSorting()) + 'comments' => $this->commentModel->getAll($task['id'], $commentSortingDirection) ))); } diff --git a/app/Controller/CommentController.php b/app/Controller/CommentController.php index 2a8c258a1..c61a0602f 100644 --- a/app/Controller/CommentController.php +++ b/app/Controller/CommentController.php @@ -4,6 +4,7 @@ namespace Kanboard\Controller; use Kanboard\Core\Controller\AccessForbiddenException; use Kanboard\Core\Controller\PageNotFoundException; +use Kanboard\Model\UserMetadataModel; /** * Comment Controller @@ -82,10 +83,10 @@ class CommentController extends BaseController $this->flash->failure(t('Unable to create your comment.')); } - return $this->response->redirect($this->helper->url->to('TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), 'comments'), true); + $this->response->redirect($this->helper->url->to('TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), 'comments'), true); + } else { + $this->create($values, $errors); } - - return $this->create($values, $errors); } /** @@ -183,9 +184,16 @@ class CommentController extends BaseController { $task = $this->getTask(); - $order = $this->userSession->getCommentSorting() === 'ASC' ? 'DESC' : 'ASC'; - $this->userSession->setCommentSorting($order); + $oldDirection = $this->userMetadataCacheDecorator->get(UserMetadataModel::KEY_COMMENT_SORTING_DIRECTION, 'ASC'); + $newDirection = $oldDirection === 'ASC' ? 'DESC' : 'ASC'; - $this->response->redirect($this->helper->url->to('TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), 'comments')); + $this->userMetadataCacheDecorator->set(UserMetadataModel::KEY_COMMENT_SORTING_DIRECTION, $newDirection); + + $this->response->redirect($this->helper->url->to( + 'TaskViewController', + 'show', + array('task_id' => $task['id'], 'project_id' => $task['project_id']), + 'comments' + )); } } diff --git a/app/Controller/TaskViewController.php b/app/Controller/TaskViewController.php index 36597457e..31b9de11b 100644 --- a/app/Controller/TaskViewController.php +++ b/app/Controller/TaskViewController.php @@ -4,6 +4,7 @@ namespace Kanboard\Controller; use Kanboard\Core\Controller\AccessForbiddenException; use Kanboard\Core\Controller\PageNotFoundException; +use Kanboard\Model\UserMetadataModel; /** * Task Controller @@ -61,13 +62,14 @@ class TaskViewController extends BaseController { $task = $this->getTask(); $subtasks = $this->subtaskModel->getAll($task['id']); + $commentSortingDirection = $this->userMetadataCacheDecorator->get(UserMetadataModel::KEY_COMMENT_SORTING_DIRECTION, 'ASC'); $this->response->html($this->helper->layout->task('task/show', array( 'task' => $task, 'project' => $this->projectModel->getById($task['project_id']), 'files' => $this->taskFileModel->getAllDocuments($task['id']), 'images' => $this->taskFileModel->getAllImages($task['id']), - 'comments' => $this->commentModel->getAll($task['id'], $this->userSession->getCommentSorting()), + 'comments' => $this->commentModel->getAll($task['id'], $commentSortingDirection), 'subtasks' => $subtasks, 'internal_links' => $this->taskLinkModel->getAllGroupedByLabel($task['id']), 'external_links' => $this->taskExternalLinkModel->getAll($task['id']), diff --git a/app/Core/Base.php b/app/Core/Base.php index df82febde..3b7c5e669 100644 --- a/app/Core/Base.php +++ b/app/Core/Base.php @@ -56,6 +56,7 @@ use Pimple\Container; * @property \Kanboard\Core\Helper $helper * @property \Kanboard\Core\Paginator $paginator * @property \Kanboard\Core\Template $template + * @property \Kanboard\Decorator\MetadataCacheDecorator $userMetadataCacheDecorator * @property \Kanboard\Model\ActionModel $actionModel * @property \Kanboard\Model\ActionParameterModel $actionParameterModel * @property \Kanboard\Model\AvatarFileModel $avatarFileModel diff --git a/app/Core/Cache/BaseCache.php b/app/Core/Cache/BaseCache.php index 04f8d2206..b51c4c0c4 100644 --- a/app/Core/Cache/BaseCache.php +++ b/app/Core/Cache/BaseCache.php @@ -8,41 +8,8 @@ namespace Kanboard\Core\Cache; * @package Kanboard\Core\Cache * @author Frederic Guillot */ -abstract class BaseCache +abstract class BaseCache implements CacheInterface { - /** - * Store an item in the cache - * - * @access public - * @param string $key - * @param string $value - */ - abstract public function set($key, $value); - - /** - * Retrieve an item from the cache by key - * - * @access public - * @param string $key - * @return mixed Null when not found, cached value otherwise - */ - abstract public function get($key); - - /** - * Remove all items from the cache - * - * @access public - */ - abstract public function flush(); - - /** - * Remove an item from the cache - * - * @access public - * @param string $key - */ - abstract public function remove($key); - /** * Proxy cache * diff --git a/app/Core/Cache/CacheInterface.php b/app/Core/Cache/CacheInterface.php new file mode 100644 index 000000000..19bd6ef73 --- /dev/null +++ b/app/Core/Cache/CacheInterface.php @@ -0,0 +1,45 @@ +sessionStorage->boardCollapsed[$project_id] = $is_collapsed; } - - /** - * Set comments sorting - * - * @access public - * @param string $order - */ - public function setCommentSorting($order) - { - $this->sessionStorage->commentSorting = $order; - } - - /** - * Get comments sorting direction - * - * @access public - * @return string - */ - public function getCommentSorting() - { - return empty($this->sessionStorage->commentSorting) ? 'ASC' : $this->sessionStorage->commentSorting; - } } diff --git a/app/Decorator/MetadataCacheDecorator.php b/app/Decorator/MetadataCacheDecorator.php new file mode 100644 index 000000000..0897b51cc --- /dev/null +++ b/app/Decorator/MetadataCacheDecorator.php @@ -0,0 +1,96 @@ +cache = $cache; + $this->metadataModel = $metadataModel; + $this->cachePrefix = $cachePrefix; + $this->entityId = $entityId; + } + + /** + * Get metadata value by key + * + * @param string $key + * @param mixed $default + * @return mixed + */ + public function get($key, $default) + { + $metadata = $this->cache->get($this->getCacheKey()); + + if ($metadata === null) { + $metadata = $this->metadataModel->getAll($this->entityId); + $this->cache->set($this->getCacheKey(), $metadata); + } + + return isset($metadata[$key]) ? $metadata[$key] : $default; + } + + /** + * Set new metadata value + * + * @param $key + * @param $value + */ + public function set($key, $value) + { + $this->metadataModel->save($this->entityId, array( + $key => $value, + )); + + $metadata = $this->metadataModel->getAll($this->entityId); + $this->cache->set($this->getCacheKey(), $metadata); + } + + /** + * Get cache key + * + * @return string + */ + protected function getCacheKey() + { + return $this->cachePrefix.$this->entityId; + } +} diff --git a/app/Model/UserMetadataModel.php b/app/Model/UserMetadataModel.php index e931d3ba8..e285b98ee 100644 --- a/app/Model/UserMetadataModel.php +++ b/app/Model/UserMetadataModel.php @@ -10,6 +10,8 @@ namespace Kanboard\Model; */ class UserMetadataModel extends MetadataModel { + const KEY_COMMENT_SORTING_DIRECTION = 'comment.sorting.direction'; + /** * Get the table * diff --git a/app/ServiceProvider/CacheProvider.php b/app/ServiceProvider/CacheProvider.php index 0d56e6019..fac44d53a 100644 --- a/app/ServiceProvider/CacheProvider.php +++ b/app/ServiceProvider/CacheProvider.php @@ -4,6 +4,7 @@ namespace Kanboard\ServiceProvider; use Kanboard\Core\Cache\FileCache; use Kanboard\Core\Cache\MemoryCache; +use Kanboard\Decorator\MetadataCacheDecorator; use Pimple\Container; use Pimple\ServiceProviderInterface; @@ -36,6 +37,15 @@ class CacheProvider implements ServiceProviderInterface $container['cacheDriver'] = $container['memoryCache']; } + $container['userMetadataCacheDecorator'] = function($c) { + return new MetadataCacheDecorator( + $c['cacheDriver'], + $c['userMetadataModel'], + 'user.metadata.', + $c['userSession']->getId() + ); + }; + return $container; } } diff --git a/tests/units/Core/User/UserSessionTest.php b/tests/units/Core/User/UserSessionTest.php index 64413f98a..7e9674c07 100644 --- a/tests/units/Core/User/UserSessionTest.php +++ b/tests/units/Core/User/UserSessionTest.php @@ -83,15 +83,6 @@ class UserSessionTest extends Base $this->assertFalse($us->isAdmin()); } - public function testCommentSorting() - { - $us = new UserSession($this->container); - $this->assertEquals('ASC', $us->getCommentSorting()); - - $us->setCommentSorting('DESC'); - $this->assertEquals('DESC', $us->getCommentSorting()); - } - public function testBoardCollapseMode() { $us = new UserSession($this->container); diff --git a/tests/units/Decorator/MetadataCacheDecoratorTest.php b/tests/units/Decorator/MetadataCacheDecoratorTest.php new file mode 100644 index 000000000..3e4dd3203 --- /dev/null +++ b/tests/units/Decorator/MetadataCacheDecoratorTest.php @@ -0,0 +1,127 @@ +cacheMock = $this + ->getMockBuilder('\Kanboard\Core\Cache\MemoryCache') + ->setMethods(array( + 'set', + 'get', + )) + ->getMock(); + + $this->metadataModelMock = $this + ->getMockBuilder('\Kanboard\Model\UserMetadataModel') + ->setConstructorArgs(array($this->container)) + ->setMethods(array( + 'getAll', + 'save', + )) + ->getMock() + ; + + $this->metadataCacheDecorator = new MetadataCacheDecorator( + $this->cacheMock, + $this->metadataModelMock, + $this->cachePrefix, + $this->entityId + ); + } + + public function testSet() + { + $this->cacheMock + ->expects($this->once()) + ->method('set'); + + $this->metadataModelMock + ->expects($this->at(0)) + ->method('save'); + + $this->metadataModelMock + ->expects($this->at(1)) + ->method('getAll') + ->with($this->entityId) + ; + + $this->metadataCacheDecorator->set('key', 'value'); + } + + public function testGetWithCache() + { + $this->cacheMock + ->expects($this->once()) + ->method('get') + ->with($this->cachePrefix.$this->entityId) + ->will($this->returnValue(array('key' => 'foobar'))) + ; + + $this->assertEquals('foobar', $this->metadataCacheDecorator->get('key', 'default')); + } + + public function testGetWithCacheAndDefaultValue() + { + $this->cacheMock + ->expects($this->once()) + ->method('get') + ->with($this->cachePrefix.$this->entityId) + ->will($this->returnValue(array('key1' => 'foobar'))) + ; + + $this->assertEquals('default', $this->metadataCacheDecorator->get('key', 'default')); + } + + public function testGetWithoutCache() + { + $this->cacheMock + ->expects($this->at(0)) + ->method('get') + ->with($this->cachePrefix.$this->entityId) + ->will($this->returnValue(null)) + ; + + $this->cacheMock + ->expects($this->at(1)) + ->method('set') + ->with( + $this->cachePrefix.$this->entityId, + array('key' => 'something') + ) + ; + + $this->metadataModelMock + ->expects($this->once()) + ->method('getAll') + ->with($this->entityId) + ->will($this->returnValue(array('key' => 'something'))) + ; + + $this->assertEquals('something', $this->metadataCacheDecorator->get('key', 'default')); + } +}