This repository was archived by the owner on May 29, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Time Tracking MVP — CRUD backend + TimeLog frontend #22
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
3cd0494
feat: add TimeEntry CRUD backend (service, controller, routes)
d0dbb26
test: add PHPUnit tests for TimeEntry controller and service
59539bd
feat: add frontend time log component and task detail view
5877e77
fix: resolve PHPCS violations in TimeEntry files
4e24838
chore: mark all spec tasks complete, update status to pr-created
fe4f31e
fix iteration 1: security hardening and quality fixes from code review
350b064
fix iteration 2: close IDOR gaps, fix field name mismatch, harden CSRF
f0a727a
fix iteration 3: security reviewer findings — breadcrumb param, null …
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,12 @@ | |
| ['name' => 'settings#create', 'url' => '/api/settings', 'verb' => 'POST'], | ||
| ['name' => 'settings#load', 'url' => '/api/settings/load', 'verb' => 'POST'], | ||
|
|
||
| // Time entry CRUD endpoints. | ||
| ['name' => 'time_entry#create', 'url' => '/api/time-entries', 'verb' => 'POST'], | ||
| ['name' => 'time_entry#index', 'url' => '/api/time-entries', 'verb' => 'GET'], | ||
| ['name' => 'time_entry#update', 'url' => '/api/time-entries/{id}', 'verb' => 'PUT'], | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [WARNING] PUT used for partial-update semantics — should be PATCH (NL API strategie) The route registers
The implementation is partial-update behaviour, which means the correct HTTP verb is PATCH. Required action: Change the route verb from |
||
| ['name' => 'time_entry#destroy', 'url' => '/api/time-entries/{id}', 'verb' => 'DELETE'], | ||
|
|
||
| // Prometheus metrics endpoint. | ||
| ['name' => 'metrics#index', 'url' => '/api/metrics', 'verb' => 'GET'], | ||
| // Health check endpoint. | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,234 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * Planix Time Entry Controller | ||
| * | ||
| * Controller for time entry CRUD operations. | ||
| * | ||
| * @category Controller | ||
| * @package OCA\Planix\Controller | ||
| * | ||
| * @author Conduction Development Team <dev@conductio.nl> | ||
| * @copyright 2026 Conduction B.V. | ||
| * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 | ||
| * | ||
| * @version GIT: <git-id> | ||
| * | ||
| * @link https://conduction.nl | ||
| */ | ||
|
|
||
| // SPDX-License-Identifier: EUPL-1.2 | ||
| // Copyright (C) 2026 Conduction B.V. | ||
| declare(strict_types=1); | ||
|
|
||
| namespace OCA\Planix\Controller; | ||
|
|
||
| use OCA\Planix\AppInfo\Application; | ||
| use OCA\Planix\Service\TimeEntryService; | ||
| use OCP\AppFramework\Controller; | ||
| use OCP\AppFramework\Http; | ||
| use OCP\AppFramework\Http\JSONResponse; | ||
| use OCP\IRequest; | ||
|
|
||
| /** | ||
| * Controller for time entry CRUD operations. | ||
| */ | ||
| class TimeEntryController extends Controller | ||
| { | ||
| /** | ||
| * Constructor for the TimeEntryController. | ||
| * | ||
| * @param IRequest $request The request object | ||
| * @param TimeEntryService $timeEntryService The time entry service | ||
| * | ||
| * @return void | ||
| */ | ||
| public function __construct( | ||
| IRequest $request, | ||
| private TimeEntryService $timeEntryService, | ||
| ) { | ||
| parent::__construct(appName: Application::APP_ID, request: $request); | ||
| }//end __construct() | ||
|
|
||
| /** | ||
| * Create a new time entry. | ||
| * | ||
| * Expects JSON body with: taskId, duration (minutes, > 0), date (ISO 8601), description (optional). | ||
| * | ||
| * @NoAdminRequired | ||
| * | ||
| * @return JSONResponse | ||
| */ | ||
| public function create(): JSONResponse | ||
| { | ||
| $userId = $this->timeEntryService->getCurrentUserId(); | ||
| if ($userId === null) { | ||
| return new JSONResponse( | ||
| ['error' => 'Authentication required.'], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| $data = $this->request->getParams(); | ||
| $entry = $this->timeEntryService->createTimeEntry($data); | ||
|
|
||
| return new JSONResponse($entry, Http::STATUS_CREATED); | ||
| } catch (\InvalidArgumentException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } catch (\RuntimeException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_INTERNAL_SERVER_ERROR | ||
| ); | ||
| } | ||
|
|
||
| }//end create() | ||
|
|
||
| /** | ||
| * List time entries for a task. | ||
| * | ||
| * Expects query parameter: taskId. | ||
| * | ||
| * @NoAdminRequired | ||
| * @NoCSRFRequired | ||
| * | ||
| * @return JSONResponse | ||
| */ | ||
| public function index(): JSONResponse | ||
| { | ||
| $userId = $this->timeEntryService->getCurrentUserId(); | ||
| if ($userId === null) { | ||
| return new JSONResponse( | ||
| ['error' => 'Authentication required.'], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| $taskId = $this->request->getParam('taskId', ''); | ||
| if (empty($taskId) === true) { | ||
| return new JSONResponse( | ||
| ['error' => 'taskId query parameter is required.'], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } | ||
|
|
||
| if (preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $taskId) !== 1) { | ||
| return new JSONResponse( | ||
| ['error' => 'taskId must be a valid UUID.'], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| $entries = $this->timeEntryService->listTimeEntries($taskId); | ||
|
|
||
| return new JSONResponse($entries); | ||
| } catch (\InvalidArgumentException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_NOT_FOUND | ||
| ); | ||
| } catch (\RuntimeException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| }//end index() | ||
|
|
||
| /** | ||
| * Update a time entry (owner only). | ||
| * | ||
| * Expects JSON body with any of: duration (minutes, > 0), date (ISO 8601), description. | ||
| * | ||
| * @param string $id The time entry UUID | ||
| * | ||
| * @NoAdminRequired | ||
| * | ||
| * @return JSONResponse | ||
| */ | ||
| public function update(string $id): JSONResponse | ||
| { | ||
| if (preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $id) !== 1) { | ||
| return new JSONResponse( | ||
| ['error' => 'Invalid identifier format.'], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } | ||
|
|
||
| $userId = $this->timeEntryService->getCurrentUserId(); | ||
| if ($userId === null) { | ||
| return new JSONResponse( | ||
| ['error' => 'Authentication required.'], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| $data = $this->request->getParams(); | ||
| $entry = $this->timeEntryService->updateTimeEntry($id, $data); | ||
|
|
||
| return new JSONResponse($entry); | ||
| } catch (\InvalidArgumentException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } catch (\RuntimeException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| }//end update() | ||
|
|
||
| /** | ||
| * Delete a time entry (owner only). | ||
| * | ||
| * @param string $id The time entry UUID | ||
| * | ||
| * @NoAdminRequired | ||
| * | ||
| * @return JSONResponse | ||
| */ | ||
| public function destroy(string $id): JSONResponse | ||
| { | ||
| if (preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $id) !== 1) { | ||
| return new JSONResponse( | ||
| ['error' => 'Invalid identifier format.'], | ||
| Http::STATUS_BAD_REQUEST | ||
| ); | ||
| } | ||
|
|
||
| $userId = $this->timeEntryService->getCurrentUserId(); | ||
| if ($userId === null) { | ||
| return new JSONResponse( | ||
| ['error' => 'Authentication required.'], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| try { | ||
| $this->timeEntryService->deleteTimeEntry($id); | ||
|
|
||
| return new JSONResponse(['success' => true]); | ||
| } catch (\InvalidArgumentException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_NOT_FOUND | ||
| ); | ||
| } catch (\RuntimeException $e) { | ||
| return new JSONResponse( | ||
| ['error' => $e->getMessage()], | ||
| Http::STATUS_FORBIDDEN | ||
| ); | ||
| } | ||
|
|
||
| }//end destroy() | ||
| }//end class |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL] Missing OAS 3.0 OpenAPI specification
The Conduction company-wide conventions require an OAS 3.0 OpenAPI Specification for all new or changed endpoints. This PR adds four new REST endpoints (
POST /api/time-entries,GET /api/time-entries,PUT /api/time-entries/{id},DELETE /api/time-entries/{id}) with no accompanying OpenAPI spec file. Without the spec, downstream consumers, API gateways, and the Common Ground ecosystem cannot discover or validate against these endpoints.Required action: Add an
openapi.yamldocumenting all four endpoints — request bodies, query parameters, response schemas, and error responses.