From 88ee691bb9c17bd6d2b93873ed789d2edc120b37 Mon Sep 17 00:00:00 2001
From: Frederic Guillot <fred@kanboard.net>
Date: Sat, 28 May 2016 17:26:33 -0400
Subject: [PATCH] Fix wrong redirect after removing a task from the task view
 page

---
 ChangeLog                                     |  1 +
 app/Controller/TaskSuppressionController.php  | 53 +++++++++++++++++++
 app/Controller/TaskViewController.php         | 30 -----------
 .../AuthenticationProvider.php                |  2 +-
 app/Template/task/dropdown.php                |  2 +-
 app/Template/task/sidebar.php                 |  2 +-
 .../{task => task_suppression}/remove.php     |  2 +-
 7 files changed, 58 insertions(+), 34 deletions(-)
 create mode 100644 app/Controller/TaskSuppressionController.php
 rename app/Template/{task => task_suppression}/remove.php (69%)

diff --git a/ChangeLog b/ChangeLog
index 1342c159b..0a63fa445 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -22,6 +22,7 @@ Improvements:
 Bug fixes:
 
 * Do not send notifications to disabled users
+* Fix wrong redirect when removing a task from the task view page
 
 Breaking changes:
 
diff --git a/app/Controller/TaskSuppressionController.php b/app/Controller/TaskSuppressionController.php
new file mode 100644
index 000000000..7c9165eb8
--- /dev/null
+++ b/app/Controller/TaskSuppressionController.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace Kanboard\Controller;
+
+use Kanboard\Core\Controller\AccessForbiddenException;
+
+/**
+ * Class TaskSuppressionController
+ *
+ * @package Kanboard\Controller
+ * @author  Frederic Guillot
+ */
+class TaskSuppressionController extends BaseController
+{
+    /**
+     * Confirmation dialog box before to remove the task
+     */
+    public function confirm()
+    {
+        $task = $this->getTask();
+
+        if (! $this->helper->user->canRemoveTask($task)) {
+            throw new AccessForbiddenException();
+        }
+
+        $this->response->html($this->template->render('task_suppression/remove', array(
+            'task' => $task,
+            'redirect' => $this->request->getStringParam('redirect'),
+        )));
+    }
+
+    /**
+     * Remove a task
+     */
+    public function remove()
+    {
+        $task = $this->getTask();
+        $this->checkCSRFParam();
+
+        if (! $this->helper->user->canRemoveTask($task)) {
+            throw new AccessForbiddenException();
+        }
+
+        if ($this->task->remove($task['id'])) {
+            $this->flash->success(t('Task removed successfully.'));
+        } else {
+            $this->flash->failure(t('Unable to remove this task.'));
+        }
+
+        $redirect = $this->request->getStringParam('redirect') === '';
+        $this->response->redirect($this->helper->url->to('BoardViewController', 'show', array('project_id' => $task['project_id'])), $redirect);
+    }
+}
diff --git a/app/Controller/TaskViewController.php b/app/Controller/TaskViewController.php
index 6d3cc5c5b..b16c15dea 100644
--- a/app/Controller/TaskViewController.php
+++ b/app/Controller/TaskViewController.php
@@ -143,34 +143,4 @@ class TaskViewController extends BaseController
             'transitions' => $this->transition->getAllByTask($task['id']),
         )));
     }
-
-    /**
-     * Remove a task
-     *
-     * @access public
-     */
-    public function remove()
-    {
-        $task = $this->getTask();
-
-        if (! $this->helper->user->canRemoveTask($task)) {
-            throw new AccessForbiddenException();
-        }
-
-        if ($this->request->getStringParam('confirmation') === 'yes') {
-            $this->checkCSRFParam();
-
-            if ($this->task->remove($task['id'])) {
-                $this->flash->success(t('Task removed successfully.'));
-            } else {
-                $this->flash->failure(t('Unable to remove this task.'));
-            }
-
-            return $this->response->redirect($this->helper->url->to('BoardViewController', 'show', array('project_id' => $task['project_id'])), true);
-        }
-
-        return $this->response->html($this->template->render('task/remove', array(
-            'task' => $task,
-        )));
-    }
 }
diff --git a/app/ServiceProvider/AuthenticationProvider.php b/app/ServiceProvider/AuthenticationProvider.php
index f8ea9b19b..2fad8a3a7 100644
--- a/app/ServiceProvider/AuthenticationProvider.php
+++ b/app/ServiceProvider/AuthenticationProvider.php
@@ -92,7 +92,7 @@ class AuthenticationProvider implements ServiceProviderInterface
         $acl->add('SubtaskRestrictionController', '*', Role::PROJECT_MEMBER);
         $acl->add('SubtaskStatusController', '*', Role::PROJECT_MEMBER);
         $acl->add('SwimlaneController', '*', Role::PROJECT_MANAGER);
-        $acl->add('TaskViewController', 'remove', Role::PROJECT_MEMBER);
+        $acl->add('TaskSuppressionController', '*', Role::PROJECT_MEMBER);
         $acl->add('TaskCreationController', '*', Role::PROJECT_MEMBER);
         $acl->add('TaskBulkController', '*', Role::PROJECT_MEMBER);
         $acl->add('TaskDuplicationController', '*', Role::PROJECT_MEMBER);
diff --git a/app/Template/task/dropdown.php b/app/Template/task/dropdown.php
index 472c819f5..b6b9c789d 100644
--- a/app/Template/task/dropdown.php
+++ b/app/Template/task/dropdown.php
@@ -58,7 +58,7 @@
         <?php if ($this->user->canRemoveTask($task)): ?>
             <li>
                 <i class="fa fa-trash-o fa-fw"></i>
-                <?= $this->url->link(t('Remove'), 'TaskViewController', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+                <?= $this->url->link(t('Remove'), 'TaskSuppressionController', 'confirm', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
             </li>
         <?php endif ?>
         <?php if (isset($task['is_active'])): ?>
diff --git a/app/Template/task/sidebar.php b/app/Template/task/sidebar.php
index ff3883e17..e77ec18a4 100644
--- a/app/Template/task/sidebar.php
+++ b/app/Template/task/sidebar.php
@@ -90,7 +90,7 @@
         <?php if ($this->user->canRemoveTask($task)): ?>
             <li>
                 <i class="fa fa-trash-o fa-fw"></i>
-                <?= $this->url->link(t('Remove'), 'TaskViewController', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'popover') ?>
+                <?= $this->url->link(t('Remove'), 'TaskSuppressionController', 'confirm', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'redirect' => 'board'), false, 'popover') ?>
             </li>
         <?php endif ?>
 
diff --git a/app/Template/task/remove.php b/app/Template/task_suppression/remove.php
similarity index 69%
rename from app/Template/task/remove.php
rename to app/Template/task_suppression/remove.php
index f963a325d..5d0f77200 100644
--- a/app/Template/task/remove.php
+++ b/app/Template/task_suppression/remove.php
@@ -8,7 +8,7 @@
     </p>
 
     <div class="form-actions">
-        <?= $this->url->link(t('Yes'), 'TaskViewController', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'confirmation' => 'yes'), true, 'btn btn-red popover-link') ?>
+        <?= $this->url->link(t('Yes'), 'TaskSuppressionController', 'remove', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'redirect' => $redirect), true, 'btn btn-red popover-link') ?>
         <?= t('or') ?>
         <?= $this->url->link(t('cancel'), 'TaskViewController', 'show', array('task_id' => $task['id'], 'project_id' => $task['project_id']), false, 'close-popover') ?>
     </div>