From c63ffa413d2543bc20bd23228155b6dcd82c2fe3 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Fri, 22 May 2026 07:24:34 +0200 Subject: [PATCH] refactor: Use controllers rather than globals. Addresses horde/nag#19 --- config/routes.php | 7 + src/Controller/ResponseTrait.php | 55 ++++++ src/Controller/SaveTaskController.php | 13 +- src/Controller/TaskFormController.php | 259 ++++++++++++++++++++++++++ task.php | 177 +++--------------- 5 files changed, 350 insertions(+), 161 deletions(-) create mode 100644 src/Controller/ResponseTrait.php create mode 100644 src/Controller/TaskFormController.php diff --git a/config/routes.php b/config/routes.php index f26664e9..1cfb30b1 100644 --- a/config/routes.php +++ b/config/routes.php @@ -9,6 +9,13 @@ use Psr\Http\Server\RequestHandlerInterface; use Horde\Nag\Controller\CompleteTaskController; use Horde\Nag\Controller\SaveTaskController; +use Horde\Nag\Controller\TaskFormController; + +$mapper->buildRoute(uri: '/t/task', name: 'TaskForm') + ->withController(TaskFormController::class) + ->withMiddleware(DefaultStack::get()) + ->withSecondaryRoute('/task.php') + ->add(); $mapper->buildRoute(uri: '/t/complete', name: 'CompleteTask') ->withController(CompleteTaskController::class) diff --git a/src/Controller/ResponseTrait.php b/src/Controller/ResponseTrait.php new file mode 100644 index 00000000..84608d76 --- /dev/null +++ b/src/Controller/ResponseTrait.php @@ -0,0 +1,55 @@ +pageOutput->header(['title' => $title]); + $this->notification->notify(['listeners' => 'status']); + $renderBody(); + $this->pageOutput->footer(); + + return Horde::endBuffer(); + } +} diff --git a/src/Controller/SaveTaskController.php b/src/Controller/SaveTaskController.php index acab630e..683d3684 100644 --- a/src/Controller/SaveTaskController.php +++ b/src/Controller/SaveTaskController.php @@ -16,14 +16,14 @@ namespace Horde\Nag\Controller; use Horde; -use Horde\Core\Controller\Traits\RedirectResponseTrait; use Horde_Core_Perms; use Horde_Notification_Handler; +use Horde_PageOutput; use Horde_Perms; use Horde_Registry; use Horde_Share_Exception; -use Horde_Util; use Horde_Variables; +use Horde\Util\Util; use Nag; use Nag_Exception; use Nag_Factory_Driver; @@ -31,7 +31,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Horde\Util\Util; /** * PSR-15 controller for saving (create/update/delete) a task. @@ -42,13 +41,15 @@ */ class SaveTaskController implements RequestHandlerInterface { - use RedirectResponseTrait; + use ResponseTrait; public function __construct( private readonly Horde_Registry $registry, private readonly Horde_Notification_Handler $notification, + private readonly Horde_PageOutput $pageOutput, private readonly Nag_Factory_Driver $driverFactory, private readonly Horde_Core_Perms $perms, + private readonly TaskFormController $taskFormController, ) {} public function handle(ServerRequestInterface $request): ResponseInterface @@ -65,9 +66,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface ); if (!$form->validate($vars)) { - $_REQUEST['actionID'] = 'task_form'; - require NAG_BASE . '/task.php'; - exit; + return $this->taskFormController->renderTaskForm($form); } $info = $form->getInfo($vars); diff --git a/src/Controller/TaskFormController.php b/src/Controller/TaskFormController.php new file mode 100644 index 00000000..2f5e3cbd --- /dev/null +++ b/src/Controller/TaskFormController.php @@ -0,0 +1,259 @@ +getQueryParams(), + (array) $request->getParsedBody() + ); + + $actionID = $params['actionID'] ?? null; + + if ($actionID === null) { + return $this->redirect((string) Horde::url('list.php', true)); + } + + return match ($actionID) { + 'add_task' => $this->addTask($params), + 'modify_task' => $this->modifyTask($params, $nag_shares), + 'delete_task' => $this->deleteTask($params, $nag_shares), + 'task_form' => $this->redisplayForm(), + default => $this->redirect((string) Horde::url('list.php', true)), + }; + } + + /** + * Render a task form with full page chrome. + * + * Called directly by SaveTaskController on validation failure. + */ + public function renderTaskForm(Nag_Form_Task $form): ResponseInterface + { + $datejs = str_replace('_', '-', $GLOBALS['language']) . '.js'; + if (!file_exists($this->registry->get('jsfs', 'horde') . '/date/' . $datejs)) { + $datejs = 'en-US.js'; + } + + Horde::startBuffer(); + $form->renderActive(); + $formHtml = Horde::endBuffer(); + + $html = $this->renderChrome($form->getTitle(), function () use ($datejs, $formHtml) { + $this->pageOutput->addScriptFile('date/' . $datejs, 'horde'); + $this->pageOutput->addScriptFile('date/date.js', 'horde'); + $this->pageOutput->addScriptFile('task.js'); + $this->pageOutput->addScriptPackage(Horde_Core_Script_Package_Keynavlist::class); + require NAG_TEMPLATES . '/javascript_defs.php'; + Nag::status(); + echo $formHtml; + }); + + return $this->htmlResponse($html); + } + + private function addTask(array $params): ResponseInterface + { + if ($this->perms->hasAppPermission('max_tasks') !== true + && $this->perms->hasAppPermission('max_tasks') <= Nag::countTasks()) { + Horde::permissionDeniedError( + 'nag', + 'max_tasks', + sprintf( + _("You are not allowed to create more than %d tasks."), + $this->perms->hasAppPermission('max_tasks') + ) + ); + return $this->redirect((string) Horde::url('list.php', true)); + } + + $vars = Horde_Variables::getDefaultVariables(); + if (!$vars->exists('tasklist_id')) { + $vars->set('tasklist_id', Nag::getDefaultTasklist(Horde_Perms::EDIT)); + } + if (!empty($params['parent_task'])) { + $vars->set('parent', $params['parent_task']); + } + + $form = new Nag_Form_Task($vars, _("New Task")); + + return $this->renderTaskForm($form); + } + + private function modifyTask(array $params, $nag_shares): ResponseInterface + { + $taskId = $params['task'] ?? null; + $tasklistId = $params['tasklist'] ?? null; + + if (!$taskId || !$tasklistId) { + return $this->redirect((string) Horde::url('list.php', true)); + } + + try { + $share = $nag_shares->getShare($tasklistId); + } catch (Horde_Share_Exception $e) { + $this->notification->push( + sprintf(_("Access denied editing task: %s"), $e->getMessage()), + 'horde.error' + ); + return $this->redirect((string) Horde::url('list.php', true)); + } + + if (!$share->hasPermission($this->registry->getAuth(), Horde_Perms::EDIT)) { + $this->notification->push(_("Access denied editing task."), 'horde.error'); + return $this->redirect((string) Horde::url('list.php', true)); + } + + try { + $task = Nag::getTask($tasklistId, $taskId); + } catch (Nag_Exception $e) { + $this->notification->push(_("Task not found."), 'horde.error'); + return $this->redirect((string) Horde::url('list.php', true)); + } + + if (!isset($task) || !isset($task->id)) { + $this->notification->push(_("Task not found."), 'horde.error'); + return $this->redirect((string) Horde::url('list.php', true)); + } + + if ($task->private && $task->owner != $this->registry->getAuth()) { + $this->notification->push(_("Access denied editing task."), 'horde.error'); + return $this->redirect((string) Horde::url('list.php', true)); + } + + $h = $task->toHash(); + $h['tags'] = implode(',', $h['tags']); + $vars = new Horde_Variables($h); + $vars->set('old_tasklist', $task->tasklist); + $vars->set('url', $params['url'] ?? null); + if (!empty($params['list'])) { + $vars->set('list', $params['list']); + } + if (!empty($params['tab_name'])) { + $vars->set('tab_name', $params['tab_name']); + } + + $form = new Nag_Form_Task($vars, sprintf(_("Edit: %s"), $task->name)); + if (!$task->completed) { + $task->loadChildren(); + $form->setTask($task); + } + + return $this->renderTaskForm($form); + } + + private function deleteTask(array $params, $nag_shares): ResponseInterface + { + $taskId = $params['task'] ?? null; + $tasklistId = $params['tasklist'] ?? null; + + if (!empty($taskId)) { + try { + $task = Nag::getTask($tasklistId, $taskId); + $task->loadChildren(); + $share = $nag_shares->getShare($tasklistId); + + if (!$share->hasPermission($this->registry->getAuth(), Horde_Perms::DELETE)) { + $this->notification->push(_("Access denied deleting task."), 'horde.error'); + } else { + $storage = $this->driverFactory->create($tasklistId); + try { + $storage->delete($taskId); + } catch (Nag_Exception $e) { + $this->notification->push( + sprintf(_("There was a problem deleting %s: %s"), $task->name, $e->getMessage()), + 'horde.error' + ); + } + $this->notification->push( + sprintf(_("Deleted %s."), $task->name), + 'horde.success' + ); + } + } catch (Horde_Share_Exception $e) { + $this->notification->push( + sprintf(_("Error deleting task: %s"), $e->getMessage()), + 'horde.error' + ); + } catch (Nag_Exception $e) { + $this->notification->push( + sprintf(_("Error deleting task: %s"), $e->getMessage()), + 'horde.error' + ); + } + } + + $url = Horde::verifySignedUrl($params['url'] ?? ''); + if ($url) { + return $this->redirect($url); + } + + return $this->redirect((string) Horde::url('list.php', true)); + } + + private function redisplayForm(): ResponseInterface + { + $vars = Horde_Variables::getDefaultVariables(); + $form = new Nag_Form_Task( + $vars, + $vars->get('task_id') + ? sprintf(_("Edit: %s"), $vars->get('name')) + : _("New Task") + ); + + return $this->renderTaskForm($form); + } +} diff --git a/task.php b/task.php index d4c45c72..da431812 100644 --- a/task.php +++ b/task.php @@ -1,8 +1,11 @@ */ -function _delete($task_id, $tasklist_id) -{ - global $injector, $nag_shares, $notification, $registry; - - if (!empty($task_id)) { - try { - $task = Nag::getTask($tasklist_id, $task_id); - $task->loadChildren(); - try { - $share = $nag_shares->getShare($tasklist_id); - } catch (Horde_Share_Exception $e) { - throw new Nag_Exception($e); - } - if (!$share->hasPermission($registry->getAuth(), Horde_Perms::DELETE)) { - $notification->push(_("Access denied deleting task."), 'horde.error'); - } else { - $storage = $injector->getInstance('Nag_Factory_Driver')->create($tasklist_id); - try { - $storage->delete($task_id); - } catch (Nag_Exception $e) { - $notification->push( - sprintf( - _("There was a problem deleting %s: %s"), - $task->name, - $e->getMessage() - ), - 'horde.error' - ); - } - $notification->push( - sprintf(_("Deleted %s."), $task->name), - 'horde.success' - ); - } - } catch (Nag_Exception $e) { - $notification->push( - sprintf( - _("Error deleting task: %s"), - $e->getMessage() - ), - 'horde.error' - ); - } - } - - /* Return to the last page or to the task list. */ - if ($url = Horde::verifySignedUrl(Util::getFormData('url'))) { - header('Location: ' . $url); - exit; - } - Horde::url('list.php', true)->redirect(); -} +use Horde\Http\Server\RequestBuilder; +use Horde\Nag\Controller\TaskFormController; require_once __DIR__ . '/lib/Application.php'; Horde_Registry::appInit('nag'); -$vars = Horde_Variables::getDefaultVariables(); - -/* Redirect to the task list if no action has been requested. */ -$actionID = $vars->get('actionID'); -if (is_null($actionID)) { - Horde::url('list.php', true)->redirect(); -} +$injector = $GLOBALS['injector']; -/* Run through the action handlers. */ -switch ($actionID) { - case 'add_task': - /* Check permissions. */ - $perms = $injector->getInstance('Horde_Core_Perms'); - if ($perms->hasAppPermission('max_tasks') !== true - && $perms->hasAppPermission('max_tasks') <= Nag::countTasks()) { - Horde::permissionDeniedError( - 'nag', - 'max_tasks', - sprintf(_("You are not allowed to create more than %d tasks."), $perms->hasAppPermission('max_tasks')) - ); - Horde::url('list.php', true)->redirect(); - } +$controller = new TaskFormController( + $injector->getInstance('Horde_Registry'), + $injector->getInstance('Horde_Notification_Handler'), + $injector->getInstance('Horde_PageOutput'), + $injector->getInstance('Horde_Core_Perms'), + $injector->getInstance('Nag_Factory_Driver'), +); - if (!$vars->exists('tasklist_id')) { - $vars->set('tasklist_id', Nag::getDefaultTasklist(Horde_Perms::EDIT)); - } - if ($parent = Util::getFormData('parent_task')) { - $vars->set('parent', $parent); - } - $form = new Nag_Form_Task($vars, _("New Task")); - break; +$request = (new RequestBuilder())->withGlobalVariables()->build(); +$response = $controller->handle($request); - case 'modify_task': - $task_id = $vars->get('task'); - $tasklist_id = $vars->get('tasklist'); - try { - $share = $nag_shares->getShare($tasklist_id); - } catch (Horde_Share_Exception $e) { - $notification->push(sprintf(_("Access denied editing task: %s"), $e->getMessage()), 'horde.error'); - } - if (!$share->hasPermission($registry->getAuth(), Horde_Perms::EDIT)) { - $notification->push(_("Access denied editing task."), 'horde.error'); - } else { - $task = Nag::getTask($tasklist_id, $task_id); - if (!isset($task) || !isset($task->id)) { - $notification->push(_("Task not found."), 'horde.error'); - } elseif ($task->private && $task->owner != $registry->getAuth()) { - $notification->push(_("Access denied editing task."), 'horde.error'); - } else { - $h = $task->toHash(); - $h['tags'] = implode(',', $h['tags']); - $vars = new Horde_Variables($h); - $vars->set('old_tasklist', $task->tasklist); - $vars->set('url', Util::getFormData('url')); - if ($sl = Util::getFormData('list')) { - $vars->set('list', $sl); - } - if ($tn = Util::getFormData('tab_name')) { - $vars->set('tab_name', $tn); - } - $form = new Nag_Form_Task($vars, sprintf(_("Edit: %s"), $task->name)); - if (!$task->completed) { - $task->loadChildren(); - $form->setTask($task); - } - break; - } - } - - /* Return to the task list. */ - Horde::url('list.php', true)->redirect(); - - case 'delete_task': - /* Delete the task if we're provided with a valid task ID. */ - _delete(Util::getFormData('task'), Util::getFormData('tasklist')); - break; - - case 'task_form': - break; - - default: - Horde::url('list.php', true)->redirect(); -} - -$datejs = str_replace('_', '-', $GLOBALS['language']) . '.js'; -if (!file_exists($registry->get('jsfs', 'horde') . '/date/' . $datejs)) { - $datejs = 'en-US.js'; +http_response_code($response->getStatusCode()); +foreach ($response->getHeaders() as $name => $values) { + foreach ($values as $value) { + header($name . ': ' . $value, false); + } } -Horde::startBuffer(); -$form->renderActive(); -$formhtml = Horde::endBuffer(); - -$GLOBALS['page_output']->addScriptFile('date/' . $datejs, 'horde'); -$GLOBALS['page_output']->addScriptFile('date/date.js', 'horde'); -$GLOBALS['page_output']->addScriptFile('task.js'); -$GLOBALS['page_output']->addScriptPackage('Horde_Core_Script_Package_Keynavlist'); - -$GLOBALS['page_output']->header([ - 'title' => $form->getTitle(), -]); -require NAG_TEMPLATES . '/javascript_defs.php'; -Nag::status(); -echo $formhtml; -$GLOBALS['page_output']->footer(); +echo (string) $response->getBody();