From 8c9083c3c4fbe691ddd685b507a242357529a429 Mon Sep 17 00:00:00 2001 From: Hydra Builder Date: Mon, 6 Apr 2026 11:31:31 +0000 Subject: [PATCH 1/6] =?UTF-8?q?feat:=20add=20kanban=20board=20MVP=20?= =?UTF-8?q?=E2=80=94=20column=20CRUD=20backend=20+=20board=20view=20fronte?= =?UTF-8?q?nd?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ColumnController with GET/POST/PUT/DELETE endpoints for columns - Add ColumnService with OpenRegister CRUD and project-membership checks - Add column API routes to appinfo/routes.php - Replace ProjectBoard.vue placeholder with full kanban board view showing columns left-to-right with task cards (title, assignee, priority, due date) - Add "Add column" button at right edge of board - Extend projects store with fetchColumns, fetchTasks, fetchBoard actions - Add /projects/:id/board route alias - Add ColumnControllerTest with 8 unit tests covering CRUD + auth - Copy OpenSpec change proposal into openspec/changes/spec/ Co-Authored-By: Claude Opus 4.6 --- appinfo/routes.php | 6 + lib/Controller/ColumnController.php | 212 +++++++++++ lib/Service/ColumnService.php | 223 ++++++++++++ openspec/changes/spec/design.md | 40 +-- openspec/changes/spec/specs/kanban-board.md | 100 ++++++ openspec/changes/spec/tasks.md | 63 +--- src/router/index.js | 5 + src/store/projects.js | 101 ++++++ src/views/ProjectBoard.vue | 339 +++++++++++++++++- .../unit/Controller/ColumnControllerTest.php | 284 +++++++++++++++ 10 files changed, 1286 insertions(+), 87 deletions(-) create mode 100644 lib/Controller/ColumnController.php create mode 100644 lib/Service/ColumnService.php create mode 100644 openspec/changes/spec/specs/kanban-board.md create mode 100644 tests/unit/Controller/ColumnControllerTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index b7487b7..c8142a1 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -10,6 +10,12 @@ ['name' => 'settings#create', 'url' => '/api/settings', 'verb' => 'POST'], ['name' => 'settings#load', 'url' => '/api/settings/load', 'verb' => 'POST'], + // Column CRUD endpoints. + ['name' => 'column#index', 'url' => '/api/columns', 'verb' => 'GET'], + ['name' => 'column#create', 'url' => '/api/columns', 'verb' => 'POST'], + ['name' => 'column#update', 'url' => '/api/columns/{id}', 'verb' => 'PUT'], + ['name' => 'column#destroy', 'url' => '/api/columns/{id}', 'verb' => 'DELETE'], + // Prometheus metrics endpoint. ['name' => 'metrics#index', 'url' => '/api/metrics', 'verb' => 'GET'], // Health check endpoint. diff --git a/lib/Controller/ColumnController.php b/lib/Controller/ColumnController.php new file mode 100644 index 0000000..14ce42a --- /dev/null +++ b/lib/Controller/ColumnController.php @@ -0,0 +1,212 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + */ + +declare(strict_types=1); + +namespace OCA\Planix\Controller; + +use OCA\Planix\AppInfo\Application; +use OCA\Planix\Service\ColumnService; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; + +/** + * Controller for managing kanban board columns. + */ +class ColumnController extends Controller +{ + + /** + * Constructor for the ColumnController. + * + * @param IRequest $request The request object + * @param ColumnService $columnService The column service + * + * @return void + */ + public function __construct( + IRequest $request, + private ColumnService $columnService, + ) { + parent::__construct(appName: Application::APP_ID, request: $request); + }//end __construct() + + /** + * List columns for a project, ordered by position. + * + * @NoAdminRequired + * + * @return JSONResponse + */ + public function index(): JSONResponse + { + $projectId = $this->request->getParam('projectId'); + + if (empty($projectId) === true) { + return new JSONResponse( + ['error' => 'The projectId query parameter is required.'], + Http::STATUS_BAD_REQUEST + ); + } + + if ($this->columnService->isProjectMember($projectId) === false) { + return new JSONResponse( + ['error' => 'You are not a member of this project.'], + Http::STATUS_FORBIDDEN + ); + } + + $columns = $this->columnService->listColumns($projectId); + + return new JSONResponse($columns); + + }//end index() + + /** + * Create a new column. + * + * @NoAdminRequired + * + * @return JSONResponse + */ + public function create(): JSONResponse + { + $data = $this->request->getParams(); + $projectId = $data['project'] ?? ($data['projectId'] ?? null); + + if (empty($projectId) === true) { + return new JSONResponse( + ['error' => 'The project field is required.'], + Http::STATUS_BAD_REQUEST + ); + } + + if ($this->columnService->isProjectMember($projectId) === false) { + return new JSONResponse( + ['error' => 'You are not a member of this project.'], + Http::STATUS_FORBIDDEN + ); + } + + $columnData = [ + 'title' => $data['title'] ?? '', + 'project' => $projectId, + 'order' => (int) ($data['order'] ?? ($data['position'] ?? 0)), + 'wipLimit' => isset($data['wipLimit']) ? (int) $data['wipLimit'] : null, + 'color' => $data['color'] ?? null, + 'type' => $data['type'] ?? 'active', + ]; + + $column = $this->columnService->createColumn($columnData); + + return new JSONResponse($column, Http::STATUS_CREATED); + + }//end create() + + /** + * Update an existing column. + * + * @NoAdminRequired + * + * @param string $id The column UUID + * + * @return JSONResponse + */ + public function update(string $id): JSONResponse + { + $column = $this->columnService->findColumn($id); + if ($column === null) { + return new JSONResponse( + ['error' => 'Column not found.'], + Http::STATUS_NOT_FOUND + ); + } + + $projectId = $column['project'] ?? ''; + if ($this->columnService->isProjectMember($projectId) === false) { + return new JSONResponse( + ['error' => 'You are not a member of this project.'], + Http::STATUS_FORBIDDEN + ); + } + + $data = $this->request->getParams(); + $updateData = []; + + if (isset($data['title']) === true) { + $updateData['title'] = $data['title']; + } + + if (isset($data['order']) === true || isset($data['position']) === true) { + $updateData['order'] = (int) ($data['order'] ?? $data['position']); + } + + if (array_key_exists('wipLimit', $data) === true) { + $updateData['wipLimit'] = $data['wipLimit'] !== null ? (int) $data['wipLimit'] : null; + } + + if (isset($data['color']) === true) { + $updateData['color'] = $data['color']; + } + + if (isset($data['type']) === true) { + $updateData['type'] = $data['type']; + } + + $updated = $this->columnService->updateColumn($id, $updateData); + + return new JSONResponse($updated); + + }//end update() + + /** + * Delete a column. Tasks in this column are moved to the backlog. + * + * @NoAdminRequired + * + * @param string $id The column UUID + * + * @return JSONResponse + */ + public function destroy(string $id): JSONResponse + { + $column = $this->columnService->findColumn($id); + if ($column === null) { + return new JSONResponse( + ['error' => 'Column not found.'], + Http::STATUS_NOT_FOUND + ); + } + + $projectId = $column['project'] ?? ''; + if ($this->columnService->isProjectMember($projectId) === false) { + return new JSONResponse( + ['error' => 'You are not a member of this project.'], + Http::STATUS_FORBIDDEN + ); + } + + $this->columnService->deleteColumn($id); + + return new JSONResponse(['success' => true]); + + }//end destroy() +}//end class diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php new file mode 100644 index 0000000..856b204 --- /dev/null +++ b/lib/Service/ColumnService.php @@ -0,0 +1,223 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + */ + +declare(strict_types=1); + +namespace OCA\Planix\Service; + +use OCP\IUserSession; +use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; + +/** + * Service for managing kanban board columns. + * + * Wraps OpenRegister CRUD for the 'column' schema and adds + * project-membership authorization checks. + */ +class ColumnService +{ + + /** + * Constructor for the ColumnService. + * + * @param ContainerInterface $container The DI container + * @param IUserSession $userSession The user session + * @param LoggerInterface $logger The logger + * + * @return void + */ + public function __construct( + private ContainerInterface $container, + private IUserSession $userSession, + private LoggerInterface $logger, + ) { + }//end __construct() + + /** + * Get the OpenRegister object service. + * + * @return object The ObjectService instance + * + * @throws \RuntimeException When OpenRegister is not available + */ + private function getObjectService(): object + { + try { + return $this->container->get('OCA\OpenRegister\Service\ObjectService'); + } catch (\Throwable $e) { + $this->logger->error('Planix: OpenRegister ObjectService not available', ['exception' => $e->getMessage()]); + throw new \RuntimeException('OpenRegister is not available.'); + } + + }//end getObjectService() + + /** + * Get the current user UID. + * + * @return string + */ + private function getCurrentUid(): string + { + $user = $this->userSession->getUser(); + return $user !== null ? $user->getUID() : ''; + + }//end getCurrentUid() + + /** + * Check whether the current user is a member of the given project. + * + * @param string $projectId The project UUID + * + * @return bool + */ + public function isProjectMember(string $projectId): bool + { + try { + $objectService = $this->getObjectService(); + $project = $objectService->findObject(register: 'planix', schema: 'project', id: $projectId); + + if ($project === null) { + return false; + } + + $members = $project['members'] ?? []; + $uid = $this->getCurrentUid(); + + return in_array($uid, $members, true); + } catch (\Throwable $e) { + $this->logger->warning('Planix: membership check failed', ['exception' => $e->getMessage()]); + return false; + } + + }//end isProjectMember() + + /** + * List columns for a project, ordered by position. + * + * @param string $projectId The project UUID + * + * @return array List of column objects + */ + public function listColumns(string $projectId): array + { + $objectService = $this->getObjectService(); + $columns = $objectService->findObjects( + register: 'planix', + schema: 'column', + filters: ['project' => $projectId], + ); + + usort($columns, static function (array $a, array $b): int { + return ($a['order'] ?? 0) <=> ($b['order'] ?? 0); + }); + + return $columns; + + }//end listColumns() + + /** + * Find a single column by ID. + * + * @param string $id The column UUID + * + * @return array|null Column data or null if not found + */ + public function findColumn(string $id): ?array + { + $objectService = $this->getObjectService(); + $column = $objectService->findObject(register: 'planix', schema: 'column', id: $id); + + return $column ?: null; + + }//end findColumn() + + /** + * Create a new column. + * + * @param array $data Column fields (title, projectId, position) + * + * @return array The created column + */ + public function createColumn(array $data): array + { + $objectService = $this->getObjectService(); + + return $objectService->saveObject( + register: 'planix', + schema: 'column', + object: $data, + ); + + }//end createColumn() + + /** + * Update an existing column. + * + * @param string $id The column UUID + * @param array $data Updated fields + * + * @return array The updated column + */ + public function updateColumn(string $id, array $data): array + { + $objectService = $this->getObjectService(); + + return $objectService->saveObject( + register: 'planix', + schema: 'column', + object: array_merge($data, ['id' => $id]), + ); + + }//end updateColumn() + + /** + * Delete a column and move its tasks to the backlog (column = null). + * + * @param string $id The column UUID + * + * @return bool True on success + */ + public function deleteColumn(string $id): bool + { + $objectService = $this->getObjectService(); + + // Move tasks assigned to this column to backlog (column = null). + $tasks = $objectService->findObjects( + register: 'planix', + schema: 'task', + filters: ['column' => $id], + ); + + foreach ($tasks as $task) { + $objectService->saveObject( + register: 'planix', + schema: 'task', + object: [ + 'id' => $task['id'], + 'column' => null, + 'columnOrder' => 0, + ], + ); + } + + return $objectService->deleteObject(register: 'planix', schema: 'column', id: $id); + + }//end deleteColumn() +}//end class diff --git a/openspec/changes/spec/design.md b/openspec/changes/spec/design.md index fd6ae51..641d65e 100644 --- a/openspec/changes/spec/design.md +++ b/openspec/changes/spec/design.md @@ -1,39 +1,27 @@ -# Admin Settings MVP +# Kanban Board MVP -**Status**: pr-created -**Spec reference**: [admin-user-settings](../../specs/admin-user-settings.md) +**Status**: approved +**Spec reference**: [kanban-board](../../specs/kanban-board.md) **Priority**: MVP ## Summary -Implement the admin settings page for Planix. This is the first MVP feature that wires up -the backend settings API with a proper frontend admin page. The user settings dialog is -deferred to a follow-up change — this change focuses on the admin side only. +Add kanban board view to Planix. Users can see tasks organized in columns per project, +with drag-and-drop between columns. Each column represents a workflow stage. -## Scope +## Scope (2 tasks) -- Admin settings page under Nextcloud Administration → Planix -- CnVersionInfoCard as the first section -- Default columns configuration (editable ordered list) -- OpenRegister initialization status and trigger button -- Backend: SettingsController with read/write endpoints via IAppConfig -- Frontend: AdminRoot.vue with CnVersionInfoCard + CnSettingsSection components +- Backend: ColumnController for managing columns per project +- Frontend: KanbanBoard.vue with columns and task cards ## Out of scope -- User settings dialog (NcAppSettingsDialog) — separate change -- Procest bridge settings — V1, separate change -- Notification preferences — separate change +- WIP limits (V1) +- Swimlanes (V1) +- Column templates (V1) ## Architecture -- Backend: `SettingsController` exposes `GET /api/settings` and `POST /api/settings` -- Settings stored via `OCP\IAppConfig` (server-side, admin-only) -- Frontend: Vue 2 + `@conduction/nextcloud-vue` components -- No new database tables or OpenRegister entities - -## Risks - -- CnVersionInfoCard and CnSettingsSection require `@conduction/nextcloud-vue` — verify the - dependency is declared in package.json -- OpenRegister initialization depends on OpenRegister being installed and enabled +- Columns stored as OpenRegister objects (schema already defined) +- Backend: ColumnController with CRUD + reorder endpoint +- Frontend: KanbanBoard.vue using CnDataTable for now (drag-drop in V1) diff --git a/openspec/changes/spec/specs/kanban-board.md b/openspec/changes/spec/specs/kanban-board.md new file mode 100644 index 0000000..425afb9 --- /dev/null +++ b/openspec/changes/spec/specs/kanban-board.md @@ -0,0 +1,100 @@ +# Kanban Board Specification + +**Status**: idea + +**Standards**: Schema.org ItemList (board), DefinedTerm (column), Kanban Guide (kanban.university) +**Feature tier**: MVP + +**OpenSpec changes:** _(links to openspec/changes/ directories when in-progress or done)_ + +## Purpose + +The kanban board is the primary visual interface for a project in Planix. It shows tasks as cards organized into configurable columns (stages). Users drag and drop cards between columns to update task status. WIP limits on columns enforce flow discipline. Boards are filtered by assignee, label, or priority to focus attention. Each project has exactly one kanban board; columns are managed as part of the project. + +## Data Model + +See [ARCHITECTURE.md](../../docs/ARCHITECTURE.md) for full Column entity definition. + +**Column summary**: + +| Property | Type | Required | Default | +|----------|------|----------|---------| +| `title` | string | Yes | — | +| `project` | reference (Project) | Yes | — | +| `order` | integer | Yes | 0 | +| `wipLimit` | integer \| null | No | null | +| `color` | string (hex) | No | — | +| `type` | enum: active, done | No | `active` | + +## Requirements + +### Requirement: Kanban Board View [MVP] +The system MUST render a kanban board with columns and task cards for each project. + +#### Scenario: Display board +- GIVEN a project has columns and tasks +- WHEN a project member opens the board view +- THEN the system MUST display columns in their configured order +- AND each task card MUST show: title, assignee avatar, due date, priority indicator, label chips +- AND the system MUST show the task count and WIP limit indicator per column + +#### Scenario: Drag-and-drop task to column +- GIVEN a task card is visible on the board +- WHEN a user drags a task from one column to another +- THEN the system MUST update the task's `column` reference and `columnOrder` in OpenRegister +- AND the card MUST appear in the new column position immediately (optimistic UI) +- AND if the backend update fails the system MUST revert the card to its original position + +#### Scenario: WIP limit visual warning +- GIVEN column "In Progress" has wipLimit = 3 and contains 3 tasks +- WHEN a fourth task is dragged into the column +- THEN the column header MUST turn orange/red indicating a limit violation +- AND a tooltip MUST explain "WIP limit (3) exceeded" +- AND the task MUST be placed in the column despite the violation (soft limit) + +#### Scenario: Column management +- GIVEN a project member is in the project settings +- WHEN the user adds a new column with title and optional WIP limit +- THEN the system MUST create the column and append it to the board +- AND the user MUST be able to reorder columns via drag-and-drop + +#### Scenario: Board filter +- GIVEN the board shows multiple tasks from multiple assignees +- WHEN the user selects a filter (e.g., "Assignee: me") +- THEN the system MUST visually dim or hide tasks that do not match the filter +- AND the URL MUST reflect the active filter (for shareable filtered views) + +#### Scenario: Drag task from backlog to board +- GIVEN the backlog panel is open alongside the board +- WHEN a user drags a task from the backlog into a board column +- THEN the system MUST assign the task to that column +- AND the task MUST disappear from the backlog panel + +## User Stories + +- As a developer, I want to see all project tasks as cards on a board so that I understand the current state of work at a glance +- As a team member, I want to drag a task card to "Done" so that I can mark it as complete without opening the detail view +- As a project lead, I want to set WIP limits on columns so that the team doesn't overload any single stage +- As a user, I want to filter the board by my name so that I can see only my tasks +- As an admin, I want to add and reorder columns so that the board matches our team's workflow + +## Acceptance Criteria + +- [ ] Board columns render in configured order; columns are horizontally scrollable if there are many +- [ ] Task cards show title, assignee avatar (NC user avatar), due date (red if overdue), priority dot, label chips +- [ ] Drag-and-drop updates task column and order; optimistic UI with rollback on failure +- [ ] WIP limit exceeded shows a red column header and tooltip +- [ ] Filter by assignee, label, and priority works without full page reload +- [ ] Filter state is reflected in the URL hash (shareable) +- [ ] Columns can be created, renamed, reordered, and deleted via project settings +- [ ] Deleting a column with tasks prompts: "Move tasks to backlog" or "Move to another column" +- [ ] Board is keyboard-navigable (WCAG AA) +- [ ] Empty columns show a clear "Empty" state with a "+ Add task" button + +## Notes + +- The kanban board is a custom Planix Vue component (drag-and-drop via SortableJS/vue-draggable), not from the `@conduction/nextcloud-vue` library. It uses `useObjectStore` for data operations and `CnStatusBadge` for card status indicators. +- Swimlanes (V1): group cards horizontally by assignee or priority within each column +- Card quick-edit (V1): inline editing of title, due date, assignee on hover without opening detail +- Collapsed columns (V1): columns can be collapsed to save horizontal space +- Blocked task indicators (V1): tasks with unresolved `blocks` dependencies show a lock icon diff --git a/openspec/changes/spec/tasks.md b/openspec/changes/spec/tasks.md index db8f61b..c3b7092 100644 --- a/openspec/changes/spec/tasks.md +++ b/openspec/changes/spec/tasks.md @@ -1,48 +1,23 @@ -# Tasks — Admin Settings MVP +# Tasks — Kanban Board MVP -## Task 1: Backend — SettingsController admin endpoints -**spec_ref**: admin-user-settings.md → Requirement: Admin Settings Page -**files_likely_affected**: lib/Controller/SettingsController.php, appinfo/routes.php +## Task 1: Backend — Column CRUD endpoints +**spec_ref**: kanban-board.md → Requirement: Column Management +**files_likely_affected**: lib/Controller/ColumnController.php (new), appinfo/routes.php **acceptance_criteria**: -- [x] `GET /api/settings` returns current admin settings as JSON -- [x] `POST /api/settings` accepts a JSON body and stores values via IAppConfig -- [x] Settings include: `default_columns` (JSON string), `allow_project_creation` (string) -- [x] Only admin users can write settings (middleware or annotation check) -- [x] Returns 403 for non-admin write attempts +- [ ] `GET /api/columns?projectId={id}` lists columns for a project, ordered by position +- [ ] `POST /api/columns` creates a column (title, projectId, position) +- [ ] `PUT /api/columns/{id}` updates a column (title, position) +- [ ] `DELETE /api/columns/{id}` deletes a column (moves tasks to backlog) +- [ ] Returns 404 for non-existent columns +- [ ] Returns 403 for non-project-members -## Task 2: Backend — SettingsService business logic -**spec_ref**: admin-user-settings.md → Data Model -**files_likely_affected**: lib/Service/SettingsService.php +## Task 2: Frontend — Kanban board view +**spec_ref**: kanban-board.md → Scenario: View kanban board +**files_likely_affected**: src/views/KanbanBoard.vue (new), src/router/index.js **acceptance_criteria**: -- [x] `getAdminSettings()` reads all planix admin keys from IAppConfig with defaults -- [x] `setAdminSettings(array $settings)` validates and stores each key -- [x] Default values match the spec: `default_columns = ["To Do","In Progress","Review","Done"]` -- [x] Unknown keys are silently ignored (no error, no storage) - -## Task 3: Frontend — AdminRoot with CnVersionInfoCard -**spec_ref**: admin-user-settings.md → Scenario: View admin settings -**files_likely_affected**: src/views/settings/AdminRoot.vue, src/views/settings/Settings.vue -**acceptance_criteria**: -- [x] Admin settings page renders under Nextcloud Administration → Planix -- [x] First section is CnVersionInfoCard showing app name and version -- [x] Page uses CnSettingsSection for each logical group -- [x] Loads current settings from `GET /api/settings` on mount - -## Task 4: Frontend — Default columns editor -**spec_ref**: admin-user-settings.md → Scenario: Configure default columns -**files_likely_affected**: src/views/settings/Settings.vue or new component -**acceptance_criteria**: -- [x] Shows current default columns as an editable ordered list -- [x] Admin can add, remove, and reorder column names -- [x] Changes are saved via `POST /api/settings` on save button click -- [x] Shows success/error feedback after save - -## Task 5: Frontend — OpenRegister initialization section -**spec_ref**: admin-user-settings.md → Scenario: OpenRegister initialization -**files_likely_affected**: src/views/settings/Settings.vue or new component -**acceptance_criteria**: -- [x] Shows whether the Planix register is initialized (green check / warning) -- [x] If not initialized, shows "Initialize register" button -- [x] Button triggers register initialization (calls backend endpoint) -- [x] Shows loading state during initialization -- [x] Shows success or error result after completion +- [ ] /projects/:id/board route shows the kanban board for a project +- [ ] Board displays columns left-to-right with task cards in each +- [ ] Each task card shows: title, assignee avatar, priority indicator, due date +- [ ] Empty columns show "No tasks" placeholder +- [ ] "Add column" button at the right edge of the board +- [ ] Column header shows title and task count diff --git a/src/router/index.js b/src/router/index.js index a0352f8..ecb5e33 100644 --- a/src/router/index.js +++ b/src/router/index.js @@ -22,6 +22,11 @@ export default new Router({ name: 'ProjectBoard', component: () => import('../views/ProjectBoard.vue'), }, + { + path: '/projects/:id/board', + name: 'ProjectBoardView', + component: () => import('../views/ProjectBoard.vue'), + }, { path: '/projects/:id/backlog', name: 'ProjectBacklog', diff --git a/src/store/projects.js b/src/store/projects.js index 8095e7a..5c6ae58 100644 --- a/src/store/projects.js +++ b/src/store/projects.js @@ -43,6 +43,9 @@ export const useProjectsStore = defineStore('projects', { /** @type {object|null} */ activeProject: null, /** @type {boolean} */ loading: false, /** @type {string|null} */ error: null, + /** @type {Array} */ columns: [], + /** @type {Array} */ tasks: [], + /** @type {boolean} */ boardLoading: false, }), actions: { @@ -261,6 +264,104 @@ export const useProjectsStore = defineStore('projects', { return { created, failed: failedTitles.length } }, + // ── 2.6b fetchColumns ───────────────────────────────────────────── + + /** + * Fetch columns for a project, ordered by position. + * + * @param {string} projectId Parent project ID + * @return {Promise} + */ + async fetchColumns(projectId) { + try { + const objectStore = this._objectStore() + const results = await objectStore.fetchCollection(COLUMN_SCHEMA, { project: projectId }) + this.columns = [...results].sort((a, b) => (a.order ?? 0) - (b.order ?? 0)) + return this.columns + } catch (err) { + console.error('fetchColumns error:', err) + this.columns = [] + return [] + } + }, + + // ── 2.6c fetchTasks ─────────────────────────────────────────────── + + /** + * Fetch tasks for a project. + * + * @param {string} projectId Parent project ID + * @return {Promise} + */ + async fetchTasks(projectId) { + try { + const objectStore = this._objectStore() + const results = await objectStore.fetchCollection(TASK_SCHEMA, { project: projectId }) + this.tasks = results + return this.tasks + } catch (err) { + console.error('fetchTasks error:', err) + this.tasks = [] + return [] + } + }, + + // ── 2.6d fetchBoard ─────────────────────────────────────────────── + + /** + * Fetch all board data (columns + tasks) for a project. + * + * @param {string} projectId Parent project ID + * @return {Promise<{columns: Array, tasks: Array}>} + */ + async fetchBoard(projectId) { + this.boardLoading = true + try { + const [columns, tasks] = await Promise.all([ + this.fetchColumns(projectId), + this.fetchTasks(projectId), + ]) + return { columns, tasks } + } finally { + this.boardLoading = false + } + }, + + // ── 2.6e createColumn ───────────────────────────────────────────── + + /** + * Create a new column for a project. + * + * @param {object} data Column fields + * @return {Promise} + */ + async createNewColumn(data) { + const objectStore = this._objectStore() + const column = await objectStore.saveObject(COLUMN_SCHEMA, data) + if (column) { + this.columns = [...this.columns, column].sort((a, b) => (a.order ?? 0) - (b.order ?? 0)) + } + return column + }, + + // ── 2.6f updateTask ─────────────────────────────────────────────── + + /** + * Update a task (e.g. move to a different column). + * + * @param {string} taskId Task ID + * @param {object} data Updated fields + * @return {Promise} + */ + async updateTask(taskId, data) { + const objectStore = this._objectStore() + const updated = await objectStore.saveObject(TASK_SCHEMA, { id: taskId, ...data }) + if (updated) { + this.tasks = this.tasks.map((t) => (t.id === taskId ? { ...t, ...updated } : t)) + } + return updated + }, + // ── 2.7 archiveProject ──────────────────────────────────────────── /** diff --git a/src/views/ProjectBoard.vue b/src/views/ProjectBoard.vue index 9273409..8f56b47 100644 --- a/src/views/ProjectBoard.vue +++ b/src/views/ProjectBoard.vue @@ -57,21 +57,98 @@ - - - - - + +
+ +
+ + +
+
+
+ +
+

