Subtasks events refactoring and show delete in activity stream

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

app/EventBuilder/SubtaskEventBuilder.php

@@ -0,0 +1,79 @@
+<?php
+
+namespace Kanboard\EventBuilder;
+
+use Kanboard\Event\SubtaskEvent;
+use Kanboard\Event\GenericEvent;
+
+/**
+ * Class SubtaskEventBuilder
+ *
+ * @package Kanboard\EventBuilder
+ * @author  Frederic Guillot
+ */
+class SubtaskEventBuilder extends BaseEventBuilder
+{
+    /**
+     * SubtaskId
+     *
+     * @access protected
+     * @var int
+     */
+    protected $subtaskId = 0;
+
+    /**
+     * Changed values
+     *
+     * @access protected
+     * @var array
+     */
+    protected $values = array();
+
+    /**
+     * Set SubtaskId
+     *
+     * @param  int $subtaskId
+     * @return $this
+     */
+    public function withSubtaskId($subtaskId)
+    {
+        $this->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);
+    }
+}

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;
         }

app/Job/SubtaskEventJob.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Kanboard\Job;
+
+use Kanboard\EventBuilder\SubtaskEventBuilder;
+
+/**
+ * Class SubtaskEventJob
+ *
+ * @package Kanboard\Job
+ * @author  Frederic Guillot
+ */
+class SubtaskEventJob extends BaseJob
+{
+    /**
+     * Set job params
+     *
+     * @param  int     $subtaskId
+     * @param  string  $eventName
+     * @param  array   $values
+     * @return $this
+     */
+    public function withParams($subtaskId, $eventName, array $values = array())
+    {
+        $this->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);
+        }
+    }
+}

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:

app/Model/SubtaskModel.php

@@ -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(
+            $this->queueManager->push($this->subtaskEventJob->withParams($subtask_id, self::EVENT_CREATE));
-                self::EVENT_CREATE,
-                new SubtaskEvent(array('id' => $subtask_id) + $values)
-            );
         }
 
         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;
+            $this->queueManager->push($this->subtaskEventJob->withParams($values['id'], self::EVENT_UPDATE, $values));
-            $event['changes'] = array_diff_assoc($values, $subtask);
-            $this->container['dispatcher']->dispatch(self::EVENT_UPDATE, new SubtaskEvent($event));
         }
 
         return $result;
@@ -377,14 +371,8 @@ class SubtaskModel extends Base
      */
     public function remove($subtask_id)
     {
-        $subtask = $this->getById($subtask_id);
+        $this->subtaskEventJob->execute($subtask_id, self::EVENT_DELETE);
-        $result = $this->db->table(self::TABLE)->eq('id', $subtask_id)->remove();
+        return $this->db->table(self::TABLE)->eq('id', $subtask_id)->remove();
-
-        if ($result) {
-            $this->container['dispatcher']->dispatch(self::EVENT_DELETE, new SubtaskEvent($subtask));
-        }
-
-        return $result;
     }
 
     /**

app/ServiceProvider/ClassProvider.php

@@ -26,12 +26,6 @@ class ClassProvider implements ServiceProviderInterface
             'AverageLeadCycleTimeAnalytic',
             'AverageTimeSpentColumnAnalytic',
         ),
-        'Job' => array(
-            'CommentEventJob',
-            'TaskFileEventJob',
-            'ProjectFileEventJob',
-            'NotificationJob',
-        ),
         'Model' => array(
             'ActionModel',
             'ActionParameterModel',

app/ServiceProvider/JobProvider.php

@@ -0,0 +1,52 @@
+<?php
+
+namespace Kanboard\ServiceProvider;
+
+use Kanboard\Job\CommentEventJob;
+use Kanboard\Job\NotificationJob;
+use Kanboard\Job\ProjectFileEventJob;
+use Kanboard\Job\SubtaskEventJob;
+use Kanboard\Job\TaskFileEventJob;
+use Pimple\Container;
+use Pimple\ServiceProviderInterface;
+
+/**
+ * Class JobProvider
+ *
+ * @package Kanboard\ServiceProvider
+ * @author  Frederic Guillot
+ */
+class JobProvider implements ServiceProviderInterface
+{
+    /**
+     * Register providers
+     *
+     * @access public
+     * @param  \Pimple\Container $container
+     * @return \Pimple\Container
+     */
+    public function register(Container $container)
+    {
+        $container['commentEventJob'] = $container->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;
+    }
+}

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)
     {

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',

app/Template/event/subtask_delete.php

@@ -0,0 +1,15 @@
+<p class="activity-title">
+    <?= e('%s removed a subtask for the task %s',
+        $this->text->e($author),
+        $this->url->link(t('#%d', $task['id']), 'TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']))
+    ) ?>
+    <span class="activity-date"><?= $this->dt->datetime($date_creation) ?></span>
+</p>
+<div class="activity-description">
+    <p class="activity-task-title"><?= $this->text->e($task['title']) ?></p>
+    <ul>
+        <li>
+            <?= $this->text->e($subtask['title']) ?> (<strong><?= $this->text->e($subtask['status_name']) ?></strong>)
+        </li>
+    </ul>
+</div>

app/Template/notification/subtask_delete.php

@@ -0,0 +1,11 @@
+<h2><?= $this->text->e($task['title']) ?> (#<?= $task['id'] ?>)</h2>
+
+<h3><?= t('Subtask removed') ?></h3>
+
+<ul>
+    <li><?= t('Title:') ?> <?= $this->text->e($subtask['title']) ?></li>
+    <li><?= t('Status:') ?> <?= $this->text->e($subtask['status_name']) ?></li>
+    <li><?= t('Assignee:') ?> <?= $this->text->e($subtask['name'] ?: $subtask['username'] ?: '?') ?></li>
+</ul>
+
+<?= $this->render('notification/footer', array('task' => $task, 'application_url' => $application_url)) ?>

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());

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(

tests/units/EventBuilder/SubtaskEventBuilderTest.php

@@ -0,0 +1,62 @@
+<?php
+
+use Kanboard\EventBuilder\SubtaskEventBuilder;
+use Kanboard\Model\ProjectModel;
+use Kanboard\Model\SubtaskModel;
+use Kanboard\Model\TaskCreationModel;
+
+require_once __DIR__.'/../Base.php';
+
+class SubtaskEventBuilderTest extends Base
+{
+    public function testWithMissingSubtask()
+    {
+        $subtaskEventBuilder = new SubtaskEventBuilder($this->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']);
+    }
+}

tests/units/Job/SubtaskEventJobTest.php

@@ -0,0 +1,52 @@
+<?php
+
+use Kanboard\Job\SubtaskEventJob;
+use Kanboard\Model\SubtaskModel;
+use Kanboard\Model\ProjectModel;
+use Kanboard\Model\TaskCreationModel;
+
+require_once __DIR__.'/../Base.php';
+
+class SubtaskEventJobTest extends Base
+{
+    public function testJobParams()
+    {
+        $subtaskEventJob = new SubtaskEventJob($this->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);
+    }
+}

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));