From 41e796c52aa693d13636b54efe15705eaec27f3f Mon Sep 17 00:00:00 2001 From: Frédéric Guillot Date: Sat, 20 Sep 2014 14:49:31 +0200 Subject: Models refactoring/improvements --- app/Controller/Base.php | 2 +- app/Controller/Task.php | 2 +- app/Controller/User.php | 2 +- app/Event/CommentNotificationListener.php | 2 +- app/Event/FileNotificationListener.php | 2 +- app/Event/SubTaskNotificationListener.php | 2 +- app/Event/TaskNotificationListener.php | 2 +- app/Model/Base.php | 50 ++++++++ app/Model/SubTask.php | 13 +- app/Model/Task.php | 194 ++++++++++++++++-------------- app/Model/User.php | 17 +-- 11 files changed, 162 insertions(+), 126 deletions(-) (limited to 'app') diff --git a/app/Controller/Base.php b/app/Controller/Base.php index cc180158..8f822f3d 100644 --- a/app/Controller/Base.php +++ b/app/Controller/Base.php @@ -269,7 +269,7 @@ abstract class Base */ protected function getTask() { - $task = $this->task->getById($this->request->getIntegerParam('task_id'), true); + $task = $this->task->getDetails($this->request->getIntegerParam('task_id')); if (! $task) { $this->notfound(); diff --git a/app/Controller/Task.php b/app/Controller/Task.php index 5de25a0e..eddb61d5 100644 --- a/app/Controller/Task.php +++ b/app/Controller/Task.php @@ -60,7 +60,7 @@ class Task extends Base $this->forbidden(true); } - $task = $this->task->getById($this->request->getIntegerParam('task_id'), true); + $task = $this->task->getDetails($this->request->getIntegerParam('task_id')); if (! $task) { $this->notfound(true); diff --git a/app/Controller/User.php b/app/Controller/User.php index 504f4aed..c94c2c88 100644 --- a/app/Controller/User.php +++ b/app/Controller/User.php @@ -353,7 +353,7 @@ class User extends Base $this->response->html($this->layout('user_edit', array( 'values' => $values, 'errors' => $errors, - 'projects' => $this->projectPermission->getAllowedProjects($user['id']), + 'projects' => $this->projectPermission->filterProjects($this->project->getList(), $user['id']), 'user' => $user, ))); } diff --git a/app/Event/CommentNotificationListener.php b/app/Event/CommentNotificationListener.php index 0aa061f3..36b7ea4b 100644 --- a/app/Event/CommentNotificationListener.php +++ b/app/Event/CommentNotificationListener.php @@ -23,7 +23,7 @@ class CommentNotificationListener extends BaseNotificationListener { $values = array(); $values['comment'] = $this->notification->comment->getById($data['id']); - $values['task'] = $this->notification->task->getById($values['comment']['task_id'], true); + $values['task'] = $this->notification->task->getDetails($values['comment']['task_id']); return $values; } diff --git a/app/Event/FileNotificationListener.php b/app/Event/FileNotificationListener.php index 98fc4260..ed5d78ed 100644 --- a/app/Event/FileNotificationListener.php +++ b/app/Event/FileNotificationListener.php @@ -23,7 +23,7 @@ class FileNotificationListener extends BaseNotificationListener { $values = array(); $values['file'] = $data; - $values['task'] = $this->notification->task->getById($data['task_id'], true); + $values['task'] = $this->notification->task->getDetails($data['task_id']); return $values; } diff --git a/app/Event/SubTaskNotificationListener.php b/app/Event/SubTaskNotificationListener.php index 0a239421..299ae00b 100644 --- a/app/Event/SubTaskNotificationListener.php +++ b/app/Event/SubTaskNotificationListener.php @@ -23,7 +23,7 @@ class SubTaskNotificationListener extends BaseNotificationListener { $values = array(); $values['subtask'] = $this->notification->subtask->getById($data['id'], true); - $values['task'] = $this->notification->task->getById($data['task_id'], true); + $values['task'] = $this->notification->task->getDetails($data['task_id']); return $values; } diff --git a/app/Event/TaskNotificationListener.php b/app/Event/TaskNotificationListener.php index ffbe7a8c..466a8eb4 100644 --- a/app/Event/TaskNotificationListener.php +++ b/app/Event/TaskNotificationListener.php @@ -22,7 +22,7 @@ class TaskNotificationListener extends BaseNotificationListener public function getTemplateData(array $data) { $values = array(); - $values['task'] = $this->notification->task->getById($data['task_id'], true); + $values['task'] = $this->notification->task->getDetails($data['task_id']); return $values; } diff --git a/app/Model/Base.php b/app/Model/Base.php index e3f194b5..2b13e815 100644 --- a/app/Model/Base.php +++ b/app/Model/Base.php @@ -19,6 +19,7 @@ use PicoDb\Database; * @property \Model\Board $board * @property \Model\Category $category * @property \Model\Comment $comment + * @property \Model\CommentHistory $commentHistory * @property \Model\Color $color * @property \Model\Config $config * @property \Model\DateParser $dateParser @@ -28,6 +29,7 @@ use PicoDb\Database; * @property \Model\Project $project * @property \Model\ProjectPermission $projectPermission * @property \Model\SubTask $subTask + * @property \Model\SubtaskHistory $subtaskHistory * @property \Model\Task $task * @property \Model\TaskExport $taskExport * @property \Model\TaskHistory $taskHistory @@ -85,4 +87,52 @@ abstract class Base { return Tool::loadModel($this->registry, $name); } + + /** + * Remove keys from an array + * + * @access public + * @param array $values Input array + * @param array $keys List of keys to remove + */ + public function removeFields(array &$values, array $keys) + { + foreach ($keys as $key) { + if (isset($values[$key])) { + unset($values[$key]); + } + } + } + + /** + * Force some fields to be at 0 if empty + * + * @access public + * @param array $values Input array + * @param array $keys List of keys + */ + public function resetFields(array &$values, array $keys) + { + foreach ($keys as $key) { + if (isset($values[$key]) && empty($values[$key])) { + $values[$key] = 0; + } + } + } + + /** + * Force some fields to be integer + * + * @access public + * @param array $values Input array + * @param array $keys List of keys + */ + public function convertIntegerFields(array &$values, array $keys) + { + foreach ($keys as $key) { + if (isset($values[$key])) { + $values[$key] = (int) $values[$key]; + } + } + } } diff --git a/app/Model/SubTask.php b/app/Model/SubTask.php index 400c79f4..0126030a 100644 --- a/app/Model/SubTask.php +++ b/app/Model/SubTask.php @@ -128,17 +128,8 @@ class SubTask extends Base */ public function prepare(array &$values) { - if (isset($values['another_subtask'])) { - unset($values['another_subtask']); - } - - if (isset($values['time_estimated']) && empty($values['time_estimated'])) { - $values['time_estimated'] = 0; - } - - if (isset($values['time_spent']) && empty($values['time_spent'])) { - $values['time_spent'] = 0; - } + $this->removeFields($values, array('another_subtask')); + $this->resetFields($values, array('time_estimated', 'time_spent')); } /** diff --git a/app/Model/Task.php b/app/Model/Task.php index 8f544ac9..262e8d76 100644 --- a/app/Model/Task.php +++ b/app/Model/Task.php @@ -41,8 +41,6 @@ class Task extends Base const EVENT_CREATE_UPDATE = 'task.create_update'; const EVENT_ASSIGNEE_CHANGE = 'task.assignee_change'; - - /** * Get a list of due tasks for all projects * @@ -73,58 +71,62 @@ class Task extends Base } /** - * Fetch one task + * Get task details (fetch more information from other tables) * * @access public * @param integer $task_id Task id - * @param boolean $more If true, fetch all related information * @return array */ - public function getById($task_id, $more = false) + public function getDetails($task_id) { - if ($more) { - - $sql = ' - SELECT - tasks.id, - tasks.title, - tasks.description, - tasks.date_creation, - tasks.date_completed, - tasks.date_modification, - tasks.date_due, - tasks.color_id, - tasks.project_id, - tasks.column_id, - tasks.owner_id, - tasks.creator_id, - tasks.position, - tasks.is_active, - tasks.score, - tasks.category_id, - project_has_categories.name AS category_name, - projects.name AS project_name, - columns.title AS column_title, - users.username AS assignee_username, - users.name AS assignee_name, - creators.username AS creator_username, - creators.name AS creator_name - FROM tasks - LEFT JOIN users ON users.id = tasks.owner_id - LEFT JOIN users AS creators ON creators.id = tasks.creator_id - LEFT JOIN project_has_categories ON project_has_categories.id = tasks.category_id - LEFT JOIN projects ON projects.id = tasks.project_id - LEFT JOIN columns ON columns.id = tasks.column_id - WHERE tasks.id = ? - '; - - $rq = $this->db->execute($sql, array($task_id)); - return $rq->fetch(PDO::FETCH_ASSOC); - } - else { + $sql = ' + SELECT + tasks.id, + tasks.title, + tasks.description, + tasks.date_creation, + tasks.date_completed, + tasks.date_modification, + tasks.date_due, + tasks.color_id, + tasks.project_id, + tasks.column_id, + tasks.owner_id, + tasks.creator_id, + tasks.position, + tasks.is_active, + tasks.score, + tasks.category_id, + project_has_categories.name AS category_name, + projects.name AS project_name, + columns.title AS column_title, + users.username AS assignee_username, + users.name AS assignee_name, + creators.username AS creator_username, + creators.name AS creator_name + FROM tasks + LEFT JOIN users ON users.id = tasks.owner_id + LEFT JOIN users AS creators ON creators.id = tasks.creator_id + LEFT JOIN project_has_categories ON project_has_categories.id = tasks.category_id + LEFT JOIN projects ON projects.id = tasks.project_id + LEFT JOIN columns ON columns.id = tasks.column_id + WHERE tasks.id = ? + '; + + $rq = $this->db->execute($sql, array($task_id)); + return $rq->fetch(PDO::FETCH_ASSOC); + } - return $this->db->table(self::TABLE)->eq('id', $task_id)->findOne(); - } + /** + * Fetch one task + * + * @access public + * @param integer $task_id Task id + * @return array + */ + public function getById($task_id) + { + return $this->db->table(self::TABLE)->eq('id', $task_id)->findOne(); } /** @@ -161,6 +163,25 @@ class Task extends Base ->count(); } + /** + * Count the number of tasks for a given column and status + * + * @access public + * @param integer $project_id Project id + * @param integer $column_id Column id + * @param array $status List of status id + * @return integer + */ + public function countByColumnId($project_id, $column_id, array $status = array(self::STATUS_OPEN)) + { + return $this->db + ->table(self::TABLE) + ->eq('project_id', $project_id) + ->eq('column_id', $column_id) + ->in('is_active', $status) + ->count(); + } + /** * Get tasks that match defined filters * @@ -226,25 +247,6 @@ class Task extends Base return $table->findAll(); } - /** - * Count the number of tasks for a given column and status - * - * @access public - * @param integer $project_id Project id - * @param integer $column_id Column id - * @param array $status List of status id - * @return integer - */ - public function countByColumnId($project_id, $column_id, array $status = array(self::STATUS_OPEN)) - { - return $this->db - ->table(self::TABLE) - ->eq('project_id', $project_id) - ->eq('column_id', $column_id) - ->in('is_active', $status) - ->count(); - } - /** * Generic method to duplicate a task * @@ -347,40 +349,23 @@ class Task extends Base */ public function prepare(array &$values) { - if (isset($values['another_task'])) { - unset($values['another_task']); - } - if (! empty($values['date_due']) && ! is_numeric($values['date_due'])) { $values['date_due'] = $this->dateParser->getTimestamp($values['date_due']); } - // Force integer fields at 0 (for Postgresql) - if (isset($values['date_due']) && empty($values['date_due'])) { - $values['date_due'] = 0; - } - - if (isset($values['score']) && empty($values['score'])) { - $values['score'] = 0; - } - - if (isset($values['is_active'])) { - $values['is_active'] = (int) $values['is_active']; - } + $this->removeFields($values, array('another_task', 'id')); + $this->resetFields($values, array('date_due', 'score', 'category_id')); + $this->convertIntegerFields($values, array('is_active')); } /** - * Create a task + * Prepare data before task creation * * @access public - * @param array $values Form values - * @return boolean + * @param array $values Form values */ - public function create(array $values) + public function prepareCreation(array &$values) { - $this->db->startTransaction(); - - // Prepare data $this->prepare($values); if (empty($values['column_id'])) { @@ -395,8 +380,33 @@ class Task extends Base $values['date_creation'] = time(); $values['date_modification'] = $values['date_creation']; $values['position'] = $this->countByColumnId($values['project_id'], $values['column_id']) + 1; + } + + /** + * Prepare data before task modification + * + * @access public + * @param array $values Form values + */ + public function prepareModification(array &$values) + { + $this->prepare($values); + $values['date_modification'] = time(); + } + + /** + * Create a task + * + * @access public + * @param array $values Form values + * @return boolean|integer + */ + public function create(array $values) + { + $this->db->startTransaction(); + + $this->prepareCreation($values); - // Save task if (! $this->db->table(self::TABLE)->save($values)) { $this->db->cancelTransaction(); return false; @@ -431,10 +441,8 @@ class Task extends Base } // Prepare data - $this->prepare($values); $updated_task = $values; - $updated_task['date_modification'] = time(); - unset($updated_task['id']); + $this->prepareModification($updated_task); $result = $this->db->table(self::TABLE)->eq('id', $values['id'])->update($updated_task); diff --git a/app/Model/User.php b/app/Model/User.php index 54588cbe..8add9722 100644 --- a/app/Model/User.php +++ b/app/Model/User.php @@ -163,21 +163,8 @@ class User extends Base } } - if (isset($values['confirmation'])) { - unset($values['confirmation']); - } - - if (isset($values['current_password'])) { - unset($values['current_password']); - } - - if (isset($values['is_admin']) && empty($values['is_admin'])) { - $values['is_admin'] = 0; - } - - if (isset($values['is_ldap_user']) && empty($values['is_ldap_user'])) { - $values['is_ldap_user'] = 0; - } + $this->removeFields($values, array('confirmation', 'current_password')); + $this->resetFields($values, array('is_admin', 'is_ldap_user')); } /** -- cgit v1.2.3