From ec0ecc5b0387924f061865f4ec12dbfc5b7018fe Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 17 Jul 2016 17:15:14 -0400 Subject: [PATCH] Added event for removed comments with some refactoring --- ChangeLog | 1 + app/Core/Base.php | 2 + app/Core/Queue/JobHandler.php | 24 ++++++--- app/Core/Queue/QueueManager.php | 6 ++- app/EventBuilder/BaseEventBuilder.php | 23 ++++++++ app/EventBuilder/CommentEventBuilder.php | 48 +++++++++++++++++ app/Job/CommentEventJob.php | 50 ++++++++++++++++++ app/Job/NotificationJob.php | 3 +- app/Model/CommentModel.php | 9 ++-- app/Model/NotificationModel.php | 5 ++ app/ServiceProvider/ClassProvider.php | 4 ++ app/Subscriber/NotificationSubscriber.php | 1 + app/Template/event/comment_remove.php | 11 ++++ app/Template/event/comment_update.php | 3 ++ app/Template/notification/comment_remove.php | 7 +++ .../EventBuilder/CommentEventBuilderTest.php | 37 +++++++++++++ tests/units/Job/CommentEventJobTest.php | 52 +++++++++++++++++++ .../{CommentTest.php => CommentModelTest.php} | 4 +- 18 files changed, 272 insertions(+), 18 deletions(-) create mode 100644 app/EventBuilder/BaseEventBuilder.php create mode 100644 app/EventBuilder/CommentEventBuilder.php create mode 100644 app/Job/CommentEventJob.php create mode 100644 app/Template/event/comment_remove.php create mode 100644 app/Template/notification/comment_remove.php create mode 100644 tests/units/EventBuilder/CommentEventBuilderTest.php create mode 100644 tests/units/Job/CommentEventJobTest.php rename tests/units/Model/{CommentTest.php => CommentModelTest.php} (98%) diff --git a/ChangeLog b/ChangeLog index e87c0965a..a1e394360 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ Version 1.0.32 (unreleased) New features: * New automated action to close tasks without activity in a specific column +* Added new event for removed comments * Added search filter for task priority * Added the possibility to hide tasks in dashboard for a specific column diff --git a/app/Core/Base.php b/app/Core/Base.php index 8103ec14f..e413a4ac6 100644 --- a/app/Core/Base.php +++ b/app/Core/Base.php @@ -150,6 +150,8 @@ use Pimple\Container; * @property \Kanboard\Core\Filter\QueryBuilder $taskQuery * @property \Kanboard\Core\Filter\LexerBuilder $taskLexer * @property \Kanboard\Core\Filter\LexerBuilder $projectActivityLexer + * @property \Kanboard\Job\CommentEventJob $commentEventJob + * @property \Kanboard\Job\NotificationJob $notificationJob * @property \Psr\Log\LoggerInterface $logger * @property \PicoDb\Database $db * @property \Symfony\Component\EventDispatcher\EventDispatcher $dispatcher diff --git a/app/Core/Queue/JobHandler.php b/app/Core/Queue/JobHandler.php index 7ca363280..326f3cefe 100644 --- a/app/Core/Queue/JobHandler.php +++ b/app/Core/Queue/JobHandler.php @@ -2,6 +2,7 @@ namespace Kanboard\Core\Queue; +use Exception; use Kanboard\Core\Base; use Kanboard\Job\BaseJob; use SimpleQueue\Job; @@ -39,16 +40,23 @@ class JobHandler extends Base public function executeJob(Job $job) { $payload = $job->getBody(); - $className = $payload['class']; - $this->memoryCache->flush(); - $this->prepareJobSession($payload['user_id']); - if (DEBUG) { - $this->logger->debug(__METHOD__.' Received job => '.$className.' ('.getmypid().')'); + try { + $className = $payload['class']; + $this->memoryCache->flush(); + $this->prepareJobSession($payload['user_id']); + + if (DEBUG) { + $this->logger->debug(__METHOD__.' Received job => '.$className.' ('.getmypid().')'); + $this->logger->debug(__METHOD__.' => '.json_encode($payload)); + } + + $worker = new $className($this->container); + call_user_func_array(array($worker, 'execute'), $payload['params']); + } catch (Exception $e) { + $this->logger->error(__METHOD__.': Error during job execution: '.$e->getMessage()); + $this->logger->error(__METHOD__ .' => '.json_encode($payload)); } - - $worker = new $className($this->container); - call_user_func_array(array($worker, 'execute'), $payload['params']); } /** diff --git a/app/Core/Queue/QueueManager.php b/app/Core/Queue/QueueManager.php index f34cb220e..dcf0ebf51 100644 --- a/app/Core/Queue/QueueManager.php +++ b/app/Core/Queue/QueueManager.php @@ -42,9 +42,13 @@ class QueueManager extends Base */ public function push(BaseJob $job) { + $jobClassName = get_class($job); + if ($this->queue !== null) { + $this->logger->debug(__METHOD__.': Job pushed in queue: '.$jobClassName); $this->queue->push(JobHandler::getInstance($this->container)->serializeJob($job)); } else { + $this->logger->debug(__METHOD__.': Job executed synchronously: '.$jobClassName); call_user_func_array(array($job, 'execute'), $job->getJobParams()); } @@ -60,7 +64,7 @@ class QueueManager extends Base public function listen() { if ($this->queue === null) { - throw new LogicException('No Queue Driver defined!'); + throw new LogicException('No queue driver defined!'); } while ($job = $this->queue->pull()) { diff --git a/app/EventBuilder/BaseEventBuilder.php b/app/EventBuilder/BaseEventBuilder.php new file mode 100644 index 000000000..c677563e5 --- /dev/null +++ b/app/EventBuilder/BaseEventBuilder.php @@ -0,0 +1,23 @@ +commentId = $commentId; + return $this; + } + + /** + * Build event data + * + * @access public + * @return CommentEvent|null + */ + public function build() + { + $comment = $this->commentModel->getById($this->commentId); + + if (empty($comment)) { + return null; + } + + return new CommentEvent(array( + 'comment' => $comment, + 'task' => $this->taskFinderModel->getDetails($comment['task_id']), + )); + } +} diff --git a/app/Job/CommentEventJob.php b/app/Job/CommentEventJob.php new file mode 100644 index 000000000..c89350edd --- /dev/null +++ b/app/Job/CommentEventJob.php @@ -0,0 +1,50 @@ +jobParams = array($commentId, $eventName); + return $this; + } + + /** + * Execute job + * + * @param int $commentId + * @param string $eventName + * @return $this + */ + public function execute($commentId, $eventName) + { + $event = CommentEventBuilder::getInstance($this->container) + ->withCommentId($commentId) + ->build(); + + if ($event !== null) { + $this->dispatcher->dispatch($eventName, $event); + + if ($eventName === CommentModel::EVENT_CREATE) { + $this->userMentionModel->fireEvents($event['comment']['comment'], CommentModel::EVENT_USER_MENTION, $event); + } + } + } +} diff --git a/app/Job/NotificationJob.php b/app/Job/NotificationJob.php index 904a92735..cfd0699d3 100644 --- a/app/Job/NotificationJob.php +++ b/app/Job/NotificationJob.php @@ -75,8 +75,7 @@ class NotificationJob extends BaseJob $values['task'] = $this->taskFinderModel->getDetails($values['file']['task_id']); break; case 'Kanboard\Event\CommentEvent': - $values['comment'] = $this->commentModel->getById($event['id']); - $values['task'] = $this->taskFinderModel->getDetails($values['comment']['task_id']); + $values = $event; break; } diff --git a/app/Model/CommentModel.php b/app/Model/CommentModel.php index 4231f29d4..6b983858e 100644 --- a/app/Model/CommentModel.php +++ b/app/Model/CommentModel.php @@ -2,7 +2,6 @@ namespace Kanboard\Model; -use Kanboard\Event\CommentEvent; use Kanboard\Core\Base; /** @@ -27,6 +26,7 @@ class CommentModel extends Base */ const EVENT_UPDATE = 'comment.update'; const EVENT_CREATE = 'comment.create'; + const EVENT_REMOVE = 'comment.remove'; const EVENT_USER_MENTION = 'comment.user.mention'; /** @@ -130,9 +130,7 @@ class CommentModel extends Base $comment_id = $this->db->table(self::TABLE)->persist($values); if ($comment_id !== false) { - $event = new CommentEvent(array('id' => $comment_id) + $values); - $this->dispatcher->dispatch(self::EVENT_CREATE, $event); - $this->userMentionModel->fireEvents($values['comment'], self::EVENT_USER_MENTION, $event); + $this->queueManager->push($this->commentEventJob->withParams($comment_id, self::EVENT_CREATE)); } return $comment_id; @@ -153,7 +151,7 @@ class CommentModel extends Base ->update(array('comment' => $values['comment'])); if ($result) { - $this->container['dispatcher']->dispatch(self::EVENT_UPDATE, new CommentEvent($values)); + $this->queueManager->push($this->commentEventJob->withParams($values['id'], self::EVENT_UPDATE)); } return $result; @@ -168,6 +166,7 @@ class CommentModel extends Base */ public function remove($comment_id) { + $this->commentEventJob->execute($comment_id, self::EVENT_REMOVE); return $this->db->table(self::TABLE)->eq('id', $comment_id)->remove(); } } diff --git a/app/Model/NotificationModel.php b/app/Model/NotificationModel.php index 4d697b5e9..df481fc70 100644 --- a/app/Model/NotificationModel.php +++ b/app/Model/NotificationModel.php @@ -74,6 +74,8 @@ class NotificationModel extends Base return e('%s updated a comment on the task #%d', $event_author, $event_data['task']['id']); case CommentModel::EVENT_CREATE: return e('%s commented on the task #%d', $event_author, $event_data['task']['id']); + case CommentModel::EVENT_REMOVE: + return e('%s removed a comment on the task #%d', $event_author, $event_data['task']['id']); case TaskFileModel::EVENT_CREATE: return e('%s attached a file to the task #%d', $event_author, $event_data['task']['id']); case TaskModel::EVENT_USER_MENTION: @@ -102,6 +104,8 @@ class NotificationModel extends Base return e('New comment on task #%d', $event_data['comment']['task_id']); case CommentModel::EVENT_UPDATE: return e('Comment updated on task #%d', $event_data['comment']['task_id']); + case CommentModel::EVENT_REMOVE: + return e('Comment removed on task #%d', $event_data['comment']['task_id']); case SubtaskModel::EVENT_CREATE: return e('New subtask on task #%d', $event_data['subtask']['task_id']); case SubtaskModel::EVENT_UPDATE: @@ -149,6 +153,7 @@ class NotificationModel extends Base return $event_data['file']['task_id']; case CommentModel::EVENT_CREATE: case CommentModel::EVENT_UPDATE: + case CommentModel::EVENT_REMOVE: return $event_data['comment']['task_id']; case SubtaskModel::EVENT_CREATE: case SubtaskModel::EVENT_UPDATE: diff --git a/app/ServiceProvider/ClassProvider.php b/app/ServiceProvider/ClassProvider.php index e32c0d437..43ade56ec 100644 --- a/app/ServiceProvider/ClassProvider.php +++ b/app/ServiceProvider/ClassProvider.php @@ -26,6 +26,10 @@ class ClassProvider implements ServiceProviderInterface 'AverageLeadCycleTimeAnalytic', 'AverageTimeSpentColumnAnalytic', ), + 'Job' => array( + 'CommentEventJob', + 'NotificationJob', + ), 'Model' => array( 'ActionModel', 'ActionParameterModel', diff --git a/app/Subscriber/NotificationSubscriber.php b/app/Subscriber/NotificationSubscriber.php index db11e585e..6cd7675c2 100644 --- a/app/Subscriber/NotificationSubscriber.php +++ b/app/Subscriber/NotificationSubscriber.php @@ -28,6 +28,7 @@ class NotificationSubscriber extends BaseSubscriber implements EventSubscriberIn SubtaskModel::EVENT_UPDATE => 'handleEvent', CommentModel::EVENT_CREATE => 'handleEvent', CommentModel::EVENT_UPDATE => 'handleEvent', + CommentModel::EVENT_REMOVE => 'handleEvent', CommentModel::EVENT_USER_MENTION => 'handleEvent', TaskFileModel::EVENT_CREATE => 'handleEvent', ); diff --git a/app/Template/event/comment_remove.php b/app/Template/event/comment_remove.php new file mode 100644 index 000000000..ead7d56ac --- /dev/null +++ b/app/Template/event/comment_remove.php @@ -0,0 +1,11 @@ +