+ {{ column.title }} +

+ + {{ tasksForColumn(column.id).length }} + +
+ + +
+
+
+ + {{ task.title }} +
+
+ + + + + {{ formatDate(task.dueDate) }} + +
+
+ + +
+ {{ t('planix', 'No tasks') }} +
+
+
+ + +
+ + + {{ t('planix', 'Add column') }} + +
+ +
+ + {{ t('planix', 'Add') }} + + + {{ t('planix', 'Cancel') }} + +
+
+
+
+
@@ -79,9 +156,10 @@ @@ -155,7 +299,6 @@ export default { diff --git a/tests/unit/Controller/ColumnControllerTest.php b/tests/unit/Controller/ColumnControllerTest.php new file mode 100644 index 0000000..aef0bd9 --- /dev/null +++ b/tests/unit/Controller/ColumnControllerTest.php @@ -0,0 +1,284 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * @version GIT: + * + * @link https://conduction.nl + */ + +declare(strict_types=1); + +namespace OCA\Planix\Tests\Unit\Controller; + +use OCA\Planix\Controller\ColumnController; +use OCA\Planix\Service\ColumnService; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; + +/** + * Tests for ColumnController. + */ +class ColumnControllerTest extends TestCase +{ + + /** + * The controller under test. + * + * @var ColumnController + */ + private ColumnController $controller; + + /** + * Mock IRequest. + * + * @var IRequest&MockObject + */ + private IRequest&MockObject $request; + + /** + * Mock ColumnService. + * + * @var ColumnService&MockObject + */ + private ColumnService&MockObject $columnService; + + /** + * Set up test fixtures. + * + * @return void + */ + protected function setUp(): void + { + parent::setUp(); + + $this->request = $this->createMock(originalClassName: IRequest::class); + $this->columnService = $this->createMock(originalClassName: ColumnService::class); + + $this->controller = new ColumnController( + request: $this->request, + columnService: $this->columnService, + ); + + }//end setUp() + + /** + * Test that index() returns columns for a valid project member. + * + * @return void + */ + public function testIndexReturnsColumnsForProjectMember(): void + { + $projectId = 'proj-uuid-1'; + $columns = [ + ['id' => 'col-1', 'title' => 'To Do', 'order' => 0], + ['id' => 'col-2', 'title' => 'In Progress', 'order' => 1], + ]; + + $this->request->expects($this->once()) + ->method('getParam') + ->with('projectId') + ->willReturn($projectId); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with($projectId) + ->willReturn(true); + + $this->columnService->expects($this->once()) + ->method('listColumns') + ->with($projectId) + ->willReturn($columns); + + $result = $this->controller->index(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: 200, actual: $result->getStatus()); + self::assertSame(expected: $columns, actual: $result->getData()); + + }//end testIndexReturnsColumnsForProjectMember() + + /** + * Test that index() returns 403 for non-project-members. + * + * @return void + */ + public function testIndexReturnsForbiddenForNonMember(): void + { + $projectId = 'proj-uuid-1'; + + $this->request->expects($this->once()) + ->method('getParam') + ->with('projectId') + ->willReturn($projectId); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with($projectId) + ->willReturn(false); + + $this->columnService->expects($this->never()) + ->method('listColumns'); + + $result = $this->controller->index(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_FORBIDDEN, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testIndexReturnsForbiddenForNonMember() + + /** + * Test that index() returns 400 when projectId is missing. + * + * @return void + */ + public function testIndexReturnsBadRequestWithoutProjectId(): void + { + $this->request->expects($this->once()) + ->method('getParam') + ->with('projectId') + ->willReturn(null); + + $result = $this->controller->index(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + + }//end testIndexReturnsBadRequestWithoutProjectId() + + /** + * Test that create() creates a column successfully. + * + * @return void + */ + public function testCreateReturnsCreatedColumn(): void + { + $params = ['title' => 'Testing', 'project' => 'proj-uuid-1', 'order' => 4, 'type' => 'active']; + $created = array_merge($params, ['id' => 'col-new']); + + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn($params); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->columnService->expects($this->once()) + ->method('createColumn') + ->willReturn($created); + + $result = $this->controller->create(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_CREATED, actual: $result->getStatus()); + self::assertSame(expected: 'col-new', actual: $result->getData()['id']); + + }//end testCreateReturnsCreatedColumn() + + /** + * Test that update() returns 404 for non-existent column. + * + * @return void + */ + public function testUpdateReturnsNotFoundForMissingColumn(): void + { + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('nonexistent') + ->willReturn(null); + + $result = $this->controller->update('nonexistent'); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_NOT_FOUND, actual: $result->getStatus()); + + }//end testUpdateReturnsNotFoundForMissingColumn() + + /** + * Test that update() returns 403 for non-project-member. + * + * @return void + */ + public function testUpdateReturnsForbiddenForNonMember(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(false); + + $result = $this->controller->update('col-1'); + + self::assertSame(expected: Http::STATUS_FORBIDDEN, actual: $result->getStatus()); + + }//end testUpdateReturnsForbiddenForNonMember() + + /** + * Test that destroy() returns 404 for non-existent column. + * + * @return void + */ + public function testDestroyReturnsNotFoundForMissingColumn(): void + { + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('nonexistent') + ->willReturn(null); + + $result = $this->controller->destroy('nonexistent'); + + self::assertSame(expected: Http::STATUS_NOT_FOUND, actual: $result->getStatus()); + + }//end testDestroyReturnsNotFoundForMissingColumn() + + /** + * Test that destroy() deletes column and returns success for project member. + * + * @return void + */ + public function testDestroyDeletesColumnForMember(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->columnService->expects($this->once()) + ->method('deleteColumn') + ->with('col-1') + ->willReturn(true); + + $result = $this->controller->destroy('col-1'); + + self::assertSame(expected: 200, actual: $result->getStatus()); + self::assertTrue(condition: $result->getData()['success']); + + }//end testDestroyDeletesColumnForMember() +}//end class From 0335578f6f7814508f4a529a8bdf6503c2ae208a Mon Sep 17 00:00:00 2001 From: Hydra Builder Date: Mon, 6 Apr 2026 11:37:18 +0000 Subject: [PATCH 2/6] fix: resolve PHPCS and ESLint violations in new files - Replace ternary operators with if/else blocks (PHPCS inline-IF rule) - Fix implicit true comparisons (PHPCS strict comparison rule) - Reorder @param before @NoAdminRequired in docblocks - Remove unused ViewColumnOutline component import - Fix max-attributes-per-line on kanban board container Co-Authored-By: Claude Opus 4.6 --- lib/Controller/ColumnController.php | 22 +++++++++++++++------- lib/Service/ColumnService.php | 22 ++++++++++++++++------ src/views/ProjectBoard.vue | 9 +++++---- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/lib/Controller/ColumnController.php b/lib/Controller/ColumnController.php index 14ce42a..de43ba0 100644 --- a/lib/Controller/ColumnController.php +++ b/lib/Controller/ColumnController.php @@ -33,7 +33,6 @@ */ class ColumnController extends Controller { - /** * Constructor for the ColumnController. * @@ -106,11 +105,16 @@ public function create(): JSONResponse ); } + $wipLimit = null; + if (isset($data['wipLimit']) === true) { + $wipLimit = (int) $data['wipLimit']; + } + $columnData = [ 'title' => $data['title'] ?? '', 'project' => $projectId, 'order' => (int) ($data['order'] ?? ($data['position'] ?? 0)), - 'wipLimit' => isset($data['wipLimit']) ? (int) $data['wipLimit'] : null, + 'wipLimit' => $wipLimit, 'color' => $data['color'] ?? null, 'type' => $data['type'] ?? 'active', ]; @@ -124,10 +128,10 @@ public function create(): JSONResponse /** * Update an existing column. * - * @NoAdminRequired - * * @param string $id The column UUID * + * @NoAdminRequired + * * @return JSONResponse */ public function update(string $id): JSONResponse @@ -160,7 +164,11 @@ public function update(string $id): JSONResponse } if (array_key_exists('wipLimit', $data) === true) { - $updateData['wipLimit'] = $data['wipLimit'] !== null ? (int) $data['wipLimit'] : null; + if ($data['wipLimit'] !== null) { + $updateData['wipLimit'] = (int) $data['wipLimit']; + } else { + $updateData['wipLimit'] = null; + } } if (isset($data['color']) === true) { @@ -180,10 +188,10 @@ public function update(string $id): JSONResponse /** * Delete a column. Tasks in this column are moved to the backlog. * - * @NoAdminRequired - * * @param string $id The column UUID * + * @NoAdminRequired + * * @return JSONResponse */ public function destroy(string $id): JSONResponse diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 856b204..e3f6dcf 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -33,7 +33,6 @@ */ class ColumnService { - /** * Constructor for the ColumnService. * @@ -76,7 +75,11 @@ private function getObjectService(): object private function getCurrentUid(): string { $user = $this->userSession->getUser(); - return $user !== null ? $user->getUID() : ''; + if ($user !== null) { + return $user->getUID(); + } + + return ''; }//end getCurrentUid() @@ -124,9 +127,12 @@ public function listColumns(string $projectId): array filters: ['project' => $projectId], ); - usort($columns, static function (array $a, array $b): int { - return ($a['order'] ?? 0) <=> ($b['order'] ?? 0); - }); + usort( + $columns, + static function (array $a, array $b): int { + return ($a['order'] ?? 0) <=> ($b['order'] ?? 0); + } + ); return $columns; @@ -144,7 +150,11 @@ public function findColumn(string $id): ?array $objectService = $this->getObjectService(); $column = $objectService->findObject(register: 'planix', schema: 'column', id: $id); - return $column ?: null; + if ($column === false || $column === null) { + return null; + } + + return $column; }//end findColumn() diff --git a/src/views/ProjectBoard.vue b/src/views/ProjectBoard.vue index 8f56b47..5deb5a6 100644 --- a/src/views/ProjectBoard.vue +++ b/src/views/ProjectBoard.vue @@ -63,7 +63,11 @@ -
+
Date: Mon, 6 Apr 2026 11:37:53 +0000 Subject: [PATCH 3/6] chore: mark all kanban board MVP tasks complete, update status to pr-created Co-Authored-By: Claude Opus 4.6 --- openspec/changes/spec/design.md | 2 +- openspec/changes/spec/tasks.md | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/openspec/changes/spec/design.md b/openspec/changes/spec/design.md index 641d65e..afec6bf 100644 --- a/openspec/changes/spec/design.md +++ b/openspec/changes/spec/design.md @@ -1,6 +1,6 @@ # Kanban Board MVP -**Status**: approved +**Status**: pr-created **Spec reference**: [kanban-board](../../specs/kanban-board.md) **Priority**: MVP diff --git a/openspec/changes/spec/tasks.md b/openspec/changes/spec/tasks.md index c3b7092..5ae2cef 100644 --- a/openspec/changes/spec/tasks.md +++ b/openspec/changes/spec/tasks.md @@ -4,20 +4,20 @@ **spec_ref**: kanban-board.md → Requirement: Column Management **files_likely_affected**: lib/Controller/ColumnController.php (new), appinfo/routes.php **acceptance_criteria**: -- [ ] `GET /api/columns?projectId={id}` lists columns for a project, ordered by position -- [ ] `POST /api/columns` creates a column (title, projectId, position) -- [ ] `PUT /api/columns/{id}` updates a column (title, position) -- [ ] `DELETE /api/columns/{id}` deletes a column (moves tasks to backlog) -- [ ] Returns 404 for non-existent columns -- [ ] Returns 403 for non-project-members +- [x] `GET /api/columns?projectId={id}` lists columns for a project, ordered by position +- [x] `POST /api/columns` creates a column (title, projectId, position) +- [x] `PUT /api/columns/{id}` updates a column (title, position) +- [x] `DELETE /api/columns/{id}` deletes a column (moves tasks to backlog) +- [x] Returns 404 for non-existent columns +- [x] Returns 403 for non-project-members ## Task 2: Frontend — Kanban board view **spec_ref**: kanban-board.md → Scenario: View kanban board **files_likely_affected**: src/views/KanbanBoard.vue (new), src/router/index.js **acceptance_criteria**: -- [ ] /projects/:id/board route shows the kanban board for a project -- [ ] Board displays columns left-to-right with task cards in each -- [ ] Each task card shows: title, assignee avatar, priority indicator, due date -- [ ] Empty columns show "No tasks" placeholder -- [ ] "Add column" button at the right edge of the board -- [ ] Column header shows title and task count +- [x] /projects/:id/board route shows the kanban board for a project +- [x] Board displays columns left-to-right with task cards in each +- [x] Each task card shows: title, assignee avatar, priority indicator, due date +- [x] Empty columns show "No tasks" placeholder +- [x] "Add column" button at the right edge of the board +- [x] Column header shows title and task count From ebffcd870f291377640ec430026cb91bfbf5f89b Mon Sep 17 00:00:00 2001 From: Hydra Builder Date: Mon, 6 Apr 2026 11:58:27 +0000 Subject: [PATCH 4/6] fix iteration 1: address all CRITICAL and WARNING review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F-01: destroy() now returns 204 No Content on success (NL API strategie) and checks deleteColumn() return value — storage failures now surface as 500. F-02: create() rejects empty title with 400 Bad Request; title is required per spec. F-03: PATCH replaces PUT in routes.php — partial-update semantics per RFC 7231 and NL API strategie. F-04: index() and create() now call findProject() first; non-existent project returns 404 Not Found rather than 403 Forbidden. F-05: Added testCreateReturnsBadRequestWithoutProject(), testCreateReturnsBadRequestWithEmptyTitle(), testCreateReturnsForbiddenForNonMember(), and testIndexReturnsNotFoundForNonExistentProject(). Updated existing tests for new findProject() call and 204 status on destroy. CB-001: task.priority is now validated through sanitizePriority() against allow-list ['low','normal','high','urgent'] before use as a CSS class token. CB-002: createColumn(), updateColumn(), deleteColumn() each contain an independent isProjectMember() guard for defense-in-depth; direct service invocations cannot bypass the access check. Co-Authored-By: Claude Sonnet 4.6 --- appinfo/routes.php | 2 +- lib/Controller/ColumnController.php | 35 ++++- lib/Service/ColumnService.php | 82 +++++++++-- src/views/ProjectBoard.vue | 9 +- .../unit/Controller/ColumnControllerTest.php | 136 +++++++++++++++++- 5 files changed, 245 insertions(+), 19 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index c8142a1..c911021 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -13,7 +13,7 @@ // Column CRUD endpoints. ['name' => 'column#index', 'url' => '/api/columns', 'verb' => 'GET'], ['name' => 'column#create', 'url' => '/api/columns', 'verb' => 'POST'], - ['name' => 'column#update', 'url' => '/api/columns/{id}', 'verb' => 'PUT'], + ['name' => 'column#update', 'url' => '/api/columns/{id}', 'verb' => 'PATCH'], ['name' => 'column#destroy', 'url' => '/api/columns/{id}', 'verb' => 'DELETE'], // Prometheus metrics endpoint. diff --git a/lib/Controller/ColumnController.php b/lib/Controller/ColumnController.php index de43ba0..307c840 100644 --- a/lib/Controller/ColumnController.php +++ b/lib/Controller/ColumnController.php @@ -66,6 +66,13 @@ public function index(): JSONResponse ); } + if ($this->columnService->findProject($projectId) === null) { + return new JSONResponse( + ['error' => 'Project not found.'], + Http::STATUS_NOT_FOUND + ); + } + if ($this->columnService->isProjectMember($projectId) === false) { return new JSONResponse( ['error' => 'You are not a member of this project.'], @@ -98,6 +105,13 @@ public function create(): JSONResponse ); } + if ($this->columnService->findProject($projectId) === null) { + return new JSONResponse( + ['error' => 'Project not found.'], + Http::STATUS_NOT_FOUND + ); + } + if ($this->columnService->isProjectMember($projectId) === false) { return new JSONResponse( ['error' => 'You are not a member of this project.'], @@ -105,13 +119,21 @@ public function create(): JSONResponse ); } + $title = $data['title'] ?? ''; + if (empty($title) === true) { + return new JSONResponse( + ['error' => 'The title field is required.'], + Http::STATUS_BAD_REQUEST + ); + } + $wipLimit = null; if (isset($data['wipLimit']) === true) { $wipLimit = (int) $data['wipLimit']; } $columnData = [ - 'title' => $data['title'] ?? '', + 'title' => $title, 'project' => $projectId, 'order' => (int) ($data['order'] ?? ($data['position'] ?? 0)), 'wipLimit' => $wipLimit, @@ -212,9 +234,16 @@ public function destroy(string $id): JSONResponse ); } - $this->columnService->deleteColumn($id); + $deleted = $this->columnService->deleteColumn($id); + + if ($deleted === false) { + return new JSONResponse( + ['error' => 'Failed to delete column.'], + Http::STATUS_INTERNAL_SERVER_ERROR + ); + } - return new JSONResponse(['success' => true]); + return new JSONResponse(null, Http::STATUS_NO_CONTENT); }//end destroy() }//end class diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index e3f6dcf..f7a6c55 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -84,31 +84,53 @@ private function getCurrentUid(): string }//end getCurrentUid() /** - * Check whether the current user is a member of the given project. + * Find a project by ID. * * @param string $projectId The project UUID * - * @return bool + * @return array|null Project data or null if not found */ - public function isProjectMember(string $projectId): bool + public function findProject(string $projectId): ?array { try { $objectService = $this->getObjectService(); $project = $objectService->findObject(register: 'planix', schema: 'project', id: $projectId); - if ($project === null) { - return false; + if ($project === null || $project === false) { + return null; } - $members = $project['members'] ?? []; - $uid = $this->getCurrentUid(); - - return in_array($uid, $members, true); + return $project; } catch (\Throwable $e) { - $this->logger->warning('Planix: membership check failed', ['exception' => $e->getMessage()]); + $this->logger->warning('Planix: project lookup failed', ['exception' => $e->getMessage()]); + return null; + } + + }//end findProject() + + /** + * Check whether the current user is a member of the given project. + * + * @param string $projectId The project UUID + * + * @return bool + */ + public function isProjectMember(string $projectId): bool + { + $uid = $this->getCurrentUid(); + if ($uid === '') { + return false; + } + + $project = $this->findProject(projectId: $projectId); + if ($project === null) { return false; } + $members = $project['members'] ?? []; + + return in_array($uid, $members, true); + }//end isProjectMember() /** @@ -161,12 +183,22 @@ public function findColumn(string $id): ?array /** * Create a new column. * + * Callers must have verified project membership before calling this method. + * This guard provides defense-in-depth by re-checking membership. + * * @param array $data Column fields (title, projectId, position) * * @return array The created column + * + * @throws \RuntimeException When caller is not a project member */ public function createColumn(array $data): array { + $projectId = $data['project'] ?? ''; + if ($this->isProjectMember(projectId: $projectId) === false) { + throw new \RuntimeException('Forbidden: caller is not a member of project '.$projectId); + } + $objectService = $this->getObjectService(); return $objectService->saveObject( @@ -180,13 +212,28 @@ public function createColumn(array $data): array /** * Update an existing column. * + * Callers must have verified project membership before calling this method. + * This guard provides defense-in-depth by re-checking membership via the column's project. + * * @param string $id The column UUID * @param array $data Updated fields * * @return array The updated column + * + * @throws \RuntimeException When caller is not a project member or column not found */ public function updateColumn(string $id, array $data): array { + $column = $this->findColumn(id: $id); + if ($column === null) { + throw new \RuntimeException('Column not found: '.$id); + } + + $projectId = $column['project'] ?? ''; + if ($this->isProjectMember(projectId: $projectId) === false) { + throw new \RuntimeException('Forbidden: caller is not a member of project '.$projectId); + } + $objectService = $this->getObjectService(); return $objectService->saveObject( @@ -200,12 +247,27 @@ public function updateColumn(string $id, array $data): array /** * Delete a column and move its tasks to the backlog (column = null). * + * Callers must have verified project membership before calling this method. + * This guard provides defense-in-depth by re-checking membership via the column's project. + * * @param string $id The column UUID * * @return bool True on success + * + * @throws \RuntimeException When caller is not a project member or column not found */ public function deleteColumn(string $id): bool { + $column = $this->findColumn(id: $id); + if ($column === null) { + throw new \RuntimeException('Column not found: '.$id); + } + + $projectId = $column['project'] ?? ''; + if ($this->isProjectMember(projectId: $projectId) === false) { + throw new \RuntimeException('Forbidden: caller is not a member of project '.$projectId); + } + $objectService = $this->getObjectService(); // Move tasks assigned to this column to backlog (column = null). diff --git a/src/views/ProjectBoard.vue b/src/views/ProjectBoard.vue index 5deb5a6..50ebdd7 100644 --- a/src/views/ProjectBoard.vue +++ b/src/views/ProjectBoard.vue @@ -96,8 +96,8 @@
+ :class="'kanban-card__priority--' + sanitizePriority(task.priority)" + :title="sanitizePriority(task.priority)" /> {{ task.title }}
@@ -250,6 +250,11 @@ export default { }) }, + sanitizePriority(priority) { + const allowed = ['low', 'normal', 'high', 'urgent'] + return allowed.includes(priority) ? priority : 'normal' + }, + tasksForColumn(columnId) { return this.tasks .filter((task) => task.column === columnId) diff --git a/tests/unit/Controller/ColumnControllerTest.php b/tests/unit/Controller/ColumnControllerTest.php index aef0bd9..903dfbc 100644 --- a/tests/unit/Controller/ColumnControllerTest.php +++ b/tests/unit/Controller/ColumnControllerTest.php @@ -91,6 +91,11 @@ public function testIndexReturnsColumnsForProjectMember(): void ->with('projectId') ->willReturn($projectId); + $this->columnService->expects($this->once()) + ->method('findProject') + ->with($projectId) + ->willReturn(['id' => $projectId, 'members' => ['user1']]); + $this->columnService->expects($this->once()) ->method('isProjectMember') ->with($projectId) @@ -109,6 +114,36 @@ public function testIndexReturnsColumnsForProjectMember(): void }//end testIndexReturnsColumnsForProjectMember() + /** + * Test that index() returns 404 for non-existent projects. + * + * @return void + */ + public function testIndexReturnsNotFoundForNonExistentProject(): void + { + $projectId = 'nonexistent'; + + $this->request->expects($this->once()) + ->method('getParam') + ->with('projectId') + ->willReturn($projectId); + + $this->columnService->expects($this->once()) + ->method('findProject') + ->with($projectId) + ->willReturn(null); + + $this->columnService->expects($this->never()) + ->method('listColumns'); + + $result = $this->controller->index(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_NOT_FOUND, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testIndexReturnsNotFoundForNonExistentProject() + /** * Test that index() returns 403 for non-project-members. * @@ -123,6 +158,11 @@ public function testIndexReturnsForbiddenForNonMember(): void ->with('projectId') ->willReturn($projectId); + $this->columnService->expects($this->once()) + ->method('findProject') + ->with($projectId) + ->willReturn(['id' => $projectId, 'members' => ['other-user']]); + $this->columnService->expects($this->once()) ->method('isProjectMember') ->with($projectId) @@ -172,6 +212,11 @@ public function testCreateReturnsCreatedColumn(): void ->method('getParams') ->willReturn($params); + $this->columnService->expects($this->once()) + ->method('findProject') + ->with('proj-uuid-1') + ->willReturn(['id' => 'proj-uuid-1', 'members' => ['user1']]); + $this->columnService->expects($this->once()) ->method('isProjectMember') ->with('proj-uuid-1') @@ -189,6 +234,92 @@ public function testCreateReturnsCreatedColumn(): void }//end testCreateReturnsCreatedColumn() + /** + * Test that create() returns 400 when project field is missing. + * + * @return void + */ + public function testCreateReturnsBadRequestWithoutProject(): void + { + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['title' => 'My Column']); + + $this->columnService->expects($this->never()) + ->method('isProjectMember'); + + $result = $this->controller->create(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testCreateReturnsBadRequestWithoutProject() + + /** + * Test that create() returns 400 when title is empty. + * + * @return void + */ + public function testCreateReturnsBadRequestWithEmptyTitle(): void + { + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['project' => 'proj-uuid-1', 'title' => '']); + + $this->columnService->expects($this->once()) + ->method('findProject') + ->with('proj-uuid-1') + ->willReturn(['id' => 'proj-uuid-1', 'members' => ['user1']]); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->columnService->expects($this->never()) + ->method('createColumn'); + + $result = $this->controller->create(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testCreateReturnsBadRequestWithEmptyTitle() + + /** + * Test that create() returns 403 for non-project-member. + * + * @return void + */ + public function testCreateReturnsForbiddenForNonMember(): void + { + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['title' => 'My Column', 'project' => 'proj-uuid-1']); + + $this->columnService->expects($this->once()) + ->method('findProject') + ->with('proj-uuid-1') + ->willReturn(['id' => 'proj-uuid-1', 'members' => ['other-user']]); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(false); + + $this->columnService->expects($this->never()) + ->method('createColumn'); + + $result = $this->controller->create(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_FORBIDDEN, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testCreateReturnsForbiddenForNonMember() + /** * Test that update() returns 404 for non-existent column. * @@ -252,7 +383,7 @@ public function testDestroyReturnsNotFoundForMissingColumn(): void }//end testDestroyReturnsNotFoundForMissingColumn() /** - * Test that destroy() deletes column and returns success for project member. + * Test that destroy() deletes column and returns 204 for project member. * * @return void */ @@ -277,8 +408,7 @@ public function testDestroyDeletesColumnForMember(): void $result = $this->controller->destroy('col-1'); - self::assertSame(expected: 200, actual: $result->getStatus()); - self::assertTrue(condition: $result->getData()['success']); + self::assertSame(expected: Http::STATUS_NO_CONTENT, actual: $result->getStatus()); }//end testDestroyDeletesColumnForMember() }//end class From 803cf835c86326b6e31c4d26acca53b19dcea6c0 Mon Sep 17 00:00:00 2001 From: Hydra Builder Date: Mon, 6 Apr 2026 12:12:36 +0000 Subject: [PATCH 5/6] fix iteration 2: findColumn() exception guard, double-lookup, uncaught RuntimeException, spec directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F-06: findColumn() now wraps getObjectService() in try/catch (like findProject()), returning null on failure and logging a warning — prevents unhandled 500 when OpenRegister is down. F-07: isProjectMember() accepts an optional pre-fetched project array; index() and create() now call findProject() once and pass the result to isProjectMember(), eliminating the double round-trip to OpenRegister per request. F-08: update() and destroy() wrap updateColumn()/deleteColumn() in try/catch(\RuntimeException) and return a JSON 500 response instead of propagating an unhandled exception. Two new tests cover these paths: testUpdateReturns500OnServiceException() and testDestroyReturns500OnServiceException(). F-09: Restored openspec/changes/spec/design.md and tasks.md to their original Admin Settings MVP content (from commit c9bdd8c). Kanban Board spec now lives in openspec/changes/kanban-board/: design.md status set to pr-created with PR link and implementation note; tasks.md gains the MVP implementation tasks with PATCH (not PUT) for the column-update endpoint, resolving the spec/implementation verb mismatch. Co-Authored-By: Claude Sonnet 4.6 --- lib/Controller/ColumnController.php | 28 ++++-- lib/Service/ColumnService.php | 25 +++-- openspec/changes/kanban-board/design.md | 8 +- openspec/changes/kanban-board/tasks.md | 32 ++++++- openspec/changes/spec/design.md | 38 +++++--- openspec/changes/spec/tasks.md | 63 ++++++++---- .../unit/Controller/ColumnControllerTest.php | 96 +++++++++++++++++-- 7 files changed, 231 insertions(+), 59 deletions(-) diff --git a/lib/Controller/ColumnController.php b/lib/Controller/ColumnController.php index 307c840..ccea65c 100644 --- a/lib/Controller/ColumnController.php +++ b/lib/Controller/ColumnController.php @@ -66,14 +66,15 @@ public function index(): JSONResponse ); } - if ($this->columnService->findProject($projectId) === null) { + $project = $this->columnService->findProject($projectId); + if ($project === null) { return new JSONResponse( ['error' => 'Project not found.'], Http::STATUS_NOT_FOUND ); } - if ($this->columnService->isProjectMember($projectId) === false) { + if ($this->columnService->isProjectMember($projectId, $project) === false) { return new JSONResponse( ['error' => 'You are not a member of this project.'], Http::STATUS_FORBIDDEN @@ -105,14 +106,15 @@ public function create(): JSONResponse ); } - if ($this->columnService->findProject($projectId) === null) { + $project = $this->columnService->findProject($projectId); + if ($project === null) { return new JSONResponse( ['error' => 'Project not found.'], Http::STATUS_NOT_FOUND ); } - if ($this->columnService->isProjectMember($projectId) === false) { + if ($this->columnService->isProjectMember($projectId, $project) === false) { return new JSONResponse( ['error' => 'You are not a member of this project.'], Http::STATUS_FORBIDDEN @@ -201,7 +203,14 @@ public function update(string $id): JSONResponse $updateData['type'] = $data['type']; } - $updated = $this->columnService->updateColumn($id, $updateData); + try { + $updated = $this->columnService->updateColumn($id, $updateData); + } catch (\RuntimeException $e) { + return new JSONResponse( + ['error' => 'Failed to update column.'], + Http::STATUS_INTERNAL_SERVER_ERROR + ); + } return new JSONResponse($updated); @@ -234,7 +243,14 @@ public function destroy(string $id): JSONResponse ); } - $deleted = $this->columnService->deleteColumn($id); + try { + $deleted = $this->columnService->deleteColumn($id); + } catch (\RuntimeException $e) { + return new JSONResponse( + ['error' => 'Failed to delete column.'], + Http::STATUS_INTERNAL_SERVER_ERROR + ); + } if ($deleted === false) { return new JSONResponse( diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index f7a6c55..2aa3756 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -111,18 +111,22 @@ public function findProject(string $projectId): ?array /** * Check whether the current user is a member of the given project. * - * @param string $projectId The project UUID + * @param string $projectId The project UUID + * @param array|null $project Pre-fetched project data to avoid a second lookup; fetched if null * * @return bool */ - public function isProjectMember(string $projectId): bool + public function isProjectMember(string $projectId, ?array $project=null): bool { $uid = $this->getCurrentUid(); if ($uid === '') { return false; } - $project = $this->findProject(projectId: $projectId); + if ($project === null) { + $project = $this->findProject(projectId: $projectId); + } + if ($project === null) { return false; } @@ -169,15 +173,20 @@ static function (array $a, array $b): int { */ public function findColumn(string $id): ?array { - $objectService = $this->getObjectService(); - $column = $objectService->findObject(register: 'planix', schema: 'column', id: $id); + try { + $objectService = $this->getObjectService(); + $column = $objectService->findObject(register: 'planix', schema: 'column', id: $id); + + if ($column === false || $column === null) { + return null; + } - if ($column === false || $column === null) { + return $column; + } catch (\Throwable $e) { + $this->logger->warning('Planix: column lookup failed', ['exception' => $e->getMessage()]); return null; } - return $column; - }//end findColumn() /** diff --git a/openspec/changes/kanban-board/design.md b/openspec/changes/kanban-board/design.md index 1d5eee3..7e1b157 100644 --- a/openspec/changes/kanban-board/design.md +++ b/openspec/changes/kanban-board/design.md @@ -1,8 +1,14 @@ # Design: kanban-board **Change ID:** kanban-board -**Status:** draft +**Status:** pr-created **Created:** 2026-04-02 +**PR:** https://github.com/ConductionNL/planix/pull/83 + +> **Implementation note:** The MVP shipped in PR #83 implements the kanban board as a +> PHP-controller-backed feature (ColumnController + ColumnService + ProjectBoard.vue) +> rather than the pure-Vue/Pinia approach described below. The full spec remains here +> as the target architecture for future iterations. --- diff --git a/openspec/changes/kanban-board/tasks.md b/openspec/changes/kanban-board/tasks.md index 1b50530..5713574 100644 --- a/openspec/changes/kanban-board/tasks.md +++ b/openspec/changes/kanban-board/tasks.md @@ -1,12 +1,40 @@ # Tasks: kanban-board **Change ID:** kanban-board -**Status:** draft +**Status:** pr-created **Created:** 2026-04-02 --- -## Implementation Tasks +## MVP Implementation Tasks (PR #83) + +These are the tasks actually implemented in PR #83 (simplified PHP-controller approach): + +### Task 1: Backend — Column CRUD endpoints +**spec_ref**: kanban-board.md → Requirement: Column Management +**files_likely_affected**: lib/Controller/ColumnController.php (new), appinfo/routes.php +**acceptance_criteria**: +- [x] `GET /api/columns?projectId={id}` lists columns for a project, ordered by position +- [x] `POST /api/columns` creates a column (title, projectId, position) +- [x] `PATCH /api/columns/{id}` updates a column (title, position) — partial update per RFC 7231 / NL API strategie +- [x] `DELETE /api/columns/{id}` deletes a column (moves tasks to backlog), returns 204 No Content +- [x] Returns 404 for non-existent columns or projects +- [x] Returns 403 for non-project-members + +### Task 2: Frontend — Kanban board view +**spec_ref**: kanban-board.md → Scenario: View kanban board +**files_likely_affected**: src/views/ProjectBoard.vue, src/store/projects.js +**acceptance_criteria**: +- [x] /projects/:id/board route shows the kanban board for a project +- [x] Board displays columns left-to-right with task cards in each +- [x] Each task card shows: title, assignee avatar, priority indicator, due date +- [x] Empty columns show "No tasks" placeholder +- [x] "Add column" button at the right edge of the board +- [x] Column header shows title and task count + +--- + +## Full Spec Tasks (future iterations) ### Task 1: Setup and Dependencies - **spec_ref**: `openspec/changes/kanban-board/proposal.md` diff --git a/openspec/changes/spec/design.md b/openspec/changes/spec/design.md index afec6bf..fd6ae51 100644 --- a/openspec/changes/spec/design.md +++ b/openspec/changes/spec/design.md @@ -1,27 +1,39 @@ -# Kanban Board MVP +# Admin Settings MVP **Status**: pr-created -**Spec reference**: [kanban-board](../../specs/kanban-board.md) +**Spec reference**: [admin-user-settings](../../specs/admin-user-settings.md) **Priority**: MVP ## Summary -Add kanban board view to Planix. Users can see tasks organized in columns per project, -with drag-and-drop between columns. Each column represents a workflow stage. +Implement the admin settings page for Planix. This is the first MVP feature that wires up +the backend settings API with a proper frontend admin page. The user settings dialog is +deferred to a follow-up change — this change focuses on the admin side only. -## Scope (2 tasks) +## Scope -- Backend: ColumnController for managing columns per project -- Frontend: KanbanBoard.vue with columns and task cards +- Admin settings page under Nextcloud Administration → Planix +- CnVersionInfoCard as the first section +- Default columns configuration (editable ordered list) +- OpenRegister initialization status and trigger button +- Backend: SettingsController with read/write endpoints via IAppConfig +- Frontend: AdminRoot.vue with CnVersionInfoCard + CnSettingsSection components ## Out of scope -- WIP limits (V1) -- Swimlanes (V1) -- Column templates (V1) +- User settings dialog (NcAppSettingsDialog) — separate change +- Procest bridge settings — V1, separate change +- Notification preferences — separate change ## Architecture -- Columns stored as OpenRegister objects (schema already defined) -- Backend: ColumnController with CRUD + reorder endpoint -- Frontend: KanbanBoard.vue using CnDataTable for now (drag-drop in V1) +- Backend: `SettingsController` exposes `GET /api/settings` and `POST /api/settings` +- Settings stored via `OCP\IAppConfig` (server-side, admin-only) +- Frontend: Vue 2 + `@conduction/nextcloud-vue` components +- No new database tables or OpenRegister entities + +## Risks + +- CnVersionInfoCard and CnSettingsSection require `@conduction/nextcloud-vue` — verify the + dependency is declared in package.json +- OpenRegister initialization depends on OpenRegister being installed and enabled diff --git a/openspec/changes/spec/tasks.md b/openspec/changes/spec/tasks.md index 5ae2cef..db8f61b 100644 --- a/openspec/changes/spec/tasks.md +++ b/openspec/changes/spec/tasks.md @@ -1,23 +1,48 @@ -# Tasks — Kanban Board MVP +# Tasks — Admin Settings MVP -## Task 1: Backend — Column CRUD endpoints -**spec_ref**: kanban-board.md → Requirement: Column Management -**files_likely_affected**: lib/Controller/ColumnController.php (new), appinfo/routes.php +## Task 1: Backend — SettingsController admin endpoints +**spec_ref**: admin-user-settings.md → Requirement: Admin Settings Page +**files_likely_affected**: lib/Controller/SettingsController.php, appinfo/routes.php **acceptance_criteria**: -- [x] `GET /api/columns?projectId={id}` lists columns for a project, ordered by position -- [x] `POST /api/columns` creates a column (title, projectId, position) -- [x] `PUT /api/columns/{id}` updates a column (title, position) -- [x] `DELETE /api/columns/{id}` deletes a column (moves tasks to backlog) -- [x] Returns 404 for non-existent columns -- [x] Returns 403 for non-project-members +- [x] `GET /api/settings` returns current admin settings as JSON +- [x] `POST /api/settings` accepts a JSON body and stores values via IAppConfig +- [x] Settings include: `default_columns` (JSON string), `allow_project_creation` (string) +- [x] Only admin users can write settings (middleware or annotation check) +- [x] Returns 403 for non-admin write attempts -## Task 2: Frontend — Kanban board view -**spec_ref**: kanban-board.md → Scenario: View kanban board -**files_likely_affected**: src/views/KanbanBoard.vue (new), src/router/index.js +## Task 2: Backend — SettingsService business logic +**spec_ref**: admin-user-settings.md → Data Model +**files_likely_affected**: lib/Service/SettingsService.php **acceptance_criteria**: -- [x] /projects/:id/board route shows the kanban board for a project -- [x] Board displays columns left-to-right with task cards in each -- [x] Each task card shows: title, assignee avatar, priority indicator, due date -- [x] Empty columns show "No tasks" placeholder -- [x] "Add column" button at the right edge of the board -- [x] Column header shows title and task count +- [x] `getAdminSettings()` reads all planix admin keys from IAppConfig with defaults +- [x] `setAdminSettings(array $settings)` validates and stores each key +- [x] Default values match the spec: `default_columns = ["To Do","In Progress","Review","Done"]` +- [x] Unknown keys are silently ignored (no error, no storage) + +## Task 3: Frontend — AdminRoot with CnVersionInfoCard +**spec_ref**: admin-user-settings.md → Scenario: View admin settings +**files_likely_affected**: src/views/settings/AdminRoot.vue, src/views/settings/Settings.vue +**acceptance_criteria**: +- [x] Admin settings page renders under Nextcloud Administration → Planix +- [x] First section is CnVersionInfoCard showing app name and version +- [x] Page uses CnSettingsSection for each logical group +- [x] Loads current settings from `GET /api/settings` on mount + +## Task 4: Frontend — Default columns editor +**spec_ref**: admin-user-settings.md → Scenario: Configure default columns +**files_likely_affected**: src/views/settings/Settings.vue or new component +**acceptance_criteria**: +- [x] Shows current default columns as an editable ordered list +- [x] Admin can add, remove, and reorder column names +- [x] Changes are saved via `POST /api/settings` on save button click +- [x] Shows success/error feedback after save + +## Task 5: Frontend — OpenRegister initialization section +**spec_ref**: admin-user-settings.md → Scenario: OpenRegister initialization +**files_likely_affected**: src/views/settings/Settings.vue or new component +**acceptance_criteria**: +- [x] Shows whether the Planix register is initialized (green check / warning) +- [x] If not initialized, shows "Initialize register" button +- [x] Button triggers register initialization (calls backend endpoint) +- [x] Shows loading state during initialization +- [x] Shows success or error result after completion diff --git a/tests/unit/Controller/ColumnControllerTest.php b/tests/unit/Controller/ColumnControllerTest.php index 903dfbc..fec7070 100644 --- a/tests/unit/Controller/ColumnControllerTest.php +++ b/tests/unit/Controller/ColumnControllerTest.php @@ -91,14 +91,16 @@ public function testIndexReturnsColumnsForProjectMember(): void ->with('projectId') ->willReturn($projectId); + $project = ['id' => $projectId, 'members' => ['user1']]; + $this->columnService->expects($this->once()) ->method('findProject') ->with($projectId) - ->willReturn(['id' => $projectId, 'members' => ['user1']]); + ->willReturn($project); $this->columnService->expects($this->once()) ->method('isProjectMember') - ->with($projectId) + ->with($projectId, $project) ->willReturn(true); $this->columnService->expects($this->once()) @@ -158,14 +160,16 @@ public function testIndexReturnsForbiddenForNonMember(): void ->with('projectId') ->willReturn($projectId); + $project = ['id' => $projectId, 'members' => ['other-user']]; + $this->columnService->expects($this->once()) ->method('findProject') ->with($projectId) - ->willReturn(['id' => $projectId, 'members' => ['other-user']]); + ->willReturn($project); $this->columnService->expects($this->once()) ->method('isProjectMember') - ->with($projectId) + ->with($projectId, $project) ->willReturn(false); $this->columnService->expects($this->never()) @@ -212,14 +216,16 @@ public function testCreateReturnsCreatedColumn(): void ->method('getParams') ->willReturn($params); + $project = ['id' => 'proj-uuid-1', 'members' => ['user1']]; + $this->columnService->expects($this->once()) ->method('findProject') ->with('proj-uuid-1') - ->willReturn(['id' => 'proj-uuid-1', 'members' => ['user1']]); + ->willReturn($project); $this->columnService->expects($this->once()) ->method('isProjectMember') - ->with('proj-uuid-1') + ->with('proj-uuid-1', $project) ->willReturn(true); $this->columnService->expects($this->once()) @@ -267,14 +273,16 @@ public function testCreateReturnsBadRequestWithEmptyTitle(): void ->method('getParams') ->willReturn(['project' => 'proj-uuid-1', 'title' => '']); + $project = ['id' => 'proj-uuid-1', 'members' => ['user1']]; + $this->columnService->expects($this->once()) ->method('findProject') ->with('proj-uuid-1') - ->willReturn(['id' => 'proj-uuid-1', 'members' => ['user1']]); + ->willReturn($project); $this->columnService->expects($this->once()) ->method('isProjectMember') - ->with('proj-uuid-1') + ->with('proj-uuid-1', $project) ->willReturn(true); $this->columnService->expects($this->never()) @@ -299,14 +307,16 @@ public function testCreateReturnsForbiddenForNonMember(): void ->method('getParams') ->willReturn(['title' => 'My Column', 'project' => 'proj-uuid-1']); + $project = ['id' => 'proj-uuid-1', 'members' => ['other-user']]; + $this->columnService->expects($this->once()) ->method('findProject') ->with('proj-uuid-1') - ->willReturn(['id' => 'proj-uuid-1', 'members' => ['other-user']]); + ->willReturn($project); $this->columnService->expects($this->once()) ->method('isProjectMember') - ->with('proj-uuid-1') + ->with('proj-uuid-1', $project) ->willReturn(false); $this->columnService->expects($this->never()) @@ -320,6 +330,72 @@ public function testCreateReturnsForbiddenForNonMember(): void }//end testCreateReturnsForbiddenForNonMember() + /** + * Test that update() returns 500 when service throws RuntimeException. + * + * @return void + */ + public function testUpdateReturns500OnServiceException(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['title' => 'Updated']); + + $this->columnService->expects($this->once()) + ->method('updateColumn') + ->willThrowException(new \RuntimeException('OpenRegister unavailable')); + + $result = $this->controller->update('col-1'); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_INTERNAL_SERVER_ERROR, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testUpdateReturns500OnServiceException() + + /** + * Test that destroy() returns 500 when service throws RuntimeException. + * + * @return void + */ + public function testDestroyReturns500OnServiceException(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->columnService->expects($this->once()) + ->method('deleteColumn') + ->willThrowException(new \RuntimeException('OpenRegister unavailable')); + + $result = $this->controller->destroy('col-1'); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_INTERNAL_SERVER_ERROR, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testDestroyReturns500OnServiceException() + /** * Test that update() returns 404 for non-existent column. * From fbf97b3f261acfbd039291de578a6b3d51a2257f Mon Sep 17 00:00:00 2001 From: Hydra Builder Date: Mon, 6 Apr 2026 12:22:31 +0000 Subject: [PATCH 6/6] fix iteration 3: type enum validation, empty-PATCH guard, and redundant project lookup - [WARNING] CSR-002: add allow-list check for column type field in create() and update() against ['active','done'] before storing; invalid values return 400 Bad Request - [WARNING] W1: update() now returns 400 Bad Request when the PATCH body contains no recognised fields (empty $updateData), preventing silent no-op or full-replace - [WARNING] W2: createColumn() now accepts an optional pre-fetched ?array $project=null parameter and passes it through to isProjectMember(), eliminating the redundant findProject() round-trip in the controller while preserving the defence-in-depth re-check for any future direct service invocations - Tests: added testCreateReturnsBadRequestWithInvalidType(), testUpdateReturnsBadRequestWithInvalidType(), and testUpdateReturnsBadRequestWithEmptyBody() --- lib/Controller/ColumnController.php | 28 ++++- lib/Service/ColumnService.php | 7 +- .../unit/Controller/ColumnControllerTest.php | 101 ++++++++++++++++++ 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/lib/Controller/ColumnController.php b/lib/Controller/ColumnController.php index ccea65c..7d59e37 100644 --- a/lib/Controller/ColumnController.php +++ b/lib/Controller/ColumnController.php @@ -129,6 +129,15 @@ public function create(): JSONResponse ); } + $allowedTypes = ['active', 'done']; + $type = $data['type'] ?? 'active'; + if (in_array($type, $allowedTypes, true) === false) { + return new JSONResponse( + ['error' => 'Invalid type. Allowed values: active, done.'], + Http::STATUS_BAD_REQUEST + ); + } + $wipLimit = null; if (isset($data['wipLimit']) === true) { $wipLimit = (int) $data['wipLimit']; @@ -140,10 +149,10 @@ public function create(): JSONResponse 'order' => (int) ($data['order'] ?? ($data['position'] ?? 0)), 'wipLimit' => $wipLimit, 'color' => $data['color'] ?? null, - 'type' => $data['type'] ?? 'active', + 'type' => $type, ]; - $column = $this->columnService->createColumn($columnData); + $column = $this->columnService->createColumn($columnData, $project); return new JSONResponse($column, Http::STATUS_CREATED); @@ -200,9 +209,24 @@ public function update(string $id): JSONResponse } if (isset($data['type']) === true) { + $allowedTypes = ['active', 'done']; + if (in_array($data['type'], $allowedTypes, true) === false) { + return new JSONResponse( + ['error' => 'Invalid type. Allowed values: active, done.'], + Http::STATUS_BAD_REQUEST + ); + } + $updateData['type'] = $data['type']; } + if (empty($updateData) === true) { + return new JSONResponse( + ['error' => 'No valid fields provided for update.'], + Http::STATUS_BAD_REQUEST + ); + } + try { $updated = $this->columnService->updateColumn($id, $updateData); } catch (\RuntimeException $e) { diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index 2aa3756..d6c2619 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -195,16 +195,17 @@ public function findColumn(string $id): ?array * Callers must have verified project membership before calling this method. * This guard provides defense-in-depth by re-checking membership. * - * @param array $data Column fields (title, projectId, position) + * @param array $data Column fields (title, projectId, position) + * @param array|null $project Pre-fetched project array to avoid a second lookup; fetched if null * * @return array The created column * * @throws \RuntimeException When caller is not a project member */ - public function createColumn(array $data): array + public function createColumn(array $data, ?array $project=null): array { $projectId = $data['project'] ?? ''; - if ($this->isProjectMember(projectId: $projectId) === false) { + if ($this->isProjectMember(projectId: $projectId, project: $project) === false) { throw new \RuntimeException('Forbidden: caller is not a member of project '.$projectId); } diff --git a/tests/unit/Controller/ColumnControllerTest.php b/tests/unit/Controller/ColumnControllerTest.php index fec7070..13f8773 100644 --- a/tests/unit/Controller/ColumnControllerTest.php +++ b/tests/unit/Controller/ColumnControllerTest.php @@ -330,6 +330,107 @@ public function testCreateReturnsForbiddenForNonMember(): void }//end testCreateReturnsForbiddenForNonMember() + /** + * Test that create() returns 400 when an invalid type value is provided. + * + * @return void + */ + public function testCreateReturnsBadRequestWithInvalidType(): void + { + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['title' => 'My Column', 'project' => 'proj-uuid-1', 'type' => 'invalid-type']); + + $project = ['id' => 'proj-uuid-1', 'members' => ['user1']]; + + $this->columnService->expects($this->once()) + ->method('findProject') + ->with('proj-uuid-1') + ->willReturn($project); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->willReturn(true); + + $this->columnService->expects($this->never()) + ->method('createColumn'); + + $result = $this->controller->create(); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testCreateReturnsBadRequestWithInvalidType() + + /** + * Test that update() returns 400 when an invalid type value is provided. + * + * @return void + */ + public function testUpdateReturnsBadRequestWithInvalidType(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn(['type' => 'invalid-type']); + + $this->columnService->expects($this->never()) + ->method('updateColumn'); + + $result = $this->controller->update('col-1'); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testUpdateReturnsBadRequestWithInvalidType() + + /** + * Test that update() returns 400 when PATCH body contains no recognised fields. + * + * @return void + */ + public function testUpdateReturnsBadRequestWithEmptyBody(): void + { + $column = ['id' => 'col-1', 'project' => 'proj-uuid-1', 'title' => 'To Do']; + + $this->columnService->expects($this->once()) + ->method('findColumn') + ->with('col-1') + ->willReturn($column); + + $this->columnService->expects($this->once()) + ->method('isProjectMember') + ->with('proj-uuid-1') + ->willReturn(true); + + $this->request->expects($this->once()) + ->method('getParams') + ->willReturn([]); + + $this->columnService->expects($this->never()) + ->method('updateColumn'); + + $result = $this->controller->update('col-1'); + + self::assertInstanceOf(expected: JSONResponse::class, actual: $result); + self::assertSame(expected: Http::STATUS_BAD_REQUEST, actual: $result->getStatus()); + self::assertArrayHasKey(key: 'error', array: $result->getData()); + + }//end testUpdateReturnsBadRequestWithEmptyBody() + /** * Test that update() returns 500 when service throws RuntimeException. *