diff --git a/.planning/phases/08-backend-aggregation-filtering/08-RESEARCH.md b/.planning/phases/08-backend-aggregation-filtering/08-RESEARCH.md new file mode 100644 index 0000000..2c61e83 --- /dev/null +++ b/.planning/phases/08-backend-aggregation-filtering/08-RESEARCH.md @@ -0,0 +1,443 @@ +# Phase 8: Backend Aggregation + Filtering - Research + +**Researched:** 2026-04-09 +**Domain:** Symfony/Doctrine PHP backend, SQL aggregation, timezone handling +**Confidence:** HIGH + +## Summary + +Phase 8 extends the existing `HeatmapService` and `HeatmapController` with two new aggregation modes (hourly, day/hour) and three filter parameters (activity, customer, plus existing project). It also adds three cascade entity endpoints for the Phase 10 TomSelect pickers. All changes are backend PHP + PHPUnit tests with minimal frontend touch (URL parameter construction only). + +The critical technical challenge is **timezone-correct hour extraction**. Kimai stores `start_time` in UTC but users operate in local timezones. MariaDB's `CONVERT_TZ` with numeric offsets (e.g., `'+02:00'`) is the most reliable approach -- it does not depend on MySQL timezone tables being loaded. The `date_tz` column (already in user timezone) can be used for day-of-week grouping. + +**Primary recommendation:** Use native SQL queries (via DBAL Connection) for the hourly and day/hour aggregations since Kimai only registers DATE/DAY/MONTH/YEAR as custom DQL functions -- there is no HOUR function. Keep the existing DQL QueryBuilder pattern for the daily aggregation and cascade endpoints. + + +## User Constraints (from CONTEXT.md) + +### Locked Decisions +No locked decisions -- user deferred all to Claude's discretion. + +### Claude's Discretion +- D-01: Extend HeatmapService with separate getHourlyAggregation() and getDayHourAggregation() methods +- D-02: Extend existing /heatmap/data endpoint with optional mode query param (daily/hourly/dayhour) +- D-03: Add activity and customer query params to /heatmap/data endpoint with AND logic +- D-04: Cascade endpoints: /heatmap/customers, /heatmap/projects?customer={id}, /heatmap/activities?project={id} +- D-05: Cascade response format: [{id: number, name: string}] +- D-06: Hourly aggregation returns {hour: 0-23, hours: float, count: int} +- D-07: Day/hour aggregation returns {day: 0-6, hour: 0-23, hours: float, count: int} +- D-08: Use Kimai user timezone for hour grouping +- D-09: Frontend extends existing fetch with mode/filter params + +### Deferred Ideas (OUT OF SCOPE) +None. + + + +## Phase Requirements + +| ID | Description | Research Support | +|----|-------------|------------------| +| API-01 | API endpoint accepts mode param returning hour-level and day/hour aggregation data | New service methods + mode param dispatch in controller (see Architecture Patterns) | +| API-02 | API endpoint accepts activity and customer filter params with own controller endpoints | Filter params on /heatmap/data + 3 cascade endpoints (see Cascade Endpoints pattern) | +| FILT-02 | Activity filtering narrows heatmap data to a specific activity | QueryBuilder WHERE on t.activity (see Filter Implementation pattern) | +| FILT-03 | Customer filtering narrows heatmap data to all projects under a customer | JOIN through t.project -> p.customer (see Filter Implementation pattern) | +| TEST-03 | PHPUnit tests for hour-level and day/hour aggregation queries | Extend existing mock pattern from HeatmapServiceTest (see Validation Architecture) | + + +## Architecture Patterns + +### Recommended Changes Structure + +``` +Controller/HeatmapController.php # extend with mode dispatch + cascade actions +Service/HeatmapService.php # add 3 new methods + extend existing with filters +Tests/Controller/HeatmapControllerTest.php # extend with new action tests +Tests/Service/HeatmapServiceTest.php # extend with new method tests +assets/src/types.ts # add HourEntry, DayHourEntry, HourlyData, DayHourData types +``` + +### Pattern 1: Mode Dispatch in Controller + +**What:** The existing `data()` action reads a `mode` query param and dispatches to the appropriate service method. [VERIFIED: existing controller code] + +**Example:** +```php +// Source: extending existing HeatmapController.php +#[Route(path: '/data', name: 'heatmap_data', methods: ['GET'])] +#[IsGranted('view_own_timesheet')] +public function data(Request $request, HeatmapService $service): JsonResponse +{ + $user = $this->getUser(); + $end = new \DateTimeImmutable('today'); + $begin = $end->modify('-1 year'); + + $mode = $request->query->getString('mode', 'daily'); + $projectId = $request->query->getInt('project') ?: null; + $customerId = $request->query->getInt('customer') ?: null; + $activityId = $request->query->getInt('activity') ?: null; + + return match ($mode) { + 'hourly' => new JsonResponse([ + 'hours' => $service->getHourlyAggregation($user, $begin, $end, $projectId, $customerId, $activityId), + 'range' => ['begin' => $begin->format('Y-m-d'), 'end' => $end->format('Y-m-d')], + ]), + 'dayhour' => new JsonResponse([ + 'matrix' => $service->getDayHourAggregation($user, $begin, $end, $projectId, $customerId, $activityId), + 'range' => ['begin' => $begin->format('Y-m-d'), 'end' => $end->format('Y-m-d')], + ]), + default => new JsonResponse([ + 'days' => $service->getDailyAggregation($user, $begin, $end, $projectId, $customerId, $activityId), + 'range' => ['begin' => $begin->format('Y-m-d'), 'end' => $end->format('Y-m-d')], + ]), + }; +} +``` + +### Pattern 2: Timezone-Correct Hour Extraction via Native SQL + +**What:** Use DBAL native SQL with `CONVERT_TZ` for hour grouping since Kimai has no HOUR DQL function registered. [VERIFIED: doctrine.yaml only registers DATE/DAY/MONTH/YEAR] + +**Why native SQL:** Kimai's custom DQL functions (in `App\Doctrine\Extensions\`) only cover DATE, DAY, MONTH, YEAR. Adding a plugin-level DQL function would require modifying Kimai's Doctrine config, which plugins should not do. Native SQL via DBAL Connection is clean and explicit. + +**Critical timezone detail:** `start_time` is stored in UTC. Each timesheet has a `timezone` column. The user's current timezone is available via `$user->getTimezone()`. We compute a numeric offset string (e.g., `'+02:00'`) from the user timezone and use `CONVERT_TZ(t.start_time, '+00:00', :tz_offset)`. This avoids requiring MySQL timezone tables. [VERIFIED: UTCDateTimeType.php confirms UTC storage; Timesheet entity confirms timezone column] + +**Example:** +```php +// Source: pattern derived from existing codebase analysis +public function getHourlyAggregation(User $user, \DateTimeInterface $begin, \DateTimeInterface $end, ?int $projectId = null, ?int $customerId = null, ?int $activityId = null): array +{ + $tz = new \DateTimeZone($user->getTimezone()); + $offset = $tz->getOffset(new \DateTime('now', $tz)); + $offsetStr = sprintf('%+03d:%02d', intdiv($offset, 3600), abs($offset % 3600) / 60); + + $conn = $this->repository->getEntityManager()->getConnection(); + $sql = 'SELECT HOUR(CONVERT_TZ(t.start_time, \'+00:00\', :tz)) as hour_slot, + COALESCE(SUM(t.duration), 0) as duration, + COUNT(t.id) as count + FROM kimai2_timesheet t + WHERE t.user = :user + AND t.date_tz BETWEEN :begin AND :end + AND t.end_time IS NOT NULL'; + + $params = [ + 'tz' => $offsetStr, + 'user' => $user->getId(), + 'begin' => $begin->format('Y-m-d'), + 'end' => $end->format('Y-m-d'), + ]; + + // append optional filters... + // $sql .= ' GROUP BY hour_slot ORDER BY hour_slot ASC'; + + $results = $conn->executeQuery($sql, $params)->fetchAllAssociative(); + // map to response format +} +``` + +**Offset caveat:** A single offset computed at request time is correct for users in fixed-offset timezones. For DST timezones, entries created during summer will be off by 1 hour when viewed in winter (and vice versa). This is an acceptable tradeoff for an aggregation heatmap -- Kimai's own `date_tz` column has the same limitation (stores the date at creation time, not retroactively adjusted). [ASSUMED] + +### Pattern 3: Filter Implementation + +**What:** Activity and customer filters are additive WHERE clauses on all aggregation queries. [VERIFIED: existing project filter pattern in HeatmapService] + +**Customer filter requires a JOIN** since timesheets reference projects, and projects reference customers: +```sql +-- Activity filter (direct relation) +AND t.activity_id = :activity + +-- Customer filter (through project) +INNER JOIN kimai2_projects p ON t.project_id = p.id +AND p.customer_id = :customer +``` + +For DQL (daily aggregation extending existing QueryBuilder): +```php +if ($activityId !== null) { + $qb->andWhere($qb->expr()->eq('t.activity', ':activity')) + ->setParameter('activity', $activityId); +} +if ($customerId !== null) { + $qb->join('t.project', 'p') + ->andWhere($qb->expr()->eq('p.customer', ':customer')) + ->setParameter('customer', $customerId); +} +``` + +### Pattern 4: Cascade Entity Endpoints + +**What:** Three new controller actions returning entity lists scoped to user's own timesheets. [VERIFIED: getUserProjects() pattern in HeatmapService] + +```php +#[Route(path: '/customers', name: 'heatmap_customers', methods: ['GET'])] +#[IsGranted('view_own_timesheet')] +public function customers(HeatmapService $service): JsonResponse +{ + return new JsonResponse($service->getUserCustomers($this->getUser())); +} +``` + +Service methods follow the existing `getUserProjects()` pattern -- query timesheets, join to related entity, select distinct, return `[{id, name}]`. + +### Pattern 5: Day/Hour Aggregation with weekStart + +**What:** The day/hour (punchcard) mode groups by day-of-week AND hour-of-day. Day index is relative to user's `weekStart` preference. + +**Day-of-week from `date_tz`:** Since `date_tz` already stores the date in user timezone, `DAYOFWEEK(date_tz)` gives the correct day. MySQL's DAYOFWEEK returns 1=Sunday through 7=Saturday. We need to remap to 0-6 relative to weekStart in PHP. [VERIFIED: Timesheet entity doc comment confirms date_tz is in user timezone] + +```sql +SELECT DAYOFWEEK(t.date_tz) as dow, + HOUR(CONVERT_TZ(t.start_time, '+00:00', :tz)) as hour_slot, + COALESCE(SUM(t.duration), 0) as duration, + COUNT(t.id) as count +FROM kimai2_timesheet t +WHERE ... +GROUP BY dow, hour_slot +ORDER BY dow, hour_slot +``` + +Then in PHP, remap MySQL's DAYOFWEEK (1=Sun,2=Mon,...,7=Sat) to 0-6 relative to weekStart: +```php +// MySQL DAYOFWEEK: 1=Sun, 2=Mon, ... 7=Sat +// If weekStart=monday: Mon=0, Tue=1, ..., Sun=6 +// If weekStart=sunday: Sun=0, Mon=1, ..., Sat=6 +$weekStartOffset = ($weekStart === 'sunday') ? 1 : 2; +$dayIndex = ($mysqlDow - $weekStartOffset + 7) % 7; +``` + +### Anti-Patterns to Avoid + +- **Registering custom DQL functions from a plugin:** Plugins should not modify Kimai's Doctrine configuration. Use native SQL instead. [VERIFIED: plugin architecture does not support custom DQL registration] +- **Using `CONVERT_TZ` with named timezones:** Requires MySQL `mysql.time_zone_name` table to be populated (often empty in Docker/dev setups). Use numeric offsets only. [ASSUMED] +- **Ignoring the `end IS NOT NULL` filter:** Running timesheets have no duration. Existing code correctly filters these. New queries must too. [VERIFIED: existing service code] + +## Don't Hand-Roll + +| Problem | Don't Build | Use Instead | Why | +|---------|-------------|-------------|-----| +| Timezone offset calculation | Manual hour math | PHP DateTimeZone::getOffset() | Handles DST transitions correctly | +| Day-of-week calculation | PHP date() on each row | MySQL DAYOFWEEK() on date_tz | Database-side grouping is faster | +| SQL injection protection | String concatenation | Parameterized queries (DBAL params) | Standard security practice | +| JSON serialization | Manual json_encode | Symfony JsonResponse | Handles encoding, content-type headers | + +## Common Pitfalls + +### Pitfall 1: CONVERT_TZ Returns NULL +**What goes wrong:** `CONVERT_TZ` returns NULL when using named timezones (e.g., 'Europe/Berlin') and MySQL timezone tables are not loaded. +**Why it happens:** MariaDB/MySQL ship with empty timezone tables by default. Named timezone lookups fail silently. +**How to avoid:** Always use numeric offset format (`'+02:00'`), never named timezones. +**Warning signs:** Aggregation returns zero/empty results when data exists. [ASSUMED] + +### Pitfall 2: DST Offset Mismatch +**What goes wrong:** A single offset computed at request time doesn't match offsets at the time entries were created. Summer entries shift by 1 hour when viewed in winter. +**Why it happens:** Timezones with DST have different UTC offsets at different times of year. +**How to avoid:** For a heatmap aggregation, this 1-hour shift is acceptable. If precision is needed later, each timesheet's own `timezone` column could be used per-row (expensive). +**Warning signs:** Hour bins near DST transitions show unexpected clustering. [ASSUMED] + +### Pitfall 3: Table Name Mismatch +**What goes wrong:** Native SQL uses actual database table names, not Doctrine entity names. +**Why it happens:** DQL uses entity names (`Timesheet`, `Project`), but raw SQL needs table names (`kimai2_timesheet`, `kimai2_projects`). +**How to avoid:** Check the `@ORM\Table` annotation or use `$em->getClassMetadata(Timesheet::class)->getTableName()` to resolve dynamically. +**Warning signs:** "Table doesn't exist" SQL errors. [VERIFIED: Timesheet entity uses kimai2_timesheet table] + +### Pitfall 4: Customer Filter Without Project Join +**What goes wrong:** Trying to filter by customer directly on the timesheet table fails -- there is no customer_id column on kimai2_timesheet. +**Why it happens:** Kimai's entity model is Timesheet -> Project -> Customer (no direct Timesheet -> Customer relation). +**How to avoid:** Always JOIN kimai2_projects when filtering by customer. +**Warning signs:** SQL column not found error. [VERIFIED: Timesheet entity has project and activity relations only] + +### Pitfall 5: Missing Query Builder Methods in Mock +**What goes wrong:** PHPUnit tests fail because the mock QueryBuilder doesn't stub new chained methods (e.g., `join()`). +**Why it happens:** Existing test helper `createServiceWithResults()` stubs a fixed set of QB methods. New filter logic adds `join()`. +**How to avoid:** Add `$qb->method('join')->willReturnSelf()` to the test helper. For native SQL tests, mock the DBAL Connection instead. +**Warning signs:** "Call to undefined method" in test output. [VERIFIED: existing test mock pattern] + +## Code Examples + +### Native SQL Query with DBAL Connection + +```php +// Source: Doctrine DBAL pattern [VERIFIED: TimesheetRepository uses getEntityManager()] +$conn = $this->repository->getEntityManager()->getConnection(); +$result = $conn->executeQuery($sql, $params); +$rows = $result->fetchAllAssociative(); +``` + +### Timezone Offset from User + +```php +// Source: PHP DateTimeZone API [VERIFIED: User::getTimezone() returns timezone string] +$tz = new \DateTimeZone($user->getTimezone()); +$offset = $tz->getOffset(new \DateTime('now', new \DateTimeZone('UTC'))); +$hours = intdiv($offset, 3600); +$minutes = abs(intdiv($offset % 3600, 60)); +$offsetStr = sprintf('%+03d:%02d', $hours, $minutes); +// e.g., "Europe/Berlin" in summer -> "+02:00" +``` + +### Cascade Query (getUserCustomers example) + +```php +// Source: extending existing getUserProjects() pattern [VERIFIED: HeatmapService.php] +public function getUserCustomers(User $user): array +{ + $qb = $this->repository->createQueryBuilder('t'); + $qb->select('DISTINCT IDENTITY(p.customer) as customerId, c.name') + ->join('t.project', 'p') + ->join('p.customer', 'c') + ->andWhere($qb->expr()->eq('t.user', ':user')) + ->andWhere($qb->expr()->isNotNull('t.end')) + ->setParameter('user', $user) + ->orderBy('c.name', 'ASC'); + + return array_map(fn(array $row) => [ + 'id' => (int) $row['customerId'], + 'name' => $row['name'], + ], $qb->getQuery()->getResult()); +} +``` + +### getUserActivities with Optional Project Scope + +```php +// Source: extending getUserProjects() pattern [VERIFIED: HeatmapService.php] +public function getUserActivities(User $user, ?int $projectId = null): array +{ + $qb = $this->repository->createQueryBuilder('t'); + $qb->select('DISTINCT IDENTITY(t.activity) as activityId, a.name') + ->join('t.activity', 'a') + ->andWhere($qb->expr()->eq('t.user', ':user')) + ->andWhere($qb->expr()->isNotNull('t.end')) + ->setParameter('user', $user) + ->orderBy('a.name', 'ASC'); + + if ($projectId !== null) { + $qb->andWhere($qb->expr()->eq('t.project', ':project')) + ->setParameter('project', $projectId); + } + + return array_map(fn(array $row) => [ + 'id' => (int) $row['activityId'], + 'name' => $row['name'], + ], $qb->getQuery()->getResult()); +} +``` + +### Testing Native SQL with Mocked Connection + +```php +// Source: PHPUnit mock pattern for DBAL [ASSUMED] +private function createServiceWithNativeResults(array $results): HeatmapService +{ + // For native SQL methods, mock the DBAL connection + $statement = $this->createMock(\Doctrine\DBAL\Result::class); + $statement->method('fetchAllAssociative')->willReturn($results); + + $connection = $this->createMock(\Doctrine\DBAL\Connection::class); + $connection->method('executeQuery')->willReturn($statement); + + $em = $this->createMock(\Doctrine\ORM\EntityManagerInterface::class); + $em->method('getConnection')->willReturn($connection); + + $repo = $this->createMock(TimesheetRepository::class); + $repo->method('getEntityManager')->willReturn($em); + // Also set up createQueryBuilder for DQL methods + // ... + + return new HeatmapService($repo); +} +``` + +## Validation Architecture + +### Test Framework +| Property | Value | +|----------|-------| +| Framework | PHPUnit 10.5 | +| Config file | `Tests/phpunit.xml` | +| Quick run command | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml` | +| Full suite command | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml` | + +### Phase Requirements -> Test Map +| Req ID | Behavior | Test Type | Automated Command | File Exists? | +|--------|----------|-----------|-------------------|-------------| +| API-01 | Mode param dispatches to correct service method | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testDataReturnsHourly` | Wave 0 | +| API-01 | Hourly aggregation groups by hour, returns correct shape | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testGetHourlyAggregation` | Wave 0 | +| API-01 | Day/hour aggregation groups by day+hour, returns correct shape | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testGetDayHourAggregation` | Wave 0 | +| API-02 | Activity filter param narrows results | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testActivityFilter` | Wave 0 | +| API-02 | Customer filter param narrows results via project join | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testCustomerFilter` | Wave 0 | +| API-02 | Cascade endpoints return entity lists | unit | `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml --filter testCustomersEndpoint` | Wave 0 | +| FILT-02 | Activity filter applied to all aggregation modes | unit | covered by API-02 activity filter tests | Wave 0 | +| FILT-03 | Customer filter applied to all aggregation modes | unit | covered by API-02 customer filter tests | Wave 0 | +| TEST-03 | PHPUnit tests for aggregation queries | unit | full suite command | Wave 0 | + +### Sampling Rate +- **Per task commit:** `php dev/kimai/vendor/bin/phpunit --configuration Tests/phpunit.xml` +- **Per wave merge:** same (single test suite) +- **Phase gate:** Full suite green before `/gsd-verify-work` + +### Wave 0 Gaps +None -- existing test infrastructure (`Tests/phpunit.xml`, `Tests/bootstrap.php`, mock helpers in `HeatmapServiceTest.php`) covers all needs. New test methods extend existing test classes. + +## Security Domain + +### Applicable ASVS Categories + +| ASVS Category | Applies | Standard Control | +|---------------|---------|-----------------| +| V2 Authentication | no | Handled by Kimai (session auth) | +| V3 Session Management | no | Handled by Kimai | +| V4 Access Control | yes | `#[IsGranted('view_own_timesheet')]` + user-scoped queries | +| V5 Input Validation | yes | Query param type casting (`getInt()`, `getString()`) + parameterized SQL | +| V6 Cryptography | no | No crypto in this phase | + +### Known Threat Patterns + +| Pattern | STRIDE | Standard Mitigation | +|---------|--------|---------------------| +| SQL injection via filter params | Tampering | Parameterized queries (DBAL params for native SQL, QueryBuilder setParameter for DQL) | +| IDOR on customer/project/activity IDs | Information Disclosure | All queries scoped to `t.user = :user` -- user can only see their own entities | +| Mode param injection | Tampering | `match()` with `default` case prevents invalid modes from reaching queries | + +## Assumptions Log + +| # | Claim | Section | Risk if Wrong | +|---|-------|---------|---------------| +| A1 | CONVERT_TZ with numeric offsets works without timezone tables loaded | Pitfall 1 | Hour grouping returns NULL; fallback would be PHP-side processing | +| A2 | Single offset for DST timezones is acceptable for heatmap aggregation | Pitfall 2 | 1-hour shift in some entries; low visual impact | +| A3 | DBAL Result::fetchAllAssociative() is mockable in PHPUnit | Code Examples | Tests need different mock approach if class is final | + +## Open Questions + +1. **Table name resolution in native SQL** + - What we know: Timesheet table is `kimai2_timesheet`, projects is `kimai2_projects`, customers is `kimai2_customers`, activities is `kimai2_activities` + - What's unclear: Whether these names are configurable via Doctrine prefix + - Recommendation: Use `$em->getClassMetadata(Timesheet::class)->getTableName()` for safety, or hardcode since Kimai's table names are stable [ASSUMED] + +## Sources + +### Primary (HIGH confidence) +- `Controller/HeatmapController.php` -- existing endpoint pattern, route attributes, auth checks +- `Service/HeatmapService.php` -- existing QueryBuilder pattern, getUserProjects() shape +- `Tests/Service/HeatmapServiceTest.php` -- existing mock pattern for service tests +- `dev/kimai/config/packages/doctrine.yaml` -- confirmed DQL functions limited to DATE/DAY/MONTH/YEAR +- `dev/kimai/src/Doctrine/UTCDateTimeType.php` -- confirmed UTC storage +- `dev/kimai/src/Entity/Timesheet.php` -- confirmed entity relations, date_tz column, timezone column +- `dev/kimai/src/Entity/User.php` -- confirmed getTimezone() and getFirstDayOfWeek() APIs +- `dev/kimai/src/Entity/Project.php` -- confirmed customer relation + +### Secondary (MEDIUM confidence) +- UI-SPEC (`08-UI-SPEC.md`) -- API response contracts for downstream phases + +### Tertiary (LOW confidence) +- MariaDB CONVERT_TZ behavior with numeric offsets (not verified against running instance) + +## Metadata + +**Confidence breakdown:** +- Standard stack: HIGH -- extending existing PHP patterns, no new libraries +- Architecture: HIGH -- all patterns verified against existing codebase +- Pitfalls: MEDIUM -- timezone handling assumptions need runtime validation + +**Research date:** 2026-04-09 +**Valid until:** 2026-05-09 (stable -- Kimai internals unlikely to change)