+ text->e($author), + $this->url->link(t('#%d', $task['id']), 'TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) + ) ?> + dt->datetime($date_creation) ?> +

+
+

text->e($task['title']) ?>

+
text->markdown($comment['comment']) ?>
+
diff --git a/app/Template/event/comment_update.php b/app/Template/event/comment_update.php index 5a0821bd0..5be598ac1 100644 --- a/app/Template/event/comment_update.php +++ b/app/Template/event/comment_update.php @@ -7,4 +7,7 @@

text->e($task['title']) ?>

+ +
text->markdown($comment['comment']) ?>
+
diff --git a/app/Template/notification/comment_remove.php b/app/Template/notification/comment_remove.php new file mode 100644 index 000000000..928623ec6 --- /dev/null +++ b/app/Template/notification/comment_remove.php @@ -0,0 +1,7 @@ +

text->e($task['title']) ?> (#)

+ +

+ +text->markdown($comment['comment']) ?> + +render('notification/footer', array('task' => $task, 'application_url' => $application_url)) ?> diff --git a/tests/units/EventBuilder/CommentEventBuilderTest.php b/tests/units/EventBuilder/CommentEventBuilderTest.php new file mode 100644 index 000000000..a490799e7 --- /dev/null +++ b/tests/units/EventBuilder/CommentEventBuilderTest.php @@ -0,0 +1,37 @@ +container); + $commentEventBuilder->withCommentId(42); + $this->assertNull($commentEventBuilder->build()); + } + + public function testBuild() + { + $commentModel = new CommentModel($this->container); + $taskCreationModel = new TaskCreationModel($this->container); + $projectModel = new ProjectModel($this->container); + $commentEventBuilder = new CommentEventBuilder($this->container); + + $this->assertEquals(1, $projectModel->create(array('name' => 'test1'))); + $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1))); + $this->assertEquals(1, $commentModel->create(array('task_id' => 1, 'comment' => 'bla bla', 'user_id' => 1))); + + $commentEventBuilder->withCommentId(1); + $event = $commentEventBuilder->build(); + + $this->assertInstanceOf('Kanboard\Event\CommentEvent', $event); + $this->assertNotEmpty($event['comment']); + $this->assertNotEmpty($event['task']); + } +} diff --git a/tests/units/Job/CommentEventJobTest.php b/tests/units/Job/CommentEventJobTest.php new file mode 100644 index 000000000..88742feb4 --- /dev/null +++ b/tests/units/Job/CommentEventJobTest.php @@ -0,0 +1,52 @@ +container); + $commentEventJob->withParams(123, 'foobar'); + + $this->assertSame(array(123, 'foobar'), $commentEventJob->getJobParams()); + } + + public function testWithMissingComment() + { + $this->container['dispatcher']->addListener(CommentModel::EVENT_CREATE, function() {}); + + $commentEventJob = new CommentEventJob($this->container); + $commentEventJob->execute(42, CommentModel::EVENT_CREATE); + + $called = $this->container['dispatcher']->getCalledListeners(); + $this->assertEmpty($called); + } + + public function testTriggerEvents() + { + $this->container['dispatcher']->addListener(CommentModel::EVENT_CREATE, function() {}); + $this->container['dispatcher']->addListener(CommentModel::EVENT_UPDATE, function() {}); + $this->container['dispatcher']->addListener(CommentModel::EVENT_REMOVE, function() {}); + + $commentModel = new CommentModel($this->container); + $taskCreationModel = new TaskCreationModel($this->container); + $projectModel = new ProjectModel($this->container); + + $this->assertEquals(1, $projectModel->create(array('name' => 'test1'))); + $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1))); + $this->assertEquals(1, $commentModel->create(array('task_id' => 1, 'comment' => 'foobar', 'user_id' => 1))); + $this->assertTrue($commentModel->update(array('id' => 1, 'comment' => 'test'))); + $this->assertTrue($commentModel->remove(1)); + + $called = $this->container['dispatcher']->getCalledListeners(); + $this->assertArrayHasKey(CommentModel::EVENT_CREATE.'.closure', $called); + $this->assertArrayHasKey(CommentModel::EVENT_UPDATE.'.closure', $called); + $this->assertArrayHasKey(CommentModel::EVENT_REMOVE.'.closure', $called); + } +} diff --git a/tests/units/Model/CommentTest.php b/tests/units/Model/CommentModelTest.php similarity index 98% rename from tests/units/Model/CommentTest.php rename to tests/units/Model/CommentModelTest.php index 574b5a878..4178a839c 100644 --- a/tests/units/Model/CommentTest.php +++ b/tests/units/Model/CommentModelTest.php @@ -6,7 +6,7 @@ use Kanboard\Model\TaskCreationModel; use Kanboard\Model\ProjectModel; use Kanboard\Model\CommentModel; -class CommentTest extends Base +class CommentModelTest extends Base { public function testCreate() { @@ -75,7 +75,7 @@ class CommentTest extends Base $this->assertEquals('bla', $comment['comment']); } - public function validateRemove() + public function testRemove() { $commentModel = new CommentModel($this->container); $taskCreationModel = new TaskCreationModel($this->container);