From 59fec5529ff0d2df52cdcb761b5c0ded6a3101f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emmanuel=20Beno=C3=AEt?= Date: Mon, 4 Jan 2016 11:01:28 +0100 Subject: [PATCH] Moving tasks - Preliminary version * Works when moving tasks without dependencies. Crashes with a SQL error due to FK violation when moving tasks with interdependencies (LTC ID?) * The form is unable to force removal of external dependencies at this time. * Related PL/PgSQL code stored in database/temp.sql at this time. --- database/tasks-move-sub.sql | 20 +++ database/temp.sql | 168 ++++++++++++++++++++++ includes/t-data/dao_tasks.inc.php | 30 ++++ includes/t-tasks/controllers.inc.php | 122 ++++++++++++++++ includes/t-tasks/package.inc.php | 1 + includes/t-tasks/page_controllers.inc.php | 60 +++++++- includes/t-tasks/pages.inc.php | 2 +- 7 files changed, 395 insertions(+), 8 deletions(-) create mode 100644 database/temp.sql diff --git a/database/tasks-move-sub.sql b/database/tasks-move-sub.sql index 858b827..7c1905a 100644 --- a/database/tasks-move-sub.sql +++ b/database/tasks-move-sub.sql @@ -145,3 +145,23 @@ $tasks_move_down$; REVOKE EXECUTE ON FUNCTION tasks_move_down( INT , INT , BOOLEAN ) FROM PUBLIC; GRANT EXECUTE ON FUNCTION tasks_move_down( INT , INT , BOOLEAN ) TO :webapp_user; + + +/* + * Move a set of tasks from a container to another. + */ +DROP FUNCTION IF EXISTS tasks_move( _fromTask BOOLEAN , _fromId INT , _toTask BOOLEAN , _toId INT , _force BOOLEAN , _tasks INT[] ); +CREATE FUNCTION tasks_move( _fromTask BOOLEAN , _fromId INT , _toTask BOOLEAN , _toId INT , _force BOOLEAN , _tasks INT[] ) + RETURNS INT + LANGUAGE PLPGSQL + STRICT VOLATILE + SECURITY DEFINER + AS $tasks_move$ + +BEGIN + RETURN 0; +END; +$tasks_move$; + +REVOKE EXECUTE ON FUNCTION tasks_move( INT , INT , BOOLEAN ) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION tasks_move( INT , INT , BOOLEAN ) TO :webapp_user; diff --git a/database/temp.sql b/database/temp.sql new file mode 100644 index 0000000..97f2240 --- /dev/null +++ b/database/temp.sql @@ -0,0 +1,168 @@ +\i database/config.sql + +/* + * Move a set of tasks from a container to another. + * Returns the following error codes: + * 0 No error + * 1 Deleted tasks + * 2 Moved tasks (not in specified container) + * 3 Target item/task deleted + * 4 Moving tasks to one of their children + * 5 Dependencies would be broken, and _force is FALSE + */ +DROP FUNCTION IF EXISTS tasks_move( BOOLEAN , INT , BOOLEAN , INT , BOOLEAN , INT[] ); +CREATE FUNCTION tasks_move( _fromTask BOOLEAN , _fromId INT , _toTask BOOLEAN , _toId INT , _force BOOLEAN , _tasks INT[] ) + RETURNS INT + LANGUAGE PLPGSQL + STRICT VOLATILE + SECURITY DEFINER + AS $tasks_move$ + +DECLARE + _i INT; + _ltc_source INT; + _ltc_dest INT; + +BEGIN + -- Create the temporary table for tasks + PERFORM _tm_create_temp( _tasks ); + + -- Make sure all specified tasks exists + SELECT INTO _i COUNT( * ) + FROM _tm_tasks + INNER JOIN tasks USING( task_id ); + IF _i <> array_length( _tasks , 1 ) THEN + RETURN 1; + END IF; + + -- Make sure all tasks match the specified container + PERFORM task_id + FROM _tm_tasks + INNER JOIN tasks USING( task_id ) + WHERE ( task_id_parent IS NOT NULL ) <> _fromTask + OR ( CASE + WHEN task_id_parent IS NULL THEN + item_id + ELSE + task_id_parent + END ) <> _fromId; + IF FOUND THEN + RETURN 2; + END IF; + + -- If the source and destination are the same, we're done. + IF _fromTask = _toTask AND _fromId = _toId THEN + RETURN 0; + END IF; + + -- Make sure that the destination exists. Also get the target LTC ID. + IF _toTask THEN + SELECT INTO _ltc_dest ltc_id + FROM logical_task_containers + WHERE task_id = _toId; + ELSE + SELECT INTO _ltc_dest ltc_id + FROM logical_task_containers + WHERE task_id IS NULL; + PERFORM item_id FROM items WHERE item_id = _toId; + END IF; + IF NOT FOUND THEN + RETURN 3; + END IF; + + -- If we're moving to a task, make sure it isn't a child of any of the + -- tasks that are being moved. + IF _toTask THEN + PERFORM * FROM _tm_tasks _mv + INNER JOIN tasks_tree _tree + ON _mv.task_id = _tree.task_id_parent + WHERE task_id_child = _toId; + IF FOUND THEN + RETURN 4; + END IF; + END IF; + + -- Get the source LTC ID. + IF _fromTask THEN + SELECT INTO _ltc_source ltc_id + FROM logical_task_containers + WHERE task_id = _fromId; + ELSE + SELECT INTO _ltc_source ltc_id + FROM logical_task_containers + WHERE task_id IS NULL; + END IF; + + -- If we're changing the LTC, handle dependencies. + IF _ltc_source <> _ltc_dest AND NOT _force THEN + -- Check them if we're not forcing the move. + PERFORM _tdn.task_id + FROM taskdep_nodes _tdn + INNER JOIN _tm_tasks _tmt + USING( task_id ) + LEFT OUTER JOIN _tm_tasks _tmt2 + ON _tmt2.task_id = _tdn.task_id_copyof + WHERE _tdn.tnode_depth = 1 AND _tmt2.task_id IS NULL; + IF FOUND THEN + RETURN 5; + END IF; + ELSIF _ltc_source <> _ltc_dest AND _force THEN + -- Otherwise, break them. + DELETE FROM task_dependencies + WHERE taskdep_id IN ( + SELECT DISTINCT _tdn.taskdep_id + FROM taskdep_nodes _tdn + INNER JOIN _tm_tasks _tmt + USING( task_id ) + LEFT OUTER JOIN _tm_tasks _tmt2 + ON _tmt2.task_id = _tdn.task_id_copyof + WHERE _tdn.tnode_depth = 1 AND _tmt2.task_id IS NULL + ); + END IF; + + -- We're ready to move the tasks themselves. + IF _toTask THEN + SELECT INTO _i item_id FROM tasks WHERE task_id = _toId; + ELSE + _i := _toId; + END IF; + UPDATE tasks SET item_id = _i , ltc_id = _ltc_dest + WHERE task_id IN ( + SELECT task_id FROM _tm_tasks + ); + + RETURN 0; +END; +$tasks_move$; + +REVOKE EXECUTE ON FUNCTION tasks_move( BOOLEAN , INT , BOOLEAN , INT , BOOLEAN , INT[] ) FROM PUBLIC; +GRANT EXECUTE ON FUNCTION tasks_move( BOOLEAN , INT , BOOLEAN , INT , BOOLEAN , INT[] ) TO :webapp_user; + + +/* + * Function used by tasks_move to insert all tasks into a temporary table. + */ +DROP FUNCTION IF EXISTS _tm_create_temp( INT[] ); +CREATE FUNCTION _tm_create_temp( _tasks INT[] ) + RETURNS VOID + LANGUAGE PLPGSQL + STRICT VOLATILE + SECURITY INVOKER + AS $_tm_create_temp$ +DECLARE + _i INT; +BEGIN + SET LOCAL client_min_messages=warning; + DROP TABLE IF EXISTS _tm_tasks; + RESET client_min_messages; + + CREATE TEMPORARY TABLE _tm_tasks( + task_id INT + ) ON COMMIT DROP; + FOR _i IN array_lower( _tasks , 1 ) .. array_upper( _tasks , 1 ) + LOOP + INSERT INTO _tm_tasks( task_id ) VALUES ( _tasks[ _i ] ); + END LOOP; +END; +$_tm_create_temp$; +REVOKE EXECUTE ON FUNCTION _tm_create_temp( INT[] ) FROM PUBLIC; diff --git a/includes/t-data/dao_tasks.inc.php b/includes/t-data/dao_tasks.inc.php index eafb798..7cbb3a4 100644 --- a/includes/t-data/dao_tasks.inc.php +++ b/includes/t-data/dao_tasks.inc.php @@ -29,6 +29,25 @@ class DAO_Tasks } + public function getActiveTasksAssoc( ) + { + $qr = $this->query( + 'SELECT id , item , parent_task , title FROM tasks_list ' + . 'WHERE completed_at IS NULL ' + . 'ORDER BY priority DESC , badness , added_at DESC' )->execute( ); + $result = array( ); + foreach ( $qr as $task ) { + $container = $task->parent_task === null ? 'I' : 'T'; + $container .= $container == 'I' ? $task->item : $task->parent_task; + if ( !array_key_exists( $container , $result ) ) { + $result[ $container ] = array( ); + } + array_push( $result[ $container ] , $task ); + } + return $result; + } + + public function getAllTasks( ) { return $this->query( @@ -326,4 +345,15 @@ class DAO_Tasks ->execute( $task->id , $sibling , $force ? 't' : 'f' ); return ( $result[0]->success == 't' ); } + + + public function moveTasks( $fromTask , $parentId , $toTask , $targetId , $tasks , $force ) + { + $result = $this->query( 'SELECT tasks_move( $1 , $2 , $3 , $4 , $5 , $6::int[] ) AS error' ) + ->execute( $fromTask ? 't' : 'f' , $parentId , + $toTask ? 't' : 'f' , $targetId , + $force ? 't' : 'f' , + '{' . join( ',' , $tasks ) . '}' ); + return $result[0]->error; + } } diff --git a/includes/t-tasks/controllers.inc.php b/includes/t-tasks/controllers.inc.php index 09a1ff1..30f16a9 100644 --- a/includes/t-tasks/controllers.inc.php +++ b/includes/t-tasks/controllers.inc.php @@ -566,3 +566,125 @@ class Ctrl_TaskClaim } } + + +class Ctrl_TaskMove + extends Controller + implements FormAware +{ + private $form; + + public function setForm( Form $form ) + { + $this->form = $form; + } + + public function handle( Page $page ) + { + $type = $this->form->field( 'type' ); + $id = $this->form->field( 'id' ); + $target = $this->form->field( 'target' ); + $tasks = $this->form->field( 'tasks[]' ); + + $tFull = $target->value( ); + if ( strlen( $tFull ) < 2 ) { + $target->putError( 'Invalid target.' ); + return null; + } + $toTask = ( substr( $tFull , 0 , 1 ) == 'T' ); + $toId = (int) substr( $tFull , 1 ); + + $error = Loader::DAO( 'tasks' )->moveTasks( + $type->value( ) === 's' , (int) $id->value( ) , + $toTask , $toId , $tasks->value( ) , + false ); // FIXME: should be read from the form + + switch ( $error ) { + + case 0: + return true; + + case 1: + $tasks->putError( 'Selected tasks deleted.' ); + break; + + case 2: + $tasks->putError( 'Selected tasks moved.' ); + break; + + case 3: + $target->putError( 'Target has been deleted.' ); + break; + + case 4: + $target->putError( 'This is a child of a selected task.' ); + break; + + case 5: + $tasks->putError( 'Dependencies would be broken (FIXME: force mode)' ); + break; + + default: + $target->putError( "An unknown error occurred ($error)" ); + break; + } + + return null; + } + + private function addTopLevelTask( ) + { + $item = $this->form->field( 'item' ); + $name = $this->form->field( 'title' ); + $priority = $this->form->field( 'priority' ); + $description = $this->form->field( 'description' ); + + $error = Loader::DAO( 'tasks' )->addTask( (int) $item->value( ) , $name->value( ) , + (int) $priority->value( ) , $description->value( ) ); + switch ( $error ) { + + case 0: + return true; + + case 1: + $name->putError( 'Duplicate task name for this item.' ); + break; + + case 2: + $item->putError( 'This item has been deleted' ); + break; + + default: + $name->putError( "An unknown error occurred ($error)" ); + break; + } + + return null; + } + + private function addNestedTask( ) + { + $parent = $this->form->field( 'parent' ); + $name = $this->form->field( 'title' ); + $priority = $this->form->field( 'priority' ); + $description = $this->form->field( 'description' ); + + $error = Loader::DAO( 'tasks' )->addNestedTask( (int) $parent->value( ) , + $name->value( ) , (int) $priority->value( ) , $description->value( ) ); + switch ( $error ) { + + case 0: + return true; + + case 1: + $name->putError( 'Duplicate sub-task name.' ); + break; + + default: + $name->putError( "An unknown error occurred ($error)" ); + break; + } + + return null; + } +} diff --git a/includes/t-tasks/package.inc.php b/includes/t-tasks/package.inc.php index 66a097a..3c13298 100644 --- a/includes/t-tasks/package.inc.php +++ b/includes/t-tasks/package.inc.php @@ -32,6 +32,7 @@ $package[ 'ctrls' ][] = 'task_details'; $package[ 'ctrls' ][] = 'task_list_subtasks'; $package[ 'ctrls' ][] = 'task_move_down'; $package[ 'ctrls' ][] = 'task_move_up'; +$package[ 'ctrls' ][] = 'task_move_form'; $package[ 'ctrls' ][] = 'task_move'; $package[ 'ctrls' ][] = 'task_notes'; $package[ 'ctrls' ][] = 'toggle_task'; diff --git a/includes/t-tasks/page_controllers.inc.php b/includes/t-tasks/page_controllers.inc.php index 57017c0..7c53711 100644 --- a/includes/t-tasks/page_controllers.inc.php +++ b/includes/t-tasks/page_controllers.inc.php @@ -788,7 +788,7 @@ class Ctrl_TaskMoveUp } -class Ctrl_TaskMove +class Ctrl_TaskMoveForm extends Controller { @@ -833,16 +833,62 @@ class Ctrl_TaskMove return $failure; } - // Generate form + // Form header $page->setTitle( $name . ': move tasks' ); $form = Loader::Create( 'Form' , 'Move tasks' , 'move-tasks' ) + ->setURL( $failure ) + ->addController( Loader::Ctrl( 'task_move' ) ) ->addField( Loader::Create( 'Field' , 'type' , 'hidden' ) ->setDefaultValue( $subtasks ? 's' : 'i' ) ) ->addField( Loader::Create( 'Field' , 'id' , 'hidden' ) - ->setDefaultValue( $id ) ) ; -// $this->addDependencySelector( $form , $task->possibleDependencies , $task->parent_task === null ); - return $form->setURL( $failure ) -// ->addController( Loader::Ctrl( 'dependency_add' ) ) - ->controller( ); + ->setDefaultValue( $id ) ); + + // List of targets + $tSel = Loader::Create( 'Field' , 'target' , 'select' ) + ->setDescription( 'Move to:' ) + ->addOption( '' , '(please select a target)' ); + $this->addTargets( $tSel , $subtasks , $id ); + $form->addField( $tSel ); + + // List of tasks + $tSel = Loader::Create( 'Field' , 'tasks[]' , 'select' , array( 'multiple' ) ) + ->setDescription( 'Tasks to move:' ); + foreach ( $tasks as $t ) { + $tSel->addOption( $t->id , $t->title ); + } + $form->addField( $tSel ); + + return $form->controller( ); } + + + private function addTargets( Field $field , $isTask , $id ) + { + $items = $this->dItems->getTreeList( ); + $tasks = $this->dTasks->getActiveTasksAssoc( ); + + foreach ( $items as $item ) { + $title = str_repeat( '--' , $item->depth ) . ' ' . strtoupper( $item->name ); + $disabled = !$isTask && $item->id == $id; + $iid = 'I' . $item->id; + $field->addOption( $iid , $title , $disabled ); + $this->addTargetTasks( $field , $iid , $tasks , $isTask , $id , $item->depth + 1 ); + } + } + + private function addTargetTasks( Field $field , $iid , $tasks , $isTask , $id , $depth ) + { + if ( !array_key_exists( $iid , $tasks ) ) { + return; + } + + foreach ( $tasks[ $iid ] as $task ) { + $title = str_repeat( '--' , $depth ) . '> ' . $task->title; + $disabled = $isTask && $task->id == $id; + $tid = 'T' . $task->id; + $field->addOption( $tid , $title , $disabled ); + $this->addTargetTasks( $field , $tid , $tasks , $isTask , $id , $depth + 1 ); + } + } + } diff --git a/includes/t-tasks/pages.inc.php b/includes/t-tasks/pages.inc.php index 36ca7b1..428b44a 100644 --- a/includes/t-tasks/pages.inc.php +++ b/includes/t-tasks/pages.inc.php @@ -17,7 +17,7 @@ class Page_TasksTasks 'view' => 'view_task' , 'deps/add' => 'dependency_add_form' , 'deps/delete' => 'dependency_delete_form' , - 'move' => 'task_move' , + 'move' => 'task_move_form' , 'move/down' => 'task_move_down' , 'move/up' => 'task_move_up' , 'notes/edit' => 'edit_note_form' ,