From 4003b122d085b58ad7acb31bafa44121ed94c37f Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Wed, 23 Dec 2015 15:39:37 +0100 Subject: Improving performance during task position change (3 times faster than before) --- app/Model/TaskPosition.php | 180 ++++++++++++++++----------------------------- 1 file changed, 64 insertions(+), 116 deletions(-) (limited to 'app/Model/TaskPosition.php') diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index da363cb3..0a8f6c4c 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -26,28 +26,34 @@ class TaskPosition extends Base */ public function movePosition($project_id, $task_id, $column_id, $position, $swimlane_id = 0, $fire_events = true) { + $result = false; + if ($position < 1) { - return false; + return $result; } $task = $this->taskFinder->getById($task_id); - // Ignore closed tasks if ($task['is_active'] == Task::STATUS_CLOSED) { return true; } - $result = false; + $this->db->startTransaction(); - if ($task['swimlane_id'] != $swimlane_id) { - $result = $this->saveSwimlaneChange($project_id, $task_id, $position, $task['column_id'], $column_id, $task['swimlane_id'], $swimlane_id); - } elseif ($task['column_id'] != $column_id) { - $result = $this->saveColumnChange($project_id, $task_id, $position, $swimlane_id, $task['column_id'], $column_id); + if ($task['swimlane_id'] != $swimlane_id || $task['column_id'] != $column_id) { + $result = $this->moveTaskToAnotherColumn($task, $swimlane_id, $column_id, $position); } elseif ($task['position'] != $position) { - $result = $this->savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id); + $result = $this->moveTaskWithinSameColumn($task, $position); } - if ($result && $fire_events) { + if (! $result) { + $this->db->cancelTransaction(); + return false; + } + + $this->db->closeTransaction(); + + if ($fire_events) { $this->fireEvents($task, $column_id, $position, $swimlane_id); } @@ -55,145 +61,87 @@ class TaskPosition extends Base } /** - * Move a task to another swimlane + * Move a task to another column/swimlane * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $original_column_id - * @param integer $new_column_id - * @param integer $original_swimlane_id - * @param integer $new_swimlane_id + * @param array $task + * @param integer $swimlane_id + * @param integer $column_id + * @param integer $position * @return boolean */ - private function saveSwimlaneChange($project_id, $task_id, $position, $original_column_id, $new_column_id, $original_swimlane_id, $new_swimlane_id) + private function moveTaskToAnotherColumn(array $task, $swimlane_id, $column_id, $position) { - $this->db->startTransaction(); - $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $original_swimlane_id); - $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $new_swimlane_id); - $this->db->closeTransaction(); + $results = array(); + $max = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->count(); + $position = $max > 0 && $position > $max ? $max + 1 : $position; - return $r1 && $r2; - } + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gt('position', $task['position'])->decrement('position', 1); + $results[] = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->gte('position', $position)->increment('position', 1); + $results[] = $this->updateTaskPosition($task['id'], $swimlane_id, $column_id, $position); - /** - * Move a task to another column - * - * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $swimlane_id - * @param integer $original_column_id - * @param integer $new_column_id - * @return boolean - */ - private function saveColumnChange($project_id, $task_id, $position, $swimlane_id, $original_column_id, $new_column_id) - { - $this->db->startTransaction(); - $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $swimlane_id); - $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $swimlane_id); - $this->db->closeTransaction(); - - return $r1 && $r2; + return !in_array(false, $results, true); } /** - * Move a task to another position in the same column + * Move a task within the same column * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id + * @param array $task + * @param integer $position * @return boolean */ - private function savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id) + private function moveTaskWithinSameColumn(array $task, $position) { - $this->db->startTransaction(); - $result = $this->saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id); - $this->db->closeTransaction(); + $results = array(); + $max = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->count(); + $position = $max > 0 && $position > $max ? $max : $position; + + if ($position >= $max) { + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->lte('position', $position)->decrement('position', 1); + } else { + $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gte('position', $position)->increment('position', 1); + } - return $result; + $results[] = $this->updateTaskPosition($task['id'], $task['swimlane_id'], $task['column_id'], $position); + + return !in_array(false, $results, true); } /** - * Save all task positions for one column + * Update final task position * * @access private - * @param integer $project_id - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id + * @param integer $task_id + * @param integer $swimlane_id + * @param integer $column_id + * @param integer $position * @return boolean */ - private function saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id) + private function updateTaskPosition($task_id, $swimlane_id, $column_id, $position) { - $tasks_ids = $this->db->table(Task::TABLE) + return $this->db->table(Task::TABLE) + ->eq('id', $task_id) ->eq('is_active', 1) - ->eq('swimlane_id', $swimlane_id) - ->eq('project_id', $project_id) - ->eq('column_id', $column_id) - ->neq('id', $task_id) - ->asc('position') - ->asc('id') - ->findAllByColumn('id'); - - $offset = 1; - - foreach ($tasks_ids as $current_task_id) { - - // Insert the new task - if ($position == $offset) { - if (! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - $offset++; - } - - // Rewrite other tasks position - if (! $this->saveTaskPosition($current_task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - - $offset++; - } - - // Insert the new task at the bottom and normalize bad position - if ($position >= $offset && ! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { - return false; - } - - return true; + ->update(array('position' => $position, 'column_id' => $column_id, 'swimlane_id' => $swimlane_id)); } /** - * Save new task position + * Get common query * * @access private - * @param integer $task_id - * @param integer $position - * @param integer $column_id - * @param integer $swimlane_id - * @return boolean + * @param integer $project_id + * @param integer $swimlane_id + * @param integer $column_id + * @return \PicoDb\Table */ - private function saveTaskPosition($task_id, $position, $column_id, $swimlane_id) + private function getQuery($project_id, $swimlane_id, $column_id) { - $result = $this->db->table(Task::TABLE)->eq('id', $task_id)->update(array( - 'position' => $position, - 'column_id' => $column_id, - 'swimlane_id' => $swimlane_id, - )); - - if (! $result) { - $this->db->cancelTransaction(); - return false; - } - - return true; + return $this->db->table(Task::TABLE) + ->eq('project_id', $project_id) + ->eq('swimlane_id', $swimlane_id) + ->eq('column_id', $column_id) + ->eq('is_active', 1); } /** -- cgit v1.2.3 From b4c5e36ee4cbf34020eb043e54e9b7294062acee Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Tue, 29 Dec 2015 09:42:32 +0100 Subject: Update modification date during task move --- app/Model/TaskPosition.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/Model/TaskPosition.php') diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index 0a8f6c4c..53193c9b 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -123,7 +123,7 @@ class TaskPosition extends Base return $this->db->table(Task::TABLE) ->eq('id', $task_id) ->eq('is_active', 1) - ->update(array('position' => $position, 'column_id' => $column_id, 'swimlane_id' => $swimlane_id)); + ->update(array('position' => $position, 'column_id' => $column_id, 'swimlane_id' => $swimlane_id, 'date_modification' => time())); } /** -- cgit v1.2.3 From 6a0895ef765ea7b83df02bb9789fda7415dee9a5 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sat, 16 Jan 2016 19:59:44 -0500 Subject: Remove class TaskMovedDateSubscriber --- ChangeLog | 1 + app/Model/TaskPosition.php | 10 ++- app/ServiceProvider/EventDispatcherProvider.php | 1 - app/Subscriber/TaskMovedDateSubscriber.php | 25 -------- tests/units/Model/TaskMovedDateSubscriberTest.php | 77 ----------------------- 5 files changed, 10 insertions(+), 104 deletions(-) delete mode 100644 app/Subscriber/TaskMovedDateSubscriber.php delete mode 100644 tests/units/Model/TaskMovedDateSubscriberTest.php (limited to 'app/Model/TaskPosition.php') diff --git a/ChangeLog b/ChangeLog index f11edbac..46669325 100644 --- a/ChangeLog +++ b/ChangeLog @@ -14,6 +14,7 @@ Improvements: * Move validators to a separate namespace * Improve and write unit tests for reports * Reduce the number of SQL queries for project daily column stats +* Remove subscriber to update date_moved field Bug fixes: diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index 53193c9b..762f7fe3 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -120,10 +120,18 @@ class TaskPosition extends Base */ private function updateTaskPosition($task_id, $swimlane_id, $column_id, $position) { + $now = time(); + return $this->db->table(Task::TABLE) ->eq('id', $task_id) ->eq('is_active', 1) - ->update(array('position' => $position, 'column_id' => $column_id, 'swimlane_id' => $swimlane_id, 'date_modification' => time())); + ->update(array( + 'position' => $position, + 'column_id' => $column_id, + 'swimlane_id' => $swimlane_id, + 'date_modification' => $now, + 'date_moved' => $now, + )); } /** diff --git a/app/ServiceProvider/EventDispatcherProvider.php b/app/ServiceProvider/EventDispatcherProvider.php index 6cb302e6..8280a138 100644 --- a/app/ServiceProvider/EventDispatcherProvider.php +++ b/app/ServiceProvider/EventDispatcherProvider.php @@ -26,7 +26,6 @@ class EventDispatcherProvider implements ServiceProviderInterface $container['dispatcher']->addSubscriber(new ProjectModificationDateSubscriber($container)); $container['dispatcher']->addSubscriber(new NotificationSubscriber($container)); $container['dispatcher']->addSubscriber(new SubtaskTimeTrackingSubscriber($container)); - $container['dispatcher']->addSubscriber(new TaskMovedDateSubscriber($container)); $container['dispatcher']->addSubscriber(new TransitionSubscriber($container)); $container['dispatcher']->addSubscriber(new RecurringTaskSubscriber($container)); diff --git a/app/Subscriber/TaskMovedDateSubscriber.php b/app/Subscriber/TaskMovedDateSubscriber.php deleted file mode 100644 index 9857f4b3..00000000 --- a/app/Subscriber/TaskMovedDateSubscriber.php +++ /dev/null @@ -1,25 +0,0 @@ - array('execute', 0), - Task::EVENT_MOVE_SWIMLANE => array('execute', 0), - ); - } - - public function execute(TaskEvent $event) - { - if (isset($event['task_id'])) { - $this->container['db']->table(Task::TABLE)->eq('id', $event['task_id'])->update(array('date_moved' => time())); - } - } -} diff --git a/tests/units/Model/TaskMovedDateSubscriberTest.php b/tests/units/Model/TaskMovedDateSubscriberTest.php deleted file mode 100644 index 0dd6e995..00000000 --- a/tests/units/Model/TaskMovedDateSubscriberTest.php +++ /dev/null @@ -1,77 +0,0 @@ -container); - $tc = new TaskCreation($this->container); - $p = new Project($this->container); - $tf = new TaskFinder($this->container); - - $this->container['dispatcher'] = new EventDispatcher; - $this->container['dispatcher']->addSubscriber(new TaskMovedDateSubscriber($this->container)); - - $now = time(); - - $this->assertEquals(1, $p->create(array('name' => 'Project #1'))); - $this->assertEquals(1, $tc->create(array('title' => 'Task #1', 'project_id' => 1))); - - $task = $tf->getById(1); - $this->assertNotEmpty($task); - $this->assertEquals($now, $task['date_moved'], '', 1); - - sleep(1); - - $this->assertTrue($tp->movePosition(1, 1, 2, 1)); - - $task = $tf->getById(1); - $this->assertNotEmpty($task); - $this->assertNotEquals($now, $task['date_moved']); - } - - public function testMoveTaskAnotherSwimlane() - { - $tp = new TaskPosition($this->container); - $tc = new TaskCreation($this->container); - $p = new Project($this->container); - $tf = new TaskFinder($this->container); - $s = new Swimlane($this->container); - - $this->container['dispatcher'] = new EventDispatcher; - $this->container['dispatcher']->addSubscriber(new TaskMovedDateSubscriber($this->container)); - - $now = time(); - - $this->assertEquals(1, $p->create(array('name' => 'Project #1'))); - $this->assertEquals(1, $s->create(array('project_id' => 1, 'name' => 'S1'))); - $this->assertEquals(2, $s->create(array('project_id' => 1, 'name' => 'S2'))); - $this->assertEquals(1, $tc->create(array('title' => 'Task #1', 'project_id' => 1))); - - $task = $tf->getById(1); - $this->assertNotEmpty($task); - $this->assertEquals($now, $task['date_moved'], '', 1); - $this->assertEquals(1, $task['column_id']); - $this->assertEquals(0, $task['swimlane_id']); - - sleep(1); - - $this->assertTrue($tp->movePosition(1, 1, 2, 1, 2)); - - $task = $tf->getById(1); - $this->assertNotEmpty($task); - $this->assertNotEquals($now, $task['date_moved']); - $this->assertEquals(2, $task['column_id']); - $this->assertEquals(2, $task['swimlane_id']); - } -} -- cgit v1.2.3 From e94c4cab7f79657f8b514b4af6c4e459e9b42961 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sun, 17 Jan 2016 14:56:31 -0500 Subject: Avoid automatic actions that change the color to fire subsequent events --- ChangeLog | 1 + app/Action/Base.php | 19 +++++++++++++------ app/Action/TaskAssignColorCategory.php | 2 +- app/Action/TaskAssignColorColumn.php | 2 +- app/Action/TaskAssignColorLink.php | 2 +- app/Action/TaskAssignColorUser.php | 2 +- app/Model/TaskCreation.php | 3 +++ app/Model/TaskModification.php | 8 +++++--- app/Model/TaskPosition.php | 9 ++++++--- app/Model/TaskStatus.php | 6 ++---- 10 files changed, 34 insertions(+), 20 deletions(-) (limited to 'app/Model/TaskPosition.php') diff --git a/ChangeLog b/ChangeLog index 39394db6..b0d7ccd7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,6 +24,7 @@ Bug fixes: * Automatic action listeners were using the same instance * Fix wrong link for category in task footer * Unable to set currency rate with Postgres database +* Avoid automatic actions that change the color to fire subsequent events Version 1.0.23 -------------- diff --git a/app/Action/Base.php b/app/Action/Base.php index 1298aec2..efc52f04 100644 --- a/app/Action/Base.php +++ b/app/Action/Base.php @@ -119,7 +119,13 @@ abstract class Base extends \Kanboard\Core\Base */ public function __toString() { - return $this->getName(); + $params = array(); + + foreach ($this->params as $key => $value) { + $params[] = $key.'='.var_export($value, true); + } + + return $this->getName().'('.implode('|', $params).'])'; } /** @@ -246,16 +252,17 @@ abstract class Base extends \Kanboard\Core\Base } $data = $event->getAll(); - $result = false; + $executable = $this->isExecutable($data, $eventName); + $executed = false; - if ($this->isExecutable($data, $eventName)) { + if ($executable) { $this->called = true; - $result = $this->doAction($data); + $executed = $this->doAction($data); } - $this->logger->debug('AutomaticAction '.$this->getName().' => '.($result ? 'true' : 'false')); + $this->logger->debug($this.' ['.$eventName.'] => executable='.var_export($executable, true).' exec_success='.var_export($executed, true)); - return $result; + return $executed; } /** diff --git a/app/Action/TaskAssignColorCategory.php b/app/Action/TaskAssignColorCategory.php index 3a15b15f..139c24cb 100644 --- a/app/Action/TaskAssignColorCategory.php +++ b/app/Action/TaskAssignColorCategory.php @@ -78,7 +78,7 @@ class TaskAssignColorCategory extends Base 'color_id' => $this->getParam('color_id'), ); - return $this->taskModification->update($values); + return $this->taskModification->update($values, false); } /** diff --git a/app/Action/TaskAssignColorColumn.php b/app/Action/TaskAssignColorColumn.php index 7474045b..92412739 100644 --- a/app/Action/TaskAssignColorColumn.php +++ b/app/Action/TaskAssignColorColumn.php @@ -79,7 +79,7 @@ class TaskAssignColorColumn extends Base 'color_id' => $this->getParam('color_id'), ); - return $this->taskModification->update($values); + return $this->taskModification->update($values, false); } /** diff --git a/app/Action/TaskAssignColorLink.php b/app/Action/TaskAssignColorLink.php index f71df70e..12ceabb3 100644 --- a/app/Action/TaskAssignColorLink.php +++ b/app/Action/TaskAssignColorLink.php @@ -78,7 +78,7 @@ class TaskAssignColorLink extends Base 'color_id' => $this->getParam('color_id'), ); - return $this->taskModification->update($values); + return $this->taskModification->update($values, false); } /** diff --git a/app/Action/TaskAssignColorUser.php b/app/Action/TaskAssignColorUser.php index 6e56bdc5..6ec8ce95 100644 --- a/app/Action/TaskAssignColorUser.php +++ b/app/Action/TaskAssignColorUser.php @@ -79,7 +79,7 @@ class TaskAssignColorUser extends Base 'color_id' => $this->getParam('color_id'), ); - return $this->taskModification->update($values); + return $this->taskModification->update($values, false); } /** diff --git a/app/Model/TaskCreation.php b/app/Model/TaskCreation.php index 88912d4d..975cc7a3 100644 --- a/app/Model/TaskCreation.php +++ b/app/Model/TaskCreation.php @@ -88,6 +88,9 @@ class TaskCreation extends Base { $event = new TaskEvent(array('task_id' => $task_id) + $values); + $this->logger->debug('Event fired: '.Task::EVENT_CREATE_UPDATE); + $this->logger->debug('Event fired: '.Task::EVENT_CREATE); + $this->dispatcher->dispatch(Task::EVENT_CREATE_UPDATE, $event); $this->dispatcher->dispatch(Task::EVENT_CREATE, $event); diff --git a/app/Model/TaskModification.php b/app/Model/TaskModification.php index 781646b8..5f0fd074 100644 --- a/app/Model/TaskModification.php +++ b/app/Model/TaskModification.php @@ -17,16 +17,17 @@ class TaskModification extends Base * * @access public * @param array $values + * @param boolean $fire_events * @return boolean */ - public function update(array $values) + public function update(array $values, $fire_events = true) { $original_task = $this->taskFinder->getById($values['id']); $this->prepare($values); $result = $this->db->table(Task::TABLE)->eq('id', $original_task['id'])->update($values); - if ($result) { + if ($fire_events && $result) { $this->fireEvents($original_task, $values); } @@ -57,7 +58,8 @@ class TaskModification extends Base } foreach ($events as $event) { - $this->container['dispatcher']->dispatch($event, new TaskEvent($event_data)); + $this->logger->debug('Event fired: '.$event); + $this->dispatcher->dispatch($event, new TaskEvent($event_data)); } } diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index 762f7fe3..091d908d 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -177,11 +177,14 @@ class TaskPosition extends Base ); if ($task['swimlane_id'] != $new_swimlane_id) { - $this->container['dispatcher']->dispatch(Task::EVENT_MOVE_SWIMLANE, new TaskEvent($event_data)); + $this->logger->debug('Event fired: '.Task::EVENT_MOVE_SWIMLANE); + $this->dispatcher->dispatch(Task::EVENT_MOVE_SWIMLANE, new TaskEvent($event_data)); } elseif ($task['column_id'] != $new_column_id) { - $this->container['dispatcher']->dispatch(Task::EVENT_MOVE_COLUMN, new TaskEvent($event_data)); + $this->logger->debug('Event fired: '.Task::EVENT_MOVE_COLUMN); + $this->dispatcher->dispatch(Task::EVENT_MOVE_COLUMN, new TaskEvent($event_data)); } elseif ($task['position'] != $new_position) { - $this->container['dispatcher']->dispatch(Task::EVENT_MOVE_POSITION, new TaskEvent($event_data)); + $this->logger->debug('Event fired: '.Task::EVENT_MOVE_POSITION); + $this->dispatcher->dispatch(Task::EVENT_MOVE_POSITION, new TaskEvent($event_data)); } } } diff --git a/app/Model/TaskStatus.php b/app/Model/TaskStatus.php index afb5ffb5..2b902815 100644 --- a/app/Model/TaskStatus.php +++ b/app/Model/TaskStatus.php @@ -113,10 +113,8 @@ class TaskStatus extends Base )); if ($result) { - $this->container['dispatcher']->dispatch( - $event, - new TaskEvent(array('task_id' => $task_id) + $this->taskFinder->getById($task_id)) - ); + $this->logger->debug('Event fired: '.$event); + $this->dispatcher->dispatch($event, new TaskEvent(array('task_id' => $task_id) + $this->taskFinder->getById($task_id))); } return $result; -- cgit v1.2.3 From 5311cf3a16b13a39b501dd85c62594a09bb59f9a Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sat, 23 Jan 2016 14:43:22 -0500 Subject: Revert back optimization of TaskPosition --- ChangeLog | 1 + app/Model/TaskPosition.php | 185 ++++++++++++++++++++++++++++----------------- 2 files changed, 115 insertions(+), 71 deletions(-) (limited to 'app/Model/TaskPosition.php') diff --git a/ChangeLog b/ChangeLog index a9510c00..0307a99b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -38,6 +38,7 @@ Bug fixes: * Unable to set currency rate with Postgres database * Avoid automatic actions that change the color to fire subsequent events * Unable to unassign a task from the API +* Revert back previous optimizations of TaskPosition Version 1.0.23 -------------- diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index 091d908d..0e29c4b1 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -26,10 +26,8 @@ class TaskPosition extends Base */ public function movePosition($project_id, $task_id, $column_id, $position, $swimlane_id = 0, $fire_events = true) { - $result = false; - if ($position < 1) { - return $result; + return false; } $task = $this->taskFinder->getById($task_id); @@ -38,22 +36,17 @@ class TaskPosition extends Base return true; } - $this->db->startTransaction(); + $result = false; - if ($task['swimlane_id'] != $swimlane_id || $task['column_id'] != $column_id) { - $result = $this->moveTaskToAnotherColumn($task, $swimlane_id, $column_id, $position); + if ($task['swimlane_id'] != $swimlane_id) { + $result = $this->saveSwimlaneChange($project_id, $task_id, $position, $task['column_id'], $column_id, $task['swimlane_id'], $swimlane_id); + } elseif ($task['column_id'] != $column_id) { + $result = $this->saveColumnChange($project_id, $task_id, $position, $swimlane_id, $task['column_id'], $column_id); } elseif ($task['position'] != $position) { - $result = $this->moveTaskWithinSameColumn($task, $position); + $result = $this->savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id); } - if (! $result) { - $this->db->cancelTransaction(); - return false; - } - - $this->db->closeTransaction(); - - if ($fire_events) { + if ($result && $fire_events) { $this->fireEvents($task, $column_id, $position, $swimlane_id); } @@ -61,95 +54,145 @@ class TaskPosition extends Base } /** - * Move a task to another column/swimlane + * Move a task to another swimlane * * @access private - * @param array $task - * @param integer $swimlane_id - * @param integer $column_id - * @param integer $position + * @param integer $project_id + * @param integer $task_id + * @param integer $position + * @param integer $original_column_id + * @param integer $new_column_id + * @param integer $original_swimlane_id + * @param integer $new_swimlane_id * @return boolean */ - private function moveTaskToAnotherColumn(array $task, $swimlane_id, $column_id, $position) + private function saveSwimlaneChange($project_id, $task_id, $position, $original_column_id, $new_column_id, $original_swimlane_id, $new_swimlane_id) { - $results = array(); - $max = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->count(); - $position = $max > 0 && $position > $max ? $max + 1 : $position; - - $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gt('position', $task['position'])->decrement('position', 1); - $results[] = $this->getQuery($task['project_id'], $swimlane_id, $column_id)->gte('position', $position)->increment('position', 1); - $results[] = $this->updateTaskPosition($task['id'], $swimlane_id, $column_id, $position); + $this->db->startTransaction(); + $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $original_swimlane_id); + $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $new_swimlane_id); + $this->db->closeTransaction(); - return !in_array(false, $results, true); + return $r1 && $r2; } /** - * Move a task within the same column + * Move a task to another column * * @access private - * @param array $task - * @param integer $position + * @param integer $project_id + * @param integer $task_id + * @param integer $position + * @param integer $swimlane_id + * @param integer $original_column_id + * @param integer $new_column_id * @return boolean */ - private function moveTaskWithinSameColumn(array $task, $position) + private function saveColumnChange($project_id, $task_id, $position, $swimlane_id, $original_column_id, $new_column_id) { - $results = array(); - $max = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->count(); - $position = $max > 0 && $position > $max ? $max : $position; - - if ($position >= $max) { - $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->lte('position', $position)->decrement('position', 1); - } else { - $results[] = $this->getQuery($task['project_id'], $task['swimlane_id'], $task['column_id'])->gte('position', $position)->increment('position', 1); - } - - $results[] = $this->updateTaskPosition($task['id'], $task['swimlane_id'], $task['column_id'], $position); + $this->db->startTransaction(); + $r1 = $this->saveTaskPositions($project_id, $task_id, 0, $original_column_id, $swimlane_id); + $r2 = $this->saveTaskPositions($project_id, $task_id, $position, $new_column_id, $swimlane_id); + $this->db->closeTransaction(); - return !in_array(false, $results, true); + return $r1 && $r2; } /** - * Update final task position + * Move a task to another position in the same column * * @access private - * @param integer $task_id - * @param integer $swimlane_id - * @param integer $column_id - * @param integer $position + * @param integer $project_id + * @param integer $task_id + * @param integer $position + * @param integer $column_id + * @param integer $swimlane_id * @return boolean */ - private function updateTaskPosition($task_id, $swimlane_id, $column_id, $position) + private function savePositionChange($project_id, $task_id, $position, $column_id, $swimlane_id) { - $now = time(); + $this->db->startTransaction(); + $result = $this->saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id); + $this->db->closeTransaction(); - return $this->db->table(Task::TABLE) - ->eq('id', $task_id) - ->eq('is_active', 1) - ->update(array( - 'position' => $position, - 'column_id' => $column_id, - 'swimlane_id' => $swimlane_id, - 'date_modification' => $now, - 'date_moved' => $now, - )); + return $result; } /** - * Get common query + * Save all task positions for one column * * @access private - * @param integer $project_id - * @param integer $swimlane_id - * @param integer $column_id - * @return \PicoDb\Table + * @param integer $project_id + * @param integer $task_id + * @param integer $position + * @param integer $column_id + * @param integer $swimlane_id + * @return boolean */ - private function getQuery($project_id, $swimlane_id, $column_id) + private function saveTaskPositions($project_id, $task_id, $position, $column_id, $swimlane_id) { - return $this->db->table(Task::TABLE) - ->eq('project_id', $project_id) + $tasks_ids = $this->db->table(Task::TABLE) + ->eq('is_active', 1) ->eq('swimlane_id', $swimlane_id) + ->eq('project_id', $project_id) ->eq('column_id', $column_id) - ->eq('is_active', 1); + ->neq('id', $task_id) + ->asc('position') + ->asc('id') + ->findAllByColumn('id'); + + $offset = 1; + + foreach ($tasks_ids as $current_task_id) { + + // Insert the new task + if ($position == $offset) { + if (! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { + return false; + } + $offset++; + } + + // Rewrite other tasks position + if (! $this->saveTaskPosition($current_task_id, $offset, $column_id, $swimlane_id)) { + return false; + } + + $offset++; + } + + // Insert the new task at the bottom and normalize bad position + if ($position >= $offset && ! $this->saveTaskPosition($task_id, $offset, $column_id, $swimlane_id)) { + return false; + } + + return true; + } + + /** + * Save new task position + * + * @access private + * @param integer $task_id + * @param integer $position + * @param integer $column_id + * @param integer $swimlane_id + * @return boolean + */ + private function saveTaskPosition($task_id, $position, $column_id, $swimlane_id) + { + $result = $this->db->table(Task::TABLE)->eq('id', $task_id)->update(array( + 'position' => $position, + 'column_id' => $column_id, + 'swimlane_id' => $swimlane_id, + )); + + if (! $result) { + $this->db->cancelTransaction(); + return false; + } + + return true; } /** -- cgit v1.2.3 From 6371d3272e4fcb964170cbb14e2dfec9c665769e Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Sat, 23 Jan 2016 15:00:35 -0500 Subject: Put back date_moved for the task (revert) --- app/Model/TaskPosition.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'app/Model/TaskPosition.php') diff --git a/app/Model/TaskPosition.php b/app/Model/TaskPosition.php index 0e29c4b1..4c9928d7 100644 --- a/app/Model/TaskPosition.php +++ b/app/Model/TaskPosition.php @@ -166,7 +166,12 @@ class TaskPosition extends Base return false; } - return true; + $now = time(); + + return $this->db->table(Task::TABLE)->eq('id', $task_id)->update(array( + 'date_moved' => $now, + 'date_modification' => $now, + )); } /** -- cgit v1.2.3