From d9d37882228771bca0c7f53f5ffcef90ba7ac1c5 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 17 Jul 2016 20:33:27 -0400 Subject: Subtasks events refactoring and show delete in activity stream --- app/Core/Base.php | 1 + app/EventBuilder/SubtaskEventBuilder.php | 79 ++++++++++++++++++++++ app/Job/NotificationJob.php | 4 -- app/Job/SubtaskEventJob.php | 48 +++++++++++++ app/Model/NotificationModel.php | 5 ++ app/Model/SubtaskModel.php | 22 ++---- app/ServiceProvider/ClassProvider.php | 6 -- app/ServiceProvider/JobProvider.php | 52 ++++++++++++++ app/ServiceProvider/QueueProvider.php | 6 +- app/Subscriber/NotificationSubscriber.php | 1 + app/Template/event/subtask_delete.php | 15 ++++ app/Template/notification/subtask_delete.php | 11 +++ app/common.php | 1 + tests/units/Base.php | 1 + .../units/EventBuilder/SubtaskEventBuilderTest.php | 62 +++++++++++++++++ tests/units/Job/SubtaskEventJobTest.php | 52 ++++++++++++++ tests/units/Model/SubtaskModelTest.php | 70 +------------------ 17 files changed, 339 insertions(+), 97 deletions(-) create mode 100644 app/EventBuilder/SubtaskEventBuilder.php create mode 100644 app/Job/SubtaskEventJob.php create mode 100644 app/ServiceProvider/JobProvider.php create mode 100644 app/Template/event/subtask_delete.php create mode 100644 app/Template/notification/subtask_delete.php create mode 100644 tests/units/EventBuilder/SubtaskEventBuilderTest.php create mode 100644 tests/units/Job/SubtaskEventJobTest.php diff --git a/app/Core/Base.php b/app/Core/Base.php index 09e04456..6650680b 100644 --- a/app/Core/Base.php +++ b/app/Core/Base.php @@ -151,6 +151,7 @@ use Pimple\Container; * @property \Kanboard\Core\Filter\LexerBuilder $taskLexer * @property \Kanboard\Core\Filter\LexerBuilder $projectActivityLexer * @property \Kanboard\Job\CommentEventJob $commentEventJob + * @property \Kanboard\Job\SubtaskEventJob $subtaskEventJob * @property \Kanboard\Job\TaskFileEventJob $taskFileEventJob * @property \Kanboard\Job\ProjectFileEventJob $projectFileEventJob * @property \Kanboard\Job\NotificationJob $notificationJob diff --git a/app/EventBuilder/SubtaskEventBuilder.php b/app/EventBuilder/SubtaskEventBuilder.php new file mode 100644 index 00000000..f0271257 --- /dev/null +++ b/app/EventBuilder/SubtaskEventBuilder.php @@ -0,0 +1,79 @@ +subtaskId = $subtaskId; + return $this; + } + + /** + * Set values + * + * @param array $values + * @return $this + */ + public function withValues(array $values) + { + $this->values = $values; + return $this; + } + + /** + * Build event data + * + * @access public + * @return GenericEvent|null + */ + public function build() + { + $eventData = array(); + $eventData['subtask'] = $this->subtaskModel->getById($this->subtaskId, true); + + if (empty($eventData['subtask'])) { + $this->logger->debug(__METHOD__.': Subtask not found'); + return null; + } + + if (! empty($this->values)) { + $eventData['changes'] = array_diff_assoc($this->values, $eventData['subtask']); + } + + $eventData['task'] = $this->taskFinderModel->getDetails($eventData['subtask']['task_id']); + return new SubtaskEvent($eventData); + } +} diff --git a/app/Job/NotificationJob.php b/app/Job/NotificationJob.php index ed568e47..5a52eb31 100644 --- a/app/Job/NotificationJob.php +++ b/app/Job/NotificationJob.php @@ -66,10 +66,6 @@ class NotificationJob extends BaseJob case 'Kanboard\Event\TaskEvent': $values['task'] = $this->taskFinderModel->getDetails($event['task_id']); break; - case 'Kanboard\Event\SubtaskEvent': - $values['subtask'] = $this->subtaskModel->getById($event['id'], true); - $values['task'] = $this->taskFinderModel->getDetails($values['subtask']['task_id']); - break; default: $values = $event; } diff --git a/app/Job/SubtaskEventJob.php b/app/Job/SubtaskEventJob.php new file mode 100644 index 00000000..1dc243ef --- /dev/null +++ b/app/Job/SubtaskEventJob.php @@ -0,0 +1,48 @@ +jobParams = array($subtaskId, $eventName, $values); + return $this; + } + + /** + * Execute job + * + * @param int $subtaskId + * @param string $eventName + * @param array $values + * @return $this + */ + public function execute($subtaskId, $eventName, array $values = array()) + { + $event = SubtaskEventBuilder::getInstance($this->container) + ->withSubtaskId($subtaskId) + ->withValues($values) + ->build(); + + if ($event !== null) { + $this->dispatcher->dispatch($eventName, $event); + } + } +} diff --git a/app/Model/NotificationModel.php b/app/Model/NotificationModel.php index df481fc7..ac8d9bae 100644 --- a/app/Model/NotificationModel.php +++ b/app/Model/NotificationModel.php @@ -70,6 +70,8 @@ class NotificationModel extends Base return e('%s updated a subtask for the task #%d', $event_author, $event_data['task']['id']); case SubtaskModel::EVENT_CREATE: return e('%s created a subtask for the task #%d', $event_author, $event_data['task']['id']); + case SubtaskModel::EVENT_DELETE: + return e('%s removed a subtask for the task #%d', $event_author, $event_data['task']['id']); case CommentModel::EVENT_UPDATE: return e('%s updated a comment on the task #%d', $event_author, $event_data['task']['id']); case CommentModel::EVENT_CREATE: @@ -110,6 +112,8 @@ class NotificationModel extends Base return e('New subtask on task #%d', $event_data['subtask']['task_id']); case SubtaskModel::EVENT_UPDATE: return e('Subtask updated on task #%d', $event_data['subtask']['task_id']); + case SubtaskModel::EVENT_DELETE: + return e('Subtask removed on task #%d', $event_data['subtask']['task_id']); case TaskModel::EVENT_CREATE: return e('New task #%d: %s', $event_data['task']['id'], $event_data['task']['title']); case TaskModel::EVENT_UPDATE: @@ -157,6 +161,7 @@ class NotificationModel extends Base return $event_data['comment']['task_id']; case SubtaskModel::EVENT_CREATE: case SubtaskModel::EVENT_UPDATE: + case SubtaskModel::EVENT_DELETE: return $event_data['subtask']['task_id']; case TaskModel::EVENT_CREATE: case TaskModel::EVENT_UPDATE: diff --git a/app/Model/SubtaskModel.php b/app/Model/SubtaskModel.php index a97bddbf..6dd1f26a 100644 --- a/app/Model/SubtaskModel.php +++ b/app/Model/SubtaskModel.php @@ -66,7 +66,7 @@ class SubtaskModel extends Base ->join(TaskModel::TABLE, 'id', 'task_id') ->findOneColumn(TaskModel::TABLE . '.project_id') ?: 0; } - + /** * Get available status * @@ -235,10 +235,7 @@ class SubtaskModel extends Base $subtask_id = $this->db->table(self::TABLE)->persist($values); if ($subtask_id !== false) { - $this->container['dispatcher']->dispatch( - self::EVENT_CREATE, - new SubtaskEvent(array('id' => $subtask_id) + $values) - ); + $this->queueManager->push($this->subtaskEventJob->withParams($subtask_id, self::EVENT_CREATE)); } return $subtask_id; @@ -255,13 +252,10 @@ class SubtaskModel extends Base public function update(array $values, $fire_events = true) { $this->prepare($values); - $subtask = $this->getById($values['id']); $result = $this->db->table(self::TABLE)->eq('id', $values['id'])->save($values); if ($result && $fire_events) { - $event = $subtask; - $event['changes'] = array_diff_assoc($values, $subtask); - $this->container['dispatcher']->dispatch(self::EVENT_UPDATE, new SubtaskEvent($event)); + $this->queueManager->push($this->subtaskEventJob->withParams($values['id'], self::EVENT_UPDATE, $values)); } return $result; @@ -377,14 +371,8 @@ class SubtaskModel extends Base */ public function remove($subtask_id) { - $subtask = $this->getById($subtask_id); - $result = $this->db->table(self::TABLE)->eq('id', $subtask_id)->remove(); - - if ($result) { - $this->container['dispatcher']->dispatch(self::EVENT_DELETE, new SubtaskEvent($subtask)); - } - - return $result; + $this->subtaskEventJob->execute($subtask_id, self::EVENT_DELETE); + return $this->db->table(self::TABLE)->eq('id', $subtask_id)->remove(); } /** diff --git a/app/ServiceProvider/ClassProvider.php b/app/ServiceProvider/ClassProvider.php index 428a8015..e32c0d43 100644 --- a/app/ServiceProvider/ClassProvider.php +++ b/app/ServiceProvider/ClassProvider.php @@ -26,12 +26,6 @@ class ClassProvider implements ServiceProviderInterface 'AverageLeadCycleTimeAnalytic', 'AverageTimeSpentColumnAnalytic', ), - 'Job' => array( - 'CommentEventJob', - 'TaskFileEventJob', - 'ProjectFileEventJob', - 'NotificationJob', - ), 'Model' => array( 'ActionModel', 'ActionParameterModel', diff --git a/app/ServiceProvider/JobProvider.php b/app/ServiceProvider/JobProvider.php new file mode 100644 index 00000000..bfea6e6e --- /dev/null +++ b/app/ServiceProvider/JobProvider.php @@ -0,0 +1,52 @@ +factory(function ($c) { + return new CommentEventJob($c); + }); + + $container['subtaskEventJob'] = $container->factory(function ($c) { + return new SubtaskEventJob($c); + }); + + $container['taskFileEventJob'] = $container->factory(function ($c) { + return new TaskFileEventJob($c); + }); + + $container['projectFileEventJob'] = $container->factory(function ($c) { + return new ProjectFileEventJob($c); + }); + + $container['notificationJob'] = $container->factory(function ($c) { + return new NotificationJob($c); + }); + + return $container; + } +} diff --git a/app/ServiceProvider/QueueProvider.php b/app/ServiceProvider/QueueProvider.php index 946b436a..570f2e77 100644 --- a/app/ServiceProvider/QueueProvider.php +++ b/app/ServiceProvider/QueueProvider.php @@ -15,9 +15,11 @@ use Pimple\ServiceProviderInterface; class QueueProvider implements ServiceProviderInterface { /** - * Registers services on the given container. + * Register providers * - * @param Container $container + * @access public + * @param \Pimple\Container $container + * @return \Pimple\Container */ public function register(Container $container) { diff --git a/app/Subscriber/NotificationSubscriber.php b/app/Subscriber/NotificationSubscriber.php index 47222f73..0b3760c4 100644 --- a/app/Subscriber/NotificationSubscriber.php +++ b/app/Subscriber/NotificationSubscriber.php @@ -26,6 +26,7 @@ class NotificationSubscriber extends BaseSubscriber implements EventSubscriberIn TaskModel::EVENT_ASSIGNEE_CHANGE => 'handleEvent', SubtaskModel::EVENT_CREATE => 'handleEvent', SubtaskModel::EVENT_UPDATE => 'handleEvent', + SubtaskModel::EVENT_DELETE => 'handleEvent', CommentModel::EVENT_CREATE => 'handleEvent', CommentModel::EVENT_UPDATE => 'handleEvent', CommentModel::EVENT_REMOVE => 'handleEvent', diff --git a/app/Template/event/subtask_delete.php b/app/Template/event/subtask_delete.php new file mode 100644 index 00000000..8ac11853 --- /dev/null +++ b/app/Template/event/subtask_delete.php @@ -0,0 +1,15 @@ +

+ 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']) ?>

+ +
diff --git a/app/Template/notification/subtask_delete.php b/app/Template/notification/subtask_delete.php new file mode 100644 index 00000000..8c5f262c --- /dev/null +++ b/app/Template/notification/subtask_delete.php @@ -0,0 +1,11 @@ +

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

+ +

+ + + +render('notification/footer', array('task' => $task, 'application_url' => $application_url)) ?> diff --git a/app/common.php b/app/common.php index 72be3603..15fd7a75 100644 --- a/app/common.php +++ b/app/common.php @@ -46,6 +46,7 @@ $container->register(new Kanboard\ServiceProvider\ActionProvider()); $container->register(new Kanboard\ServiceProvider\ExternalLinkProvider()); $container->register(new Kanboard\ServiceProvider\AvatarProvider()); $container->register(new Kanboard\ServiceProvider\FilterProvider()); +$container->register(new Kanboard\ServiceProvider\JobProvider()); $container->register(new Kanboard\ServiceProvider\QueueProvider()); $container->register(new Kanboard\ServiceProvider\ApiProvider()); $container->register(new Kanboard\ServiceProvider\CommandProvider()); diff --git a/tests/units/Base.php b/tests/units/Base.php index 9dbfb280..c471ee31 100644 --- a/tests/units/Base.php +++ b/tests/units/Base.php @@ -41,6 +41,7 @@ abstract class Base extends PHPUnit_Framework_TestCase $this->container->register(new Kanboard\ServiceProvider\RouteProvider()); $this->container->register(new Kanboard\ServiceProvider\AvatarProvider()); $this->container->register(new Kanboard\ServiceProvider\FilterProvider()); + $this->container->register(new Kanboard\ServiceProvider\JobProvider()); $this->container->register(new Kanboard\ServiceProvider\QueueProvider()); $this->container['dispatcher'] = new TraceableEventDispatcher( diff --git a/tests/units/EventBuilder/SubtaskEventBuilderTest.php b/tests/units/EventBuilder/SubtaskEventBuilderTest.php new file mode 100644 index 00000000..062bdfb4 --- /dev/null +++ b/tests/units/EventBuilder/SubtaskEventBuilderTest.php @@ -0,0 +1,62 @@ +container); + $subtaskEventBuilder->withSubtaskId(42); + $this->assertNull($subtaskEventBuilder->build()); + } + + public function testBuildWithoutChanges() + { + $subtaskModel = new SubtaskModel($this->container); + $taskCreationModel = new TaskCreationModel($this->container); + $projectModel = new ProjectModel($this->container); + $subtaskEventBuilder = new SubtaskEventBuilder($this->container); + + $this->assertEquals(1, $projectModel->create(array('name' => 'test1'))); + $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1))); + $this->assertEquals(1, $subtaskModel->create(array('task_id' => 1, 'title' => 'test'))); + + $event = $subtaskEventBuilder->withSubtaskId(1)->build(); + + $this->assertInstanceOf('Kanboard\Event\SubtaskEvent', $event); + $this->assertNotEmpty($event['subtask']); + $this->assertNotEmpty($event['task']); + $this->assertArrayNotHasKey('changes', $event); + } + + public function testBuildWithChanges() + { + $subtaskModel = new SubtaskModel($this->container); + $taskCreationModel = new TaskCreationModel($this->container); + $projectModel = new ProjectModel($this->container); + $subtaskEventBuilder = new SubtaskEventBuilder($this->container); + + $this->assertEquals(1, $projectModel->create(array('name' => 'test1'))); + $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test', 'project_id' => 1))); + $this->assertEquals(1, $subtaskModel->create(array('task_id' => 1, 'title' => 'test'))); + + $event = $subtaskEventBuilder + ->withSubtaskId(1) + ->withValues(array('title' => 'new title', 'user_id' => 1)) + ->build(); + + $this->assertInstanceOf('Kanboard\Event\SubtaskEvent', $event); + $this->assertNotEmpty($event['subtask']); + $this->assertNotEmpty($event['task']); + $this->assertNotEmpty($event['changes']); + $this->assertCount(2, $event['changes']); + $this->assertEquals('new title', $event['changes']['title']); + $this->assertEquals(1, $event['changes']['user_id']); + } +} diff --git a/tests/units/Job/SubtaskEventJobTest.php b/tests/units/Job/SubtaskEventJobTest.php new file mode 100644 index 00000000..265a8e2d --- /dev/null +++ b/tests/units/Job/SubtaskEventJobTest.php @@ -0,0 +1,52 @@ +container); + $subtaskEventJob->withParams(123, 'foobar', array('k' => 'v')); + + $this->assertSame(array(123, 'foobar', array('k' => 'v')), $subtaskEventJob->getJobParams()); + } + + public function testWithMissingSubtask() + { + $this->container['dispatcher']->addListener(SubtaskModel::EVENT_CREATE, function() {}); + + $SubtaskEventJob = new SubtaskEventJob($this->container); + $SubtaskEventJob->execute(42, SubtaskModel::EVENT_CREATE); + + $called = $this->container['dispatcher']->getCalledListeners(); + $this->assertEmpty($called); + } + + public function testTriggerEvents() + { + $this->container['dispatcher']->addListener(SubtaskModel::EVENT_CREATE, function() {}); + $this->container['dispatcher']->addListener(SubtaskModel::EVENT_UPDATE, function() {}); + $this->container['dispatcher']->addListener(SubtaskModel::EVENT_DELETE, function() {}); + + $subtaskModel = new SubtaskModel($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, $subtaskModel->create(array('task_id' => 1, 'title' => 'before'))); + $this->assertTrue($subtaskModel->update(array('id' => 1, 'title' => 'after'))); + $this->assertTrue($subtaskModel->remove(1)); + + $called = $this->container['dispatcher']->getCalledListeners(); + $this->assertArrayHasKey(SubtaskModel::EVENT_CREATE.'.closure', $called); + $this->assertArrayHasKey(SubtaskModel::EVENT_UPDATE.'.closure', $called); + $this->assertArrayHasKey(SubtaskModel::EVENT_DELETE.'.closure', $called); + } +} diff --git a/tests/units/Model/SubtaskModelTest.php b/tests/units/Model/SubtaskModelTest.php index 6451189d..7e438651 100644 --- a/tests/units/Model/SubtaskModelTest.php +++ b/tests/units/Model/SubtaskModelTest.php @@ -9,64 +9,6 @@ use Kanboard\Model\TaskFinderModel; class SubtaskModelTest extends Base { - public function onSubtaskCreated($event) - { - $this->assertInstanceOf('Kanboard\Event\SubtaskEvent', $event); - $data = $event->getAll(); - - $this->assertArrayHasKey('id', $data); - $this->assertArrayHasKey('title', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('time_estimated', $data); - $this->assertArrayHasKey('time_spent', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('task_id', $data); - $this->assertArrayHasKey('user_id', $data); - $this->assertArrayHasKey('position', $data); - $this->assertNotEmpty($data['task_id']); - $this->assertNotEmpty($data['id']); - } - - public function onSubtaskUpdated($event) - { - $this->assertInstanceOf('Kanboard\Event\SubtaskEvent', $event); - $data = $event->getAll(); - - $this->assertArrayHasKey('id', $data); - $this->assertArrayHasKey('title', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('time_estimated', $data); - $this->assertArrayHasKey('time_spent', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('task_id', $data); - $this->assertArrayHasKey('user_id', $data); - $this->assertArrayHasKey('position', $data); - $this->assertArrayHasKey('changes', $data); - $this->assertArrayHasKey('user_id', $data['changes']); - $this->assertArrayHasKey('status', $data['changes']); - - $this->assertEquals(SubtaskModel::STATUS_INPROGRESS, $data['changes']['status']); - $this->assertEquals(1, $data['changes']['user_id']); - } - - public function onSubtaskDeleted($event) - { - $this->assertInstanceOf('Kanboard\Event\SubtaskEvent', $event); - $data = $event->getAll(); - - $this->assertArrayHasKey('id', $data); - $this->assertArrayHasKey('title', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('time_estimated', $data); - $this->assertArrayHasKey('time_spent', $data); - $this->assertArrayHasKey('status', $data); - $this->assertArrayHasKey('task_id', $data); - $this->assertArrayHasKey('user_id', $data); - $this->assertArrayHasKey('position', $data); - $this->assertNotEmpty($data['task_id']); - $this->assertNotEmpty($data['id']); - } - public function testCreation() { $taskCreationModel = new TaskCreationModel($this->container); @@ -75,9 +17,6 @@ class SubtaskModelTest extends Base $this->assertEquals(1, $projectModel->create(array('name' => 'test'))); $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test 1', 'project_id' => 1))); - - $this->container['dispatcher']->addListener(SubtaskModel::EVENT_CREATE, array($this, 'onSubtaskCreated')); - $this->assertEquals(1, $subtaskModel->create(array('title' => 'subtask #1', 'task_id' => 1))); $subtask = $subtaskModel->getById(1); @@ -101,8 +40,6 @@ class SubtaskModelTest extends Base $this->assertEquals(1, $projectModel->create(array('name' => 'test'))); $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test 1', 'project_id' => 1))); - $this->container['dispatcher']->addListener(SubtaskModel::EVENT_UPDATE, array($this, 'onSubtaskUpdated')); - $this->assertEquals(1, $subtaskModel->create(array('title' => 'subtask #1', 'task_id' => 1))); $this->assertTrue($subtaskModel->update(array('id' => 1, 'user_id' => 1, 'status' => SubtaskModel::STATUS_INPROGRESS))); @@ -128,8 +65,6 @@ class SubtaskModelTest extends Base $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test 1', 'project_id' => 1))); $this->assertEquals(1, $subtaskModel->create(array('title' => 'subtask #1', 'task_id' => 1))); - $this->container['dispatcher']->addListener(SubtaskModel::EVENT_DELETE, array($this, 'onSubtaskDeleted')); - $subtask = $subtaskModel->getById(1); $this->assertNotEmpty($subtask); @@ -269,7 +204,6 @@ class SubtaskModelTest extends Base $this->assertTrue($subtaskModel->duplicate(1, 2)); $subtasks = $subtaskModel->getAll(2); - $this->assertNotFalse($subtasks); $this->assertNotEmpty($subtasks); $this->assertEquals(2, count($subtasks)); @@ -383,7 +317,7 @@ class SubtaskModelTest extends Base $this->assertEquals(2, $task['time_spent']); $this->assertEquals(3, $task['time_estimated']); } - + public function testGetProjectId() { $taskCreationModel = new TaskCreationModel($this->container); @@ -393,7 +327,7 @@ class SubtaskModelTest extends Base $this->assertEquals(1, $projectModel->create(array('name' => 'test1'))); $this->assertEquals(1, $taskCreationModel->create(array('title' => 'test 1', 'project_id' => 1))); $this->assertEquals(1, $subtaskModel->create(array('title' => 'subtask #1', 'task_id' => 1))); - + $this->assertEquals(1, $subtaskModel->getProjectId(1)); $this->assertEquals(0, $subtaskModel->getProjectId(2)); } -- cgit v1.2.3